Refactor michelle#114
Conversation
📝 WalkthroughWalkthroughAligned JWT dependency versions, versioned BookController endpoints to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🎉 Thank you @yjtpj4zf99-art for your contribution! Your pull request has been submitted successfully, and a maintainer will review it soon. We’re excited to have you on board! Remember to star the repo 🌟 to help us grow, and follow @ajaynegi45 to stay in the loop and increase the visibility of your contributions!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/main/java/com/libraryman_api/book/BookController.java (2)
96-107: Delete response change looks fine; consider not echoing the raw ID.Returning
ResponseEntity<String>with a confirmation message is a reasonable improvement overvoid(previously Spring returned 200 with empty body anyway). Two small thoughts:
- Echoing
idin the response body is harmless here (it's the caller-supplied path variable), but if you ever want a structured response, a small DTO ({"message": "...", "id": ...}) is easier for clients to parse than a plain string.- Since
MemberController#deleteMemberstill returnsvoid, the API is now inconsistent across resources — worth aligning in a follow-up.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/libraryman_api/book/BookController.java` around lines 96 - 107, The deleteBook method in BookController currently returns a plain string that echoes the raw path id; change it to return a small structured DTO (e.g., DeleteResponse with fields message and optionally id) instead of ResponseEntity<String>, and use bookService.deleteBook(id) as before but wrap the confirmation in ResponseEntity<DeleteResponse>. Update the method signature (deleteBook) and response creation to build and return the DTO, and consider aligning MemberController#deleteMember to the same DTO shape in a follow-up for API consistency.
38-125: Inconsistent leading slashes & redundant path segments.Two nits in the sub-mappings:
- Leading slash is inconsistent:
/get-all-books,/add-bookhave one, whileget-book-by-id/{id}(L63),update-book/{id}(L90),delete-book/{id}(L102),book/search(L119) don't. Spring tolerates both, but consistency aids readability and grep-ability.- With the new base
/api/v1/books, the per-method verbs (get-all-books,add-book,book/search, etc.) duplicate the resource noun and fight REST conventions. Idiomatic mapping would beGET /api/v1/books,GET /api/v1/books/{id},POST /api/v1/books,PUT /api/v1/books/{id},DELETE /api/v1/books/{id},GET /api/v1/books/search. Since this PR is already breaking the URL contract, it's a good moment to clean this up in one go rather than breaking clients a second time later.Proposed cleaner REST mapping
- `@GetMapping`("/get-all-books") + `@GetMapping` public Page<BookDto> getAllBooks(...) { ... } - `@GetMapping`("get-book-by-id/{id}") + `@GetMapping`("/{id}") public ResponseEntity<BookDto> getBookById(`@PathVariable` int id) { ... } - `@PostMapping`("/add-book") + `@PostMapping` public BookDto addBook(`@Valid` `@RequestBody` BookDto bookDto) { ... } - `@PutMapping`("update-book/{id}") + `@PutMapping`("/{id}") public BookDto updateBook(...) { ... } - `@DeleteMapping`("delete-book/{id}") + `@DeleteMapping`("/{id}") public ResponseEntity<String> deleteBook(`@PathVariable` int id) { ... } - `@GetMapping`("book/search") + `@GetMapping`("/search") public ResponseEntity<Page<Book>> searchBook(...) { ... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/libraryman_api/book/BookController.java` around lines 38 - 125, The controller mixes leading-slash styles and non-RESTful per-method paths; update the method mappings in BookController so they are consistent and RESTful relative to the base "/api/v1/books": change getAllBooks to `@GetMapping`("/") or `@GetMapping`("") (GET /api/v1/books), getBookById to `@GetMapping`("/{id}"), addBook to `@PostMapping`("") (POST /api/v1/books), updateBook to `@PutMapping`("/{id}"), deleteBook to `@DeleteMapping`("/{id}"), and searchBook to `@GetMapping`("/search"); ensure every mapping uses a leading slash consistently (or none) across methods and adjust any client-facing docs/tests accordingly while leaving method names (getAllBooks, getBookById, addBook, updateBook, deleteBook, searchBook) and service calls unchanged.
🤖 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/main/java/com/libraryman_api/book/BookController.java`:
- Around line 22-23: Your BookController class changed its base path to
"/api/v1/books" but security rules in WebConfiguration still permit the old
"/api/..." paths, causing new endpoints to be blocked; update WebConfiguration
to replace any permitted patterns like "/api/get-all-books/**" and
"/api/book/search**" with "/api/v1/books/**" (or add explicit permits for the
new pattern) so requests to BookController (class BookController,
`@RequestMapping`("/api/v1/books")) are allowed, update any MockMvc tests in
BookControllerTest to use the new "/api/v1/books" URLs, and audit
MemberController and API docs/tests/Postman collections to either migrate to the
v1 path or document/version the difference for consistency.
---
Nitpick comments:
In `@src/main/java/com/libraryman_api/book/BookController.java`:
- Around line 96-107: The deleteBook method in BookController currently returns
a plain string that echoes the raw path id; change it to return a small
structured DTO (e.g., DeleteResponse with fields message and optionally id)
instead of ResponseEntity<String>, and use bookService.deleteBook(id) as before
but wrap the confirmation in ResponseEntity<DeleteResponse>. Update the method
signature (deleteBook) and response creation to build and return the DTO, and
consider aligning MemberController#deleteMember to the same DTO shape in a
follow-up for API consistency.
- Around line 38-125: The controller mixes leading-slash styles and non-RESTful
per-method paths; update the method mappings in BookController so they are
consistent and RESTful relative to the base "/api/v1/books": change getAllBooks
to `@GetMapping`("/") or `@GetMapping`("") (GET /api/v1/books), getBookById to
`@GetMapping`("/{id}"), addBook to `@PostMapping`("") (POST /api/v1/books),
updateBook to `@PutMapping`("/{id}"), deleteBook to `@DeleteMapping`("/{id}"), and
searchBook to `@GetMapping`("/search"); ensure every mapping uses a leading slash
consistently (or none) across methods and adjust any client-facing docs/tests
accordingly while leaving method names (getAllBooks, getBookById, addBook,
updateBook, deleteBook, searchBook) and service calls unchanged.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: addae607-9e9d-464c-8bd8-33408fad372f
📒 Files selected for processing (2)
pom.xmlsrc/main/java/com/libraryman_api/book/BookController.java
| @RestController | ||
| @RequestMapping("/api") | ||
| @RequestMapping("/api/v1/books") //Versioning API path -> provide more context |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find references to old BookController paths anywhere in the repo.
rg -nP '/api/(get-all-books|get-book-by-id|add-book|update-book|delete-book|book/search)' -C1
# Find MockMvc or test usages of book endpoints.
rg -nP --type=java -C2 '(perform|get|post|put|delete)\s*\(\s*"[^"]*(book|api)[^"]*"'
# Check MemberController base path for cross-controller consistency.
rg -nP --type=java -C1 '@RequestMapping\('Repository: ajaynegi45/LibraryMan-API
Length of output: 2183
🏁 Script executed:
# Check what methods exist in BookController now
rg -A 30 '@RestController' src/main/java/com/libraryman_api/book/BookController.java | head -50
# Verify WebConfiguration is actually using those old paths
cat src/main/java/com/libraryman_api/security/config/WebConfiguration.java | grep -A 2 -B 2 '/api/get-all-books\|/api/book/search'
# Check if there are any other references to the old /api paths for books
rg -i '/api/(add-book|get-book|update-book|delete-book)' --type=javaRepository: ajaynegi45/LibraryMan-API
Length of output: 2023
🏁 Script executed:
# Check if there are test files for BookController
find . -name "*BookControllerTest*" -o -name "*BookTest*" | grep -i test
# Check the README or API documentation for endpoint references
rg -i 'api/.*book' README.md OpenAPI* docs* 2>/dev/null || echo "No docs found matching pattern"
# See the full WebConfiguration to understand the impact
cat src/main/java/com/libraryman_api/security/config/WebConfiguration.javaRepository: ajaynegi45/LibraryMan-API
Length of output: 4313
🏁 Script executed:
# Check if WebConfiguration has been updated to permit the new /api/v1/books paths
rg '/api/v1' src/main/java/com/libraryman_api/security/config/WebConfiguration.java
# List all endpoints in BookController to understand the full scope of the breaking change
rg -B 1 '@(GetMapping|PostMapping|PutMapping|DeleteMapping|PatchMapping)' src/main/java/com/libraryman_api/book/BookController.java | head -40Repository: ajaynegi45/LibraryMan-API
Length of output: 340
🏁 Script executed:
# Confirm there are no permission rules for /api/v1/books in WebConfiguration
rg '/api/v1/books' src/main/java/com/libraryman_api/security/config/WebConfiguration.java
# Check if there are any test files and their current endpoints
find . -path "*/test/*" -name "*BookController*" -type f
find . -path "*/test/*" -name "*Test.java" | xargs grep -l "book" 2>/dev/null | head -5Repository: ajaynegi45/LibraryMan-API
Length of output: 348
🏁 Script executed:
# Check if BookServiceTest or any test actually tests the endpoints
cat src/test/java/com/libraryman_api/book/BookServiceTest.java | head -30
# Search for any MockMvc tests that test book endpoints
rg -i 'mockmvc|perform.*book' --type=java src/test/
# Check if there's a general integration test or e2e test
find . -path "*/test/*" -name "*IntegrationTest*" -o -path "*/test/*" -name "*E2E*"Repository: ajaynegi45/LibraryMan-API
Length of output: 1082
🏁 Script executed:
# Check the other endpoints and see what methods they reference
rg -A 1 '@(GetMapping|PostMapping|PutMapping|DeleteMapping)' src/main/java/com/libraryman_api/book/BookController.java | grep -E '@|value|path'
# Verify what the actual endpoint paths will be when fully qualified (base path + method mapping)
echo "BookController base: /api/v1/books"
rg '@(GetMapping|PostMapping|PutMapping|DeleteMapping)\(' src/main/java/com/libraryman_api/book/BookController.javaRepository: ajaynegi45/LibraryMan-API
Length of output: 1040
Security configuration out of sync with new BookController paths — endpoints will be inaccessible.
The base path change from /api to /api/v1/books breaks existing clients, but there's a critical accompanying issue: WebConfiguration.java (lines 43-44) still permits the old /api/get-all-books/** and /api/book/search** paths, while the new /api/v1/books/* paths have no matching security rules. This means:
- Old endpoints (
/api/get-all-books, etc.) return 404 (no longer exist). - New endpoints (
/api/v1/books/get-all-books, etc.) likely fail with 403 Forbidden (no security rule permits them), unless they accidentally fall under.anyRequest().authenticated()in a way that breaks the intended behavior.
Required fixes:
- Update
WebConfiguration.javato permit/api/v1/books/**instead of/api/get-all-books/**and/api/book/search**. - Update MockMvc tests (if
BookControllerTestexists) to use the new/api/v1/bookspaths. - Update frontend, API docs (README/OpenAPI), and Postman collections to reflect
/api/v1/books. - Align
MemberController(still on/api,src/main/java/com/libraryman_api/member/MemberController.java:22) for API consistency, or document why BookController alone uses versioning. - Consider a deprecation window if external clients are consuming this API.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/libraryman_api/book/BookController.java` around lines 22 -
23, Your BookController class changed its base path to "/api/v1/books" but
security rules in WebConfiguration still permit the old "/api/..." paths,
causing new endpoints to be blocked; update WebConfiguration to replace any
permitted patterns like "/api/get-all-books/**" and "/api/book/search**" with
"/api/v1/books/**" (or add explicit permits for the new pattern) so requests to
BookController (class BookController, `@RequestMapping`("/api/v1/books")) are
allowed, update any MockMvc tests in BookControllerTest to use the new
"/api/v1/books" URLs, and audit MemberController and API docs/tests/Postman
collections to either migrate to the v1 path or document/version the difference
for consistency.
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker-compose.yml`:
- Around line 24-26: Replace use of MySQL root credentials by creating and
referencing a dedicated, least-privilege DB user: set MYSQL_USER and
MYSQL_PASSWORD on the db service to create a non-root account scoped to the
libraryman schema, update SPRING_DATASOURCE_USERNAME and
SPRING_DATASOURCE_PASSWORD to reference those values (use ${MYSQL_USER} and
${MYSQL_PASSWORD} from an .env file rather than literals), and ensure the DB
initialization creates/grants that user only the needed privileges for the
libraryman database so the application does not connect as root.
- Around line 5-9: The docker-compose service currently hardcodes
MYSQL_ROOT_PASSWORD and exposes port 3306 to the host and lacks persistent
storage; change to use a least-privileged DB user by replacing
MYSQL_ROOT_PASSWORD/root with MYSQL_USER and MYSQL_PASSWORD (read values from an
.env file and add that .env to .gitignore), remove the host port mapping
("3306:3306") so the DB is not published (use expose: ["3306"] or omit ports
entirely) and add a named volume mounted at /var/lib/mysql for persistence
(declare a top-level volume such as db-data). Ensure references to MYSQL_USER,
MYSQL_PASSWORD, ports/expose, and the volume name (db-data) are updated in the
service definition.
- Around line 32-37: Remove the dummy OAuth2 credential environment variables
(SPRING_SECURITY_OAUTH2_CLIENT_REGISTRATION_GOOGLE_CLIENT_ID and
SPRING_SECURITY_OAUTH2_CLIENT_REGISTRATION_GOOGLE_CLIENT_SECRET) and stop
relying on non-standard flags SPRING_SECURITY_OAUTH2_CLIENT_ENABLED /
SPRING_SECURITY_OAUTH2_RESOURCESERVER_ENABLED; instead add
SPRING_AUTOCONFIGURE_EXCLUDE with the value
org.springframework.boot.autoconfigure.security.oauth2.client.servlet.OAuth2ClientAutoConfiguration
to the compose config to fully disable OAuth2 client auto-configuration, or
alternatively implement a Spring profile that excludes oauth2Login() from your
SecurityFilterChain for local/staging environments.
In `@Dockerfile`:
- Line 2: The Dockerfile uses "FROM eclipse-temurin:21-jdk-alpine AS build"
which mismatches the project's <java.version> (17) in pom.xml; align them by
either updating the pom.xml <java.version> to 21 or changing both Dockerfile
images (build and runtime) to eclipse-temurin:17-... so the container JDK/JRE
matches the declared target Java version; locate the image references in the
Dockerfile (the FROM lines) and the <java.version> property in pom.xml and make
the corresponding one-line change to keep build/runtime and project target
consistent.
- Around line 14-18: The runtime Dockerfile currently runs as root and copies
all jars with a broad glob; update the runtime stage to create and switch to an
unprivileged user (e.g., add a non-root user/group, chown the app.jar and add a
USER directive) and narrow the COPY from the build stage by pinning the artifact
name or using a specific pattern instead of /app/target/*.jar (replace COPY
--from=build /app/target/*.jar app.jar with a copy that targets the final
artifact name or a more specific glob), ensuring the jar ownership is set for
the new user before switching to it.
- Around line 5-7: The Dockerfile uses the Maven wrapper (./mvnw and
.mvn/wrapper/maven-wrapper.properties) but the maven-wrapper.jar is not present
and mvnw may lack the executable bit, so the RUN ./mvnw ... steps will fail; fix
by either committing .mvn/wrapper/maven-wrapper.jar (generate it locally with
mvn -N wrapper:wrapper) and ensure mvnw is executable (git update-index --add
--chmod=+x mvnw or set chmod +x in the Dockerfile before calling it), or change
the build stage to a Maven base image (e.g., maven:3.9-eclipse-temurin-21) and
call mvn dependency:go-offline / mvn clean package directly instead of ./mvnw.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0edf7b37-2eb5-4d54-ae3c-3993c29a22ca
📒 Files selected for processing (2)
Dockerfiledocker-compose.yml
| environment: | ||
| MYSQL_DATABASE: libraryman | ||
| MYSQL_ROOT_PASSWORD: root | ||
| ports: | ||
| - "3306:3306" |
There was a problem hiding this comment.
Hardcoded root credentials and DB port exposed on host.
MYSQL_ROOT_PASSWORD: rootand usingrootas the application user is insecure even for staging — app should use a dedicated, least-privileged user. PreferMYSQL_USER/MYSQL_PASSWORDand sourcing secrets via an.envfile (and.gitignoreit).- Publishing
3306:3306exposes MySQL on the host interface. Since only theapiservice needs it, drop the host mapping and rely on the internal Docker network (expose: ["3306"]or omit entirely). - No volume is declared, so DB data is lost on
docker compose down. Add a named volume mounted at/var/lib/mysql.
🔧 Suggested adjustment
db:
image: mysql:8.0
container_name: libraryman-db
environment:
MYSQL_DATABASE: libraryman
- MYSQL_ROOT_PASSWORD: root
- ports:
- - "3306:3306"
+ MYSQL_ROOT_PASSWORD: ${MYSQL_ROOT_PASSWORD}
+ MYSQL_USER: ${MYSQL_USER}
+ MYSQL_PASSWORD: ${MYSQL_PASSWORD}
+ expose:
+ - "3306"
+ volumes:
+ - db-data:/var/lib/mysqland add at the bottom:
volumes:
db-data:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docker-compose.yml` around lines 5 - 9, The docker-compose service currently
hardcodes MYSQL_ROOT_PASSWORD and exposes port 3306 to the host and lacks
persistent storage; change to use a least-privileged DB user by replacing
MYSQL_ROOT_PASSWORD/root with MYSQL_USER and MYSQL_PASSWORD (read values from an
.env file and add that .env to .gitignore), remove the host port mapping
("3306:3306") so the DB is not published (use expose: ["3306"] or omit ports
entirely) and add a named volume mounted at /var/lib/mysql for persistence
(declare a top-level volume such as db-data). Ensure references to MYSQL_USER,
MYSQL_PASSWORD, ports/expose, and the volume name (db-data) are updated in the
service definition.
| - SPRING_DATASOURCE_URL=jdbc:mysql://db:3306/libraryman | ||
| - SPRING_DATASOURCE_USERNAME=root | ||
| - SPRING_DATASOURCE_PASSWORD=root |
There was a problem hiding this comment.
App should not connect as MySQL root.
SPRING_DATASOURCE_USERNAME=root/PASSWORD=root grants the application full administrative privileges on the DB server — a clear least-privilege violation. Create a dedicated user (via MYSQL_USER/MYSQL_PASSWORD on the db service) scoped to the libraryman schema and reference those here, ideally via ${...} from an .env file rather than inline literals.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docker-compose.yml` around lines 24 - 26, Replace use of MySQL root
credentials by creating and referencing a dedicated, least-privilege DB user:
set MYSQL_USER and MYSQL_PASSWORD on the db service to create a non-root account
scoped to the libraryman schema, update SPRING_DATASOURCE_USERNAME and
SPRING_DATASOURCE_PASSWORD to reference those values (use ${MYSQL_USER} and
${MYSQL_PASSWORD} from an .env file rather than literals), and ensure the DB
initialization creates/grants that user only the needed privileges for the
libraryman database so the application does not connect as root.
| # DISABLE OAUTH2 FEATURES COMPLETELY | ||
| - SPRING_SECURITY_OAUTH2_CLIENT_ENABLED=false | ||
| - SPRING_SECURITY_OAUTH2_RESOURCESERVER_ENABLED=false | ||
| # PROVIDE A DUMMY REGISTRATION JUST IN CASE THE BEAN SCANNER STILL LOOKS | ||
| - SPRING_SECURITY_OAUTH2_CLIENT_REGISTRATION_GOOGLE_CLIENT_ID=dummy | ||
| - SPRING_SECURITY_OAUTH2_CLIENT_REGISTRATION_GOOGLE_CLIENT_SECRET=dummy No newline at end of file |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Is there a Spring Boot property spring.security.oauth2.client.enabled that disables the OAuth2 client auto-configuration, or is SPRING_AUTOCONFIGURE_EXCLUDE the correct mechanism?
💡 Result:
No, there is no Spring Boot property spring.security.oauth2.client.enabled that disables the OAuth2 client auto-configuration. The correct mechanism is to use spring.autoconfigure.exclude (or the exclude attribute on @SpringBootApplication) to exclude the OAuth2ClientAutoConfiguration class. The OAuth2 client auto-configuration class is org.springframework.boot.autoconfigure.security.oauth2.client.servlet.OAuth2ClientAutoConfiguration, which imports the registration repository and web security configurations when spring-security-oauth2-client is on the classpath and it's a servlet web application. To disable it via properties: spring.autoconfigure.exclude=org.springframework.boot.autoconfigure.security.oauth2.client.servlet.OAuth2ClientAutoConfiguration Official Spring Boot documentation on disabling auto-configuration confirms this approach, and the GitHub issue #6474 in Spring Security explicitly states that Spring Boot provides this capability for OAuth2ClientAutoConfiguration rather than a dedicated disable property. No sources mention or document a spring.security.oauth2.client.enabled property; all OAuth2 client properties are under spring.security.oauth2.client.registration.* and provider.* for configuring registrations, not enabling/disabling the auto-config itself.
Citations:
- 1: Disable OAuth2WebSecurityConfiguration through property. spring-projects/spring-security#6474
- 2: https://docs.spring.vmware.com/spring-boot/reference/using/auto-configuration.html
- 3: https://docs.spring.vmware.com/spring-boot/3.3/reference/using/auto-configuration.html
- 4: https://docs.spring.vmware.com/spring-boot/docs/2.7.25.1/api/org/springframework/boot/autoconfigure/security/oauth2/client/servlet/OAuth2ClientAutoConfiguration.html
- 5: https://docs.spring.io/spring-boot/docs/2.7.x/api/org/springframework/boot/autoconfigure/security/oauth2/client/servlet/OAuth2ClientAutoConfiguration.html
- 6: https://github.com/spring-projects/spring-boot/blob/v2.7.18/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/oauth2/client/servlet/OAuth2ClientAutoConfiguration.java
- 7: https://github.com/spring-projects/spring-boot/blob/v3.3.13/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/oauth2/client/servlet/OAuth2ClientAutoConfiguration.java
- 8: https://docs.enterprise.spring.io/spring-boot/3.3/api/java/org/springframework/boot/autoconfigure/security/oauth2/client/servlet/OAuth2ClientAutoConfiguration.html
- 9: https://docs.spring.io/spring-boot/3.3/api/java/org/springframework/boot/autoconfigure/security/oauth2/client/servlet/OAuth2ClientAutoConfiguration.html
- 10: https://github.com/spring-projects/spring-boot/blob/v3.4.13/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/oauth2/client/servlet/OAuth2ClientAutoConfiguration.java
🏁 Script executed:
# Search for WebConfiguration file
fd -i WebConfiguration
# Search for oauth2Login references
rg "oauth2Login" -i --type javaRepository: ajaynegi45/LibraryMan-API
Length of output: 252
🏁 Script executed:
# Check docker-compose.yml full content
cat -n docker-compose.yml | head -60
# Search for SPRING_AUTOCONFIGURE_EXCLUDE usage
rg "SPRING_AUTOCONFIGURE_EXCLUDE|spring.autoconfigure.exclude" -iRepository: ajaynegi45/LibraryMan-API
Length of output: 1563
Remove dummy OAuth2 credentials; use SPRING_AUTOCONFIGURE_EXCLUDE to properly disable OAuth2 auto-configuration.
The SPRING_SECURITY_OAUTH2_CLIENT_ENABLED and SPRING_SECURITY_OAUTH2_RESOURCESERVER_ENABLED properties are not standard Spring Boot properties and will not prevent OAuth2 auto-configuration. With dummy credentials present, the application may start, but OAuth2 login attempts will fail at runtime. Instead, exclude the auto-configuration classes entirely by setting:
SPRING_AUTOCONFIGURE_EXCLUDE=org.springframework.boot.autoconfigure.security.oauth2.client.servlet.OAuth2ClientAutoConfiguration
Alternatively, define a Spring profile that excludes oauth2Login() from the security chain for local/staging environments.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docker-compose.yml` around lines 32 - 37, Remove the dummy OAuth2 credential
environment variables
(SPRING_SECURITY_OAUTH2_CLIENT_REGISTRATION_GOOGLE_CLIENT_ID and
SPRING_SECURITY_OAUTH2_CLIENT_REGISTRATION_GOOGLE_CLIENT_SECRET) and stop
relying on non-standard flags SPRING_SECURITY_OAUTH2_CLIENT_ENABLED /
SPRING_SECURITY_OAUTH2_RESOURCESERVER_ENABLED; instead add
SPRING_AUTOCONFIGURE_EXCLUDE with the value
org.springframework.boot.autoconfigure.security.oauth2.client.servlet.OAuth2ClientAutoConfiguration
to the compose config to fully disable OAuth2 client auto-configuration, or
alternatively implement a Spring profile that excludes oauth2Login() from your
SecurityFilterChain for local/staging environments.
| @@ -0,0 +1,18 @@ | |||
|
|
|||
| FROM eclipse-temurin:21-jdk-alpine AS build | |||
There was a problem hiding this comment.
JDK version mismatch with pom.xml (java.version=17).
The build and runtime images use JDK/JRE 21 while pom.xml targets Java 17. The build will succeed (21 ≥ 17), but running the resulting JAR on a JRE that differs from the declared target can hide compatibility issues and drift between local/CI and container runtimes. Either bump <java.version> to 21 or switch the images to eclipse-temurin:17-... to keep them aligned.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Dockerfile` at line 2, The Dockerfile uses "FROM
eclipse-temurin:21-jdk-alpine AS build" which mismatches the project's
<java.version> (17) in pom.xml; align them by either updating the pom.xml
<java.version> to 21 or changing both Dockerfile images (build and runtime) to
eclipse-temurin:17-... so the container JDK/JRE matches the declared target Java
version; locate the image references in the Dockerfile (the FROM lines) and the
<java.version> property in pom.xml and make the corresponding one-line change to
keep build/runtime and project target consistent.
| COPY .mvn/ .mvn | ||
| COPY mvnw pom.xml ./ | ||
| RUN ./mvnw dependency:go-offline |
There was a problem hiding this comment.
Maven wrapper will fail: maven-wrapper.jar missing and mvnw likely not executable.
Per the cross-file context, .mvn/wrapper/ contains only maven-wrapper.properties — maven-wrapper.jar is absent, so ./mvnw dependency:go-offline (line 7) and ./mvnw clean package (line 11) will fail inside the container. Additionally, mvnw needs the executable bit; on hosts where it isn't set in git, the RUN ./mvnw ... step fails with "permission denied".
🛠️ Suggested fix
COPY .mvn/ .mvn
COPY mvnw pom.xml ./
+RUN chmod +x mvnw
RUN ./mvnw dependency:go-offlineAlso commit .mvn/wrapper/maven-wrapper.jar (run mvn -N wrapper:wrapper locally) or switch the build stage to a maven:3.9-eclipse-temurin-21 image and invoke mvn directly instead of the wrapper.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Dockerfile` around lines 5 - 7, The Dockerfile uses the Maven wrapper (./mvnw
and .mvn/wrapper/maven-wrapper.properties) but the maven-wrapper.jar is not
present and mvnw may lack the executable bit, so the RUN ./mvnw ... steps will
fail; fix by either committing .mvn/wrapper/maven-wrapper.jar (generate it
locally with mvn -N wrapper:wrapper) and ensure mvnw is executable (git
update-index --add --chmod=+x mvnw or set chmod +x in the Dockerfile before
calling it), or change the build stage to a Maven base image (e.g.,
maven:3.9-eclipse-temurin-21) and call mvn dependency:go-offline / mvn clean
package directly instead of ./mvnw.
| FROM eclipse-temurin:21-jre-alpine | ||
| WORKDIR /app | ||
| COPY --from=build /app/target/*.jar app.jar | ||
| EXPOSE 8080 | ||
| ENTRYPOINT ["java", "-jar", "app.jar"] |
There was a problem hiding this comment.
Run as a non-root user and narrow the JAR copy glob.
Trivy DS-0002: no USER directive, so the container runs as root. Add an unprivileged user in the runtime stage. Also, COPY --from=build /app/target/*.jar app.jar silently misbehaves if Maven produces more than one jar (e.g., *-sources.jar, *-javadoc.jar) — pin the finalName or use a more specific pattern.
🔒 Suggested fix
FROM eclipse-temurin:21-jre-alpine
WORKDIR /app
-COPY --from=build /app/target/*.jar app.jar
+RUN addgroup -S app && adduser -S app -G app
+COPY --from=build /app/target/*.jar app.jar
+RUN chown app:app app.jar
+USER app
EXPOSE 8080
ENTRYPOINT ["java", "-jar", "app.jar"]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| FROM eclipse-temurin:21-jre-alpine | |
| WORKDIR /app | |
| COPY --from=build /app/target/*.jar app.jar | |
| EXPOSE 8080 | |
| ENTRYPOINT ["java", "-jar", "app.jar"] | |
| FROM eclipse-temurin:21-jre-alpine | |
| WORKDIR /app | |
| RUN addgroup -S app && adduser -S app -G app | |
| COPY --from=build /app/target/*.jar app.jar | |
| RUN chown app:app app.jar | |
| USER app | |
| EXPOSE 8080 | |
| ENTRYPOINT ["java", "-jar", "app.jar"] |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Dockerfile` around lines 14 - 18, The runtime Dockerfile currently runs as
root and copies all jars with a broad glob; update the runtime stage to create
and switch to an unprivileged user (e.g., add a non-root user/group, chown the
app.jar and add a USER directive) and narrow the COPY from the build stage by
pinning the artifact name or using a specific pattern instead of
/app/target/*.jar (replace COPY --from=build /app/target/*.jar app.jar with a
copy that targets the final artifact name or a more specific glob), ensuring the
jar ownership is set for the new user before switching to it.
1 participant
Summary by CodeRabbit
Release Notes
New Features
Chores