feat(Reservation): 관리자 접근 제한 #65
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에 인증된 사용자 정보 기반의 권한 검증 로직이 추가되었습니다. 권한이 없는 사용자가 접근 시, 새로운 예외 클래스와 글로벌 예외 핸들러를 통해 403 Forbidden과 함께 명확한 에러 메시지를 반환하도록 구현되었습니다. 관련 에러 메시지도 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant Service
participant UserRepository
participant ExceptionHandler
Client->>Controller: getReservationListByStoreId(storeId, memberDetails)
Controller->>Service: getReservationListByStoreId(storeId, memberDetails)
Service->>UserRepository: findById(memberDetails.memberId)
UserRepository-->>Service: User 반환
alt 권한 없음
Service-->>ExceptionHandler: ReservationViewUnauthorizedException 발생
ExceptionHandler-->>Client: 403 Forbidden + 에러 메시지
else 권한 있음
Service-->>Controller: 예약 목록 반환
Controller-->>Client: 예약 목록 응답
end
sequenceDiagram
participant Client
participant Controller
participant Service
participant UserRepository
participant ExceptionHandler
Client->>Controller: updateReservationStatus(reservationId, requestDto, memberDetails)
Controller->>Service: updateReservationStatus(reservationId, requestDto, memberDetails)
Service->>UserRepository: findById(memberDetails.memberId)
UserRepository-->>Service: User 반환
alt 권한 없음
Service-->>ExceptionHandler: ReservationUpdateUnauthorizedException 발생
ExceptionHandler-->>Client: 403 Forbidden + 에러 메시지
else 권한 있음
Service-->>Controller: 상태 변경 결과 반환
Controller-->>Client: 결과 응답
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/reservation/service/ReservationService.java (2)
14-15: 불필요한 임포트 제거 필요
OrderUpdateUnauthorizedException은 이 파일에서 사용되지 않습니다.import com.nowait.common.enums.Role; -import com.nowait.domaincorerdb.order.exception.OrderUpdateUnauthorizedException;
37-37: 성능 최적화 고려사항메서드 호출 시마다 사용자 정보를 데이터베이스에서 조회하는 것은 성능에 영향을 줄 수 있습니다. 자주 호출되는 API라면 캐싱을 고려해보세요.
예를 들어 Spring Cache를 활용하거나, MemberDetails에 storeId 정보를 포함시키는 방법을 고려할 수 있습니다.
Also applies to: 68-68
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
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/reservation/controller/ReservationController.java(4 hunks)nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/reservation/service/ReservationService.java(3 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/reservation/exception/ReservationUpdateUnauthorizedException.java(1 hunks)nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/reservation/exception/ReservationViewUnauthorizedException.java(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/exception/GlobalExceptionHandler.java (2)
nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/reservation/exception/ReservationUpdateUnauthorizedException.java (1)
ReservationUpdateUnauthorizedException(5-9)nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/reservation/exception/ReservationViewUnauthorizedException.java (1)
ReservationViewUnauthorizedException(5-9)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/reservation/service/ReservationService.java (3)
nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/order/exception/OrderUpdateUnauthorizedException.java (1)
OrderUpdateUnauthorizedException(5-9)nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/reservation/exception/ReservationUpdateUnauthorizedException.java (1)
ReservationUpdateUnauthorizedException(5-9)nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/reservation/exception/ReservationViewUnauthorizedException.java (1)
ReservationViewUnauthorizedException(5-9)
🔇 Additional comments (10)
nowait-common/src/main/java/com/nowait/common/exception/ErrorMessage.java (1)
28-29: 새로운 예약 권한 에러 메시지 추가가 적절합니다.기존 패턴과 일치하며 다른 엔티티의 권한 에러 메시지들(ORDER_VIEW_UNAUTHORIZED, ORDER_UPDATE_UNAUTHORIZED 등)과 일관성 있게 구현되었습니다.
nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/reservation/exception/ReservationViewUnauthorizedException.java (1)
1-10: 예약 조회 권한 예외 클래스가 올바르게 구현되었습니다.RuntimeException을 상속하고 적절한 에러 메시지를 사용하여 기존 패턴과 일치합니다.
nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/reservation/exception/ReservationUpdateUnauthorizedException.java (1)
1-10: 예약 수정 권한 예외 클래스가 올바르게 구현되었습니다.ReservationViewUnauthorizedException과 동일한 패턴을 따르며 적절한 에러 메시지를 사용합니다.
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/exception/GlobalExceptionHandler.java (3)
35-36: 새로운 예외 클래스 import가 올바르게 추가되었습니다.필요한 예외 클래스들이 적절히 import되었습니다.
161-166: 예약 조회 권한 예외 핸들러가 올바르게 구현되었습니다.기존 OrderViewUnauthorizedException 핸들러와 동일한 패턴을 따르며 적절한 HTTP 403 상태 코드를 반환합니다.
168-173: 예약 수정 권한 예외 핸들러가 올바르게 구현되었습니다.기존 OrderUpdateUnauthorizedException 핸들러와 동일한 패턴을 따르며 일관성 있게 구현되었습니다.
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/reservation/controller/ReservationController.java (4)
5-5: 인증 관련 import가 올바르게 추가되었습니다.@AuthenticationPrincipal 어노테이션 사용을 위한 import가 적절히 추가되었습니다.
18-18: MemberDetails import가 올바르게 추가되었습니다.인증된 사용자 정보를 위한 MemberDetails 클래스 import가 적절히 추가되었습니다.
36-38: 예약 조회 메서드에 인증 정보 추가가 적절합니다.@AuthenticationPrincipal 어노테이션을 사용하여 인증된 사용자 정보를 받고 서비스 레이어로 전달하는 구조가 올바르게 구현되었습니다.
51-54: 예약 상태 변경 메서드에 인증 정보 추가가 적절합니다.예약 조회 메서드와 동일한 패턴으로 인증 정보를 받고 전달하는 구조가 일관성 있게 구현되었습니다.
Comment on lines
66
to
73
| public CallGetResponseDto updateReservationStatus(Long reservationId, ReservationStatusUpdateRequestDto requestDto, | ||
| MemberDetails memberDetails) { | ||
| User user = userRepository.findById(memberDetails.getId()).orElseThrow(UserNotFoundException::new); | ||
| Reservation reservation = reservationRepository.findById(reservationId) | ||
| .orElseThrow(ReservationNotFoundException::new); | ||
| if (!Role.SUPER_ADMIN.equals(user.getRole()) && !user.getStoreId().equals(reservation.getStore().getStoreId())) { | ||
| throw new ReservationUpdateUnauthorizedException(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
권한 검증 로직 중복 및 null 안전성 문제
이 메서드에서도 동일한 null 안전성 문제가 있으며, 권한 검증 로직이 중복됩니다.
public CallGetResponseDto updateReservationStatus(Long reservationId, ReservationStatusUpdateRequestDto requestDto,
MemberDetails memberDetails) {
User user = userRepository.findById(memberDetails.getId()).orElseThrow(UserNotFoundException::new);
Reservation reservation = reservationRepository.findById(reservationId)
.orElseThrow(ReservationNotFoundException::new);
- if (!Role.SUPER_ADMIN.equals(user.getRole()) && !user.getStoreId().equals(reservation.getStore().getStoreId())) {
+ if (!Role.SUPER_ADMIN.equals(user.getRole()) && (user.getStoreId() == null || !user.getStoreId().equals(reservation.getStore().getStoreId()))) {
throw new ReservationUpdateUnauthorizedException();
}추가로 권한 검증 로직을 별도 메서드로 추출하여 중복을 제거하는 것을 권장합니다:
private void validateUserAccess(User user, Long targetStoreId, RuntimeException exception) {
if (!Role.SUPER_ADMIN.equals(user.getRole()) &&
(user.getStoreId() == null || !user.getStoreId().equals(targetStoreId))) {
throw exception;
}
}🤖 Prompt for AI Agents
In
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/reservation/service/ReservationService.java
around lines 66 to 73, the user access validation logic is duplicated and lacks
null safety checks for user storeId. Extract this logic into a separate private
method named validateUserAccess that takes a User, a targetStoreId, and an
exception to throw. Inside this method, check if the user's role is not
SUPER_ADMIN and if the user's storeId is null or does not equal the
targetStoreId, then throw the provided exception. Replace the existing inline
access checks in updateReservationStatus with a call to this new method to
remove duplication and improve null safety.
Comment on lines
+36
to
+40
| public ReservationStatusSummaryDto getReservationListByStoreId(Long storeId, MemberDetails memberDetails) { | ||
| User user = userRepository.findById(memberDetails.getId()).orElseThrow(UserNotFoundException::new); | ||
| if (!Role.SUPER_ADMIN.equals(user.getRole()) && !user.getStoreId().equals(storeId)) { | ||
| throw new ReservationViewUnauthorizedException(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
null 안전성 검증 추가 필요
권한 검증 로직이 올바르게 구현되었으나, user.getStoreId()가 null인 경우 NullPointerException이 발생할 수 있습니다.
public ReservationStatusSummaryDto getReservationListByStoreId(Long storeId, MemberDetails memberDetails) {
User user = userRepository.findById(memberDetails.getId()).orElseThrow(UserNotFoundException::new);
- 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 ReservationViewUnauthorizedException();
}🤖 Prompt for AI Agents
In
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/reservation/service/ReservationService.java
around lines 36 to 40, the authorization check uses user.getStoreId() without
null safety, which can cause a NullPointerException if getStoreId() returns
null. To fix this, add a null check for user.getStoreId() before calling equals,
or use a null-safe comparison method to ensure no NullPointerException occurs
during the role and store ID validation.
- user.getStoreId = null 일 가능성을 고려하여 nullable-false 추가 - NPE 방지
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/reservation/service/ReservationService.java(3 hunks)nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/user/entity/User.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/reservation/service/ReservationService.java
| @Enumerated(EnumType.STRING) | ||
| private Role role; | ||
|
|
||
| @Column(nullable = false) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
기존 데이터와 데이터베이스 마이그레이션에 대한 검증이 필요합니다.
storeId 필드에 nullable = false 제약조건을 추가하는 것은 권한 검증 로직에 적합하지만, 다음 사항들을 확인해야 합니다:
- 기존 데이터베이스에
storeId가 null인 사용자 레코드가 존재하는지 확인 SUPER_ADMIN역할의 사용자들도 특정 store에 연결되어야 하는지 비즈니스 로직 검토- 데이터베이스 마이그레이션 스크립트에서 기존 null 값들을 적절히 처리하는 로직 포함
다음 스크립트를 실행하여 기존 데이터의 상태를 확인하고 마이그레이션 전략을 수립하세요:
🏁 Script executed:
#!/bin/bash
# Description: Check for existing null storeId values and SUPER_ADMIN users in the codebase
# Check for database migration files that handle this constraint
fd -e sql -e xml | xargs grep -l "storeId" | head -5
# Check for SUPER_ADMIN role usage to understand business logic
rg "SUPER_ADMIN" --type java -A 3 -B 3
# Check for User entity creation patterns to ensure storeId is always set
ast-grep --pattern 'User.builder()$$$build()'Length of output: 13134
데이터베이스 마이그레이션 및 사용자 생성 로직 검증 필요
이번 변경으로 User.storeId에 @Column(nullable = false) 제약이 추가되었으나, 다음 항목을 반드시 보강해야 합니다:
- SQL/XML 마이그레이션 파일에서
storeId제약을 처리하는 스크립트가 없습니다.
→ 기존 null 값에 대한 업데이트 혹은 기본값 부여 로직을 포함한 마이그레이션 스크립트를 추가하세요. - SUPER_ADMIN 사용자도
storeId없이 생성될 가능성을 검토해야 합니다.
→ 비즈니스 정책에 따라 SUPER_ADMIN에게 기본 storeId를 할당할지 결정하고, 관련 로직을 반영하세요. - User 객체 생성 흐름(회원가입 API, 관리자 생성 서비스 등)에서
storeId가 항상 설정되는지 확인하세요.
→ 빌더 패턴이나 생성자 호출 시 누락되는 부분이 없는지 코드 전반을 점검하고, 필요하면 유효성 검사를 강화하세요.
🤖 Prompt for AI Agents
In
nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/user/entity/User.java
at line 47, the addition of @Column(nullable = false) on User.storeId requires
updating the database migration scripts to handle existing null values by
providing default values or update logic. Review and update the user creation
logic, especially for SUPER_ADMIN users, to assign a default storeId if business
rules require it. Also, audit all User object creation points, including signup
and admin creation flows, to ensure storeId is always set, adding validation or
builder pattern checks as needed.
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