Backend User Authentication & Registration#133
Merged
driedpampas merged 45 commits intomainfrom Apr 1, 2026
Merged
Conversation
Added dependencies for REST APIs and Swagger. Created a mock endpoint at /api/routing/calculate returning fake route data for frontend integration.
- Extrase clasele DTO (RoutingRequest, RoutingResponse, RoutePoint) în fișiere separate și adăugați getteri/setteri pentru a rezolva problemele de Maintainability din SonarQube. - Adăugat RoutingControllerTest.java pentru a crește Code Coverage-ul conform cerințelor Quality Gate-ului. - Adăugat excepție în application.properties pentru a ignora temporar auto-configurarea bazei de date (DataSource/Hibernate). Baza de date PostGIS va fi configurată și integrată ulterior de BE 1 în task-ul dedicat.
This reverts commit 3d9ef59.
* fix(config): update MongoDB URI property in application configuration * feat(jacoco): add JaCoCo configuration for test coverage reporting
Olivia/telemetry db setup
…ql-setup Add Spring Boot Web dependency, global exception handler, and server port configuration
Add Spring Web, OpenAPI setup and mock RoutingController
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds JWT-based authentication (controller, DTOs, service, repository, entity, security filter/config, JWT util), replaces a PostGIS DB init script with a simple Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthController
participant UserService
participant UserRepository
participant Database
participant AuthenticationManager
participant JwtUtil
rect rgba(0, 150, 200, 0.5)
Note over Client,AuthController: Registration
Client->>AuthController: POST /api/auth/register (email,password,firstName,lastName)
AuthController->>UserService: registerUser(...)
UserService->>UserRepository: existsByEmail(email)
UserRepository->>Database: SELECT
alt exists
UserService-->>AuthController: throw UserAlreadyExistsException
AuthController-->>Client: 409
else new
UserService->>UserService: encode password
UserService->>UserRepository: save(user)
UserRepository->>Database: INSERT
UserService-->>AuthController: success
AuthController-->>Client: 200
end
end
rect rgba(100, 200, 100, 0.5)
Note over Client,AuthController: Login
Client->>AuthController: POST /api/auth/login (email,password)
AuthController->>AuthenticationManager: authenticate(token)
AuthenticationManager->>UserService: loadUserByUsername(email)
UserService->>UserRepository: findByEmail(email)
UserRepository->>Database: SELECT
AuthenticationManager-->>AuthController: Authentication
alt valid
AuthController->>JwtUtil: generateToken(email)
JwtUtil-->>AuthController: token
AuthController-->>Client: 200 (token)
else invalid
AuthController-->>Client: 401
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/test/java/com/p2ps/P2PShoppingApplicationTests.java (1)
8-20:⚠️ Potential issue | 🔴 Critical
@ActiveProfiles("test")does not cover the directmain(...)startup path.Line 8 configures the Spring Test context, but Line 20 starts a separate app instance via
P2PShoppingApplication.main(...)without activating thetestprofile. The new process loads the default configuration (PostgreSQL atlocalhost:5433), causing CI failures due to connection refusal. The test profile must be explicitly passed to the launched process.Proposed fix
`@Test` void shouldStartApplicationMainWithoutThrowing() { - assertDoesNotThrow(() -> P2PShoppingApplication.main(new String[]{"--spring.main.web-application-type=none"})); + assertDoesNotThrow(() -> P2PShoppingApplication.main(new String[]{ + "--spring.main.web-application-type=none", + "--spring.profiles.active=test" + })); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/p2ps/P2PShoppingApplicationTests.java` around lines 8 - 20, The test starts a separate app instance via P2PShoppingApplication.main(...) without the test profile, causing it to load default DB config; modify the test invocation in shouldStartApplicationMainWithoutThrowing to include the test profile flag (e.g. add "--spring.profiles.active=test" to the argument array) so the launched process uses the `@ActiveProfiles`("test") configuration when calling P2PShoppingApplication.main; update the argument list passed to P2PShoppingApplication.main(...) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/test/java/com/p2ps/P2PShoppingApplicationTests.java`:
- Around line 8-20: The test starts a separate app instance via
P2PShoppingApplication.main(...) without the test profile, causing it to load
default DB config; modify the test invocation in
shouldStartApplicationMainWithoutThrowing to include the test profile flag (e.g.
add "--spring.profiles.active=test" to the argument array) so the launched
process uses the `@ActiveProfiles`("test") configuration when calling
P2PShoppingApplication.main; update the argument list passed to
P2PShoppingApplication.main(...) accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3847f2cf-4895-4dd1-887e-80afef7a630d
📒 Files selected for processing (6)
build.gradle.ktssrc/main/java/com/p2ps/auth/security/JwtUtil.javasrc/main/java/com/p2ps/auth/security/dto/LoginRequest.javasrc/main/java/com/p2ps/auth/service/UserService.javasrc/test/java/com/p2ps/P2PShoppingApplicationTests.javasrc/test/java/resources/application-test.properties
✅ Files skipped from review due to trivial changes (3)
- src/test/java/resources/application-test.properties
- src/main/java/com/p2ps/auth/security/dto/LoginRequest.java
- build.gradle.kts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/p2ps/auth/service/UserService.java
- src/main/java/com/p2ps/auth/security/JwtUtil.java
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/test/java/com/p2ps/P2PShoppingApplicationTests.java (1)
7-16: Consolidate test configuration to avoid duplication.Using both
@ActiveProfiles("test")(which loadsapplication-test.properties) and inline@SpringBootTest(properties = {...})creates two sources of truth. Inline properties override profile properties for overlapping keys, making the profile-based values effectively dead code and creating maintenance drift risk.Choose one approach:
- Option A (recommended): Keep
@ActiveProfiles("test")and move all properties toapplication-test.properties, removing the inlinepropertiesarray.- Option B: Remove
@ActiveProfiles("test")and rely solely on inline properties for this specific test.♻️ Option A: Rely on profile-based configuration
-@SpringBootTest(properties = { - "jwt.secret=test-secret-key-care-trebuie-sa-fie-foarte-lunga-32-chars", - "spring.datasource.url=jdbc:h2:mem:testdb;DB_CLOSE_DELAY=-1", - "spring.datasource.driver-class-name=org.h2.Driver", - "spring.datasource.username=sa", - "spring.datasource.password=", - "spring.jpa.database-platform=org.hibernate.dialect.H2Dialect", - "spring.jpa.hibernate.ddl-auto=create-drop" -}) +@SpringBootTest `@ActiveProfiles`("test")Ensure
src/test/resources/application-test.propertiescontains all these properties.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/p2ps/P2PShoppingApplicationTests.java` around lines 7 - 16, The test currently mixes `@SpringBootTest`(properties = { ... }) with `@ActiveProfiles`("test"), causing duplicate/conflicting configuration; pick one approach—recommended: remove the inline properties from the `@SpringBootTest` annotation and move those keys into src/test/resources/application-test.properties so `@ActiveProfiles`("test") (on class P2PShoppingApplicationTests) drives the test config; alternatively remove `@ActiveProfiles`("test") and keep the inline properties if you prefer per-test overrides—update the annotation usage on the P2PShoppingApplicationTests class and consolidate the properties accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/test/java/com/p2ps/P2PShoppingApplicationTests.java`:
- Around line 7-16: The test currently mixes `@SpringBootTest`(properties = { ...
}) with `@ActiveProfiles`("test"), causing duplicate/conflicting configuration;
pick one approach—recommended: remove the inline properties from the
`@SpringBootTest` annotation and move those keys into
src/test/resources/application-test.properties so `@ActiveProfiles`("test") (on
class P2PShoppingApplicationTests) drives the test config; alternatively remove
`@ActiveProfiles`("test") and keep the inline properties if you prefer per-test
overrides—update the annotation usage on the P2PShoppingApplicationTests class
and consolidate the properties accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c2251cb9-d39d-4b24-89cd-c56b40fa0e6b
📒 Files selected for processing (3)
src/main/java/com/p2ps/auth/security/dto/AuthResponse.javasrc/main/resources/application.propertiessrc/test/java/com/p2ps/P2PShoppingApplicationTests.java
✅ Files skipped from review due to trivial changes (1)
- src/main/java/com/p2ps/auth/security/dto/AuthResponse.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/resources/application.properties
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/com/p2ps/auth/security/JwtUtil.java (1)
19-28:⚠️ Potential issue | 🟠 MajorHarden JWT secret initialization (charset + explicit validation).
Line 27 still uses platform-default encoding and no pre-validation for null/short secrets. That can produce environment-dependent keys and fail at runtime with weak/null config.
🔧 Suggested patch
+import java.nio.charset.StandardCharsets; @@ `@PostConstruct` public void init() { - this.secretKey = Keys.hmacShaKeyFor(secretKeyString.getBytes()); + if (secretKeyString == null || secretKeyString.isBlank()) { + throw new IllegalStateException("Missing jwt.secret configuration"); + } + byte[] keyBytes = secretKeyString.getBytes(StandardCharsets.UTF_8); + if (keyBytes.length < 32) { + throw new IllegalStateException("JWT secret must be at least 32 bytes for HS256"); + } + this.secretKey = Keys.hmacShaKeyFor(keyBytes); }#!/bin/bash set -euo pipefail echo "== Verify JJWT dependency and JwtUtil init implementation ==" fd -i 'build.gradle.kts|pom.xml' -x sh -c 'echo "--- {}"; rg -n "jjwt|io\\.jsonwebtoken" "{}" || true' fd -i 'JwtUtil.java' -x sh -c 'echo "--- {}"; nl -ba "{}" | sed -n "1,120p"'Expected result: dependency uses JJWT, and
init()should show UTF-8 conversion plus explicit null/length validation beforeKeys.hmacShaKeyFor(...).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/p2ps/auth/security/JwtUtil.java` around lines 19 - 28, The JwtUtil init() uses platform-default charset and lacks validation; update the init() method to (1) validate that secretKeyString is not null/blank and that secretKeyString.getBytes(StandardCharsets.UTF_8).length meets the minimum required length for JJWT HMAC keys (e.g., >= 32 bytes for HS256) and throw a clear IllegalStateException if invalid, and (2) derive secretKey using secretKeyString.getBytes(StandardCharsets.UTF_8) passed into Keys.hmacShaKeyFor(...). Reference: secretKeyString, init(), secretKey, Keys.hmacShaKeyFor.
🧹 Nitpick comments (1)
src/test/java/com/p2ps/auth/UserServicesTest.java (1)
8-8: Remove unused import.
BeforeEachis imported but never used in this test class.-import org.junit.jupiter.api.BeforeEach;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/p2ps/auth/UserServicesTest.java` at line 8, Remove the unused import of BeforeEach from UserServicesTest.java: delete the line importing org.junit.jupiter.api.BeforeEach and verify there are no references to the BeforeEach symbol (e.g., any `@BeforeEach` annotations or methods named for setup) in the class; if a setup method is needed, replace the import with the correct annotation import or add the setup method, otherwise simply remove the import to resolve the unused import warning.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/test/java/com/p2ps/auth/UserServicesTest.java`:
- Line 22: The test class declared as UserServiceTest does not match the current
filename UserServicesTest.java; rename the file to UserServiceTest.java so the
public class UserServiceTest matches the filename exactly and the test compiles.
---
Duplicate comments:
In `@src/main/java/com/p2ps/auth/security/JwtUtil.java`:
- Around line 19-28: The JwtUtil init() uses platform-default charset and lacks
validation; update the init() method to (1) validate that secretKeyString is not
null/blank and that secretKeyString.getBytes(StandardCharsets.UTF_8).length
meets the minimum required length for JJWT HMAC keys (e.g., >= 32 bytes for
HS256) and throw a clear IllegalStateException if invalid, and (2) derive
secretKey using secretKeyString.getBytes(StandardCharsets.UTF_8) passed into
Keys.hmacShaKeyFor(...). Reference: secretKeyString, init(), secretKey,
Keys.hmacShaKeyFor.
---
Nitpick comments:
In `@src/test/java/com/p2ps/auth/UserServicesTest.java`:
- Line 8: Remove the unused import of BeforeEach from UserServicesTest.java:
delete the line importing org.junit.jupiter.api.BeforeEach and verify there are
no references to the BeforeEach symbol (e.g., any `@BeforeEach` annotations or
methods named for setup) in the class; if a setup method is needed, replace the
import with the correct annotation import or add the setup method, otherwise
simply remove the import to resolve the unused import warning.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3dc34c75-c591-4416-a1eb-afdfc89a3889
📒 Files selected for processing (7)
src/main/java/com/p2ps/auth/controller/AuthController.javasrc/main/java/com/p2ps/auth/dto/RegisterRequest.javasrc/main/java/com/p2ps/auth/model/Users.javasrc/main/java/com/p2ps/auth/security/JwtAuthFilter.javasrc/main/java/com/p2ps/auth/security/JwtUtil.javasrc/main/java/com/p2ps/auth/security/SecurityConfig.javasrc/test/java/com/p2ps/auth/UserServicesTest.java
✅ Files skipped from review due to trivial changes (1)
- src/main/java/com/p2ps/auth/model/Users.java
🚧 Files skipped from review as they are similar to previous changes (4)
- src/main/java/com/p2ps/auth/controller/AuthController.java
- src/main/java/com/p2ps/auth/dto/RegisterRequest.java
- src/main/java/com/p2ps/auth/security/JwtAuthFilter.java
- src/main/java/com/p2ps/auth/security/SecurityConfig.java
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/main/java/com/p2ps/exception/GlobalExceptionHandler.java (1)
57-65: Consider using consistent response type across handlers.This handler returns
ResponseEntity<Map<String, String>>while others returnResponseEntity<ErrorResponse>. For API consistency, consider usingErrorResponsehere as well.♻️ Proposed refactor for consistency
`@ExceptionHandler`({BadCredentialsException.class, UsernameNotFoundException.class}) - public ResponseEntity<Map<String, String>> handleAuthenticationError(Exception ex) { - return ResponseEntity - .status(HttpStatus.UNAUTHORIZED) // Trimitem 401 în loc de 500 - .body(Map.of( - "error", "Unauthorized", - "message", "Invalid email or password" // Mesaj generic, sigur - )); + public ResponseEntity<ErrorResponse> handleAuthenticationError(Exception ex) { + ErrorResponse errorResponse = new ErrorResponse( + "Unauthorized", + "Invalid email or password" + ); + return new ResponseEntity<>(errorResponse, HttpStatus.UNAUTHORIZED); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/p2ps/exception/GlobalExceptionHandler.java` around lines 57 - 65, The handleAuthenticationError method in GlobalExceptionHandler currently returns ResponseEntity<Map<String, String>>; change it to return ResponseEntity<ErrorResponse> and construct/return an ErrorResponse instance (populating the same "error" and "message" values used now) with status HttpStatus.UNAUTHORIZED so the handler matches other exception handlers that use ErrorResponse; update the method signature and body in handleAuthenticationError to create and return the ErrorResponse accordingly.src/main/java/com/p2ps/auth/controller/AuthController.java (1)
45-53: Theauthenticationresult is captured but not used.The
authenticate()call correctly validates credentials, but the returnedAuthenticationobject is assigned to a variable that's never used. You could either remove the variable assignment or use the authenticated principal to generate the token.♻️ Option 1: Remove unused variable
- Authentication authentication = authenticationManager.authenticate( + authenticationManager.authenticate( new UsernamePasswordAuthenticationToken( request.getEmail(), request.getPassword() ) ); - - String token = jwtUtil.generateToken(request.getEmail());♻️ Option 2: Use authenticated principal
Authentication authentication = authenticationManager.authenticate( new UsernamePasswordAuthenticationToken( request.getEmail(), request.getPassword() ) ); - - - String token = jwtUtil.generateToken(request.getEmail()); + String token = jwtUtil.generateToken(authentication.getName());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/p2ps/auth/controller/AuthController.java` around lines 45 - 53, AuthController captures the result of authenticationManager.authenticate(...) into the unused variable authentication; either remove the assignment and call authenticationManager.authenticate(...) for its side-effect only, or use the authenticated principal when generating the JWT (e.g., extract the principal or username from the returned Authentication and pass that into jwtUtil.generateToken instead of using request.getEmail()). Locate the call to authenticationManager.authenticate with UsernamePasswordAuthenticationToken and update the subsequent jwtUtil.generateToken invocation accordingly to use authentication.getPrincipal()/getName() or delete the unused variable.src/test/java/com/p2ps/auth/AuthControllerTest.java (1)
47-48: Test method name suggests 201 Created, but endpoint returns 200 OK.The method is named
register_ShouldReturnCreatedbut the assertion expectsstatus().isOk()(HTTP 200). Either rename the method toregister_ShouldReturnOkor update the endpoint to return201 Createdif that's the intended behavior for resource creation.♻️ Rename to match actual behavior
`@Test` - void register_ShouldReturnCreated() throws Exception { + void register_ShouldReturnOk() throws Exception {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/p2ps/auth/AuthControllerTest.java` around lines 47 - 48, The test method name register_ShouldReturnCreated in AuthControllerTest is misleading because the assertion uses status().isOk(); rename the test to register_ShouldReturnOk (or alternatively change the controller/register endpoint to return 201 Created if you intend creation semantics). Update the method name register_ShouldReturnCreated to register_ShouldReturnOk and adjust any references or test frameworks that rely on the method name; if you choose the endpoint change, modify the AuthController register handler to return ResponseEntity.status(HttpStatus.CREATED) for successful registration and update the test assertion to status().isCreated().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/main/java/com/p2ps/auth/controller/AuthController.java`:
- Around line 45-53: AuthController captures the result of
authenticationManager.authenticate(...) into the unused variable authentication;
either remove the assignment and call authenticationManager.authenticate(...)
for its side-effect only, or use the authenticated principal when generating the
JWT (e.g., extract the principal or username from the returned Authentication
and pass that into jwtUtil.generateToken instead of using request.getEmail()).
Locate the call to authenticationManager.authenticate with
UsernamePasswordAuthenticationToken and update the subsequent
jwtUtil.generateToken invocation accordingly to use
authentication.getPrincipal()/getName() or delete the unused variable.
In `@src/main/java/com/p2ps/exception/GlobalExceptionHandler.java`:
- Around line 57-65: The handleAuthenticationError method in
GlobalExceptionHandler currently returns ResponseEntity<Map<String, String>>;
change it to return ResponseEntity<ErrorResponse> and construct/return an
ErrorResponse instance (populating the same "error" and "message" values used
now) with status HttpStatus.UNAUTHORIZED so the handler matches other exception
handlers that use ErrorResponse; update the method signature and body in
handleAuthenticationError to create and return the ErrorResponse accordingly.
In `@src/test/java/com/p2ps/auth/AuthControllerTest.java`:
- Around line 47-48: The test method name register_ShouldReturnCreated in
AuthControllerTest is misleading because the assertion uses status().isOk();
rename the test to register_ShouldReturnOk (or alternatively change the
controller/register endpoint to return 201 Created if you intend creation
semantics). Update the method name register_ShouldReturnCreated to
register_ShouldReturnOk and adjust any references or test frameworks that rely
on the method name; if you choose the endpoint change, modify the AuthController
register handler to return ResponseEntity.status(HttpStatus.CREATED) for
successful registration and update the test assertion to status().isCreated().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 099b50b7-4a9a-4182-94de-829d6646413c
📒 Files selected for processing (13)
build.gradle.ktssrc/main/java/com/p2ps/auth/controller/AuthController.javasrc/main/java/com/p2ps/auth/dto/RegisterRequest.javasrc/main/java/com/p2ps/auth/model/Users.javasrc/main/java/com/p2ps/auth/security/JwtAuthFilter.javasrc/main/java/com/p2ps/auth/security/JwtUtil.javasrc/main/java/com/p2ps/auth/security/SecurityConfig.javasrc/main/java/com/p2ps/auth/security/dto/LoginRequest.javasrc/main/java/com/p2ps/auth/service/UserService.javasrc/main/java/com/p2ps/exception/GlobalExceptionHandler.javasrc/test/java/com/p2ps/auth/AuthControllerTest.javasrc/test/java/com/p2ps/auth/UserSeriviceTest.javasrc/test/java/resources/application-test.properties
✅ Files skipped from review due to trivial changes (2)
- src/test/java/resources/application-test.properties
- src/main/java/com/p2ps/auth/model/Users.java
🚧 Files skipped from review as they are similar to previous changes (5)
- src/main/java/com/p2ps/auth/dto/RegisterRequest.java
- src/main/java/com/p2ps/auth/security/dto/LoginRequest.java
- src/main/java/com/p2ps/auth/security/JwtAuthFilter.java
- src/main/java/com/p2ps/auth/security/SecurityConfig.java
- build.gradle.kts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
build.gradle.kts (1)
50-51: Align Testcontainers versions with Spring Boot 4.0.4 BOM management.Lines 50-51 pin Testcontainers to
1.19.0, which is significantly outdated. Spring Boot 4.0.4 manages Testcontainers2.0.4. Remove explicit versions to leverage the BOM and avoid version misalignment:Proposed change
- testImplementation("org.testcontainers:testcontainers:1.19.0") - testImplementation("org.testcontainers:postgresql:1.19.0") + testImplementation("org.testcontainers:testcontainers") + testImplementation("org.testcontainers:postgresql")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build.gradle.kts` around lines 50 - 51, Remove the hard-coded Testcontainers versions so the Spring Boot 4.0.4 BOM can manage them: replace the two dependency declarations that currently reference org.testcontainers:testcontainers:1.19.0 and org.testcontainers:postgresql:1.19.0 with versionless coordinates (i.e., keep "org.testcontainers:testcontainers" and "org.testcontainers:postgresql" in the testImplementation entries) so the BOM-provided 2.0.4 version is used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@build.gradle.kts`:
- Around line 38-48: Remove the duplicated dependency declaration for
org.springframework.boot:spring-boot-starter-test: locate both occurrences of
testImplementation("org.springframework.boot:spring-boot-starter-test") and
delete the redundant one so only a single testImplementation for that coordinate
remains (keep
testImplementation("org.springframework.boot:spring-boot-starter-webmvc-test")
as-is since it transitively brings in the needed test artifacts).
---
Nitpick comments:
In `@build.gradle.kts`:
- Around line 50-51: Remove the hard-coded Testcontainers versions so the Spring
Boot 4.0.4 BOM can manage them: replace the two dependency declarations that
currently reference org.testcontainers:testcontainers:1.19.0 and
org.testcontainers:postgresql:1.19.0 with versionless coordinates (i.e., keep
"org.testcontainers:testcontainers" and "org.testcontainers:postgresql" in the
testImplementation entries) so the BOM-provided 2.0.4 version is used.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9d8f7ee3-1219-4837-900d-5281fb114044
📒 Files selected for processing (4)
build.gradle.ktssrc/test/java/com/p2ps/auth/AuthControllerTest.javasrc/test/java/com/p2ps/auth/UserSeriviceTest.javasrc/test/java/resources/application-test.properties
✅ Files skipped from review due to trivial changes (1)
- src/test/java/resources/application-test.properties
🚧 Files skipped from review as they are similar to previous changes (2)
- src/test/java/com/p2ps/auth/AuthControllerTest.java
- src/test/java/com/p2ps/auth/UserSeriviceTest.java
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/test/java/com/p2ps/auth/AuthControllerTest.java (2)
85-99: Strengthen login success test by asserting the token payload.Right now this test only checks status. Add a JSON assertion so payload regressions are caught.
Suggested assertion upgrade
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; @@ mockMvc.perform(post("/api/auth/login") .contentType(MediaType.APPLICATION_JSON) .content(objectMapper.writeValueAsString(request))) - .andExpect(status().isOk()); + .andExpect(status().isOk()) + .andExpect(jsonPath("$.token").value("mocked-jwt-token-123"));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/p2ps/auth/AuthControllerTest.java` around lines 85 - 99, The test login_ShouldReturnOk in AuthControllerTest currently only asserts HTTP 200; enhance it to also assert the response JSON contains the JWT by adding a JSON body assertion on the mockMvc.perform result (e.g., andExpect(content().contentType(MediaType.APPLICATION_JSON)) andExpect(jsonPath("$.token").value("mocked-jwt-token-123"))). Keep the existing stubbing of jwtUtil.generateToken("...") and Authentication mock; update the mockMvc.perform chain to include these jsonPath assertions so the test verifies the token payload is returned.
56-70: Rename this test or assert201 Createdto match intent.The method name says “ShouldReturnCreated” but the assertion expects
200 OK. Aligning the name with actual behavior avoids confusion.Suggested minimal rename
- void register_ShouldReturnCreated() throws Exception { + void register_ShouldReturnOk() throws Exception {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/p2ps/auth/AuthControllerTest.java` around lines 56 - 70, The test method register_ShouldReturnCreated is inconsistent: its name implies it should assert HTTP 201 but the assertion uses status().isOk(); either change the assertion in the mockMvc.perform(...) chain to expect status().isCreated() or rename the test method (register_ShouldReturnOk) to match the current behavior. Locate the method register_ShouldReturnCreated in AuthControllerTest, update the expectation on mockMvc.perform(...).andExpect(...) to status().isCreated() if the controller is meant to return 201, or rename the test method to reflect 200 OK if you prefer to keep the assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/test/java/com/p2ps/auth/JwtAuthFilterTest.java`:
- Around line 47-75: Tests in JwtAuthFilterTest leak SecurityContextHolder state
across tests causing order-dependent failures; add a cleanup hook that invokes
SecurityContextHolder.clearContext() after each test (e.g., an `@AfterEach` method
in the JwtAuthFilterTest class) so doFilterInternal tests always run with a
fresh security context and no leftover Authentication.
---
Nitpick comments:
In `@src/test/java/com/p2ps/auth/AuthControllerTest.java`:
- Around line 85-99: The test login_ShouldReturnOk in AuthControllerTest
currently only asserts HTTP 200; enhance it to also assert the response JSON
contains the JWT by adding a JSON body assertion on the mockMvc.perform result
(e.g., andExpect(content().contentType(MediaType.APPLICATION_JSON))
andExpect(jsonPath("$.token").value("mocked-jwt-token-123"))). Keep the existing
stubbing of jwtUtil.generateToken("...") and Authentication mock; update the
mockMvc.perform chain to include these jsonPath assertions so the test verifies
the token payload is returned.
- Around line 56-70: The test method register_ShouldReturnCreated is
inconsistent: its name implies it should assert HTTP 201 but the assertion uses
status().isOk(); either change the assertion in the mockMvc.perform(...) chain
to expect status().isCreated() or rename the test method
(register_ShouldReturnOk) to match the current behavior. Locate the method
register_ShouldReturnCreated in AuthControllerTest, update the expectation on
mockMvc.perform(...).andExpect(...) to status().isCreated() if the controller is
meant to return 201, or rename the test method to reflect 200 OK if you prefer
to keep the assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a4cff4f7-3136-4d72-ae11-7de8e66bdb2a
📒 Files selected for processing (4)
src/main/java/com/p2ps/auth/security/JwtAuthFilter.javasrc/test/java/com/p2ps/auth/AuthControllerTest.javasrc/test/java/com/p2ps/auth/JwtAuthFilterTest.javasrc/test/java/com/p2ps/auth/JwtUtilTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/p2ps/auth/security/JwtAuthFilter.java
+ chore: misc config changes
driedpampas
force-pushed
the
feature/auth-backend-setup
branch
from
3398338 to
35ff5b4
Compare
|
This was referenced Apr 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implemented a full-stack authentication flow using JWT (JSON Web Tokens) and Spring Security.
How to test:
Summary by CodeRabbit
New Features
Bug Fixes / UX
Tests
Chores