Authentication done backend complete scan the file qodana and i will proceed to frontend #2
Conversation
Add qodana CI checks
…cloakid instead of userid done now moving to frontend
Merge branch 'master' of https://github.com/BEASTSHRIRAM/BeastxFit
Merge pull request #2 from BEASTSHRIRAM/master
Qodana for JVM46 new problems were found
☁️ View the detailed Qodana report Contact Qodana teamContact us at qodana-support@jetbrains.com
|
There was a problem hiding this comment.
Pull request overview
This PR implements Keycloak-based authentication for a fitness application by adding a gateway service that intercepts requests, validates JWT tokens, and synchronizes Keycloak users with the application's user database. The implementation includes modifications to both the gateway and beastxfit services to support user authentication and automatic user registration.
Changes:
- Added a new gateway service with Spring Cloud Gateway, OAuth2 resource server, and Keycloak integration
- Implemented KeycloakUserSyncFilter to automatically create local user records from Keycloak JWT tokens
- Updated beastxfit user service to support keycloakId and modified registration logic to return existing users instead of throwing errors
Reviewed changes
Copilot reviewed 22 out of 25 changed files in this pull request and generated 19 comments.
Show a summary per file
| File | Description |
|---|---|
| gateway/pom.xml | Maven configuration for gateway service with Spring Cloud Gateway, OAuth2, and Eureka dependencies |
| gateway/src/main/java/com/fitness/gateway/GatewayApplication.java | Main Spring Boot application class for the gateway service |
| gateway/src/main/java/com/fitness/gateway/SecurityConfig.java | OAuth2 resource server security configuration with JWT validation |
| gateway/src/main/java/com/fitness/gateway/KeycloakUserSyncFilter.java | WebFilter that extracts user info from JWT tokens and syncs users to the backend |
| gateway/src/main/java/com/fitness/gateway/user/WebClientConfig.java | WebClient configuration for calling the beastxfit user service |
| gateway/src/main/java/com/fitness/gateway/user/UserService.java | Service for validating and registering users via WebClient |
| gateway/src/main/java/com/fitness/gateway/user/RegisterRequest.java | DTO for user registration requests |
| gateway/src/main/java/com/fitness/gateway/user/UserResponse.java | DTO for user responses |
| gateway/src/main/resources/application.yaml | Gateway application configuration with config server import |
| gateway/src/test/java/com/fitness/gateway/GatewayApplicationTests.java | Empty context load test |
| configserver/src/main/resources/config/gateway-service.yml | Gateway service configuration with routing rules and OAuth2 settings |
| beastxfit/src/main/java/com/fitness/beastxfit/model/User.java | Added keycloakId field to User entity |
| beastxfit/src/main/java/com/fitness/beastxfit/dto/RegisterRequest.java | Added KeycloakId field to registration request |
| beastxfit/src/main/java/com/fitness/beastxfit/dto/UserResponse.java | Added keyCloakId field to user response |
| beastxfit/src/main/java/com/fitness/beastxfit/repository/UserRepository.java | Added methods for keycloakId lookups and finding users by email |
| beastxfit/src/main/java/com/fitness/beastxfit/services/UserService.java | Modified registration to return existing users and added keycloakId support |
Files not reviewed (3)
- .idea/compiler.xml: Language not supported
- .idea/encodings.xml: Language not supported
- .idea/misc.xml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -28,6 +38,7 @@ public UserResponse register(RegisterRequest request) { | |||
| UserResponse userResponse = new UserResponse(); | |||
| userResponse.setId(savedUser.getId()); | |||
| userResponse.setPassword(savedUser.getPassword()); | |||
There was a problem hiding this comment.
The UserResponse includes a password field which creates a security vulnerability. Passwords should never be exposed in API responses. Remove the password field from the response object.
| return Mono.error(new RuntimeException("User Not Found: " + userId)); | ||
| else if (e.getStatusCode() == HttpStatus.BAD_REQUEST) | ||
| return Mono.error(new RuntimeException("Invalid Request: " + userId)); | ||
| return Mono.error(new RuntimeException("Unexpected error: " + e.getMessage())); |
There was a problem hiding this comment.
The error messages lack context. "User Not Found: " and "Invalid Request: " followed by the userId don't provide enough information about what operation failed. Consider messages like "User validation failed - user not found: " or "User validation failed - invalid user ID format: " to clarify that this is occurring during validation.
| return Mono.error(new RuntimeException("User Not Found: " + userId)); | |
| else if (e.getStatusCode() == HttpStatus.BAD_REQUEST) | |
| return Mono.error(new RuntimeException("Invalid Request: " + userId)); | |
| return Mono.error(new RuntimeException("Unexpected error: " + e.getMessage())); | |
| return Mono.error(new RuntimeException("User validation failed - user not found: " + userId)); | |
| else if (e.getStatusCode() == HttpStatus.BAD_REQUEST) | |
| return Mono.error(new RuntimeException("User validation failed - invalid user ID format: " + userId)); | |
| return Mono.error(new RuntimeException("User validation failed - unexpected error while calling validation API: " + e.getMessage())); |
| registerRequest.setLastName(claims.getStringClaim("family_name")); | ||
| return registerRequest; | ||
| } catch (Exception e) { | ||
| e.printStackTrace(); |
There was a problem hiding this comment.
The getUserDetails method uses printStackTrace which is not appropriate for production code. Replace it with proper logging using the @slf4j logger, such as log.error("Failed to parse JWT token", e).
| e.printStackTrace(); | |
| log.error("Failed to parse JWT token", e); |
|
|
||
| private RegisterRequest getUserDetails(String token) { | ||
| try { | ||
| String tokenWithoutBearer = token.replace("Bearer ", "").trim(); |
There was a problem hiding this comment.
The getUserDetails method does not validate that the token parameter is not null before calling replace on it. This will cause a NullPointerException if no Authorization header is present. Add a null check before processing the token.
|
|
||
| @NotBlank(message="Passsword is required") | ||
| @Size(min = 6,message = "password must have 6 characters ") | ||
| private String keycloakId; |
There was a problem hiding this comment.
The field naming is inconsistent. In RegisterRequest, the field is named "keycloakId" (line 17), but the getter/setter will be "getKeycloakId/setKeycloakId" due to Lombok. However, in UserResponse the field is "keyCloakId" with a capital 'C' (different casing). This inconsistency can cause confusion and potential serialization issues. Standardize to use "keycloakId" (lowercase 'c') throughout the codebase.
| UserResponse userResponse = new UserResponse(); | ||
| userResponse.setId(savedUser.getId()); | ||
| userResponse.setPassword(savedUser.getPassword()); | ||
| userResponse.setKeyCloakId(savedUser.getKeycloakId()); |
There was a problem hiding this comment.
The setKeyCloakId() method is being called but the UserResponse field is defined as "keyCloakId" with non-standard casing. While Lombok will generate setKeyCloakId() matching the field name, this is inconsistent with the User entity which uses "keycloakId". Standardize the field name to "keycloakId" throughout the codebase.
| userResponse.setKeyCloakId(savedUser.getKeycloakId()); | |
| userResponse.setKeycloakId(savedUser.getKeycloakId()); |
| private String password; | ||
| private String firstName; | ||
| private String lastName; | ||
| //whenever we push any record its value(time bro)is automatically genereation |
There was a problem hiding this comment.
The comment contains a grammatical error. "genereation" should be "generated". The comment should read "whenever we push any record its value (time) is automatically generated".
| //whenever we push any record its value(time bro)is automatically genereation | |
| // whenever we push any record its value (time) is automatically generated |
| RegisterRequest registerRequest = new RegisterRequest(); | ||
| registerRequest.setEmail(claims.getStringClaim("email")); | ||
| registerRequest.setKeycloakId(claims.getStringClaim("sub")); | ||
| registerRequest.setPassword("dummy@123123"); |
There was a problem hiding this comment.
A hardcoded password "dummy@123123" is being set for all users during registration. This is a security vulnerability as it exposes a known password. Instead, if the password is not needed for Keycloak-authenticated users, either omit it entirely or generate a secure random password that cannot be used for authentication.
| @Email(message ="Invalid email format") | ||
| private String email; | ||
|
|
||
| private String KeycloakId; |
There was a problem hiding this comment.
The field name uses inconsistent casing. It's defined as "KeycloakId" with a capital 'K', which violates Java naming conventions where field names should start with a lowercase letter. Change this to "keycloakId" to follow standard Java conventions and maintain consistency with the User entity field naming.
| private String KeycloakId; | |
| private String keycloakId; |
| User existingUser =repository.findByEmail(request.getEmail()); | ||
| UserResponse userResponse = new UserResponse(); | ||
| userResponse.setId(existingUser.getId()); | ||
| userResponse.setPassword(existingUser.getPassword()); |
There was a problem hiding this comment.
The UserResponse includes a password field which creates a security vulnerability. Passwords should never be exposed in API responses. Remove the password field from the response object.
1 participant
No description provided.