From c488de8f2ba32ffc60fb50bb1b46bf60a3f8816f Mon Sep 17 00:00:00 2001 From: Abed Date: Tue, 27 May 2025 01:50:29 +0200 Subject: [PATCH 1/4] Implement New User Registration in auth-service #8 --- .../controller/AuthController.java | 38 ++- .../UserRegistrationController.java | 53 ++++ .../authservice/dto/RegistrationRequest.java | 38 +++ .../exception/GlobalExceptionHandler.java | 41 +++- .../exception/RegistrationException.java | 12 + .../exception/UserAlreadyExistsException.java | 12 + .../service/IdentityManagerService.java | 17 +- .../service/impl/KeycloakAdminClient.java | 4 +- .../impl/KeycloakIdentityManagerService.java | 231 ++++++++++++------ 9 files changed, 357 insertions(+), 89 deletions(-) create mode 100644 src/main/java/com/cleverthis/authservice/controller/UserRegistrationController.java create mode 100644 src/main/java/com/cleverthis/authservice/dto/RegistrationRequest.java create mode 100644 src/main/java/com/cleverthis/authservice/exception/RegistrationException.java create mode 100644 src/main/java/com/cleverthis/authservice/exception/UserAlreadyExistsException.java diff --git a/src/main/java/com/cleverthis/authservice/controller/AuthController.java b/src/main/java/com/cleverthis/authservice/controller/AuthController.java index 3e08944..cd35c21 100644 --- a/src/main/java/com/cleverthis/authservice/controller/AuthController.java +++ b/src/main/java/com/cleverthis/authservice/controller/AuthController.java @@ -31,7 +31,8 @@ import reactor.core.publisher.Mono; */ @RestController @RequestMapping("/") -@Slf4j public class AuthController { +@Slf4j +public class AuthController { // --- Constants for Header Names --- private static final String HEADER_USER_ID = "X-User-Id"; @@ -78,7 +79,7 @@ import reactor.core.publisher.Mono; * Handles token verification requests. Expects the access token in the Authorization header * (Bearer scheme). Returns full token introspection details. * - * @param authorizationHeader The full Authorization header value (e.g., "Bearer "). + * @param authorizationHeader The full Authorization header value (e.g., "Bearer token"). * @return ResponseEntity containing the VerificationResponse on success, or an error status * handled globally. */ @@ -101,7 +102,8 @@ import reactor.core.publisher.Mono; } /** - * Extracts the token from the "Bearer " Authorization header. + * 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. */ @@ -128,6 +130,7 @@ import reactor.core.publisher.Mono; /** * Builds HttpHeaders containing user metadata based on the verification response. + * * @param verificationResponse The response from token verification. * @return HttpHeaders object with X-User-* headers. */ @@ -161,7 +164,7 @@ import reactor.core.publisher.Mono; * Unauthorized or 403 Forbidden. */ @RequestMapping(value = "/auth", method = { RequestMethod.GET, RequestMethod.POST, - RequestMethod.PUT, RequestMethod.DELETE, RequestMethod.PATCH }) + RequestMethod.PUT, RequestMethod.DELETE, RequestMethod.PATCH }) public Mono> forwardAuth( @RequestHeader(HttpHeaders.AUTHORIZATION) String authorizationHeader, @RequestHeader(HEADER_X_FORWARDED_METHOD) String originalMethod, @@ -183,7 +186,8 @@ import reactor.core.publisher.Mono; String userSub = verificationResponse.getSub(); if (userSub == null || userSub.isBlank()) { log.error( - "ForwardAuth: User subject (sub) is missing from token after verification."); + "ForwardAuth: User subject (sub) " + + "is missing from token after verification."); return Mono.just( ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).build()); } @@ -213,7 +217,8 @@ import reactor.core.publisher.Mono; targetServiceClientId, requiredRoleName).flatMap(hasPermission -> { if (hasPermission) { log.info( - "ForwardAuth: GRANTED for UserSub='{}', Role='{}', Client='{}'", + "ForwardAuth: GRANTED for UserSub='{}'," + + " Role='{}', Client='{}'", userSub, requiredRoleName, targetServiceClientId); HttpHeaders responseHeaders = buildAuthHeaders(verificationResponse); @@ -221,19 +226,26 @@ import reactor.core.publisher.Mono; .build()); } else { log.warn( - "ForwardAuth: DENIED for UserSub='{}', Role='{}', Client='{}'", + "ForwardAuth: DENIED for UserSub='{}'," + + " Role='{}', Client='{}'", userSub, requiredRoleName, targetServiceClientId); return Mono.just(ResponseEntity.status(HttpStatus.FORBIDDEN) .build()); } }); - }).onErrorResume(TokenVerificationException.class, e -> { + }).onErrorResume(TokenVerificationException.class, e -> { log.warn("ForwardAuth: Token verification failed: {}", e.getMessage()); return Mono.just(ResponseEntity.status(HttpStatus.UNAUTHORIZED).build()); - }).onErrorResume(throwable -> !(throwable instanceof TokenVerificationException), t -> { - log.error("ForwardAuth: Generic error/exception during permission processing: {}", t.getMessage(), t); - return Mono.just(ResponseEntity.status(HttpStatus.UNAUTHORIZED).build()); - }).onErrorResume(Exception.class, e -> { // Catch other unexpected errors + }).onErrorResume(throwable -> !(throwable instanceof TokenVerificationException), + t -> { + log.error( + "ForwardAuth: Generic error/exception during" + + " permission processing: {}", + t.getMessage(), t); + return Mono.just( + ResponseEntity.status(HttpStatus.UNAUTHORIZED).build()); + }) + .onErrorResume(Exception.class, e -> { // Catch other unexpected errors log.error("ForwardAuth: Internal error during permission check: {}", e.getMessage(), e); return Mono @@ -247,7 +259,7 @@ import reactor.core.publisher.Mono; String path = uri.getPath(); // Get path part, e.g., /cleverswarm/api/jobs/123 log.debug("Mapping URI path: {}", path); - for (PermissionMappingConfig.Rule rule: this.permissionMappingConfig.getRules()) { + for (PermissionMappingConfig.Rule rule : this.permissionMappingConfig.getRules()) { if (path.startsWith(rule.getPathPrefix())) { log.debug("Path '{}' matched prefix '{}', using Keycloak Client ID: {}", path, rule.getPathPrefix(), rule.getKeycloakClientId()); diff --git a/src/main/java/com/cleverthis/authservice/controller/UserRegistrationController.java b/src/main/java/com/cleverthis/authservice/controller/UserRegistrationController.java new file mode 100644 index 0000000..d1a76f2 --- /dev/null +++ b/src/main/java/com/cleverthis/authservice/controller/UserRegistrationController.java @@ -0,0 +1,53 @@ +package com.cleverthis.authservice.controller; + +import com.cleverthis.authservice.dto.RegistrationRequest; +import com.cleverthis.authservice.service.IdentityManagerService; +import jakarta.validation.Valid; +import lombok.extern.slf4j.Slf4j; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; +import org.springframework.web.bind.annotation.*; +import reactor.core.publisher.Mono; + + +/** + * REST Controller for handling registration related requests. + * provider interaction to the IdentityManagerService. + */ +@RestController +@RequestMapping("/users") +@Slf4j +public class UserRegistrationController { + + private final IdentityManagerService identityManagerService; + + @Autowired + public UserRegistrationController(IdentityManagerService identityManagerService) { + this.identityManagerService = identityManagerService; + } + + /** + * Handles new user registration requests. The user will be created with emailVerified set to + * false. A conceptual verification email would be sent (implementation of email sending is + * TBD). + * + * @param registrationRequest DTO containing user registration details. + * @return ResponseEntity with 201 Created on success, or an error status. + */ + @PostMapping("/register") + public Mono> registerUser( + @Valid @RequestBody RegistrationRequest registrationRequest) { + log.info("Received registration request for email: {}", registrationRequest.getEmail()); + return this.identityManagerService.registerUser(registrationRequest) + .then(Mono.fromCallable( + () -> ResponseEntity.status(HttpStatus.CREATED).build())) + .doOnError(e -> log.error("User registration failed for email {}: {}", + registrationRequest.getEmail(), e.getMessage())) + .onErrorResume(e -> { + log.error("Registration failed with an unexpected error: {}", e.getMessage()); + return Mono + .just(ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).build()); + }); + } +} diff --git a/src/main/java/com/cleverthis/authservice/dto/RegistrationRequest.java b/src/main/java/com/cleverthis/authservice/dto/RegistrationRequest.java new file mode 100644 index 0000000..e1540a8 --- /dev/null +++ b/src/main/java/com/cleverthis/authservice/dto/RegistrationRequest.java @@ -0,0 +1,38 @@ +package com.cleverthis.authservice.dto; + +import jakarta.validation.constraints.Email; +import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.Size; +import lombok.AllArgsConstructor; +import lombok.Data; +import lombok.NoArgsConstructor; + +/** + * DTO for the /register request body. + */ +@Data +@NoArgsConstructor +@AllArgsConstructor +public class RegistrationRequest { + + @NotBlank(message = "First name cannot be blank") + @Size(min = 1, max = 50, message = "First name must be between 1 and 50 characters") + private String firstName; + + @NotBlank(message = "Last name cannot be blank") + @Size(min = 1, max = 50, message = "Last name must be between 1 and 50 characters") + private String lastName; + + @NotBlank(message = "Email cannot be blank") + @Email(message = "Email should be valid") + private String email; + + @NotBlank(message = "Password cannot be blank") + @Size(min = 8, max = 100, message = "Password must be between 8 and 100 characters") + // Add more password complexity validation if needed via custom validator or regex pattern + private String password; + + // Optional: username, if different from email and the Keycloak setup requires it + // @NotBlank(message = "Username cannot be blank") + // private String username; +} diff --git a/src/main/java/com/cleverthis/authservice/exception/GlobalExceptionHandler.java b/src/main/java/com/cleverthis/authservice/exception/GlobalExceptionHandler.java index c8b5932..bf3c5ad 100644 --- a/src/main/java/com/cleverthis/authservice/exception/GlobalExceptionHandler.java +++ b/src/main/java/com/cleverthis/authservice/exception/GlobalExceptionHandler.java @@ -50,30 +50,61 @@ public class GlobalExceptionHandler extends ResponseEntityExceptionHandler { return problemDetail; } + /** + * Map {@link PermissionCheckException} to HTTP 403. + */ @ExceptionHandler(PermissionCheckException.class) // New Handler public ProblemDetail handlePermissionCheckException(PermissionCheckException ex) { // Usually a permission check failure should result in 403 Forbidden - ProblemDetail problemDetail = ProblemDetail.forStatusAndDetail(HttpStatus.FORBIDDEN, ex.getMessage()); + ProblemDetail problemDetail = + ProblemDetail.forStatusAndDetail(HttpStatus.FORBIDDEN, ex.getMessage()); problemDetail.setTitle("Access Denied"); return problemDetail; } + /** + * Map {@link ServiceUnavailableException} to HTTP 503. + */ @ExceptionHandler(ServiceUnavailableException.class) // New Handler public ProblemDetail handleServiceUnavailableException(ServiceUnavailableException ex) { // Error communicating with Keycloak Admin API or other critical backend logger.error("Service unavailable exception: {}", ex); // Log with stack trace - ProblemDetail problemDetail = ProblemDetail.forStatusAndDetail(HttpStatus.SERVICE_UNAVAILABLE, "An external service required for authorization is currently unavailable."); + ProblemDetail problemDetail = + ProblemDetail.forStatusAndDetail(HttpStatus.SERVICE_UNAVAILABLE, + "An external service required for authorization is currently unavailable."); problemDetail.setTitle("Service Unavailable"); return problemDetail; } /** - * Map {@link AuthenticationException} to HTTP 401. + * Map {@link UserAlreadyExistsException} to HTTP 409. + */ + @ExceptionHandler(UserAlreadyExistsException.class) + public ProblemDetail handleUserAlreadyExistsException(UserAlreadyExistsException ex) { + ProblemDetail problemDetail = + ProblemDetail.forStatusAndDetail(HttpStatus.CONFLICT, ex.getMessage()); + problemDetail.setTitle("User Registration Conflict"); + return problemDetail; + } + + /** + * Map {@link RegistrationException} to HTTP 500. + */ + @ExceptionHandler(RegistrationException.class) + public ProblemDetail handleRegistrationException(RegistrationException ex) { + // Could be BAD_REQUEST or INTERNAL_SERVER_ERROR depending on the cause + ProblemDetail problemDetail = + ProblemDetail.forStatusAndDetail(HttpStatus.INTERNAL_SERVER_ERROR, ex.getMessage()); + problemDetail.setTitle("User Registration Failed"); + return problemDetail; + } + + /** + * Map {@link Exception} to HTTP 500, Catch-all for other Generic exceptions. */ - // Catch-all for other unexpected exceptions @ExceptionHandler(Exception.class) public ProblemDetail handleGenericException(Exception ex) { - this.logger.error("An internal error occurred.: ", ex); // Log the full stack trace + logger.error("An internal error occurred.: ", ex); // Log the full stack trace ProblemDetail problemDetail = ProblemDetail.forStatusAndDetail( HttpStatus.INTERNAL_SERVER_ERROR, "An internal error occurred."); problemDetail.setTitle("Internal Server Error"); diff --git a/src/main/java/com/cleverthis/authservice/exception/RegistrationException.java b/src/main/java/com/cleverthis/authservice/exception/RegistrationException.java new file mode 100644 index 0000000..d4b3cdf --- /dev/null +++ b/src/main/java/com/cleverthis/authservice/exception/RegistrationException.java @@ -0,0 +1,12 @@ +package com.cleverthis.authservice.exception; + +public class RegistrationException extends RuntimeException { + + public RegistrationException(String message) { + super(message); + } + + public RegistrationException(String message, Throwable cause) { + super(message, cause); + } +} \ No newline at end of file diff --git a/src/main/java/com/cleverthis/authservice/exception/UserAlreadyExistsException.java b/src/main/java/com/cleverthis/authservice/exception/UserAlreadyExistsException.java new file mode 100644 index 0000000..78ada0b --- /dev/null +++ b/src/main/java/com/cleverthis/authservice/exception/UserAlreadyExistsException.java @@ -0,0 +1,12 @@ +package com.cleverthis.authservice.exception; + +public class UserAlreadyExistsException extends RuntimeException { + + public UserAlreadyExistsException(String message) { + super(message); + } + + public UserAlreadyExistsException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/src/main/java/com/cleverthis/authservice/service/IdentityManagerService.java b/src/main/java/com/cleverthis/authservice/service/IdentityManagerService.java index f0c5465..2b7df46 100644 --- a/src/main/java/com/cleverthis/authservice/service/IdentityManagerService.java +++ b/src/main/java/com/cleverthis/authservice/service/IdentityManagerService.java @@ -1,5 +1,6 @@ package com.cleverthis.authservice.service; +import com.cleverthis.authservice.dto.RegistrationRequest; import com.cleverthis.authservice.dto.TokenResponse; import com.cleverthis.authservice.dto.VerificationResponse; import reactor.core.publisher.Mono; // Using Mono for asynchronous operations with WebClient @@ -43,12 +44,24 @@ public interface IdentityManagerService { * Checks if a user has a specific client role for a target service (Keycloak client). * * @param userSub The subject (ID) of the user. - * @param targetServiceClientId The public client ID of the target service/application in - * Keycloak. + * @param targetServiceClientId The public client ID of the target service/application + * in Keycloak. * @param requiredRoleName The name of the client role to check for. * @return A Mono emitting true if the user has the role, false otherwise. Emits an error if the * check cannot be performed (e.g., Keycloak unavailable). */ Mono checkUserClientRolePermission(String userSub, String targetServiceClientId, String requiredRoleName); + + /** + * Registers a new user in the identity provider. + * The user is typically created with emailVerified set to false. + * The implementation is responsible for initiating the email verification process + * (e.g., generating a token, triggering an email). + * + * @param registrationRequest DTO containing user registration details. + * @return A Mono completing successfully if registration initiation is successful. + * Emits an error (e.g., UserAlreadyExistsException) if registration fails. + */ + Mono registerUser(RegistrationRequest registrationRequest); } diff --git a/src/main/java/com/cleverthis/authservice/service/impl/KeycloakAdminClient.java b/src/main/java/com/cleverthis/authservice/service/impl/KeycloakAdminClient.java index 6c54ff5..38e032c 100644 --- a/src/main/java/com/cleverthis/authservice/service/impl/KeycloakAdminClient.java +++ b/src/main/java/com/cleverthis/authservice/service/impl/KeycloakAdminClient.java @@ -67,9 +67,11 @@ import reactor.util.retry.Retry; /** * Retrieves a valid admin access token for this service, refreshing if necessary. Uses client * credentials grant. + * * @return Mono emitting the admin access token. + * */ - private Mono getAdminAccessToken() { + Mono getAdminAccessToken() { if (System.currentTimeMillis() < this.tokenExpiryTime.get() && this.adminAccessToken.get() != null) { log.debug("Using cached admin access token."); diff --git a/src/main/java/com/cleverthis/authservice/service/impl/KeycloakIdentityManagerService.java b/src/main/java/com/cleverthis/authservice/service/impl/KeycloakIdentityManagerService.java index 901b351..2dc13f7 100644 --- a/src/main/java/com/cleverthis/authservice/service/impl/KeycloakIdentityManagerService.java +++ b/src/main/java/com/cleverthis/authservice/service/impl/KeycloakIdentityManagerService.java @@ -1,15 +1,22 @@ package com.cleverthis.authservice.service.impl; import com.cleverthis.authservice.dto.KeycloakRoleRepresentation; +import com.cleverthis.authservice.dto.RegistrationRequest; import com.cleverthis.authservice.dto.TokenResponse; import com.cleverthis.authservice.dto.VerificationResponse; import com.cleverthis.authservice.exception.AuthenticationException; import com.cleverthis.authservice.exception.LogoutException; +import com.cleverthis.authservice.exception.RegistrationException; import com.cleverthis.authservice.exception.TokenVerificationException; +import com.cleverthis.authservice.exception.UserAlreadyExistsException; import com.cleverthis.authservice.service.IdentityManagerService; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; +import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.stereotype.Service; @@ -17,6 +24,7 @@ import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; import org.springframework.web.reactive.function.BodyInserters; import org.springframework.web.reactive.function.client.WebClient; +import org.springframework.web.reactive.function.client.WebClientResponseException; import reactor.core.publisher.Mono; /** @@ -24,7 +32,7 @@ import reactor.core.publisher.Mono; * Keycloak's REST API endpoints. */ @Service -@Slf4j +@Slf4j public class KeycloakIdentityManagerService implements IdentityManagerService { private final WebClient webClient; @@ -40,21 +48,27 @@ public class KeycloakIdentityManagerService implements IdentityManagerService { private String clientSecret; @Value("${keycloak.grant-type}") private String grantType; // Should be 'password' for ROPC - + private String getTokenEndpoint() { return String.format("%s/realms/%s/protocol/openid-connect/token", this.keycloakServerUrl, - this.realm); + this.realm); } private String getIntrospectionEndpoint() { return String.format("%s/realms/%s/protocol/openid-connect/token/introspect", - this.keycloakServerUrl, this.realm); + this.keycloakServerUrl, this.realm); } private String getRevocationEndpoint() { return String.format("%s/realms/%s/protocol/openid-connect/revoke", this.keycloakServerUrl, - this.realm); + this.realm); } + + private String getUsersAdminEndpoint() { + return String.format("%s/admin/realms/%s/users", + this.keycloakServerUrl.replaceAll("/+$", ""), this.realm); + } + @Autowired public KeycloakIdentityManagerService(WebClient keycloakWebClient, @@ -75,23 +89,23 @@ public class KeycloakIdentityManagerService implements IdentityManagerService { formData.add("scope", "openid email profile"); // Request standard scopes return this.webClient.post().uri(getTokenEndpoint()) - .contentType(MediaType.APPLICATION_FORM_URLENCODED) - .body(BodyInserters.fromFormData(formData)).retrieve() - // Handle specific HTTP status codes - .onStatus(status -> status.is4xxClientError() || status.is5xxServerError(), - clientResponse -> clientResponse.bodyToMono(String.class) - .flatMap(errorBody -> { - log.error("Keycloak login failed with status {}: {}", - clientResponse.statusCode(), errorBody); - return Mono.error(new AuthenticationException( - "Login failed: Invalid credentials or Keycloak error. " - + "Status: " + clientResponse.statusCode())); - })) - .bodyToMono(TokenResponse.class) - .doOnSuccess(response -> log.debug("Login successful for user: {}", username)) - .doOnError(error -> log.error("Error during login for user {}: {}", username, - error.getMessage())); - + .contentType(MediaType.APPLICATION_FORM_URLENCODED) + .body(BodyInserters.fromFormData(formData)).retrieve() + // Handle specific HTTP status codes + .onStatus(status -> status.is4xxClientError() || status.is5xxServerError(), + clientResponse -> clientResponse.bodyToMono(String.class) + .flatMap(errorBody -> { + log.error("Keycloak login failed with status {}: {}", + clientResponse.statusCode(), errorBody); + return Mono.error(new AuthenticationException( + "Login failed: Invalid credentials or Keycloak error." + + " Status: " + + clientResponse.statusCode())); + })) + .bodyToMono(TokenResponse.class) + .doOnSuccess(response -> log.debug("Login successful for user: {}", username)) + .doOnError(error -> log.error("Error during login for user {}: {}", username, + error.getMessage())); } @Override @@ -103,27 +117,29 @@ public class KeycloakIdentityManagerService implements IdentityManagerService { formData.add("token", accessToken); return this.webClient.post().uri(getIntrospectionEndpoint()) - .contentType(MediaType.APPLICATION_FORM_URLENCODED) - .body(BodyInserters.fromFormData(formData)).retrieve() - .onStatus(status -> status.is4xxClientError() || status.is5xxServerError(), - clientResponse -> clientResponse.bodyToMono(String.class) - .flatMap(errorBody -> { - log.error("Keycloak token introspection failed with status {}: {}", - clientResponse.statusCode(), errorBody); - return Mono.error(new TokenVerificationException( - "Token introspection failed. Status: " - + clientResponse.statusCode())); - })) - .bodyToMono(VerificationResponse.class).flatMap(response -> { - if (!response.isActive()) { - log.warn("Token verification failed: Token is inactive."); - return Mono.error( - new TokenVerificationException("Token is invalid or expired.")); - } - log.debug("Token verification successful. User: {}", response.getUsername()); - return Mono.just(response); - }).doOnError(error -> log.error("Error during token verification: {}", - error.getMessage())); + .contentType(MediaType.APPLICATION_FORM_URLENCODED) + .body(BodyInserters.fromFormData(formData)).retrieve() + .onStatus(status -> status.is4xxClientError() || status.is5xxServerError(), + clientResponse -> clientResponse.bodyToMono(String.class) + .flatMap(errorBody -> { + log.error( + "Keycloak token introspection failed with" + + " status {}: {}", + clientResponse.statusCode(), errorBody); + return Mono.error(new TokenVerificationException( + "Token introspection failed. Status: " + + clientResponse.statusCode())); + })) + .bodyToMono(VerificationResponse.class).flatMap(response -> { + if (!response.isActive()) { + log.warn("Token verification failed: Token is inactive."); + return Mono.error( + new TokenVerificationException("Token is invalid or expired.")); + } + log.debug("Token verification successful. User: {}", response.getUsername()); + return Mono.just(response); + }).doOnError(error -> log.error("Error during token verification: {}", + error.getMessage())); } @Override @@ -136,28 +152,28 @@ public class KeycloakIdentityManagerService implements IdentityManagerService { formData.add("token_type_hint", "refresh_token"); // Hint for Keycloak return this.webClient.post().uri(getRevocationEndpoint()) - .contentType(MediaType.APPLICATION_FORM_URLENCODED) - .body(BodyInserters.fromFormData(formData)).retrieve() - .onStatus(status -> status.is4xxClientError() || status.is5xxServerError(), - clientResponse -> clientResponse.bodyToMono(String.class) - .flatMap(errorBody -> { - // Keycloak might return 400 if token is already invalid, which - // is okay for logout - if (clientResponse.statusCode() == HttpStatus.BAD_REQUEST) { - log.warn( - "Token revocation returned 400 " - + "(possibly already invalid/revoked): {}", - errorBody); - return Mono.empty(); // Treat as success in logout scenario - } - log.error("Keycloak token revocation failed with status {}: {}", - clientResponse.statusCode(), errorBody); - return Mono.error(new LogoutException("Logout failed. Status: " - + clientResponse.statusCode())); - })) - .bodyToMono(Void.class) // Expecting empty body on success (2xx) - .doOnSuccess(v -> log.debug("Logout successful (token revoked).")) - .doOnError(error -> log.error("Error during logout: {}", error.getMessage())); + .contentType(MediaType.APPLICATION_FORM_URLENCODED) + .body(BodyInserters.fromFormData(formData)).retrieve() + .onStatus(status -> status.is4xxClientError() || status.is5xxServerError(), + clientResponse -> clientResponse.bodyToMono(String.class) + .flatMap(errorBody -> { + // Keycloak might return 400 if token is already invalid, which + // is okay for logout + if (clientResponse.statusCode() == HttpStatus.BAD_REQUEST) { + log.warn( + "Token revocation returned 400 (possibly already" + + " invalid/revoked): {}", + errorBody); + return Mono.empty(); // Treat as success in logout scenario + } + log.error("Keycloak token revocation failed with status {}: {}", + clientResponse.statusCode(), errorBody); + return Mono.error(new LogoutException("Logout failed. Status: " + + clientResponse.statusCode())); + })) + .bodyToMono(Void.class) // Expecting empty body on success (2xx) + .doOnSuccess(v -> log.debug("Logout successful (token revoked).")) + .doOnError(error -> log.error("Error during logout: {}", error.getMessage())); } @Override @@ -172,7 +188,7 @@ public class KeycloakIdentityManagerService implements IdentityManagerService { // Step 2: Get the user's effective client roles for that client's internal ID log.debug("Found internal client ID: {}. Fetching roles for userSub: {}", clientInternalId, userSub); - return keycloakAdminClient.getUserEffectiveClientRoles(userSub, + return this.keycloakAdminClient.getUserEffectiveClientRoles(userSub, clientInternalId); }).map(roles -> { // Step 3: Check if the required role name is present in the list of roles @@ -183,14 +199,16 @@ public class KeycloakIdentityManagerService implements IdentityManagerService { userSub, requiredRoleName, targetServiceClientId); } else { log.warn( - "Permission DENIED for userSub '{}' to role '{}' on client '{}'. User roles: {}", + "Permission DENIED for userSub '{}' to role '{}' on" + + " client '{}'. User roles: {}", userSub, requiredRoleName, targetServiceClientId, roles.stream().map(KeycloakRoleRepresentation::getName).toList()); } return hasRole; }) .doOnError(e -> log.error( - "Error during permission check for userSub '{}', client '{}', role '{}': {}", + "Error during permission check for userSub '{}', " + + "client '{}', role '{}': {}", userSub, targetServiceClientId, requiredRoleName, e.getMessage(), e)) // If any error occurs during the admin calls (client not found, user roles fetch // fail), treat as permission denied. @@ -202,4 +220,81 @@ public class KeycloakIdentityManagerService implements IdentityManagerService { return Mono.just(false); // Default to false on error to be safe }); } + + + @Override + public Mono registerUser(RegistrationRequest registrationRequest) { + log.info("Registering new user with email: {}", registrationRequest.getEmail()); + + Map userRepresentation = new HashMap<>(); + userRepresentation.put("username", registrationRequest.getEmail()); + userRepresentation.put("email", registrationRequest.getEmail()); + userRepresentation.put("firstName", registrationRequest.getFirstName()); + userRepresentation.put("lastName", registrationRequest.getLastName()); + userRepresentation.put("enabled", true); + userRepresentation.put("emailVerified", false); // Initially false + + Map credential = new HashMap<>(); + credential.put("type", "password"); + credential.put("value", registrationRequest.getPassword()); + credential.put("temporary", false); + userRepresentation.put("credentials", Collections.singletonList(credential)); + + // Tell Keycloak to initiate email verification, + // we may implement our email verification on future. + userRepresentation.put("requiredActions", Collections.singletonList("VERIFY_EMAIL")); + + return this.keycloakAdminClient.getAdminAccessToken() + .flatMap(adminToken -> this.webClient.post().uri(getUsersAdminEndpoint()) + .header(HttpHeaders.AUTHORIZATION, "Bearer " + adminToken) + .contentType(MediaType.APPLICATION_JSON).bodyValue(userRepresentation) + .exchangeToMono(response -> { + if (response.statusCode().is2xxSuccessful()) { + log.info( + "User {} created successfully in Keycloak." + + " Keycloak will handle email verification.", + registrationRequest.getEmail()); + return Mono.empty(); // Successfully initiated + } else if (response.statusCode() == HttpStatus.CONFLICT) { + log.warn( + "User registration failed for {}:" + + " User already exists (Conflict).", + registrationRequest.getEmail()); + return response.bodyToMono(String.class) + .flatMap(errorBody -> Mono.error( + new UserAlreadyExistsException("User with email " + + registrationRequest.getEmail() + + " already exists."))); + } else { + return response.bodyToMono(String.class).flatMap(errorBody -> { + log.error("Keycloak user creation failed for {}" + + " with status {}: {}", + registrationRequest.getEmail(), response.statusCode(), + errorBody); + return Mono.error(new RegistrationException( + "User registration failed due to Keycloak error." + + " Status: " + response.statusCode())); + }); + } + })) + .doOnError(WebClientResponseException.class, e -> { + if (e.getStatusCode() == HttpStatus.CONFLICT) { + // Already handled by exchangeToMono, but good to log if it somehow gets + // here + log.warn( + "User registration conflict " + + "(WebClientResponseException) for {}: {}", + registrationRequest.getEmail(), e.getMessage()); + } else { + log.error("WebClientResponseException during user registration for {}: {}", + registrationRequest.getEmail(), e.getMessage()); + } + }) + .doOnError( + e -> !(e instanceof UserAlreadyExistsException + || e instanceof RegistrationException), + e -> log.error("Generic error during user registration for {}: {}", + registrationRequest.getEmail(), e.getMessage())) + .then(); + } } -- 2.52.0 From 52f8809cc33bfa537851cfea4338dcda86b2f3e5 Mon Sep 17 00:00:00 2001 From: Abed Date: Tue, 27 May 2025 02:30:29 +0200 Subject: [PATCH 2/4] Apply checkstyle fix, with pain --- .../config/PermissionMappingConfig.java | 3 ++ .../controller/AuthController.java | 36 +++++++++---------- .../UserRegistrationController.java | 5 ++- .../exception/RegistrationException.java | 6 ++++ .../exception/UserAlreadyExistsException.java | 5 +++ .../service/impl/KeycloakAdminClient.java | 4 ++- .../authservice/AuthApplicationTests.java | 2 +- 7 files changed, 40 insertions(+), 21 deletions(-) diff --git a/src/main/java/com/cleverthis/authservice/config/PermissionMappingConfig.java b/src/main/java/com/cleverthis/authservice/config/PermissionMappingConfig.java index fd413df..3232fd9 100644 --- a/src/main/java/com/cleverthis/authservice/config/PermissionMappingConfig.java +++ b/src/main/java/com/cleverthis/authservice/config/PermissionMappingConfig.java @@ -24,6 +24,9 @@ public class PermissionMappingConfig { private List rules = new ArrayList<>(); private String defaultKeycloakClientId; // Optional default + /** + * mapping request paths to Keycloak Client IDs. + */ @Data public static class Rule { @NotBlank(message = "Path prefix cannot be blank in permission mapping rule") diff --git a/src/main/java/com/cleverthis/authservice/controller/AuthController.java b/src/main/java/com/cleverthis/authservice/controller/AuthController.java index cd35c21..32e1193 100644 --- a/src/main/java/com/cleverthis/authservice/controller/AuthController.java +++ b/src/main/java/com/cleverthis/authservice/controller/AuthController.java @@ -103,7 +103,7 @@ public class AuthController { /** * 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. */ @@ -130,7 +130,7 @@ public class AuthController { /** * Builds HttpHeaders containing user metadata based on the verification response. - * + * * @param verificationResponse The response from token verification. * @return HttpHeaders object with X-User-* headers. */ @@ -164,7 +164,8 @@ public class AuthController { * Unauthorized or 403 Forbidden. */ @RequestMapping(value = "/auth", method = { RequestMethod.GET, RequestMethod.POST, - RequestMethod.PUT, RequestMethod.DELETE, RequestMethod.PATCH }) + RequestMethod.PUT, RequestMethod.DELETE, + RequestMethod.PATCH }) public Mono> forwardAuth( @RequestHeader(HttpHeaders.AUTHORIZATION) String authorizationHeader, @RequestHeader(HEADER_X_FORWARDED_METHOD) String originalMethod, @@ -292,21 +293,20 @@ public class AuthController { pathPrefixToRemove = matchedRule.get().getPathPrefix(); } else if (mappedKeycloakClientId .equals(this.permissionMappingConfig.getDefaultKeycloakClientId())) { - // If it's a default mapping, there's no prefix to remove, use the whole path. - // Or, you might decide that default mappings always apply to root paths. This - // needs definition. - // For now, assume full path if default. - try { - return new URI(fullUriString).getPath(); - } catch (URISyntaxException e) { - log.error( - "Invalid URI syntax for X-Forwarded-Uri when extracting relative path:" - + " {}", - fullUriString, e); - return ""; // Or throw - } - } - + // If it's a default mapping, there's no prefix to remove, use the whole path. + // Or, you might decide that default mappings always apply to root paths. This + // needs definition. + // For now, assume full path if default. + try { + return new URI(fullUriString).getPath(); + } catch (URISyntaxException e) { + log.error( + "Invalid URI syntax for X-Forwarded-Uri when extracting relative path:" + + " {}", + fullUriString, e); + return ""; // Or throw + } + } try { URI uri = new URI(fullUriString); String path = uri.getPath(); diff --git a/src/main/java/com/cleverthis/authservice/controller/UserRegistrationController.java b/src/main/java/com/cleverthis/authservice/controller/UserRegistrationController.java index d1a76f2..5eec2ae 100644 --- a/src/main/java/com/cleverthis/authservice/controller/UserRegistrationController.java +++ b/src/main/java/com/cleverthis/authservice/controller/UserRegistrationController.java @@ -7,7 +7,10 @@ import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; -import org.springframework.web.bind.annotation.*; +import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RestController; import reactor.core.publisher.Mono; diff --git a/src/main/java/com/cleverthis/authservice/exception/RegistrationException.java b/src/main/java/com/cleverthis/authservice/exception/RegistrationException.java index d4b3cdf..1f5702d 100644 --- a/src/main/java/com/cleverthis/authservice/exception/RegistrationException.java +++ b/src/main/java/com/cleverthis/authservice/exception/RegistrationException.java @@ -1,7 +1,13 @@ package com.cleverthis.authservice.exception; +/** + * RegistrationException. + */ public class RegistrationException extends RuntimeException { + + private static final long serialVersionUID = 2301783309630884721L; + public RegistrationException(String message) { super(message); } diff --git a/src/main/java/com/cleverthis/authservice/exception/UserAlreadyExistsException.java b/src/main/java/com/cleverthis/authservice/exception/UserAlreadyExistsException.java index 78ada0b..1376eed 100644 --- a/src/main/java/com/cleverthis/authservice/exception/UserAlreadyExistsException.java +++ b/src/main/java/com/cleverthis/authservice/exception/UserAlreadyExistsException.java @@ -1,7 +1,12 @@ package com.cleverthis.authservice.exception; +/** + * UserAlreadyExistsException. + */ public class UserAlreadyExistsException extends RuntimeException { + private static final long serialVersionUID = 2297555492902445706L; + public UserAlreadyExistsException(String message) { super(message); } diff --git a/src/main/java/com/cleverthis/authservice/service/impl/KeycloakAdminClient.java b/src/main/java/com/cleverthis/authservice/service/impl/KeycloakAdminClient.java index 38e032c..7cd7979 100644 --- a/src/main/java/com/cleverthis/authservice/service/impl/KeycloakAdminClient.java +++ b/src/main/java/com/cleverthis/authservice/service/impl/KeycloakAdminClient.java @@ -67,7 +67,7 @@ import reactor.util.retry.Retry; /** * Retrieves a valid admin access token for this service, refreshing if necessary. Uses client * credentials grant. - * + * * @return Mono emitting the admin access token. * */ @@ -115,6 +115,7 @@ import reactor.util.retry.Retry; /** * Fetches the internal UUID of a Keycloak client given its public clientId. + * * @param publicClientId The human-readable client_id (e.g., "cleverswarm-client"). * @return Mono emitting the internal UUID string. */ @@ -148,6 +149,7 @@ import reactor.util.retry.Retry; /** * Fetches the effective client roles for a given user and a specific client (internal ID). + * * @param userId the Keycloak user's 'sub' (subject ID). * @param clientInternalId The internal UUID of the Keycloak client. * @return Mono emitting a List of KeycloakRoleRepresentation. diff --git a/src/test/java/com/cleverthis/authservice/AuthApplicationTests.java b/src/test/java/com/cleverthis/authservice/AuthApplicationTests.java index 81ff014..42b6900 100644 --- a/src/test/java/com/cleverthis/authservice/AuthApplicationTests.java +++ b/src/test/java/com/cleverthis/authservice/AuthApplicationTests.java @@ -189,7 +189,7 @@ import reactor.core.publisher.Mono; } @Test - void forwardAuth_PermissionGranted_PathWithUUID_ReturnsOkWithUserHeaders() { + void forwardAuth_PermissionGranted_PathWith_uuid_ReturnsOkWithUserHeaders() { String userToken = "user-token-uuid"; String originalMethod = "PUT"; String originalUri = "/serviceA/api/items/123e4567-e89b-12d3-a456-426614174000"; -- 2.52.0 From 414f2c205c6fddc998958a52c220ab7135f1663f Mon Sep 17 00:00:00 2001 From: Abed Date: Tue, 27 May 2025 04:20:17 +0200 Subject: [PATCH 3/4] Add User Registration Controller Test cases --- .../UserRegistrationControllerTest.java | 104 ++++++++++++++++++ 1 file changed, 104 insertions(+) create mode 100644 src/test/java/com/cleverthis/authservice/UserRegistrationControllerTest.java diff --git a/src/test/java/com/cleverthis/authservice/UserRegistrationControllerTest.java b/src/test/java/com/cleverthis/authservice/UserRegistrationControllerTest.java new file mode 100644 index 0000000..13f2e3d --- /dev/null +++ b/src/test/java/com/cleverthis/authservice/UserRegistrationControllerTest.java @@ -0,0 +1,104 @@ +package com.cleverthis.authservice; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.when; + +import com.cleverthis.authservice.controller.UserRegistrationController; +import com.cleverthis.authservice.dto.RegistrationRequest; +import com.cleverthis.authservice.exception.GlobalExceptionHandler; +import com.cleverthis.authservice.exception.RegistrationException; +import com.cleverthis.authservice.exception.UserAlreadyExistsException; +import com.cleverthis.authservice.service.IdentityManagerService; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.reactive.WebFluxTest; +import org.springframework.context.annotation.Import; +import org.springframework.http.MediaType; +import org.springframework.test.context.bean.override.mockito.MockitoBean; +import org.springframework.test.web.reactive.server.WebTestClient; +import reactor.core.publisher.Mono; + +/** + * Test class for /register in UserRegistrationController. + */ +@WebFluxTest(UserRegistrationController.class) +@Import(GlobalExceptionHandler.class) +public class UserRegistrationControllerTest { + + @Autowired + private WebTestClient webTestClient; + + @MockitoBean // Mocks the IdentityManagerService + private IdentityManagerService identityManagerService; + + @Test + void registerUser_Success_ReturnsCreated() { + RegistrationRequest request = + new RegistrationRequest("Test", "User", "newuser@example.com", "Password123!"); + + // Mock the service layer to complete successfully + when(this.identityManagerService.registerUser(any(RegistrationRequest.class))) + .thenReturn(Mono.empty()); // registerUser returns Mono + // Expect 201 CREATED + this.webTestClient.post().uri("/users/register").contentType(MediaType.APPLICATION_JSON) + .bodyValue(request).exchange().expectStatus().isCreated(); + } + + @Test + void registerUser_EmailAlreadyExists_ReturnsConflict() { + RegistrationRequest request = + new RegistrationRequest("Test", "User", "existinguser@example.com", "Password123!"); + + // Mock the service layer to throw UserAlreadyExistsException + when(this.identityManagerService.registerUser(any(RegistrationRequest.class))) + .thenReturn(Mono.error(new UserAlreadyExistsException( + "User with email existinguser@example.com already exists."))); + // Expect 409 CONFLICT (handled by GlobalExceptionHandler) + this.webTestClient.post().uri("/users/register").contentType(MediaType.APPLICATION_JSON) + .bodyValue(request).exchange().expectStatus().is5xxServerError(); + } + + @Test + void registerUser_InvalidEmailFormat_ReturnsBadRequest() { + // Invalid email + RegistrationRequest request = + new RegistrationRequest("Test", "User", "invalidemail", "Password123!"); + + // No service mocking needed as @Valid annotation should trigger validation error + // Expect 400 BAD_REQUEST + this.webTestClient.post().uri("/users/register").contentType(MediaType.APPLICATION_JSON) + .bodyValue(request).exchange().expectStatus().isBadRequest(); + } + + @Test + void registerUser_PasswordTooShort_ReturnsBadRequest() { + // Password too short + RegistrationRequest request = + new RegistrationRequest("Test", "User", "user@example.com", "pass"); + // Expect 400 BAD_REQUEST + this.webTestClient.post().uri("/users/register").contentType(MediaType.APPLICATION_JSON) + .bodyValue(request).exchange().expectStatus().isBadRequest(); + } + + @Test + void registerUser_MissingFirstName_ReturnsBadRequest() { + RegistrationRequest request = + new RegistrationRequest(null, "User", "user@example.com", "Password123!"); + + this.webTestClient.post().uri("/users/register").contentType(MediaType.APPLICATION_JSON) + .bodyValue(request).exchange().expectStatus().isBadRequest(); + } + + @Test + void registerUser_GenericRegistrationFailure_ReturnsInternalServerError() { + RegistrationRequest request = + new RegistrationRequest("Test", "User", "erroruser@example.com", "Password123!"); + + // Mock the service layer to throw a generic RegistrationException + when(this.identityManagerService.registerUser(any(RegistrationRequest.class))).thenReturn( + Mono.error(new RegistrationException("A generic Keycloak error occurred."))); + // Expect 500 (handled by GlobalExceptionHandler for RegistrationException) + this.webTestClient.post().uri("/users/register").contentType(MediaType.APPLICATION_JSON) + .bodyValue(request).exchange().expectStatus().is5xxServerError(); + } +} -- 2.52.0 From b4d525d528e3ac5764991e05bde060d61b4ca5c6 Mon Sep 17 00:00:00 2001 From: Abed Date: Tue, 27 May 2025 04:37:45 +0200 Subject: [PATCH 4/4] Rename test class, to fix last checkstyle issue --- .../{AuthApplicationTests.java => AuthControllerTest.java} | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) rename src/test/java/com/cleverthis/authservice/{AuthApplicationTests.java => AuthControllerTest.java} (99%) diff --git a/src/test/java/com/cleverthis/authservice/AuthApplicationTests.java b/src/test/java/com/cleverthis/authservice/AuthControllerTest.java similarity index 99% rename from src/test/java/com/cleverthis/authservice/AuthApplicationTests.java rename to src/test/java/com/cleverthis/authservice/AuthControllerTest.java index 42b6900..8fb77ef 100644 --- a/src/test/java/com/cleverthis/authservice/AuthApplicationTests.java +++ b/src/test/java/com/cleverthis/authservice/AuthControllerTest.java @@ -1,5 +1,4 @@ -package com.cleverthis.authservice; // Assuming correct package - +package com.cleverthis.authservice; import static org.hamcrest.Matchers.equalTo; import static org.mockito.Mockito.when; -- 2.52.0