From 1a76da12e50f6cbf6ed8c834dd034c3d15afed77 Mon Sep 17 00:00:00 2001 From: Rui Hu Date: Mon, 19 May 2025 16:28:59 +0800 Subject: [PATCH] style(General): add checkstyle and fix all exsiting issues. ISSUES CLOSED: clevermicro/user-management#17 --- checkstyle.xml | 380 ++++++++++++++++++ pom.xml | 219 ++++++---- .../authservice/AuthServiceApplication.java | 2 +- .../authservice/config/WebClientConfig.java | 29 +- .../authservice/dto/TokenResponse.java | 4 +- .../exception/GlobalExceptionHandler.java | 25 +- .../impl/KeycloakIdentityManagerService.java | 128 +++--- 7 files changed, 618 insertions(+), 169 deletions(-) create mode 100644 checkstyle.xml diff --git a/checkstyle.xml b/checkstyle.xml new file mode 100644 index 0000000..bb5132d --- /dev/null +++ b/checkstyle.xml @@ -0,0 +1,380 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/pom.xml b/pom.xml index 064cd5c..211786a 100644 --- a/pom.xml +++ b/pom.xml @@ -1,88 +1,139 @@ - - 4.0.0 - - org.springframework.boot - spring-boot-starter-parent - 3.4.5 - - - com.cleverthis - auth-service - 0.0.1-SNAPSHOT - auth-service - User management project for CleverThis - - - - - - - - - - - - - - - 21 - - - - org.springframework.boot - spring-boot-starter-validation - - - org.springframework.boot - spring-boot-starter-webflux - - - org.projectlombok - lombok - true - provided - - - org.springframework.boot - spring-boot-starter-test - test - - - io.projectreactor - reactor-test - test - - + + 4.0.0 + + org.springframework.boot + spring-boot-starter-parent + 3.4.5 + + + com.cleverthis + auth-service + 0.0.1-SNAPSHOT + auth-service + User management project for CleverThis + + + + + + + + + + + + + + + 21 + com.cleverthis.authservice.AuthServiceApplication + + + + org.springframework.boot + spring-boot-starter-validation + + + org.springframework.boot + spring-boot-starter-webflux + + + org.projectlombok + lombok + true + provided + + + org.springframework.boot + spring-boot-starter-test + test + + + io.projectreactor + reactor-test + test + + - - - - org.apache.maven.plugins - maven-compiler-plugin - - - - org.projectlombok - lombok - - - - - - org.springframework.boot - spring-boot-maven-plugin - - - - org.projectlombok - lombok - - - - - - + + + + org.apache.maven.plugins + maven-compiler-plugin + + + + org.projectlombok + lombok + + + + + + org.springframework.boot + spring-boot-maven-plugin + + + + org.projectlombok + lombok + + + + + + org.jacoco + jacoco-maven-plugin + 0.8.13 + + + + prepare-agent + + + + report + prepare-package + + report + + + + + + org.apache.maven.plugins + maven-checkstyle-plugin + 3.6.0 + + + com.puppycrawl.tools + checkstyle + 10.23.1 + + + + checkstyle.xml + UTF-8 + true + true + warning + true + false + + + + validate + validate + + check + + + + + + diff --git a/src/main/java/com/cleverthis/authservice/AuthServiceApplication.java b/src/main/java/com/cleverthis/authservice/AuthServiceApplication.java index a7cb2c5..cc78bea 100644 --- a/src/main/java/com/cleverthis/authservice/AuthServiceApplication.java +++ b/src/main/java/com/cleverthis/authservice/AuthServiceApplication.java @@ -4,7 +4,7 @@ import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.SpringBootApplication; /** - * Auth-Service starter + * Auth-Service starter. */ @SpringBootApplication public class AuthServiceApplication { diff --git a/src/main/java/com/cleverthis/authservice/config/WebClientConfig.java b/src/main/java/com/cleverthis/authservice/config/WebClientConfig.java index 5a7cfc9..dd65d7d 100644 --- a/src/main/java/com/cleverthis/authservice/config/WebClientConfig.java +++ b/src/main/java/com/cleverthis/authservice/config/WebClientConfig.java @@ -1,16 +1,15 @@ package com.cleverthis.authservice.config; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; -import org.springframework.web.reactive.function.client.WebClient; import io.netty.channel.ChannelOption; import io.netty.handler.timeout.ReadTimeoutHandler; import io.netty.handler.timeout.WriteTimeoutHandler; -import org.springframework.http.client.reactive.ReactorClientHttpConnector; -import reactor.netty.http.client.HttpClient; - import java.time.Duration; import java.util.concurrent.TimeUnit; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.http.client.reactive.ReactorClientHttpConnector; +import org.springframework.web.reactive.function.client.WebClient; +import reactor.netty.http.client.HttpClient; /** * Configuration class for creating and configuring the WebClient bean @@ -24,19 +23,23 @@ public class WebClientConfig { private static final int READ_TIMEOUT = 10000; // 10 seconds private static final int WRITE_TIMEOUT = 10000; // 10 seconds + /** + * Provide a {@link WebClient} for interacting with keycloak. + * */ @Bean public WebClient keycloakWebClient() { // Configure HttpClient with timeouts HttpClient httpClient = HttpClient.create() - .option(ChannelOption.CONNECT_TIMEOUT_MILLIS, CONNECT_TIMEOUT) - .responseTimeout(Duration.ofMillis(READ_TIMEOUT)) // Max time to wait for response - .doOnConnected(conn -> - conn.addHandlerLast(new ReadTimeoutHandler(READ_TIMEOUT, TimeUnit.MILLISECONDS)) - .addHandlerLast(new WriteTimeoutHandler(WRITE_TIMEOUT, TimeUnit.MILLISECONDS))); + .option(ChannelOption.CONNECT_TIMEOUT_MILLIS, CONNECT_TIMEOUT) + .responseTimeout(Duration.ofMillis(READ_TIMEOUT)) // Max time to wait for response + .doOnConnected(conn -> + conn.addHandlerLast(new ReadTimeoutHandler(READ_TIMEOUT, TimeUnit.MILLISECONDS)) + .addHandlerLast( + new WriteTimeoutHandler(WRITE_TIMEOUT, TimeUnit.MILLISECONDS))); // Build WebClient with the configured HttpClient return WebClient.builder() - .clientConnector(new ReactorClientHttpConnector(httpClient)) - .build(); + .clientConnector(new ReactorClientHttpConnector(httpClient)) + .build(); } } diff --git a/src/main/java/com/cleverthis/authservice/dto/TokenResponse.java b/src/main/java/com/cleverthis/authservice/dto/TokenResponse.java index 502643a..603b4b5 100644 --- a/src/main/java/com/cleverthis/authservice/dto/TokenResponse.java +++ b/src/main/java/com/cleverthis/authservice/dto/TokenResponse.java @@ -1,7 +1,6 @@ package com.cleverthis.authservice.dto; import com.fasterxml.jackson.annotation.JsonProperty; - import lombok.AllArgsConstructor; import lombok.Data; import lombok.NoArgsConstructor; @@ -13,7 +12,8 @@ import lombok.NoArgsConstructor; @Data @NoArgsConstructor -@AllArgsConstructor public class TokenResponse { +@AllArgsConstructor +public class TokenResponse { @JsonProperty("access_token") private String accessToken; diff --git a/src/main/java/com/cleverthis/authservice/exception/GlobalExceptionHandler.java b/src/main/java/com/cleverthis/authservice/exception/GlobalExceptionHandler.java index b2e313a..75bc3c9 100644 --- a/src/main/java/com/cleverthis/authservice/exception/GlobalExceptionHandler.java +++ b/src/main/java/com/cleverthis/authservice/exception/GlobalExceptionHandler.java @@ -11,42 +11,55 @@ import org.springframework.web.reactive.result.method.annotation.ResponseEntityE * Problem Details (RFC 7807). */ -@RestControllerAdvice public class GlobalExceptionHandler extends ResponseEntityExceptionHandler { +@RestControllerAdvice +public class GlobalExceptionHandler extends ResponseEntityExceptionHandler { + /** + * Map {@link AuthenticationException} to HTTP 401. + */ @ExceptionHandler(AuthenticationException.class) public ProblemDetail handleAuthenticationException(AuthenticationException ex) { ProblemDetail problemDetail = - ProblemDetail.forStatusAndDetail(HttpStatus.UNAUTHORIZED, ex.getMessage()); + ProblemDetail.forStatusAndDetail(HttpStatus.UNAUTHORIZED, ex.getMessage()); problemDetail.setTitle("Authentication Failed"); // Add more details if needed // problemDetail.setProperty("details", "Check username/password or Keycloak status."); return problemDetail; } + /** + * Map {@link TokenVerificationException} to HTTP 401. + */ @ExceptionHandler(TokenVerificationException.class) public ProblemDetail handleTokenVerificationException(TokenVerificationException ex) { ProblemDetail problemDetail = - ProblemDetail.forStatusAndDetail(HttpStatus.UNAUTHORIZED, ex.getMessage()); + ProblemDetail.forStatusAndDetail(HttpStatus.UNAUTHORIZED, ex.getMessage()); problemDetail.setTitle("Token Verification Failed"); return problemDetail; } + /** + * Map {@link LogoutException} to HTTP 400 or 500. + */ @ExceptionHandler(LogoutException.class) public ProblemDetail handleLogoutException(LogoutException ex) { // Depending on the cause, might return 400 or 500 ProblemDetail problemDetail = - ProblemDetail.forStatusAndDetail(HttpStatus.INTERNAL_SERVER_ERROR, ex.getMessage()); + ProblemDetail.forStatusAndDetail(HttpStatus.INTERNAL_SERVER_ERROR, ex.getMessage()); problemDetail.setTitle("Logout Failed"); return problemDetail; } + /** + * Map {@link AuthenticationException} to HTTP 401. + */ // Catch-all for other unexpected exceptions @ExceptionHandler(Exception.class) public ProblemDetail handleGenericException(Exception ex) { - logger.error("An internal error occurred.: ", ex); // Log the full stack trace + this.logger.error("An internal error occurred.: ", ex); // Log the full stack trace ProblemDetail problemDetail = ProblemDetail.forStatusAndDetail( - HttpStatus.INTERNAL_SERVER_ERROR, "An internal error occurred."); + HttpStatus.INTERNAL_SERVER_ERROR, "An internal error occurred."); problemDetail.setTitle("Internal Server Error"); return problemDetail; } 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 a5f5ed6..2ab6b66 100644 --- a/src/main/java/com/cleverthis/authservice/service/impl/KeycloakIdentityManagerService.java +++ b/src/main/java/com/cleverthis/authservice/service/impl/KeycloakIdentityManagerService.java @@ -23,12 +23,12 @@ import reactor.core.publisher.Mono; * Keycloak's REST API endpoints. */ @Service -@Slf4j +@Slf4j public class KeycloakIdentityManagerService implements IdentityManagerService { private final WebClient webClient; - + @Value("${keycloak.server-url}") private String keycloakServerUrl; @Value("${keycloak.realm}") @@ -40,20 +40,20 @@ public class KeycloakIdentityManagerService implements IdentityManagerService { @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", - 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); } @Autowired @@ -73,22 +73,22 @@ 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 @@ -100,27 +100,27 @@ 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 @@ -133,25 +133,27 @@ 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())); } }