Skip to content

Feat/adminpolicy#125

Merged
lindaeskilsson merged 4 commits into
mainfrom
feat/Adminpolicy
Apr 5, 2026
Merged

Feat/adminpolicy#125
lindaeskilsson merged 4 commits into
mainfrom
feat/Adminpolicy

Conversation

@lindaeskilsson

@lindaeskilsson lindaeskilsson commented Apr 4, 2026

Copy link
Copy Markdown
Contributor

feat: implement AdminPolicy for role-based access control

Closes #73

Vad har gjorts:

Skapar AdminPolicy som en central komponent för admin-behörighet i hela Vet1177-plattformen.

la till requireAdmin(User user) som kastar ForbiddenException (403) om användaren inte har rollen ADMIN.

Uppdaterar VetController så att createVet anropar adminPolicy.requireAdmin(user) som första rad, via @AuthenticationPrincipal.

Avvikelse från issue-beskrivningen

Issuen föreslog att använda SecurityContextHolder och ha metoden utan parameter, altså requireAdmin(). Jag valde istället att ta emot User som parameter, requireAdmin(User user), Det håller AdminPolicy konsekvent med hur alla andra policies i projektet är byggda och det gör klassen lättare att testa

Användaren hämtas via @AuthenticationPrincipal i controllern istället, vilket ska vara Spring Securitys rekommenderade sätt

Summary by CodeRabbit

  • Security
    • Veterinarian creation now requires admin-level authentication and authorization. The system checks that the requesting user holds admin privileges before processing creation requests.
    • Non-admin or unauthenticated users attempting to create veterinarians will receive an access denied error; successful requests by admins continue to return the created resource as before.

@coderabbitai

coderabbitai Bot commented Apr 4, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9051975c-7a96-4796-962a-10e500f97f54

📥 Commits

Reviewing files that changed from the base of the PR and between fab496a and 4bd07a6.

📒 Files selected for processing (1)
  • src/main/java/org/example/vet1177/policy/AdminPolicy.java
✅ Files skipped from review due to trivial changes (1)
  • src/main/java/org/example/vet1177/policy/AdminPolicy.java

📝 Walkthrough

Walkthrough

A new AdminPolicy Spring component was added for admin authorization, and VetController was updated to inject AdminPolicy, accept the authenticated User in createVet, and call adminPolicy.requireAdmin(user) before delegating to the service.

Changes

Cohort / File(s) Summary
AdminPolicy Implementation
src/main/java/org/example/vet1177/policy/AdminPolicy.java
Added new Spring @Component AdminPolicy with void requireAdmin(User user) that throws when user is null or not Role.ADMIN.
VetController Integration
src/main/java/org/example/vet1177/controller/VetController.java
Constructor signature updated to accept AdminPolicy; createVet now takes @AuthenticationPrincipal User user and calls adminPolicy.requireAdmin(user) before vetService.createVet(request).

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant VetController
    participant AdminPolicy
    participant VetService
    participant Database
    Client->>VetController: POST /vets (createVet request)
    Note over VetController: Authentication principal extracted as User
    VetController->>AdminPolicy: requireAdmin(user)
    alt user has Role.ADMIN
        AdminPolicy-->>VetController: OK
        VetController->>VetService: createVet(request)
        VetService->>Database: persist vet
        Database-->>VetService: persisted
        VetService-->>VetController: VetResponse
        VetController-->>Client: 201 Created (VetResponse)
    else not admin
        AdminPolicy-->>VetController: throws ForbiddenException
        VetController-->>Client: 403 Forbidden
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

enhancement

Suggested reviewers

  • TatjanaTrajkovic
  • johanbriger
  • annikaholmqvist94

Poem

🐇 A hop, a check, an admin gate,
I guard the petals of the state.
One policy now holds the key,
No scattered checks — just unity.
Hooray for tidy authority! 🎉

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Feat/adminpolicy' is vague and does not clearly describe the main change; it uses a generic feature branch naming convention rather than a descriptive PR title. Revise the title to be more descriptive, e.g., 'Add AdminPolicy component for role-based access control' or 'Implement admin authorization check in VetController'.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed The PR implements the core requirements from issue #73: creates AdminPolicy as a Spring component, implements requireAdmin method with ForbiddenException on unauthorized access, injects it into VetController, and integrates the check into createVet.
Out of Scope Changes check ✅ Passed The PR includes a deliberate deviation from the original proposal: requireAdmin accepts a User parameter instead of using SecurityContextHolder. While this deviates from issue #73's proposed implementation, it is explicitly justified in the PR objectives as improving testability and aligning with project patterns, and all changes directly support the main authorization goal.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/Adminpolicy

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Component
public class AdminPolicy {
public void requireAdmin(User user){
if (user.getRole() != Role.ADMIN) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

För att undvika crash i det fallet där user == null, ändra till if (user == null || user.getRole() != Role.ADMIN)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bra input! Ändrar det!

@lindaeskilsson
lindaeskilsson merged commit 9d02d6f into main Apr 5, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: Implement AdminPolicy for access control

2 participants