feat: [HYJ] role 수정#105
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request primarily focuses on cleaning up source code by removing redundant comments and refining role-based access control logic within the payment and review services. Key changes include the removal of explanatory comments in several DTOs and the implementation of role string processing in ReviewService. Feedback indicates a security concern in createReview where the user's role should be re-validated against the database to comply with architectural principles. Furthermore, the manual removal of the 'ROLE_' prefix is identified as unnecessary and inconsistent with other service implementations, with suggestions provided to simplify the role verification logic.
| String cleanedRole = userRole.replace("ROLE_", ""); | ||
| if (!"CUSTOMER".equals(cleanedRole)) { | ||
| throw new BusinessException(ErrorCode.ACCESS_DENIED); | ||
| } |
There was a problem hiding this comment.
아키텍처 원칙(Security & Auth 1번)에 따라 토큰의 role과 실제 DB의 role을 매 요청마다 재검증해야 합니다. 현재 코드는 컨트롤러에서 전달받은 userRole(JWT 기반)만 확인하고 있어, DB에서 권한이 변경된 경우를 감지할 수 없습니다. PaymentService의 구현 방식과 동일하게 DB에서 조회한 user 객체의 권한을 직접 확인하도록 수정하십시오.
if (!"CUSTOMER".equals(user.getUserRole().name())) {
throw new BusinessException(ErrorCode.ACCESS_DENIED);
}References
- JWT: 토큰 내 role과 실제 DB role을 매 요청마다 재검증하는 로직 포함 (link)
| // ROLE_ 접두사 제거 후 비교 | ||
| String cleanedRole = user.getUserRole().name().replace("ROLE_", ""); | ||
| if (!"CUSTOMER".equals(cleanedRole)) { | ||
| throw new BusinessException(ErrorCode.ACCESS_DENIED); | ||
| } |
There was a problem hiding this comment.
DB에서 조회한 Enum의 name()을 호출할 때 ROLE_ 접두사를 제거하는 로직은 불필요합니다. UserRole Enum 정의에 ROLE_ 접두사가 포함되어 있지 않다면(PaymentService의 구현을 참고할 때 포함되지 않은 것으로 보임), 단순 문자열 비교만으로 충분합니다. 불필요한 가공 로직을 제거하여 코드를 간결하게 유지하십시오.
if (!"CUSTOMER".equals(user.getUserRole().name())) {
throw new BusinessException(ErrorCode.ACCESS_DENIED);
}| // ROLE_ 접두사 제거 후 비교 | ||
| String cleanedRole = user.getUserRole().name().replace("ROLE_", ""); | ||
| if (!"CUSTOMER".equals(cleanedRole)) { | ||
| throw new BusinessException(ErrorCode.ACCESS_DENIED); | ||
| } |
There was a problem hiding this comment.
위와 동일하게, 불필요한 문자열 치환 로직을 제거하고 DB의 권한 정보를 직접 비교하는 방식을 권장합니다.
if (!"CUSTOMER".equals(user.getUserRole().name())) {
throw new BusinessException(ErrorCode.ACCESS_DENIED);
}| // DB에서 가져온 역할을 "CUSTOMER", "OWNER" 등으로 통일 | ||
| String dbRole = user.getUserRole().name().replace("ROLE_", ""); |
There was a problem hiding this comment.
Enum의 name() 결과에서 ROLE_을 제거하는 처리는 중복되거나 불필요해 보입니다. PaymentService와의 일관성을 위해 불필요한 가공 로직을 제거하고 원본 Enum 이름을 사용하십시오.
| // DB에서 가져온 역할을 "CUSTOMER", "OWNER" 등으로 통일 | |
| String dbRole = user.getUserRole().name().replace("ROLE_", ""); | |
| // DB에서 가져온 역할을 사용하여 권한 확인 | |
| String dbRole = user.getUserRole().name(); |
2 participants
💡 PR 제목
🔗 관련 이슈
📌 작업 내용 요약
🧱 변경 범위 (Scope)
Backend
Frontend
🧪 테스트 내역
Backend
Frontend
npm run dev로컬 실행 확인테스트 상세(필수)
🖼️ 화면 캡처 / 시연 영상 (Frontend 변경 시 권장)
✅ 리뷰어 체크 포인트 (선택)