[DDING-000] 폼지 지원자 조회시 권한 검증로직 추가 #266
Conversation
KoSeonJe
requested review from
wonjunYou,
5uhwann and
Seooooo24
as code owners
|
Caution Review failedThe pull request is closed. """ 워크스루이번 PR은 변경 사항
시퀀스 다이어그램sequenceDiagram
participant 사용자
participant 서비스 as FacadeCentralFormApplicationServiceImpl
participant ClubService
participant 양식 as Form
사용자->>서비스: 요청(getAllFormApplication / getFormApplication)
서비스->>ClubService: 사용자 ID로 클럽 조회 요청
ClubService-->>서비스: 클럽 정보 반환
서비스->>서비스: validateAuthority(club, form) 호출
alt 권한 일치
서비스-->>사용자: 양식 신청 정보 반환
else 권한 불일치
서비스-->>사용자: AuthenticationException.NonHaveAuthority 예외 발생
end
관련 PR
제안된 라벨
제안된 리뷰어
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/ddingdong/ddingdongBE/domain/formapplication/service/FacadeCentralFormApplicationServiceImpl.java (2)
39-50: 권한 검증 로직의 에러 메시지를 더 구체적으로 개선하면 좋겠습니다.현재 구현은 적절하나, 사용자 경험 향상을 위해 다음 사항을 고려해보세요:
- 권한 오류 발생 시 더 구체적인 에러 메시지 제공
- 로깅 추가로 권한 검증 실패 추적
예시 코드:
@Override public MyAllFormApplicationsQuery getAllFormApplication(Long formId, User user) { Club club = clubService.getByUserId(user.getId()); Form form = formService.getById(formId); - validateAuthority(club, form); + validateAuthority(club, form, + String.format("사용자의 클럽(%s)이 폼의 클럽(%s)과 일치하지 않습니다", + club.getName(), form.getClub().getName())); List<FormApplication> formApplications = formApplicationService.getAllByForm(form); // ... 나머지 코드 }
78-82: 권한 검증 메소드의 방어적 프로그래밍을 강화하면 좋겠습니다.현재 구현은 기본적인 기능은 충족하지만, 다음과 같은 개선사항을 고려해보세요:
- null 체크 추가
- 로깅 추가
- 구체적인 에러 메시지 파라미터화
-private void validateAuthority(Club club, Form form) { +private void validateAuthority(Club club, Form form, String errorMessage) { + Objects.requireNonNull(club, "클럽 정보가 없습니다"); + Objects.requireNonNull(form, "폼 정보가 없습니다"); + Objects.requireNonNull(form.getClub(), "폼의 클럽 정보가 없습니다"); + if (!Objects.equals(club.getId(), form.getClub().getId())) { - throw new NonHaveAuthority(); + log.warn("권한 검증 실패: {}", errorMessage); + throw new NonHaveAuthority(errorMessage); } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/ddingdong/ddingdongBE/domain/formapplication/service/FacadeCentralFormApplicationServiceImpl.java(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and analyze
🔇 Additional comments (2)
src/main/java/ddingdong/ddingdongBE/domain/formapplication/service/FacadeCentralFormApplicationServiceImpl.java (2)
3-5: 새로운 의존성 추가가 적절합니다!권한 검증을 위한 새로운 의존성들이 잘 추가되었습니다.
ClubService를 통한 사용자의 클럽 정보 조회는 권한 검증에 필수적인 요소입니다.Also applies to: 21-21, 37-37
52-61: 권한 검증 패턴이 일관성 있게 적용되었습니다.getAllFormApplication과 동일한 패턴으로 권한 검증이 구현되어 있어 코드의 일관성이 잘 유지되었습니다. 다만, 이 메소드에도 위에서 제안한 동일한 에러 메시지 개선사항을 적용하면 좋겠습니다.
|
🚀 작업 내용
폼지 지원자 정보는 중요 정보이기에 조회시 권한 검증로직 추가하였습니다
🤔 고민했던 내용
💬 리뷰 중점사항
Summary by CodeRabbit