feat(Menu): 관리자 접근 제한 #63
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- 본인 store외 접근 제한 로직 추가 - 예외처리 상세화
Walkthrough이 변경 사항은 메뉴 관련 API의 인증 및 권한 부여 로직을 강화합니다. 메뉴 생성, 조회, 수정, 삭제 시 사용자 인증 정보를 서비스 계층까지 전달하며, 권한이 없는 경우 명확한 예외와 에러 응답을 반환합니다. 또한 User 엔티티 및 관련 DTO에 storeId 필드가 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant MenuController
participant MenuService
participant UserRepository
Client->>MenuController: 메뉴 생성/조회/수정/삭제 요청 (인증 정보 포함)
MenuController->>MenuService: 요청 + MemberDetails 전달
MenuService->>UserRepository: 사용자 정보 조회
MenuService-->>MenuService: 권한 체크 (SUPER_ADMIN 또는 점주 여부)
alt 권한 없음
MenuService-->>MenuController: Menu*UnauthorizedException 발생
MenuController-->>Client: 403 Forbidden + 에러 메시지
else 권한 있음
MenuService-->>MenuController: 정상 처리 결과 반환
MenuController-->>Client: 성공 응답
end
Possibly related PRs
Suggested labels
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (2)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/menu/service/MenuService.java (1)
139-147: toggleSoldOut 메소드에 권한 검증이 누락되었습니다.다른 모든 메뉴 작업에는 권한 검증이 있지만, 품절 상태 토글 기능에는 권한 검증이 없습니다. 이는 보안 취약점이 될 수 있습니다.
-public Boolean toggleSoldOut(Long menuId) { +public Boolean toggleSoldOut(Long menuId, MemberDetails memberDetails) { + User user = userRepository.findById(memberDetails.getId()).orElseThrow(UserNotFoundException::new); Menu menu = menuRepository.findById(menuId) .orElseThrow(MenuNotFoundException::new); + + if (!Role.SUPER_ADMIN.equals(user.getRole()) && !user.getStoreId().equals(menu.getStoreId())) { + throw new MenuUpdateUnauthorizedException(); + } menu.toggleSoldOut(); menuRepository.save(menu); return menu.getIsSoldOut(); }nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/menu/controller/MenuController.java (1)
116-127: toggleSoldOut 메소드에도 권한 검증이 필요합니다.다른 모든 메뉴 작업 엔드포인트에는 인증 정보를 전달하지만, 품절 토글 기능에는 누락되었습니다. 일관성과 보안을 위해 추가해야 합니다.
-public ResponseEntity<?> toggleSoldOut(@PathVariable Long menuId) { +public ResponseEntity<?> toggleSoldOut(@PathVariable Long menuId, @AuthenticationPrincipal MemberDetails memberDetails) { return ResponseEntity .status(HttpStatus.OK) .body( ApiUtils.success( - menuService.toggleSoldOut(menuId) + menuService.toggleSoldOut(menuId, memberDetails) ) ); }
🧹 Nitpick comments (7)
nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/menu/exception/MenuCreationUnauthorizedException.java (1)
5-9: 예외 클래스 구현 승인 및 선택적 개선사항 제안예외 클래스가 표준적인 패턴을 잘 따르고 있으며, ErrorMessage enum을 사용하여 일관성을 유지하고 있습니다.
향후 확장성을 고려하여 추가 정보를 전달할 수 있는 생성자를 추가하는 것을 고려해볼 수 있습니다:
public class MenuCreationUnauthorizedException extends RuntimeException { public MenuCreationUnauthorizedException() { super(ErrorMessage.MENU_CREATION_UNAUTHORIZED.getMessage()); } + + public MenuCreationUnauthorizedException(String additionalMessage) { + super(ErrorMessage.MENU_CREATION_UNAUTHORIZED.getMessage() + ": " + additionalMessage); + } }nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/menu/service/MenuService.java (1)
100-123: 권한 검증 로직의 중복을 줄일 수 있습니다.
createMenu와 동일한 권한 검증 패턴이 반복되고 있습니다. 메뉴 수정 권한 검증도 별도 메소드로 추출하는 것을 고려해보세요.+private static void validateMenuModificationAuthorization(User user, Long storeId) { + if (!Role.SUPER_ADMIN.equals(user.getRole()) && !user.getStoreId().equals(storeId)) { + throw new MenuUpdateUnauthorizedException(); + } +}nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/menu/controller/MenuController.java (5)
41-43: 쉼표 뒤에 공백을 추가하세요.코드 스타일 일관성을 위해 메소드 파라미터 사이의 쉼표 뒤에 공백을 추가해야 합니다.
-MenuCreateResponse response = menuService.createMenu(request,memberDetails); +MenuCreateResponse response = menuService.createMenu(request, memberDetails);
57-64: 쉼표 뒤에 공백을 추가하세요.-menuService.getAllMenusByStoreId(storeId,memberDetails) +menuService.getAllMenusByStoreId(storeId, memberDetails)
73-81: 쉼표 뒤에 공백을 추가하세요.-menuService.getMenuById(storeId, menuId,memberDetails) +menuService.getMenuById(storeId, menuId, memberDetails)
91-99: 쉼표 뒤에 공백을 추가하세요.-menuService.updateMenu(menuId, request,memberDetails) +menuService.updateMenu(menuId, request, memberDetails)
106-112: 쉼표 뒤에 공백을 추가하세요.-public ResponseEntity<?> deleteMenu(@PathVariable Long menuId,@AuthenticationPrincipal MemberDetails memberDetails) { +public ResponseEntity<?> deleteMenu(@PathVariable Long menuId, @AuthenticationPrincipal MemberDetails memberDetails) {-menuService.deleteMenu(menuId,memberDetails) +menuService.deleteMenu(menuId, memberDetails)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/exception/GlobalExceptionHandler.java(2 hunks)nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/menu/controller/MenuController.java(6 hunks)nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/menu/service/MenuService.java(5 hunks)nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/user/dto/ManagerSignupRequestDto.java(2 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/security/jwt/JwtAuthorizationFilter.java(1 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/token/controller/TokenController.java(1 hunks)nowait-common/src/main/java/com/nowait/common/exception/ErrorMessage.java(1 hunks)nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/menu/exception/MenuCreationUnauthorizedException.java(1 hunks)nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/menu/exception/MenuDeleteUnauthorizedException.java(1 hunks)nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/menu/exception/MenuUpdateUnauthorizedException.java(1 hunks)nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/menu/exception/MenuViewUnauthorizedException.java(1 hunks)nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/user/entity/User.java(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/exception/GlobalExceptionHandler.java (4)
nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/menu/exception/MenuCreationUnauthorizedException.java (1)
MenuCreationUnauthorizedException(5-9)nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/menu/exception/MenuDeleteUnauthorizedException.java (1)
MenuDeleteUnauthorizedException(5-9)nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/menu/exception/MenuUpdateUnauthorizedException.java (1)
MenuUpdateUnauthorizedException(5-9)nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/menu/exception/MenuViewUnauthorizedException.java (1)
MenuViewUnauthorizedException(5-9)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/menu/service/MenuService.java (4)
nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/menu/exception/MenuCreationUnauthorizedException.java (1)
MenuCreationUnauthorizedException(5-9)nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/menu/exception/MenuDeleteUnauthorizedException.java (1)
MenuDeleteUnauthorizedException(5-9)nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/menu/exception/MenuUpdateUnauthorizedException.java (1)
MenuUpdateUnauthorizedException(5-9)nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/menu/exception/MenuViewUnauthorizedException.java (1)
MenuViewUnauthorizedException(5-9)
🔇 Additional comments (13)
nowait-common/src/main/java/com/nowait/common/exception/ErrorMessage.java (1)
33-36: 새로운 메뉴 권한 에러 메시지 구현 승인메뉴 작업별 세분화된 권한 에러 메시지가 잘 정의되었습니다. 각 CRUD 작업에 대해 명확한 에러 메시지와 순차적인 에러 코드를 제공하여 예외 처리의 일관성을 높였습니다.
nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/menu/exception/MenuViewUnauthorizedException.java (1)
5-9: 일관성 있는 예외 클래스 구현 승인메뉴 보기 권한 관련 예외 클래스가 표준적인 패턴을 잘 따르고 있으며, 다른 메뉴 예외 클래스들과 일관성을 유지하고 있습니다.
nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/menu/exception/MenuUpdateUnauthorizedException.java (1)
5-9: 일관성 있는 예외 클래스 구현 승인메뉴 수정 권한 관련 예외 클래스가 표준적인 패턴을 잘 따르고 있으며, 다른 메뉴 예외 클래스들과 일관성을 유지하고 있습니다.
nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/menu/exception/MenuDeleteUnauthorizedException.java (1)
1-10: 일관성 있게 잘 구현된 예외 클래스입니다.다른 메뉴 관련 예외 클래스들과 동일한 패턴을 따르고 있어 코드 일관성이 좋습니다.
nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/user/entity/User.java (2)
47-47: storeId 필드에 대한 제약 조건을 검토해보세요.storeId 필드가 추가되었지만, 다음 사항들을 고려해보세요:
- 관리자가 아닌 사용자(예: 일반 고객)의 경우 storeId가 null이어야 하는지
- 데이터베이스 제약 조건(foreign key, nullable 등)이 필요한지
- 역할(Role)에 따른 storeId 유효성 검증 로직이 필요한지
49-59: 생성자와 정적 팩토리 메서드가 올바르게 업데이트되었습니다.storeId 파라미터가 일관성 있게 추가되었습니다.
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/user/dto/ManagerSignupRequestDto.java (2)
38-40: storeId 필드가 적절히 추가되었습니다.@NotNull 검증과 스키마 문서화가 잘 되어 있습니다.
49-49: toEntity() 메서드가 올바르게 업데이트되었습니다.storeId가 User 엔티티 생성 시 정확히 전달되고 있습니다.
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/exception/GlobalExceptionHandler.java (2)
24-27: 새로운 메뉴 인증 예외 클래스들이 올바르게 import되었습니다.
135-161: 메뉴 인증 예외 핸들러들이 일관성 있게 구현되었습니다.
- 적절한 HTTP 상태 코드(403 Forbidden) 사용
- 기존 핸들러들과 동일한 패턴 준수
- 정확한 에러 코드와 메시지 사용
- 적절한 로깅 수행
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/menu/service/MenuService.java (3)
41-54: 권한 검증 로직이 적절하게 구현되었습니다.메뉴 생성 시 SUPER_ADMIN이나 해당 가게의 소유자만 접근할 수 있도록 제한하는 로직이 올바르게 구현되었습니다.
57-76: 권한 검증 로직을 별도 메소드로 추출하여 재사용한 점이 좋습니다.
validateMenuViewAuthorization메소드를 활용하여 중복 코드를 줄이고 일관성 있는 권한 검증을 구현했습니다.
79-97: 예외 타입을 더 구체적으로 변경한 점이 좋습니다.
OrderParameterEmptyException대신MenuParamEmptyException을 사용하여 메뉴 도메인에 특화된 예외 처리를 구현했습니다.
| // menu | ||
| MENU_PARAMETER_EMPTY("메뉴 생성 시 파라미터 정보가 없습니다.", "menu001"), | ||
| MENU_NOT_FOUND("해당 메뉴를 찾을 수 없습니다.", "menu001"), | ||
| MENU_NOT_FOUND("해당 메뉴를 찾을 수 없습니다.", "menu002"), |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
기존 에러 코드 변경에 대한 하위 호환성 검증 필요
MENU_NOT_FOUND의 에러 코드가 "menu001"에서 "menu002"로 변경되었습니다. 이는 클라이언트 코드에서 기존 에러 코드를 참조하고 있다면 호환성 문제를 야기할 수 있습니다.
다음 스크립트를 실행하여 기존 에러 코드 "menu001"을 참조하는 코드가 있는지 확인해주세요:
🏁 Script executed:
#!/bin/bash
# Description: 기존 "menu001" 에러 코드를 참조하는 코드 검색
# Expected: 기존 코드에서 "menu001" 참조가 있다면 업데이트 필요
echo "=== 기존 menu001 에러 코드 참조 검색 ==="
rg -n "menu001" --type javaLength of output: 235
기존 menu001 코드 재사용 확인 및 하위 호환성 보장 필요
스크립트 실행 결과, 아래 위치에서 여전히 menu001이 사용되고 있습니다.
이전에는 menu001이 MENU_NOT_FOUND에 매핑되었으나, 현재 MENU_NOT_FOUND는 menu002로 변경되어 클라이언트가 menu001을 여전히 “메뉴를 찾을 수 없음”으로 처리 중이라면 잘못된 동작이 발생할 수 있습니다.
- 확인 위치
• nowait-common/src/main/java/com/nowait/common/exception/ErrorMessage.java:31
MENU_PARAMETER_EMPTY("메뉴 생성 시 파라미터 정보가 없습니다.", "menu001"),
• nowait-common/src/main/java/com/nowait/common/exception/ErrorMessage.java:32
MENU_NOT_FOUND("해당 메뉴를 찾을 수 없습니다.", "menu002"),
제안 사항
- 기존
menu001코드 참조 클라이언트 코드 목록을 파악하고 영향도 분석 - 하위 호환을 위해
menu001을 계속 유지하거나, 별도 마이그레이션 가이드 및 릴리즈 노트 제공 - 버전 정책(major/minor) 검토 후 적절한 버전 업 조치
🤖 Prompt for AI Agents
In nowait-common/src/main/java/com/nowait/common/exception/ErrorMessage.java at
lines 31-32, the error code for MENU_NOT_FOUND was changed from "menu001" to
"menu002", but client code still uses "menu001" for this error, causing
compatibility issues. To fix this, either revert MENU_NOT_FOUND to use "menu001"
to maintain backward compatibility or ensure a clear migration path by updating
client references, providing migration guides, and adjusting versioning
accordingly.
Comment on lines
44
to
46
| if (role.equals("SUPER_ADMIN")) { | ||
| currentAccessTokenExpiration = 7L * 24 * 60 * 60 * 1000L; // 7일 | ||
| currentAccessTokenExpiration = 100L * 24 * 60 * 60 * 1000L; // 100일 | ||
| } |
There was a problem hiding this comment.
액세스 토큰 만료 시간이 너무 긴 것에 대한 보안 우려
SUPER_ADMIN 역할의 액세스 토큰 만료 시간을 100일로 설정하는 것은 보안상 위험할 수 있습니다:
- 토큰 탈취 위험: 긴 만료 시간으로 인해 토큰이 탈취되었을 때 장기간 악용될 수 있습니다
- 높은 권한 노출: SUPER_ADMIN은 최고 권한을 가진 역할로, 더 짧은 만료 시간이 적절합니다
- 보안 모범 사례: 일반적으로 액세스 토큰은 짧은 만료 시간(1-24시간)을 가지고 리프레시 토큰으로 갱신하는 것이 권장됩니다
다음과 같이 더 짧은 만료 시간으로 수정하는 것을 권장합니다:
- currentAccessTokenExpiration = 100L * 24 * 60 * 60 * 1000L; // 100일
+ currentAccessTokenExpiration = 24 * 60 * 60 * 1000L; // 24시간📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (role.equals("SUPER_ADMIN")) { | |
| currentAccessTokenExpiration = 7L * 24 * 60 * 60 * 1000L; // 7일 | |
| currentAccessTokenExpiration = 100L * 24 * 60 * 60 * 1000L; // 100일 | |
| } | |
| if (role.equals("SUPER_ADMIN")) { | |
| currentAccessTokenExpiration = 24 * 60 * 60 * 1000L; // 24시간 | |
| } |
🤖 Prompt for AI Agents
In
nowait-app-user-api/src/main/java/com/nowait/applicationuser/token/controller/TokenController.java
around lines 44 to 46, the access token expiration time for the SUPER_ADMIN role
is set to 100 days, which is too long and poses security risks. Change the
expiration time to a much shorter duration, such as between 1 to 24 hours, to
reduce the risk of token theft and misuse. Implement this by updating the
currentAccessTokenExpiration value to a shorter time span consistent with
security best practices for high-privilege roles.
Comment on lines
81
to
+82
| User user = User.createUserWithId(userId, "sampleEmail", "sampleNickname", "sampleProfileImg" | ||
| , SocialType.KAKAO, Role.fromString(roleString)); | ||
| , SocialType.KAKAO, Role.fromString(roleString),5000L); |
There was a problem hiding this comment.
하드코딩된 storeId 값을 검토해야 합니다.
모든 JWT 인증 사용자에게 동일한 storeId(5000L)가 할당되고 있습니다. 이는 관리자 접근 제한 기능의 목적에 맞지 않을 수 있습니다.
다음 중 하나를 고려해보세요:
- JWT 토큰에서 실제 storeId를 추출하여 사용
- 사용자별 storeId를 데이터베이스에서 조회하여 설정
- 임시 값이라면 TODO 주석 추가
🤖 Prompt for AI Agents
In
nowait-app-user-api/src/main/java/com/nowait/applicationuser/security/jwt/JwtAuthorizationFilter.java
at lines 81-82, the storeId is hardcoded as 5000L for all users, which may not
suit admin access restrictions. To fix this, either extract the actual storeId
from the JWT token if available, or query the user's storeId from the database
and use that value. If this is a temporary placeholder, add a TODO comment
indicating that the storeId assignment needs proper implementation later.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
작업 요약
Issue Link
#62
문제점 및 어려움
해결 방안
Reference
Summary by CodeRabbit
신규 기능
버그 수정
기타 변경