-
Notifications
You must be signed in to change notification settings - Fork 0
feat(Store): 관리자 접근 제한 #66
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
The head ref may contain hidden characters: "feature/#62-\uAD00\uB9AC\uC790store\uC811\uADFC\uC81C\uD55C"
Conversation
- store RUD 시 접근 제한 로직 추가 - 비즈니스 로직 고려하여 user 생성 후 storeId 수동 추가 로직으로 변경
Walkthrough이 변경 사항은 매장 관련 조회, 수정, 삭제 작업에 대해 사용자 인증 및 권한 검증 로직을 서비스 계층에 추가하고, 이에 맞는 예외 처리 및 에러 메시지를 도입합니다. 또한 매니저 회원가입 요청 DTO에서 storeId 필드를 제거하고, User 엔티티의 storeId 컬럼을 nullable로 변경합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Controller as StoreController
participant Service as StoreServiceImpl
participant UserRepo as UserRepository
Controller->>Service: getStoreByStoreId(storeId, memberDetails)
Service->>UserRepo: findById(memberDetails.getMemberId())
UserRepo-->>Service: User
alt User not found
Service-->>Controller: throw UserNotFoundException
else User found
alt User가 super admin
Service-->>Controller: StoreReadDto 반환
else User의 storeId != storeId
Service-->>Controller: throw StoreViewUnauthorizedException
else 권한 있음
Service-->>Controller: StoreReadDto 반환
end
end
Possibly related PRs
Suggested reviewers
✨ 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (5)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/store/controller/StoreController.java (2)
57-57: 코드 포맷팅 수정 필요
@PathVariable과@AuthenticationPrincipal사이에 공백이 누락되었습니다.-public ResponseEntity<?> getStoreById(@PathVariable Long storeId,@AuthenticationPrincipal MemberDetails memberDetails) { +public ResponseEntity<?> getStoreById(@PathVariable Long storeId, @AuthenticationPrincipal MemberDetails memberDetails) {
87-87: 코드 포맷팅 수정 필요
@PathVariable과@AuthenticationPrincipal사이에 공백이 누락되었습니다.-public ResponseEntity<?> deleteStore(@PathVariable Long storeId,@AuthenticationPrincipal MemberDetails memberDetails) { +public ResponseEntity<?> deleteStore(@PathVariable Long storeId, @AuthenticationPrincipal MemberDetails memberDetails) {nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/store/service/StoreServiceImpl.java (3)
14-14: 사용하지 않는 import 제거 필요
ReservationUpdateUnauthorizedException은 이 파일에서 사용되지 않습니다.-import com.nowait.domaincorerdb.reservation.exception.ReservationUpdateUnauthorizedException;
72-77: null 체크 순서 개선 제안파라미터 null 체크를 사용자 조회 전에 수행하면 불필요한 데이터베이스 쿼리를 방지할 수 있습니다.
public StoreReadDto updateStore(Long storeId, StoreUpdateRequest request, MemberDetails memberDetails) { - User user = userRepository.findById(memberDetails.getId()).orElseThrow(UserNotFoundException::new); if (storeId == null || request == null) throw new StoreParamEmptyException(); + User user = userRepository.findById(memberDetails.getId()).orElseThrow(UserNotFoundException::new); if (!Role.SUPER_ADMIN.equals(user.getRole()) && !user.getStoreId().equals(storeId)) {
99-106: null 체크 순서 개선 제안파라미터 null 체크를 사용자 조회 전에 수행하면 불필요한 데이터베이스 쿼리를 방지할 수 있습니다.
public String deleteStore(Long storeId, MemberDetails memberDetails) { - User user = userRepository.findById(memberDetails.getId()).orElseThrow(UserNotFoundException::new); if (storeId == null) { throw new StoreParamEmptyException(); } + User user = userRepository.findById(memberDetails.getId()).orElseThrow(UserNotFoundException::new); if (!Role.SUPER_ADMIN.equals(user.getRole()) && !user.getStoreId().equals(storeId)) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
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/store/controller/StoreController.java(4 hunks)nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/store/service/StoreService.java(1 hunks)nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/store/service/StoreServiceImpl.java(5 hunks)nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/user/dto/ManagerSignupRequestDto.java(0 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/store/exception/StoreDeleteUnauthorizedException.java(1 hunks)nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/store/exception/StoreUpdateUnauthorizedException.java(1 hunks)nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/store/exception/StoreViewUnauthorizedException.java(1 hunks)nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/user/entity/User.java(0 hunks)
💤 Files with no reviewable changes (2)
- nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/user/entity/User.java
- nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/user/dto/ManagerSignupRequestDto.java
🧰 Additional context used
🧬 Code Graph Analysis (1)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/exception/GlobalExceptionHandler.java (1)
nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/store/exception/StoreViewUnauthorizedException.java (1)
StoreViewUnauthorizedException(5-9)
🔇 Additional comments (9)
nowait-common/src/main/java/com/nowait/common/exception/ErrorMessage.java (1)
46-48: 일관성 있는 에러 메시지 추가 승인새로운 매장 관련 권한 에러 메시지들이 기존 패턴과 일관성 있게 추가되었습니다. 에러 코드 순서(store003~store005)와 한국어 메시지 형식이 적절합니다.
nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/store/exception/StoreViewUnauthorizedException.java (1)
1-9: 표준 예외 클래스 구현 승인매장 조회 권한 예외 클래스가 기존 패턴을 따라 올바르게 구현되었습니다. ErrorMessage enum을 활용한 일관성 있는 에러 메시지 처리가 적절합니다.
nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/store/exception/StoreUpdateUnauthorizedException.java (1)
1-9: 표준 예외 클래스 구현 승인매장 수정 권한 예외 클래스가 다른 권한 예외들과 동일한 패턴으로 일관성 있게 구현되었습니다.
nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/store/exception/StoreDeleteUnauthorizedException.java (1)
1-9: 표준 예외 클래스 구현 승인매장 삭제 권한 예외 클래스가 기존 패턴과 일치하게 구현되었습니다. 세 가지 권한 예외 클래스가 모두 일관성 있는 구조를 가지고 있습니다.
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/store/service/StoreService.java (2)
7-7: 인증 컨텍스트 통합을 위한 import 추가 승인MemberDetails 클래스 import가 적절하게 추가되었습니다.
13-17: 서비스 메소드 시그니처 변경 승인 및 권한 검증 구현 확인 완료StoreServiceImpl에서 MemberDetails 파라미터를 기반으로 한 권한 검증 로직이 각 메서드에 제대로 적용되어 있음을 확인했습니다.
- getStoreByStoreId (53–58행): User 조회 후 Role/SUPER_ADMIN 및 매장 ID 일치 여부 검사 → StoreViewUnauthorizedException
- updateStore (72–76행): User 조회 후 Role/SUPER_ADMIN 및 매장 ID 일치 여부 검사 → StoreUpdateUnauthorizedException
- deleteStore (99–105행): User 조회 후 Role/SUPER_ADMIN 및 매장 ID 일치 여부 검사 → StoreDeleteUnauthorizedException
위 구현이 의도대로 동작하므로 변경을 승인합니다.
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/store/controller/StoreController.java (1)
72-79: 권한 검증 로직 추가 확인매장 정보 수정 시 인증된 사용자 정보를 서비스 레이어로 전달하는 변경사항이 적절하게 구현되었습니다.
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/exception/GlobalExceptionHandler.java (1)
206-225: 매장 관련 권한 예외 처리 추가 확인매장 조회, 수정, 삭제에 대한 권한 없음 예외 처리가 기존 패턴과 일치하게 잘 구현되었습니다. 각 예외는 적절한 HTTP 403 상태 코드와 고유한 에러 코드를 반환합니다.
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/store/service/StoreServiceImpl.java (1)
53-58: 매장 조회 권한 검증 로직 구현 확인SUPER_ADMIN이 아닌 사용자는 자신의 매장만 조회할 수 있도록 권한 검증이 적절히 구현되었습니다.
| if (!Role.SUPER_ADMIN.equals(user.getRole()) && !user.getStoreId().equals(storeId)) { | ||
| throw new StoreViewUnauthorizedException(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
사용자의 storeId가 null인 경우 처리 확인 필요
권한 검증 로직에서 user.getStoreId()가 null일 수 있는 경우를 고려해야 합니다. 매니저 회원가입 시 storeId를 수동으로 추가하는 프로세스 변경으로 인해 storeId가 null인 사용자가 존재할 수 있습니다.
다음 스크립트를 실행하여 User 엔티티의 storeId 필드가 nullable인지 확인하세요:
Also applies to: 75-76, 104-105
🏁 Script executed:
#!/bin/bash
# Description: User 엔티티의 storeId 필드가 nullable인지 확인
# User 엔티티에서 storeId 필드 정의 찾기
ast-grep --pattern 'class User {
$$$
$_ storeId
$$$
}'
# @Column 어노테이션에서 nullable 속성 확인
rg -A 2 -B 2 "storeId" --glob "**/User.java"Length of output: 2939
storeId가 null일 때 NPE 방지 로직 추가 필요
권한 검증 시 user.getStoreId()가 null일 수 있어 현재 equals 호출 시 NPE가 발생할 수 있습니다. 아래 위치들에 대해 null-safe 비교 로직으로 변경해주세요.
- 대상 위치 (nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/store/service/StoreServiceImpl.java)
• 라인 56–57
• 라인 75–76
• 라인 104–105
수정 예시 1: Objects.equals 사용
+ import java.util.Objects;
…
- if (!Role.SUPER_ADMIN.equals(user.getRole()) && !user.getStoreId().equals(storeId)) {
+ if (!Role.SUPER_ADMIN.equals(user.getRole()) && !Objects.equals(user.getStoreId(), storeId)) {
throw new StoreViewUnauthorizedException();
}수정 예시 2: 명시적 null 체크
- if (!Role.SUPER_ADMIN.equals(user.getRole()) && !user.getStoreId().equals(storeId)) {
+ if (!Role.SUPER_ADMIN.equals(user.getRole())
+ && (user.getStoreId() == null || !user.getStoreId().equals(storeId))) {
throw new StoreViewUnauthorizedException();
}이렇게 변경하면 storeId가 null일 때도 안전하게 권한 검증이 수행됩니다.
📝 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.
| if (!Role.SUPER_ADMIN.equals(user.getRole()) && !user.getStoreId().equals(storeId)) { | |
| throw new StoreViewUnauthorizedException(); | |
| import java.util.Objects; | |
| … | |
| // previously: | |
| // if (!Role.SUPER_ADMIN.equals(user.getRole()) && !user.getStoreId().equals(storeId)) { | |
| if (!Role.SUPER_ADMIN.equals(user.getRole()) | |
| && !Objects.equals(user.getStoreId(), storeId)) { | |
| throw new StoreViewUnauthorizedException(); | |
| } |
🤖 Prompt for AI Agents
In
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/store/service/StoreServiceImpl.java
at lines 56-57, 75-76, and 104-105, the current role and storeId comparison uses
user.getStoreId().equals(storeId), which can throw a NullPointerException if
user.getStoreId() is null. To fix this, replace these comparisons with a
null-safe approach such as using Objects.equals(user.getStoreId(), storeId) or
explicitly checking for null before calling equals. This change will ensure safe
permission validation even when storeId is null.
작업 요약
Issue Link
#62
문제점 및 어려움
해결 방안
Reference
Summary by CodeRabbit
신규 기능
버그 수정
기타