Skip to content

Conversation

@yhi9839
Copy link
Contributor

@yhi9839 yhi9839 commented Jan 14, 2026

  1. #⃣ 연관된 이슈
  2. 📝 작업 내용
    • RoleHierarchy 추가
    • ROLE로 검사하는 로직 변경 (performer 가 아니면 에러 -> Audience 이면 에러)

Summary by CodeRabbit

  • Bug Fixes

    • Refined authorization checks so show and album operations consistently enforce role-based access (audience vs performer).
    • Service-level access now validates requestor identity before returning performer show reservations.
  • Chores

    • Added role-hierarchy configuration to formalize authorization levels (admin > performer > audience).

✏️ Tip: You can customize this high-level summary in your review settings.

@yhi9839 yhi9839 linked an issue Jan 14, 2026 that may be closed by this pull request
@coderabbitai
Copy link

coderabbitai bot commented Jan 14, 2026

📝 Walkthrough

Walkthrough

This PR adds role-hierarchy support to Spring Security and adjusts service-level authorization checks: role checks that previously denied non-PERFORMER members now explicitly deny AUDIENCE members. It also threads authenticated member ID through the performer controller to the service for owner checks.

Changes

Cohort / File(s) Summary
Role Hierarchy Configuration
src/main/java/cc/backend/config/jwt/SecurityConfig.java, src/main/java/cc/backend/config/jwt/MethodSecurityConfig.java
Added a RoleHierarchy bean (ROLE_ADMIN > ROLE_PERFORMER > ROLE_AUDIENCE) and MethodSecurityConfig to wire the hierarchy into method-level security expressions.
Amateur show authorization logic
src/main/java/cc/backend/amateurShow/service/amateurShowService/AmateurServiceImpl.java
Replaced checks of member.getRole() != Role.PERFORMER with member.getRole() == Role.AUDIENCE in enrollShow, updateShow, deleteShow, getMyAmateurShow (error still MEMBER_NOT_PERFORMER).
Photo album authorization tweak
src/main/java/cc/backend/photoAlbum/service/PhotoAlbumServiceImpl.java
Adjusted getMyShows role check to reject AUDIENCE specifically instead of rejecting non-PERFORMER.
Performer API / ownership check
src/main/java/cc/backend/performer/PerformerController.java, src/main/java/cc/backend/performer/PerformerService.java
Controller now injects authenticated Member (@AuthenticationPrincipal(expression = "member")) and passes member.getId() to getShowReservationList; service signature updated to receive memberId and enforces that the requesting member owns the show.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Controller as PerformerController
    participant Auth as AuthenticationPrincipal
    participant Service as PerformerService
    participant Repo as AmateurShowRepository
    participant DB as Database

    Client->>Controller: GET /performer/shows/{amateurShowId}/reservations?roundId=...
    Controller->>Auth: resolve authenticated principal (member)
    Auth-->>Controller: Member (id)
    Controller->>Service: getShowReservationList(memberId, amateurShowId, roundId)
    Service->>Repo: findById(amateurShowId)
    Repo->>DB: query show by id
    DB-->>Repo: show record
    Repo-->>Service: AmateurShow (with owner member)
    Service->>Service: check show.member.id == memberId -> if not throw MEMBER_NOT_AUTHORIZED
    Service->>Repo: load rounds/tickets/reservations...
    Service-->>Controller: ShowReservationResponseDTO
    Controller-->>Client: 200 OK (payload) / or error
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

Possibly related PRs

Poem

🐰
I hopped through roles and little gates,
ADMIN above, PERFORMER narrates,
AUDIENCE waits with polite applause—
A rabbit cheers for clearer laws! 🎭

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed Title clearly describes the main objective: enabling admins to access all APIs through role hierarchy support, which is the core of the changeset.
Description check ✅ Passed PR description includes related issue (#141) and summarizes the work (RoleHierarchy addition and role-checking logic changes), meeting the template requirements.

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

✨ Finishing touches
  • 📝 Generate docstrings


📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a4954b0 and 1695957.

📒 Files selected for processing (2)
  • src/main/java/cc/backend/performer/PerformerController.java
  • src/main/java/cc/backend/performer/PerformerService.java
🔇 Additional comments (3)
src/main/java/cc/backend/performer/PerformerService.java (2)

42-45: Verify admin bypass behavior for ownership check.

The ownership check correctly ensures that only the show registrant can view reservation details. However, this may conflict with the PR's stated goal of allowing admins to access all APIs.

With the current implementation:

  • Admins will pass @PreAuthorize("hasRole('PERFORMER')") due to role hierarchy
  • But admins will fail this ownership check if they don't own the show

If admins should be able to view any show's reservations, consider adding a role check:

💡 Suggested change if admin bypass is needed
         // 1-1) 로그인한 계정이 공연의 주인인지 확인
-        if (!show.getMember().getId().equals(memberId)) {
+        if (!show.getMember().getId().equals(memberId) && !isAdmin(memberId)) {
             throw new GeneralException(ErrorStatus.MEMBER_NOT_AUTHORIZED);
         }

Where isAdmin() checks if the member has the ADMIN role.


36-45: LGTM on the authorization pattern.

The method signature update to include memberId and the ownership validation pattern is a solid approach for resource-level authorization. The check at the service layer provides defense-in-depth alongside the controller-level @PreAuthorize.

src/main/java/cc/backend/performer/PerformerController.java (1)

74-83: LGTM!

The authenticated member injection and ID propagation to the service layer is correctly implemented. This pattern is consistent with the other endpoints in this controller and properly enables the ownership validation in PerformerService.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


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.

@yhi9839 yhi9839 merged commit e904105 into develop Jan 15, 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] 모든 API 접근 가능한 관리자 권한 부여

2 participants