feature/login-home-screen - #30
Conversation
…tions to redirect properly after login depending of the role
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 7 minutes and 39 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis pull request restructures the application's authentication and authorization flow by introducing role-based routing, seeding an admin user in the dev profile, separating admin and user endpoints across controllers, and updating security configuration to enforce role-specific access controls. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthHandler as AuthenticationHandler
participant SecurityContext as SecurityContext
participant AdminController as AdminController
participant UserController as UserController
Client->>AuthHandler: POST /login (credentials)
AuthHandler->>SecurityContext: Authenticate user
SecurityContext-->>AuthHandler: Authentication result + roles
alt Has ROLE_ADMIN
AuthHandler->>AdminController: Redirect to /dashboard
AdminController-->>Client: Return dashboard view
else Has ROLE_USER
AuthHandler->>UserController: Redirect to /home
UserController-->>Client: Return home view
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
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)
40-45:⚠️ Potential issue | 🔴 Critical
/admin/**is still exposed to any authenticated user.Only
/dashboardis admin-gated here.AdminControllerstill exposesGET /admin/logs,GET /admin/users,POST /admin/users, andPOST /admin/logs/delete, and those now fall through to.anyRequest().authenticated(). A regularUSERcan still hit admin pages and admin mutations.🔐 Minimal fix
- .requestMatchers("/dashboard").hasRole("ADMIN") + .requestMatchers("/dashboard", "/admin/**").hasRole("ADMIN") .requestMatchers("/home", "/profile/**").hasRole("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 40 - 45, SecurityConfig currently only restricts "/dashboard" to ADMIN, leaving "/admin/**" governed by .anyRequest().authenticated() so USERs can access AdminController endpoints; update the authorization matcher list in SecurityConfig to explicitly restrict the admin endpoints by adding a rule like requestMatchers("/admin/**").hasRole("ADMIN") (placed before .anyRequest().authenticated()) so all AdminController routes (e.g., GET/POST /admin/logs, /admin/users) require the ADMIN role.
🧹 Nitpick comments (1)
src/main/java/backendlab/team4you/Team4youApplication.java (1)
32-47: Store seeded roles in one format.This writes
ROLE_ADMINfor the admin andUSERfor the normal user. The rest of the auth flow already has to compensate for both shapes, which makes authority creation brittle. Pick one persisted role format and keep all writers on it.♻️ Example if you standardize on unprefixed stored roles
- devAdmin.setRole("ROLE_ADMIN"); + devAdmin.setRole("ADMIN"); ... devUser.setRole("USER");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/backendlab/team4you/Team4youApplication.java` around lines 32 - 47, The seeded roles are inconsistent: devAdmin uses "ROLE_ADMIN" while devUser uses "USER", which forces auth code to handle two formats; standardize persisted roles (pick one format) and update the seeders accordingly — for example change devAdmin.setRole("ROLE_ADMIN") to devAdmin.setRole("ADMIN") (or alternatively change devUser.setRole("USER") to "ROLE_USER") so both UserEntity instances (devAdmin and devUser in Team4youApplication.java) call setRole with the same normalized string; ensure any other seeding or hard-coded role writes follow the chosen format.
🤖 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/Team4youApplication.java`:
- Around line 24-50: The current seeding logic inside Team4youApplication only
runs when repository.count() == 0, so existing databases with users skip
creating the dev admin; change it to always check and create the dev admin
independently by querying for the admin user (e.g., findByName or findByEmail or
a repository.existsByRole/existsByName) and only call repository.save(new
UserEntity(...)) for the devAdmin if that lookup returns absent, while leaving
the existing repository.count() block (or other seeds) intact for creating the
devUser; reference the UserEntity devAdmin construction,
devAdmin.setRole("ROLE_ADMIN"), and repository.save(devAdmin) to locate where to
add the existence check and conditional save.
---
Outside diff comments:
In `@src/main/java/backendlab/team4you/config/SecurityConfig.java`:
- Around line 40-45: SecurityConfig currently only restricts "/dashboard" to
ADMIN, leaving "/admin/**" governed by .anyRequest().authenticated() so USERs
can access AdminController endpoints; update the authorization matcher list in
SecurityConfig to explicitly restrict the admin endpoints by adding a rule like
requestMatchers("/admin/**").hasRole("ADMIN") (placed before
.anyRequest().authenticated()) so all AdminController routes (e.g., GET/POST
/admin/logs, /admin/users) require the ADMIN role.
---
Nitpick comments:
In `@src/main/java/backendlab/team4you/Team4youApplication.java`:
- Around line 32-47: The seeded roles are inconsistent: devAdmin uses
"ROLE_ADMIN" while devUser uses "USER", which forces auth code to handle two
formats; standardize persisted roles (pick one format) and update the seeders
accordingly — for example change devAdmin.setRole("ROLE_ADMIN") to
devAdmin.setRole("ADMIN") (or alternatively change devUser.setRole("USER") to
"ROLE_USER") so both UserEntity instances (devAdmin and devUser in
Team4youApplication.java) call setRole with the same normalized string; ensure
any other seeding or hard-coded role writes follow the chosen format.
🪄 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: 3aa89a6c-5afe-4d56-8f80-873b62e7a0e3
📒 Files selected for processing (7)
src/main/java/backendlab/team4you/Team4youApplication.javasrc/main/java/backendlab/team4you/config/CustomAuthenticationSuccessHandler.javasrc/main/java/backendlab/team4you/config/SecurityConfig.javasrc/main/java/backendlab/team4you/controller/AdminController.javasrc/main/java/backendlab/team4you/controller/SignupController.javasrc/main/java/backendlab/team4you/controller/UserController.javasrc/main/resources/application.properties
💤 Files with no reviewable changes (1)
- src/main/java/backendlab/team4you/controller/SignupController.java
Summary by CodeRabbit
Release Notes
New Features
Chores