Replace-User-Metadata-#5 #16

Merged
abed.alrahman merged 2 commits from Replace-User-Metadata-#5 into develop 2025-05-22 10:07:52 +00:00
3 changed files with 222 additions and 27 deletions
@@ -4,6 +4,7 @@ import com.cleverthis.authservice.dto.LoginRequest;
import com.cleverthis.authservice.dto.LogoutRequest; import com.cleverthis.authservice.dto.LogoutRequest;
import com.cleverthis.authservice.dto.TokenResponse; import com.cleverthis.authservice.dto.TokenResponse;
import com.cleverthis.authservice.dto.VerificationResponse; import com.cleverthis.authservice.dto.VerificationResponse;
import com.cleverthis.authservice.exception.TokenVerificationException;
import com.cleverthis.authservice.service.IdentityManagerService; import com.cleverthis.authservice.service.IdentityManagerService;
import jakarta.validation.Valid; import jakarta.validation.Valid;
import lombok.extern.slf4j.Slf4j; import lombok.extern.slf4j.Slf4j;
@@ -11,6 +12,7 @@ import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.HttpHeaders; import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpStatus; import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity; import org.springframework.http.ResponseEntity;
import org.springframework.util.StringUtils;
import org.springframework.web.bind.annotation.PostMapping; import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestHeader; import org.springframework.web.bind.annotation.RequestHeader;
@@ -26,6 +28,13 @@ import reactor.core.publisher.Mono;
@RequestMapping("/") @RequestMapping("/")
@Slf4j public class AuthController { @Slf4j public class AuthController {
// --- Constants for Header Names ---
private static final String HEADER_USER_ID = "X-User-Id";
private static final String HEADER_USER_NAME = "X-User-Name";
private static final String HEADER_USER_EMAIL = "X-User-Email";
private static final String HEADER_USER_GROUPS = "X-User-Groups";
// Add more headers as needed (e.g., X-User-Roles)
private final IdentityManagerService identityManagerService; private final IdentityManagerService identityManagerService;
@Autowired @Autowired
@@ -51,26 +60,42 @@ import reactor.core.publisher.Mono;
loginRequest.getUsername(), e.getMessage())); loginRequest.getUsername(), e.getMessage()));
} }
/** /**
* Handles token verification requests. Expects the access token in the Authorization header * Handles token verification requests. Expects the access token in the Authorization header
* (Bearer scheme). * (Bearer scheme). Returns full token introspection details.
* *
* @param authorizationHeader The full Authorization header value. * @param authorizationHeader The full Authorization header value (e.g., "Bearer <token>").
* @return ResponseEntity containing the VerificationResponse on success, or an error status * @return ResponseEntity containing the VerificationResponse on success, or an error status
* handled globally. * handled globally.
*/ */
@PostMapping("/verify") @PostMapping("/verify")
public Mono<ResponseEntity<VerificationResponse>> verify( public Mono<ResponseEntity<VerificationResponse>> verify(
@RequestHeader(HttpHeaders.AUTHORIZATION) String authorizationHeader) { @RequestHeader(HttpHeaders.AUTHORIZATION) String authorizationHeader) {
log.info("Received token verification request"); Mono<ResponseEntity<VerificationResponse>> result;
if (authorizationHeader == null || !authorizationHeader.startsWith("Bearer ")) { log.info("Received token verification request (/verify)");
log.warn("Verification request missing or invalid Bearer token format. {}", String accessToken = extractBearerToken(authorizationHeader);
authorizationHeader); if (accessToken == null) {
return Mono.just(ResponseEntity.status(HttpStatus.UNAUTHORIZED).build()); log.warn("Verification request missing or invalid Bearer token format, token : {}", authorizationHeader);
result = Mono.just(ResponseEntity.status(HttpStatus.UNAUTHORIZED).build());
} else {
result = this.identityManagerService.verifyToken(accessToken).map(ResponseEntity::ok)
.doOnError(e -> log.error("Token verification failed: {}", e.getMessage()));
} }
String accessToken = authorizationHeader.substring(7);
return this.identityManagerService.verifyToken(accessToken).map(ResponseEntity::ok) return result;
.doOnError(e -> log.error("Token verification failed: {}", e.getMessage())); }
/**
* Extracts the token from the "Bearer <token>" Authorization header.
* @param authorizationHeader The full header value.
* @return The token string or null if header is invalid.
*/
private String extractBearerToken(String authorizationHeader) {
if (authorizationHeader != null && authorizationHeader.startsWith("Bearer ")) {
return authorizationHeader.substring(7);
}
return null;
} }
/** /**
@@ -86,4 +111,73 @@ import reactor.core.publisher.Mono;
.thenReturn(ResponseEntity.noContent().<Void>build()) .thenReturn(ResponseEntity.noContent().<Void>build())
.doOnError(e -> log.error("Logout failed: {}", e.getMessage())); .doOnError(e -> log.error("Logout failed: {}", e.getMessage()));
} }
/**
* Handles forward authentication requests from Traefik. Expects the access token in the
* Authorization header (Bearer scheme). On success, returns 200 OK with user metadata in
* headers. On failure, returns 401 Unauthorized.
*
* @param authorizationHeader The full Authorization header value (e.g., "Bearer <token>").
* @return ResponseEntity with status 200 and custom headers on success, or 401/500 on failure.
*/
// CHANGED: Use POST instead of GET for diagnostic purposes
@PostMapping("/auth")
hurui200320 marked this conversation as resolved
Review

