feat(Order): 관리자 접근 제한 #64
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이 변경사항은 주문 관련 작업에 대한 권한 검증을 도입하고, 이에 맞는 예외와 예외 처리 로직을 추가합니다. 주문 조회 및 상태 변경 시 인증된 사용자의 정보를 활용해 권한을 확인하며, 관련 예외 및 에러 메시지, 예외 핸들러가 새로 정의되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant Service
participant UserRepository
participant OrderRepository
Client->>Controller: getOrderListByStoreId(storeId, memberDetails)
Controller->>Service: findAllOrders(storeId, memberDetails)
Service->>UserRepository: findById(memberDetails.id)
UserRepository-->>Service: User
Service->>Service: 권한 체크 (SUPER_ADMIN/매장 소속)
alt 권한 없음
Service->>Controller: throw OrderViewUnauthorizedException
Controller->>Client: 403 Forbidden + ErrorResponse
else 권한 있음
Service->>OrderRepository: findAllByStoreId(storeId)
OrderRepository-->>Service: OrderList
Service->>Controller: OrderResponseDtoList
Controller->>Client: 200 OK + OrderResponseDtoList
end
sequenceDiagram
participant Client
participant Controller
participant Service
participant UserRepository
participant OrderRepository
Client->>Controller: updateOrderStatus(orderId, requestDto, memberDetails)
Controller->>Service: updateOrderStatus(orderId, newStatus, memberDetails)
Service->>OrderRepository: findById(orderId)
OrderRepository-->>Service: Order or null
alt 주문 없음
Service->>Controller: throw OrderNotFoundException
Controller->>Client: 404 Not Found + ErrorResponse
else 주문 있음
Service->>UserRepository: findById(memberDetails.id)
UserRepository-->>Service: User
Service->>Service: 권한 체크 (SUPER_ADMIN/매장 소속)
alt 권한 없음
Service->>Controller: throw OrderUpdateUnauthorizedException
Controller->>Client: 403 Forbidden + ErrorResponse
else 권한 있음
Service->>OrderRepository: updateStatus(orderId, newStatus)
Service->>Controller: OrderStatusUpdateResponseDto
Controller->>Client: 200 OK + OrderStatusUpdateResponseDto
end
end
Possibly related PRs
✨ 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: 1
🧹 Nitpick comments (4)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/order/controller/OrderController.java (3)
39-41: 쉼표 뒤 공백 추가 필요코드 일관성을 위해 쉼표 뒤에 공백을 추가하세요.
- List<OrderResponseDto> response = orderService.findAllOrders(storeId,memberDetails); + List<OrderResponseDto> response = orderService.findAllOrders(storeId, memberDetails);
57-57: 애노테이션 사이 공백 추가 필요
@RequestBody와@Valid사이에 공백이 필요합니다.- @RequestBody@Valid OrderStatusUpdateRequestDto requestDto, + @RequestBody @Valid OrderStatusUpdateRequestDto requestDto,
60-61: 쉼표 뒤 공백 추가 필요코드 일관성을 위해 쉼표 뒤에 공백을 추가하세요.
- OrderStatusUpdateResponseDto response = orderService.updateOrderStatus( - orderId,requestDto.getOrderStatus(),memberDetails); + OrderStatusUpdateResponseDto response = orderService.updateOrderStatus( + orderId, requestDto.getOrderStatus(), memberDetails);nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/order/service/OrderService.java (1)
33-33: 공백 일관성 개선 필요등호 주변의 공백을 일관되게 유지하세요.
- User user = userRepository.findById(memberDetails.getId()).orElseThrow(UserNotFoundException::new); + User user = userRepository.findById(memberDetails.getId()).orElseThrow(UserNotFoundException::new);Also applies to: 45-45
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
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/order/controller/OrderController.java(4 hunks)nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/order/service/OrderService.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/order/exception/OrderNotFoundException.java(1 hunks)nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/order/exception/OrderUpdateUnauthorizedException.java(1 hunks)nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/order/exception/OrderViewUnauthorizedException.java(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/exception/GlobalExceptionHandler.java (3)
nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/order/exception/OrderNotFoundException.java (1)
OrderNotFoundException(5-9)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/order/exception/OrderViewUnauthorizedException.java (1)
OrderViewUnauthorizedException(5-9)
🔇 Additional comments (10)
nowait-common/src/main/java/com/nowait/common/exception/ErrorMessage.java (1)
22-24: 새로운 주문 관련 오류 메시지가 적절하게 추가되었습니다.추가된 오류 메시지들이 기존 패턴을 잘 따르고 있으며, 한국어 메시지가 명확하고 권한 요구사항을 잘 설명하고 있습니다. 오류 코드도 순차적으로 잘 할당되었습니다.
nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/order/exception/OrderNotFoundException.java (1)
5-9: 예외 클래스 구현이 올바르게 작성되었습니다.중앙화된 오류 메시지 시스템을 사용하여 일관성을 유지하고 있으며, RuntimeException을 적절히 확장하고 있습니다. 코드베이스의 기존 패턴을 잘 따르고 있습니다.
nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/order/exception/OrderUpdateUnauthorizedException.java (1)
5-9: 주문 수정 권한 예외 클래스가 일관성 있게 구현되었습니다.다른 권한 관련 예외 클래스들과 동일한 패턴을 따르고 있으며, 적절한 오류 메시지를 사용하고 있습니다. 구현이 간결하고 올바릅니다.
nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/order/exception/OrderViewUnauthorizedException.java (1)
5-9: 주문 조회 권한 예외 클래스가 일관성 있게 구현되었습니다.다른 새로운 예외 클래스들과 동일한 패턴을 따르고 있으며, 모든 예외 클래스들이 일관된 방식으로 구현되어 있습니다. 코드 품질이 좋습니다.
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/exception/GlobalExceptionHandler.java (4)
30-33: 새로운 예외 클래스들의 import가 적절하게 추가되었습니다.새로 추가된 주문 관련 예외 클래스들이 올바르게 import되었으며, 기존 import 순서와 일관성을 유지하고 있습니다.
131-136: OrderViewUnauthorizedException 핸들러가 적절하게 구현되었습니다.HTTP 상태 코드로 403 FORBIDDEN을 사용하는 것이 권한 없는 조회 요청에 대해 적절하며, 기존 핸들러들과 일관된 패턴을 따르고 있습니다. 로깅과 오류 응답 생성도 올바르게 구현되었습니다.
138-143: OrderNotFoundException 핸들러가 적절하게 구현되었습니다.HTTP 상태 코드로 404 NOT_FOUND를 사용하는 것이 주문을 찾을 수 없는 경우에 적절하며, RESTful API 설계 원칙을 잘 따르고 있습니다. 구현이 기존 패턴과 일관성이 있습니다.
145-150: OrderUpdateUnauthorizedException 핸들러가 적절하게 구현되었습니다.HTTP 상태 코드로 403 FORBIDDEN을 사용하는 것이 권한 없는 수정 요청에 대해 적절하며, 다른 권한 관련 예외 핸들러들과 일관된 처리 방식을 사용하고 있습니다.
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/order/service/OrderService.java (2)
45-50: 정보 누출 가능성 검토 필요현재 구현에서는 주문이 존재하지 않는 경우와 권한이 없는 경우를 구분할 수 있어 정보가 누출될 수 있습니다. 권한 검증을 먼저 수행하고 있지만, 공격자가 주문 ID의 존재 여부를 파악할 수 있습니다.
보안을 강화하려면 권한이 없는 경우 주문 존재 여부와 관계없이 동일한 예외를 발생시키는 것을 고려하세요:
public OrderStatusUpdateResponseDto updateOrderStatus(Long orderId, OrderStatus newStatus, MemberDetails memberDetails) { User user = userRepository.findById(memberDetails.getId()).orElseThrow(UserNotFoundException::new); UserOrder userOrder = orderRepository.findById(orderId) .orElseThrow(OrderNotFoundException::new); if (!Role.SUPER_ADMIN.equals(user.getRole()) && !user.getStoreId().equals(userOrder.getStore().getStoreId())) { - throw new OrderUpdateUnauthorizedException(); + throw new OrderNotFoundException(); // 권한이 없는 경우에도 주문을 찾을 수 없다고 처리 } userOrder.updateStatus(newStatus); return OrderStatusUpdateResponseDto.fromEntity(userOrder); }
33-33: UserNotFoundException 처리 확인 완료GlobalExceptionHandler (
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/exception/GlobalExceptionHandler.java) 에서 이미@ExceptionHandler(UserNotFoundException.class)와@ResponseStatus(BAD_REQUEST)를 통해 예외를 400 Bad Request로 처리하고 있으므로, 추가 조치는 필요하지 않습니다.
Comment on lines
+33
to
+36
| User user = userRepository.findById(memberDetails.getId()).orElseThrow(UserNotFoundException::new); | ||
| if (!Role.SUPER_ADMIN.equals(user.getRole()) && !user.getStoreId().equals(storeId)) { | ||
| throw new OrderViewUnauthorizedException(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
중복된 권한 검증 로직 추출 제안
권한 검증 로직이 두 메서드에서 중복되고 있습니다. 별도의 메서드로 추출하여 재사용성을 높이고 유지보수성을 개선하세요.
다음과 같이 권한 검증 메서드를 추가하세요:
private void validateUserAuthorization(MemberDetails memberDetails, Long storeId) {
User user = userRepository.findById(memberDetails.getId())
.orElseThrow(UserNotFoundException::new);
if (!Role.SUPER_ADMIN.equals(user.getRole()) && !user.getStoreId().equals(storeId)) {
throw new OrderViewUnauthorizedException();
}
}그리고 기존 메서드들을 다음과 같이 수정하세요:
public List<OrderResponseDto> findAllOrders(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 OrderViewUnauthorizedException();
- }
+ validateUserAuthorization(memberDetails, storeId);
return orderRepository.findAllByStore_StoreId(storeId).stream()
.map(OrderResponseDto::fromEntity)
.collect(Collectors.toList());
}Also applies to: 45-50
🤖 Prompt for AI Agents
In
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/order/service/OrderService.java
around lines 33-36 and 45-50, the user authorization check logic is duplicated.
Extract this logic into a private method named validateUserAuthorization that
takes MemberDetails and storeId as parameters, performs the user retrieval and
role/storeId check, and throws exceptions as needed. Then replace the duplicated
code in both locations by calls to this new method to improve reusability and
maintainability.
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
신규 기능
버그 수정