feature/security-config - #13
Merged
Merged
Conversation
…or WebAuthn check
…add a new version of migration for this
Contributor
📝 WalkthroughWalkthroughAdds a CustomAuthenticationSuccessHandler that checks stored WebAuthn credentials after login to conditionally redirect users, updates SecurityConfig to wire the handler and change authorization/UserDetailsService behavior, renames/remaps user and credential JPA entities, adds DB migrations, a new WebAuthn check template, and minor controller edits. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as User (Browser)
participant AuthManager as Spring Auth System
participant SuccessHandler as CustomAuthentication<br/>SuccessHandler
participant UserService as UserService
participant CredRepo as UserCredentialRepository
participant HTTP as HTTP Response
Browser->>AuthManager: POST /login (credentials)
AuthManager->>SuccessHandler: onAuthenticationSuccess(authentication)
SuccessHandler->>UserService: findByEmail(username)
alt user found
UserService-->>SuccessHandler: userEntity
SuccessHandler->>CredRepo: findByUserId(userEntity.id)
alt credentials exist
CredRepo-->>SuccessHandler: credential records
SuccessHandler->>HTTP: sendRedirect("/webauthn-check")
else no credentials
SuccessHandler->>HTTP: sendRedirect("/dashboard")
end
else user not found
SuccessHandler->>HTTP: sendRedirect("/dashboard")
end
HTTP-->>Browser: 302 Redirect
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
src/main/resources/templates/webauthn-check.html (1)
17-21: Empty event listener serves no purpose.The
window.addEventListener('load', () => {})callback is empty and should either be removed or populated with initialization logic.♻️ Remove empty listener
-<script> - - window.addEventListener('load', () => { - }); -</script>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/templates/webauthn-check.html` around lines 17 - 21, The empty load handler window.addEventListener('load', () => {}) should be removed or replaced with actual initialization code; locate the occurrence of window.addEventListener('load', ...) in the template (webauthn-check.html) and either delete that empty listener block or populate its arrow function with the necessary startup logic (e.g., calling an init function like initWebAuthn or executing DOM/setup steps) so there are no no-op event listeners left in the file.
🤖 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/backendlab/team4you/config/CustomAuthenticationSuccessHandler.java`:
- Around line 35-42: In CustomAuthenticationSuccessHandler, the current logic
performs getRedirectStrategy().sendRedirect(..., "/webauthn-check") then falls
through to also sendRedirect(..., "/dashboard"), causing double redirects;
modify the control flow in the onAuthenticationSuccess method (or wherever this
snippet lives) so that after sending the "/webauthn-check" redirect execution
returns immediately (or use an else/early-return) to avoid the second
sendRedirect to "/dashboard" when credentials are present; ensure the method
exits after the first redirect so only one redirect is sent.
- Around line 35-41: In CustomAuthenticationSuccessHandler the check uses
credentials != null but userCredentialRepository.findByUserId(...) returns a
List that is never null; change the logic to check if (!credentials.isEmpty())
and redirect to "/webauthn-check" only when there are existing credentials,
otherwise redirect users with an empty list to the registration flow (e.g.
"/webauthn-register") before falling back to the default dashboard redirect;
update the branch that currently uses
getRedirectStrategy().sendRedirect(request, response, "/webauthn-check") to
follow this new conditional using the credentials List returned by
userCredentialRepository.findByUserId.
In `@src/main/java/backendlab/team4you/config/SecurityConfig.java`:
- Around line 65-77: The current userDetailsService method creates a UserDetails
for ANY username with a hardcoded password by calling encoder.encode("123456")
on every request, producing a critical auth bypass and performance issue;
replace this with a real lookup-backed UserDetailsService that injects your
persistence service (e.g., UserService) and returns the stored password hash
(throwing UsernameNotFoundException when not found) instead of encoding a
constant password, and if you need a dev-only fallback keep it in a separate
`@Profile`("dev") bean (e.g., devUserDetailsService) with a clear warning so the
insecure implementation never runs in production.
In `@src/main/java/backendlab/team4you/controller/RegistrationController.java`:
- Line 79: The fullName attribute is built by concatenating user.getFirstName()
and user.getLastName() without a separator, producing "JohnDoe"; update the
construction in RegistrationController (where model.addAttribute("fullName", ...
) is set) to insert a space between first and last name (e.g., combine
user.getFirstName(), a literal " " and user.getLastName()) so the rendered
fullName becomes "John Doe".
In `@src/main/java/backendlab/team4you/user/UserEntity.java`:
- Around line 56-64: Call the correct repository method findByEmail(String
email) instead of the non-existent findByUsername(); update the two call
sites—SignupController (where users.findByUsername(req.username) is used) and
CustomAuthenticationSuccessHandler (where
userEntityRepository.findByUsername(username) is used)—to invoke
users.findByEmail(...) / userEntityRepository.findByEmail(...) passing the email
value (req.username or username as appropriate) so the query matches the
UserRepository's findByEmail signature and the UserEntity email field.
- Around line 41-45: The constructor UserEntity(Bytes id, String email, String
firstName) is encoding the id with id.toString() while setId(...) uses
id.toBase64UrlString() and getId() decodes with Bytes.fromBase64(), causing
mismatched round-trips; update the constructor to encode the incoming Bytes id
using the same Base64URL method (id != null ? id.toBase64UrlString() : null) and
preserve the existing null handling so setId(), getId(), and the constructor use
a consistent Base64URL representation.
---
Nitpick comments:
In `@src/main/resources/templates/webauthn-check.html`:
- Around line 17-21: The empty load handler window.addEventListener('load', ()
=> {}) should be removed or replaced with actual initialization code; locate the
occurrence of window.addEventListener('load', ...) in the template
(webauthn-check.html) and either delete that empty listener block or populate
its arrow function with the necessary startup logic (e.g., calling an init
function like initWebAuthn or executing DOM/setup steps) so there are no no-op
event listeners left in the file.
🪄 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: f097e8dd-5b21-44f5-a605-997335af231a
📒 Files selected for processing (10)
src/main/java/backendlab/team4you/config/CustomAuthenticationSuccessHandler.javasrc/main/java/backendlab/team4you/config/SecurityConfig.javasrc/main/java/backendlab/team4you/controller/ProfileController.javasrc/main/java/backendlab/team4you/controller/RegistrationController.javasrc/main/java/backendlab/team4you/controller/SignupController.javasrc/main/java/backendlab/team4you/user/UserEntity.javasrc/main/java/backendlab/team4you/webauthn/LoginController.javasrc/main/java/backendlab/team4you/webauthn/WebAuthnCredential.javasrc/main/resources/db/migration/V5__rename_app_user_to_user_entities.sqlsrc/main/resources/templates/webauthn-check.html
💤 Files with no reviewable changes (2)
- src/main/java/backendlab/team4you/webauthn/LoginController.java
- src/main/java/backendlab/team4you/controller/ProfileController.java
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/main/java/backendlab/team4you/config/SecurityConfig.java (2)
35-36: Remove commented-out code.Line 35 contains a commented-out authorization rule. If no longer needed, remove it to keep the configuration clean.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/backendlab/team4you/config/SecurityConfig.java` around lines 35 - 36, Remove the stale commented-out authorization rule in SecurityConfig.java by deleting the commented line containing .requestMatchers("/profile", "/logout").authenticated() so the security configuration only contains active matchers (e.g., .requestMatchers("/webauthn-check").authenticated()); ensure no other commented authorization lines remain to keep the configuration clean.
15-18: Remove duplicate import.
Useris imported twice (lines 7 and 18).♻️ Proposed fix
import backendlab.team4you.user.UserEntity; import backendlab.team4you.user.UserService; import org.springframework.security.core.userdetails.UsernameNotFoundException; -import org.springframework.security.core.userdetails.User;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/backendlab/team4you/config/SecurityConfig.java` around lines 15 - 18, Remove the duplicate import of org.springframework.security.core.userdetails.User in SecurityConfig.java so the class imports list only one User; keep the existing imports for UserEntity, UserService, and UsernameNotFoundException unchanged and ensure SecurityConfig still compiles and references the single User import consistently (e.g., in any loadUserByUsername or similar methods).src/main/java/backendlab/team4you/controller/SignupController.java (1)
65-67: Consider updating the error message to reference email.Since
req.usernameappears to hold an email address (used withfindByEmail), the error message "Username already exists" may confuse users. Consider changing to "Email already exists" for clarity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/backendlab/team4you/controller/SignupController.java` around lines 65 - 67, The error message is misleading: in SignupController where you check userService.findByEmail(req.username) you should change the thrown ResponseStatusException message from "Username already exists" to "Email already exists" so it accurately reflects that req.username is an email; update the message string in that conditional (the code referencing userService.findByEmail and req.username) to "Email already exists".src/main/java/backendlab/team4you/user/UserEntity.java (1)
61-64: Display name may contain leading/trailing spaces.When
firstNameorlastNameis null, the result includes unnecessary whitespace (e.g.," Smith","John ", or just" "). Consider trimming or conditionally adding the separator.♻️ Proposed fix
`@Override` public String getDisplayName() { - return (this.firstName != null ? this.firstName : "") + " " + (this.lastName != null ? this.lastName : ""); + String first = this.firstName != null ? this.firstName : ""; + String last = this.lastName != null ? this.lastName : ""; + return (first + " " + last).trim(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/backendlab/team4you/user/UserEntity.java` around lines 61 - 64, The getDisplayName method in UserEntity can produce leading/trailing spaces when firstName or lastName is null; update getDisplayName to build the display name by conditionally joining non-null/non-empty firstName and lastName (or using String.join/collecting non-empty parts) and then trim the final result so names like " Smith", "John " or " " are avoided; ensure you reference and modify the getDisplayName method in class UserEntity and preserve null-safety.
🤖 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/backendlab/team4you/config/SecurityConfig.java`:
- Around line 68-84: The userDetailsService bean currently accepts an unused
BCryptPasswordEncoder parameter and assigns roles via a hardcoded email check;
remove the unused parameter from the UserDetailsService method signature and
instead fetch the user's role(s) from persistence by adding a role field (or a
roles/permissions relation) to UserEntity and persisting roles for users; then
update the lambda in userDetailsService (the returned UserDetails provider) to
read the role(s) from user.getRole() or user.getRoles() and map those to the
Spring Security User builder (e.g., .roles(...) or .authorities(...)) rather
than comparing username to "admin@team4you.com". Ensure UserService.findByEmail
continues to return UserEntity with the new role data and adapt any DB
migrations/repositories to support the new field/relation.
---
Nitpick comments:
In `@src/main/java/backendlab/team4you/config/SecurityConfig.java`:
- Around line 35-36: Remove the stale commented-out authorization rule in
SecurityConfig.java by deleting the commented line containing
.requestMatchers("/profile", "/logout").authenticated() so the security
configuration only contains active matchers (e.g.,
.requestMatchers("/webauthn-check").authenticated()); ensure no other commented
authorization lines remain to keep the configuration clean.
- Around line 15-18: Remove the duplicate import of
org.springframework.security.core.userdetails.User in SecurityConfig.java so the
class imports list only one User; keep the existing imports for UserEntity,
UserService, and UsernameNotFoundException unchanged and ensure SecurityConfig
still compiles and references the single User import consistently (e.g., in any
loadUserByUsername or similar methods).
In `@src/main/java/backendlab/team4you/controller/SignupController.java`:
- Around line 65-67: The error message is misleading: in SignupController where
you check userService.findByEmail(req.username) you should change the thrown
ResponseStatusException message from "Username already exists" to "Email already
exists" so it accurately reflects that req.username is an email; update the
message string in that conditional (the code referencing userService.findByEmail
and req.username) to "Email already exists".
In `@src/main/java/backendlab/team4you/user/UserEntity.java`:
- Around line 61-64: The getDisplayName method in UserEntity can produce
leading/trailing spaces when firstName or lastName is null; update
getDisplayName to build the display name by conditionally joining
non-null/non-empty firstName and lastName (or using String.join/collecting
non-empty parts) and then trim the final result so names like " Smith", "John "
or " " are avoided; ensure you reference and modify the getDisplayName method in
class UserEntity and preserve null-safety.
🪄 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: 29baa74e-a66a-47be-9009-09a9e6a7a765
📒 Files selected for processing (6)
src/main/java/backendlab/team4you/config/CustomAuthenticationSuccessHandler.javasrc/main/java/backendlab/team4you/config/SecurityConfig.javasrc/main/java/backendlab/team4you/controller/RegistrationController.javasrc/main/java/backendlab/team4you/controller/SignupController.javasrc/main/java/backendlab/team4you/user/AppUser.javasrc/main/java/backendlab/team4you/user/UserEntity.java
💤 Files with no reviewable changes (1)
- src/main/java/backendlab/team4you/user/AppUser.java
✅ Files skipped from review due to trivial changes (1)
- src/main/java/backendlab/team4you/controller/RegistrationController.java
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
Author
|
@coderabbitai resolve |
Contributor
✅ Actions performedComments resolved. Approval is disabled; enable |
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/main/java/backendlab/team4you/config/SecurityConfig.java (1)
7-7:⚠️ Potential issue | 🟡 MinorDuplicate import of
Userclass.The import
org.springframework.security.core.userdetails.Userappears twice (lines 7 and 18). Remove the duplicate to avoid compilation warnings.import backendlab.team4you.user.UserEntity; import backendlab.team4you.user.UserService; import org.springframework.security.core.userdetails.UsernameNotFoundException; -import org.springframework.security.core.userdetails.User;Also applies to: 18-18
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/backendlab/team4you/config/SecurityConfig.java` at line 7, Remove the duplicate import of org.springframework.security.core.userdetails.User in SecurityConfig.java: locate the two import statements referencing User and remove the redundant one so only a single import for the User class remains (keep the import used by methods in SecurityConfig class).
🧹 Nitpick comments (1)
src/main/java/backendlab/team4you/config/SecurityConfig.java (1)
35-36: Remove commented-out code.Line 35 contains commented-out authorization rule. If this is no longer needed, remove it to keep the configuration clean. If it's being preserved for reference, consider adding a TODO comment explaining why.
-// .requestMatchers("/profile", "/logout").authenticated() .requestMatchers("/webauthn-check").authenticated()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/backendlab/team4you/config/SecurityConfig.java` around lines 35 - 36, Remove the leftover commented-out authorization rule in SecurityConfig so the HttpSecurity configuration is clean: delete the commented line "// .requestMatchers("/profile", "/logout").authenticated()" near the chain that sets .requestMatchers("/webauthn-check").authenticated(); if you need to keep it for future reference, replace it with a brief TODO comment explaining why it's preserved instead of leaving it commented out.
🤖 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/main/java/backendlab/team4you/config/SecurityConfig.java`:
- Line 7: Remove the duplicate import of
org.springframework.security.core.userdetails.User in SecurityConfig.java:
locate the two import statements referencing User and remove the redundant one
so only a single import for the User class remains (keep the import used by
methods in SecurityConfig class).
---
Nitpick comments:
In `@src/main/java/backendlab/team4you/config/SecurityConfig.java`:
- Around line 35-36: Remove the leftover commented-out authorization rule in
SecurityConfig so the HttpSecurity configuration is clean: delete the commented
line "// .requestMatchers("/profile", "/logout").authenticated()" near the chain
that sets .requestMatchers("/webauthn-check").authenticated(); if you need to
keep it for future reference, replace it with a brief TODO comment explaining
why it's preserved instead of leaving it commented out.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e1005900-fff5-4d91-94eb-e15a4c59395a
📒 Files selected for processing (4)
src/main/java/backendlab/team4you/config/SecurityConfig.javasrc/main/java/backendlab/team4you/controller/SignupController.javasrc/main/java/backendlab/team4you/user/UserEntity.javasrc/main/resources/db/migration/V6__add_role_users.sql
✅ Files skipped from review due to trivial changes (1)
- src/main/resources/db/migration/V6__add_role_users.sql
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/backendlab/team4you/controller/SignupController.java
- src/main/java/backendlab/team4you/user/UserEntity.java
Contributor
Author
|
@coderabbitai resolve |
Contributor
✅ Actions performedComments resolved. Approval is disabled; enable |
JohanHiths
approved these changes
Apr 8, 2026
This was referenced Apr 9, 2026
This was referenced Apr 16, 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
2 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.
Summary by CodeRabbit
New Features
Chores
Style