By default, traefik's forward auth uses GET. But here you used post, based on the comment above, I assume you forget to change it back?

Also in the doc there is a config that allows traefik to use the original HTTP method. If we want finer control (check different HTTP method), then we can enable this setting and change the code accordingly.

By default, traefik's forward auth uses GET. But here you used post, based on the comment above, I assume you forget to change it back? Also in [the doc](https://doc.traefik.io/traefik/middlewares/http/forwardauth/#preserverequestmethod) there is a config that allows traefik to use the original HTTP method. If we want finer control (check different HTTP method), then we can enable this setting and change the code accordingly.
Review

This ticket is an intermediate ticket for the Access control ticket. I already changed it in the access control implementation ticket.
So, in the next PR for ticket #3, it will use ALL methods, since it will be forwarded from Trarfik

This ticket is an intermediate ticket for the Access control ticket. I already changed it in the access control implementation ticket. So, in the next PR for ticket #3, it will use ALL methods, since it will be forwarded from Trarfik
public Mono<ResponseEntity<Void>> forwardAuth(
@RequestHeader(HttpHeaders.AUTHORIZATION) String authorizationHeader) {
log.info("Received forwardAuth request (/auth)");
String accessToken = extractBearerToken(authorizationHeader);
if (accessToken == null) {
log.warn("ForwardAuth request missing or invalid Bearer token format, token : {}", authorizationHeader);
// Return 401 which Traefik forwardAuth typically interprets as "block request"
return Mono.just(ResponseEntity.status(HttpStatus.UNAUTHORIZED).build());
}
return this.identityManagerService.verifyToken(accessToken).flatMap(verificationResponse -> {
// If token is valid and active, build success response with headers
log.debug("ForwardAuth successful for user: {}",
verificationResponse.getPreferredUsername());
HttpHeaders responseHeaders = buildAuthHeaders(verificationResponse);
// Return 200 OK with the headers, no body needed.
return Mono.just(ResponseEntity.ok().headers(responseHeaders).<Void>build());
}).doOnError(e -> log.error("ForwardAuth token verification failed: {}", e.getMessage()))
// On specific verification error (invalid token), return 401
.onErrorResume(TokenVerificationException.class,
e -> Mono
.just(ResponseEntity.status(HttpStatus.UNAUTHORIZED).<Void>build()))
// Catch other potential errors during service call (e.g., Keycloak unavailable)
.onErrorResume(Exception.class, e -> {
log.error("Internal error during forwardAuth token verification: {}",
e.getMessage(), e);
// Return 500 or 401? Returning 401 might be safer for auth checks.
return Mono.just(ResponseEntity.status(HttpStatus.UNAUTHORIZED).<Void>build());
});
}
/**
* Builds HttpHeaders containing user metadata based on the verification response.
* @param verificationResponse The response from token verification.
* @return HttpHeaders object with X-User-* headers.
*/
private HttpHeaders buildAuthHeaders(VerificationResponse verificationResponse) {
HttpHeaders headers = new HttpHeaders();
// Use 'sub' (subject) as the canonical User ID
if (verificationResponse.getSub() != null) {
headers.add(HEADER_USER_ID, verificationResponse.getSub());
}
// Use 'preferredUsername' as the display/login name
if (verificationResponse.getPreferredUsername() != null) {
headers.add(HEADER_USER_NAME, verificationResponse.getPreferredUsername());
}
if (verificationResponse.getEmail() != null) {
headers.add(HEADER_USER_EMAIL, verificationResponse.getEmail());
}
// Handle groups if present
if (verificationResponse.getGroups() != null
&& !verificationResponse.getGroups().isEmpty()) {
// Convert list of groups to a comma-separated string
headers.add(HEADER_USER_GROUPS,
StringUtils.collectionToCommaDelimitedString(verificationResponse.getGroups()));
}
return headers;
}
} }
@@ -1,5 +1,9 @@
package com.cleverthis.authservice.dto; package com.cleverthis.authservice.dto;
import java.util.List;
import com.fasterxml.jackson.annotation.JsonProperty;
import lombok.AllArgsConstructor; import lombok.AllArgsConstructor;
import lombok.Data; import lombok.Data;
import lombok.NoArgsConstructor; import lombok.NoArgsConstructor;
@@ -20,11 +24,12 @@ public class VerificationResponse {
private long exp; // Expiration timestamp private long exp; // Expiration timestamp
private long iat; // Issued at timestamp private long iat; // Issued at timestamp
private long nbf; // Not before timestamp private long nbf; // Not before timestamp
private String aud; // Audience private List<String> aud; // Audience - Can be a list if token has multiple audiences
private String iss; // Issuer private String iss; // Issuer
private String typ; // Type (e.g., Bearer) private String typ; // Type (e.g., Bearer)
private String email; // Standard OIDC claim private String email; // Standard OIDC claim
private Boolean emailVerified; // Standard OIDC claim private Boolean emailVerified; // Standard OIDC claim
@JsonProperty("preferred_username")
private String preferredUsername; // Standard OIDC claim - Use this for X-User-Name private String preferredUsername; // Standard OIDC claim - Use this for X-User-Name
private List<String> groups;
} }
@@ -13,6 +13,8 @@ import com.cleverthis.authservice.exception.GlobalExceptionHandler;
import com.cleverthis.authservice.exception.LogoutException; import com.cleverthis.authservice.exception.LogoutException;
import com.cleverthis.authservice.exception.TokenVerificationException; import com.cleverthis.authservice.exception.TokenVerificationException;
import com.cleverthis.authservice.service.IdentityManagerService; import com.cleverthis.authservice.service.IdentityManagerService;
import java.util.Arrays;
import java.util.List;
import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Autowired;
@@ -29,7 +31,8 @@ import reactor.core.publisher.Mono;
* controller logic and request/response handling. * controller logic and request/response handling.
*/ */
@WebFluxTest(AuthController.class) @WebFluxTest(AuthController.class)
@Import(GlobalExceptionHandler.class) class AuthControllerTest { @Import(GlobalExceptionHandler.class)
class AuthControllerTest {
@Autowired @Autowired
private WebTestClient webTestClient; // Test client for making requests to the controller private WebTestClient webTestClient; // Test client for making requests to the controller
@@ -39,16 +42,32 @@ import reactor.core.publisher.Mono;
private TokenResponse mockTokenResponse; private TokenResponse mockTokenResponse;
private VerificationResponse mockVerificationResponse; private VerificationResponse mockVerificationResponse;
private VerificationResponse mockVerificationResponseWithGroups;
// --- Constants for Header Names (mirroring controller) ---
private static final String HEADER_USER_ID = "X-User-Id";
private static final String HEADER_USER_NAME = "X-User-Name";
private static final String HEADER_USER_EMAIL = "X-User-Email";
private static final String HEADER_USER_GROUPS = "X-User-Groups";
@BeforeEach @BeforeEach
void setUp() { void setUp() {
// Setup mock responses for successful scenarios // Setup mock responses for successful scenarios
this.mockTokenResponse = new TokenResponse("access-123", 300, 1800, "refresh-456", "Bearer", this.mockTokenResponse = new TokenResponse("access-123", 300, 1800, "refresh-456", "Bearer",
"id-789", 0, "state", "openid"); "id-789", 0, "state", "openid");
// Mock response without groups
this.mockVerificationResponse = new VerificationResponse(true, "test-client", "testuser", this.mockVerificationResponse = new VerificationResponse(true, "test-client", "testuser",
"openid", "sub-123", System.currentTimeMillis() / 1000 + 300, "openid", "user-sub-123", System.currentTimeMillis() / 1000 + 300,
System.currentTimeMillis() / 1000, System.currentTimeMillis() / 1000, "aud", "iss", System.currentTimeMillis() / 1000, System.currentTimeMillis() / 1000,
"Bearer", "test@example.com", true, "testuser"); Arrays.asList("aud"), "iss", "Bearer", "test@example.com", true, "testuser", null);
// Mock response with groups
this.mockVerificationResponseWithGroups =
new VerificationResponse(true, "test-client", "groupuser", "openid", "user-sub-456",
System.currentTimeMillis() / 1000 + 300, System.currentTimeMillis() / 1000,
System.currentTimeMillis() / 1000, Arrays.asList("aud"), "iss", "Bearer",
"group@example.com", true, "groupuser", List.of("group1", "group2"));
} }
// --- /login Tests --- // --- /login Tests ---
@@ -73,13 +92,13 @@ import reactor.core.publisher.Mono;
when(this.identityManagerService.login("wronguser", "wrongpass")) when(this.identityManagerService.login("wronguser", "wrongpass"))
.thenReturn(Mono.error(new AuthenticationException("Invalid credentials"))); .thenReturn(Mono.error(new AuthenticationException("Invalid credentials")));
this. webTestClient.post().uri("/login").contentType(MediaType.APPLICATION_JSON) this.webTestClient.post().uri("/login").contentType(MediaType.APPLICATION_JSON)
.bodyValue(loginRequest).exchange().expectStatus().isUnauthorized(); // Expect 401 .bodyValue(loginRequest).exchange().expectStatus().isUnauthorized();
} }
@Test @Test
void login_InvalidRequest_ReturnsBadRequest() { void login_InvalidRequest_ReturnsBadRequest() {
LoginRequest loginRequest = new LoginRequest("", ""); LoginRequest loginRequest = new LoginRequest("", ""); // Invalid request
// No need to mock service for validation errors // No need to mock service for validation errors
// The framework and GlobalExceptionHandler (via parent) should handle this // The framework and GlobalExceptionHandler (via parent) should handle this
@@ -124,10 +143,88 @@ import reactor.core.publisher.Mono;
@Test @Test
void verify_InvalidAuthHeaderFormat_ReturnsUnauthorized() { void verify_InvalidAuthHeaderFormat_ReturnsUnauthorized() {
// No service mocking needed, controller handles this directly // Controller logic handles this case specifically
this.webTestClient.post().uri("/verify") this.webTestClient.post().uri("/verify")
.header(HttpHeaders.AUTHORIZATION, "Basic somecredentials") // Wrong scheme .header(HttpHeaders.AUTHORIZATION, "Basic somecredentials") // Wrong scheme
.exchange().expectStatus().isUnauthorized(); // Expect 401 .exchange().expectStatus().isUnauthorized(); // Expect 401 (handled by controller
}
// --- /auth (ForwardAuth) Tests ---
@Test
void forwardAuth_Success_ReturnsOkWithHeaders() {
String validToken = "valid-token-fw-123";
// Use mock response with groups for this test
when(this.identityManagerService.verifyToken(validToken))
.thenReturn(Mono.just(this.mockVerificationResponseWithGroups));
// CHANGED: Use POST for /auth tests
this.webTestClient.post().uri("/auth")
.header(HttpHeaders.AUTHORIZATION, "Bearer " + validToken).exchange().expectStatus()
.isOk() // Expect 200 OK
.expectHeader().valueEquals(HEADER_USER_ID, "user-sub-456").expectHeader()
.valueEquals(HEADER_USER_NAME, "groupuser").expectHeader()
.valueEquals(HEADER_USER_EMAIL, "group@example.com").expectHeader()
.valueEquals(HEADER_USER_GROUPS, "group1,group2") // Comma-separated
.expectBody().isEmpty(); // No body expected
}
@Test
void forwardAuth_Success_ReturnsOkWithHeaders_NoGroups() {
String validToken = "valid-token-fw-nogroups";
// Use mock response without groups
when(this.identityManagerService.verifyToken(validToken))
.thenReturn(Mono.just(this.mockVerificationResponse));
// CHANGED: Use POST for /auth tests
this.webTestClient.post().uri("/auth")
.header(HttpHeaders.AUTHORIZATION, "Bearer " + validToken).exchange().expectStatus()
.isOk() // Expect 200 OK
.expectHeader().valueEquals(HEADER_USER_ID, "user-sub-123").expectHeader()
.valueEquals(HEADER_USER_NAME, "testuser").expectHeader()
.valueEquals(HEADER_USER_EMAIL, "test@example.com").expectHeader()
.doesNotExist(HEADER_USER_GROUPS) // Groups header should not exist
.expectBody().isEmpty();
}
@Test
void forwardAuth_ServiceThrowsVerificationException_ReturnsUnauthorized() {
String invalidToken = "invalid-token-fw-456";
when(this.identityManagerService.verifyToken(invalidToken))
.thenReturn(Mono.error(new TokenVerificationException("Token expired")));
// CHANGED: Use POST for /auth tests
this.webTestClient.post().uri("/auth")
.header(HttpHeaders.AUTHORIZATION, "Bearer " + invalidToken).exchange()
.expectStatus().isUnauthorized(); // Expect 401 (Handled by controller
}
@Test
void forwardAuth_ServiceThrowsGenericException_ReturnsUnauthorized() {
String token = "token-causing-error";
// Mock service to throw a generic exception
when(this.identityManagerService.verifyToken(token))
.thenReturn(Mono.error(new RuntimeException("Unexpected service error")));
// CHANGED: Use POST for /auth tests
this.webTestClient.post().uri("/auth").header(HttpHeaders.AUTHORIZATION, "Bearer " + token)
.exchange().expectStatus().isUnauthorized(); // Expect 401
}
@Test
void forwardAuth_MissingAuthHeader_ReturnsBadRequest() { // UPDATED EXPECTATION
// Framework handles missing required header
// CHANGED: Use POST for /auth tests
this.webTestClient.post().uri("/auth").exchange().expectStatus().isBadRequest();
}
@Test
void forwardAuth_InvalidAuthHeaderFormat_ReturnsUnauthorized() {
// Controller logic handles this case specifically
// CHANGED: Use POST for /auth tests
this.webTestClient.post().uri("/auth")
.header(HttpHeaders.AUTHORIZATION, "Basic somecredentials").exchange()
.expectStatus().isUnauthorized(); // Expect 401 (Handled by controller)
} }
// --- /logout Tests --- // --- /logout Tests ---
@@ -136,7 +233,7 @@ import reactor.core.publisher.Mono;
void logout_Success_ReturnsNoContent() { void logout_Success_ReturnsNoContent() {
LogoutRequest logoutRequest = new LogoutRequest("refresh-token-to-revoke"); LogoutRequest logoutRequest = new LogoutRequest("refresh-token-to-revoke");
when(this.identityManagerService.logout(logoutRequest.getRefreshToken())) when(this.identityManagerService.logout(logoutRequest.getRefreshToken()))
.thenReturn(Mono.empty()); // Mono<Void> completes successfully .thenReturn(Mono.empty());
this.webTestClient.post().uri("/logout").contentType(MediaType.APPLICATION_JSON) this.webTestClient.post().uri("/logout").contentType(MediaType.APPLICATION_JSON)
.bodyValue(logoutRequest).exchange().expectStatus().isNoContent(); // Expect 204 .bodyValue(logoutRequest).exchange().expectStatus().isNoContent(); // Expect 204
@@ -150,17 +247,16 @@ import reactor.core.publisher.Mono;
.thenReturn(Mono.error(new LogoutException("Keycloak unavailable"))); .thenReturn(Mono.error(new LogoutException("Keycloak unavailable")));
this.webTestClient.post().uri("/logout").contentType(MediaType.APPLICATION_JSON) this.webTestClient.post().uri("/logout").contentType(MediaType.APPLICATION_JSON)
.bodyValue(logoutRequest).exchange().expectStatus().is5xxServerError(); .bodyValue(logoutRequest).exchange().expectStatus().is5xxServerError();
} }
@Test @Test
void logout_InvalidRequest_ReturnsBadRequest() { void logout_InvalidRequest_ReturnsBadRequest() {
LogoutRequest logoutRequest = new LogoutRequest(""); LogoutRequest logoutRequest = new LogoutRequest(""); // Invalid request (blank token)
// Invalid request (blank token)
// No service mocking needed // No service mocking needed
this.webTestClient.post().uri("/logout").contentType(MediaType.APPLICATION_JSON) this.webTestClient.post().uri("/logout").contentType(MediaType.APPLICATION_JSON)
.bodyValue(logoutRequest).exchange().expectStatus().isBadRequest(); .bodyValue(logoutRequest).exchange().expectStatus().isBadRequest(); // Expect 400
} }
} }