Skip to content
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

[BE] feat: 리소스 접근 권한 검증 aop 구현 #1089

Merged
merged 28 commits into from
Feb 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
745e5db
feat: 리소스 접근 관한 관리 어노테이션 생성
nayonsoso Feb 12, 2025
9fd2cb5
feat: 리소스 접근 관한에 사용될 util 코드 생성
nayonsoso Feb 12, 2025
79ea4e8
feat: 리뷰 그룹 권한 aop 구현
nayonsoso Feb 12, 2025
15f91ff
feat: 리뷰 권한 aop 구현
nayonsoso Feb 12, 2025
5c671b9
feat: 리뷰그룹 접근 권한 aop 구현
nayonsoso Feb 12, 2025
0e4c5ac
test: 권한 aop 테스트
nayonsoso Feb 12, 2025
61f5260
test: 서비스 코드의 테스트에 aop가 동작하지 않도록 설정 변경
nayonsoso Feb 12, 2025
720db84
refactor: 하이라이트 수정 시, ReviewGroupSession 을 사용하지 않도록
nayonsoso Feb 12, 2025
a849300
refactor: 단일리뷰 조회 시, ReviewGroupSession 을 사용하지 않도록
nayonsoso Feb 12, 2025
190ca47
refactor: 리뷰 요약 조회 시, ReviewGroupSession 을 사용하지 않도록
nayonsoso Feb 12, 2025
9e753a0
refactor: 리뷰 목록 조회 시, ReviewGroupSession 을 사용하지 않도록
nayonsoso Feb 12, 2025
b06827a
refactor: 리뷰 모아보기 시, ReviewGroupSession 을 사용하지 않도록
nayonsoso Feb 12, 2025
71fb966
test: 사용하지 않는 테스트 삭제
nayonsoso Feb 12, 2025
d11211e
refactor: 권한 검증 순서 변경
nayonsoso Feb 12, 2025
5748b3c
test: 관련된 테스트 코드끼리 분류
nayonsoso Feb 12, 2025
3b8983f
test: 테스트 설정 수정
nayonsoso Feb 12, 2025
73ad384
refactor: 서비스가 아닌 컨트롤러에서 권한 검증하도록
nayonsoso Feb 12, 2025
7a87f90
refactor: aspect에 사용된 트랜잭션 제거
nayonsoso Feb 15, 2025
1cc9bf8
refactor: 회원이 만든 리뷰인지 확인하는 로직 수정
nayonsoso Feb 15, 2025
d83de97
refactor: 사용되는 예외 변경
nayonsoso Feb 15, 2025
3f94d5a
refactor: 권한 관련 예외와 함수 이름 변경
nayonsoso Feb 15, 2025
8cfdbf3
refactor: forbidden 예외 핸들러 추가
nayonsoso Feb 15, 2025
120be95
refactor: 패키지 변경
nayonsoso Feb 15, 2025
f3ccc13
fix: import 추가
nayonsoso Feb 15, 2025
de1ff1c
test: 변경된 픽스쳐 적용
nayonsoso Feb 15, 2025
e82eca8
style: 레포지토리 함수 나열 순서 변경
nayonsoso Feb 16, 2025
5fb0127
refactor: 비회원 리뷰 접근 권한 확인 로직 변경
nayonsoso Feb 16, 2025
3362087
test: 테스트 클래스 패키지 변경
nayonsoso Feb 16, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import reviewme.global.exception.BadRequestException;
import reviewme.global.exception.DataInconsistencyException;
import reviewme.global.exception.FieldErrorResponse;
import reviewme.global.exception.ForbiddenException;
import reviewme.global.exception.NotFoundException;
import reviewme.global.exception.UnauthorizedException;

Expand All @@ -45,6 +46,11 @@ public ProblemDetail handleUnauthorizedException(UnauthorizedException ex) {
return ProblemDetail.forStatusAndDetail(HttpStatus.UNAUTHORIZED, ex.getErrorMessage());
}

@ExceptionHandler(ForbiddenException.class)
public ProblemDetail handleForbiddenException(UnauthorizedException ex) {
return ProblemDetail.forStatusAndDetail(HttpStatus.FORBIDDEN, ex.getErrorMessage());
}

@ExceptionHandler(DataInconsistencyException.class)
public ProblemDetail handleDataConsistencyException(DataInconsistencyException ex) {
return ProblemDetail.forStatusAndDetail(HttpStatus.INTERNAL_SERVER_ERROR, ex.getErrorMessage());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package reviewme.global.exception;

public abstract class ForbiddenException extends ReviewMeException {

public ForbiddenException(String errorMessage) {
super(errorMessage);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,7 @@
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RestController;
import reviewme.security.resolver.GuestReviewGroupSession;
import reviewme.security.resolver.LoginMemberSession;
import reviewme.security.resolver.dto.GuestReviewGroup;
import reviewme.security.resolver.dto.LoginMember;
import reviewme.security.aspect.RequireReviewGroupAccess;
import reviewme.highlight.service.HighlightService;
import reviewme.highlight.service.dto.HighlightsRequest;

Expand All @@ -20,14 +17,10 @@ public class HighlightController {
private final HighlightService highlightService;

@PostMapping("/v2/highlight")
@RequireReviewGroupAccess(target = "#request.reviewGroupId()")
public ResponseEntity<Void> highlight(
@Valid @RequestBody HighlightsRequest request,
@LoginMemberSession(required = false) LoginMember loginMember,
@GuestReviewGroupSession(required = false) GuestReviewGroup guestReviewGroup
@Valid @RequestBody HighlightsRequest request
) {
/*
TODO : aop 인증 로직 필요 (존재하는 세션에 대해 reviewGroupId와 일치 여부 확인)
*/
highlightService.editHighlight(request);
return ResponseEntity.ok().build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;
import reviewme.security.resolver.GuestReviewGroupSession;
import reviewme.security.resolver.LoginMemberSession;
import reviewme.security.resolver.dto.GuestReviewGroup;
import reviewme.security.resolver.dto.LoginMember;
import reviewme.security.aspect.RequireReviewAccess;
import reviewme.security.aspect.RequireReviewGroupAccess;
import reviewme.review.service.ReviewDetailLookupService;
import reviewme.review.service.ReviewGatheredLookupService;
import reviewme.review.service.ReviewListLookupService;
Expand Down Expand Up @@ -48,57 +48,40 @@ public ResponseEntity<Void> createReview(
}

@GetMapping("/v2/groups/{reviewGroupId}/reviews/received")
@RequireReviewGroupAccess(target = "#reviewGroupId")
public ResponseEntity<ReceivedReviewPageResponse> findReceivedReviews(
@PathVariable long reviewGroupId,
@RequestParam(required = false) Long lastReviewId,
@RequestParam(required = false) Integer size,
@LoginMemberSession(required = false) LoginMember loginMember,
@GuestReviewGroupSession(required = false) GuestReviewGroup guestReviewGroup
@RequestParam(required = false) Integer size
) {
/*
TODO : aop 인증 로직 필요 (존재하는 세션에 대해 reviewGroupId와 일치 여부 확인)
*/
ReceivedReviewPageResponse response = reviewListLookupService.getReceivedReviews(reviewGroupId, lastReviewId,
size);
ReceivedReviewPageResponse response = reviewListLookupService.getReceivedReviews(reviewGroupId, lastReviewId, size);
return ResponseEntity.ok(response);
}

@GetMapping("/v2/reviews/{id}")
@RequireReviewAccess(target = "#id")
public ResponseEntity<ReviewDetailResponse> findReceivedReviewDetail(
@PathVariable long id,
@LoginMemberSession(required = false) LoginMember loginMember,
@GuestReviewGroupSession(required = false) GuestReviewGroup guestReviewGroup
@PathVariable long id
) {
/*
TODO : aop 인증 로직 필요 (존재하는 세션에 대해 reviewId와 일치 여부 확인)
*/
ReviewDetailResponse response = reviewDetailLookupService.getReviewDetail(id);
return ResponseEntity.ok(response);
}

@GetMapping("/v2/groups/{reviewGroupId}/reviews/summary")
@RequireReviewGroupAccess(target = "#reviewGroupId")
public ResponseEntity<ReceivedReviewsSummaryResponse> findReceivedReviewOverview(
@PathVariable long reviewGroupId,
@LoginMemberSession(required = false) LoginMember loginMember,
@GuestReviewGroupSession(required = false) GuestReviewGroup guestReviewGroup
@PathVariable long reviewGroupId
) {
/*
TODO : aop 인증 로직 필요 (존재하는 세션에 대해 reviewGroupId와 일치 여부 확인)
*/
ReceivedReviewsSummaryResponse response = reviewSummaryService.getReviewSummary(reviewGroupId);
return ResponseEntity.ok(response);
}

@GetMapping("/v2/groups/{reviewGroupId}/reviews/gather")
@RequireReviewGroupAccess(target = "#reviewGroupId")
public ResponseEntity<ReviewsGatheredBySectionResponse> getReceivedReviewsBySectionId(
@PathVariable long reviewGroupId,
@RequestParam("sectionId") long sectionId,
@LoginMemberSession(required = false) LoginMember loginMember,
@GuestReviewGroupSession(required = false) GuestReviewGroup guestReviewGroup
@RequestParam("sectionId") long sectionId
) {
/*
TODO : aop 인증 로직 필요 (존재하는 세션에 대해 reviewGroupId와 일치 여부 확인)
*/
ReviewsGatheredBySectionResponse response =
reviewGatheredLookupService.getReceivedReviewsBySectionId(reviewGroupId, sectionId);
return ResponseEntity.ok(response);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ public class ReviewDetailLookupService {
public ReviewDetailResponse getReviewDetail(long reviewId) {
Review review = reviewRepository.findById(reviewId)
.orElseThrow(() -> new ReviewNotFoundException(reviewId));

ReviewGroup reviewGroup = reviewGroupRepository.findById(review.getReviewGroupId())
.orElseThrow(() -> new ReviewGroupNotFoundException(review.getReviewGroupId()));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
import org.springframework.transaction.annotation.Transactional;
import reviewme.review.repository.ReviewRepository;
import reviewme.review.service.dto.response.list.AuthoredReviewsResponse;
import reviewme.review.service.dto.response.list.ReceivedReviewPageResponse;
import reviewme.review.service.dto.response.list.ReceivedReviewPageElementResponse;
import reviewme.review.service.dto.response.list.ReceivedReviewPageResponse;
import reviewme.review.service.mapper.ReviewListMapper;
import reviewme.reviewgroup.domain.ReviewGroup;
import reviewme.reviewgroup.domain.exception.ReviewGroupNotFoundException;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
public class ReviewNotFoundException extends NotFoundException {

public ReviewNotFoundException(long reviewId) {
super("리뷰를 찾을 수 없어요");
log.info("Review not found - reviewId: {}", reviewId);
super("리뷰를 찾을 수 없어요.");
log.info("Review not found. - reviewId: {}", reviewId);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,8 @@ rg.id, rg.reviewee, rg.projectName, rg.reviewRequestCode, rg.createdAt, COUNT(r.
List<ReviewGroupPageElementResponse> findByMemberIdWithLimit(long memberId, Long lastReviewGroupId, int limit);

boolean existsByReviewRequestCode(String reviewRequestCode);

boolean existsByIdAndReviewRequestCode(long id, String reviewRequestCode);

boolean existsByIdAndMemberId(long id, long memberId);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package reviewme.security.aspect;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

@Target(ElementType.METHOD)
@Retention(RetentionPolicy.RUNTIME)
public @interface RequireReviewAccess {

String target() default "#reviewId";
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package reviewme.security.aspect;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

@Target(ElementType.METHOD)
@Retention(RetentionPolicy.RUNTIME)
public @interface RequireReviewGroupAccess {

String target() default "#reviewGroupId";
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package reviewme.security.aspect;

import jakarta.servlet.http.HttpSession;
import java.util.Objects;
import org.aspectj.lang.ProceedingJoinPoint;
import org.aspectj.lang.reflect.MethodSignature;
import org.springframework.core.convert.TypeDescriptor;
import org.springframework.expression.Expression;
import org.springframework.expression.ExpressionParser;
import org.springframework.expression.spel.standard.SpelExpressionParser;
import org.springframework.expression.spel.support.StandardEvaluationContext;
import org.springframework.web.context.request.RequestContextHolder;
import org.springframework.web.context.request.ServletRequestAttributes;
import reviewme.security.aspect.exception.SpELEvaluationFailedException;

public class ResourceAuthorizationUtils {

private static final ExpressionParser EXPRESSION_PARSER = new SpelExpressionParser();

public static <T> T getTarget(ProceedingJoinPoint joinPoint, String targetExpression, Class<T> targetType) {
Object[] args = joinPoint.getArgs();
MethodSignature signature = (MethodSignature) joinPoint.getSignature();
StandardEvaluationContext context = new StandardEvaluationContext();
String[] paramNames = signature.getParameterNames();
for (int i = 0; i < paramNames.length; i++) {
context.setVariable(paramNames[i], args[i]);
}

try {
Expression expression = EXPRESSION_PARSER.parseExpression(targetExpression);
TypeDescriptor actualTypeDescriptor = expression.getValueTypeDescriptor(context);
TypeDescriptor expectedTypeDescriptor = TypeDescriptor.valueOf(targetType);
if (actualTypeDescriptor == null || !expectedTypeDescriptor.isAssignableTo(actualTypeDescriptor)) {
throw new SpELEvaluationFailedException(signature.getMethod().getName(), targetExpression);
}
T targetValue = expression.getValue(context, targetType);
return Objects.requireNonNull(targetValue);
} catch (Exception e) {
throw new SpELEvaluationFailedException(signature.getMethod().getName(), targetExpression);
}
}

public static HttpSession getCurrentSession() {
return ((ServletRequestAttributes) RequestContextHolder.currentRequestAttributes())
.getRequest()
.getSession(false);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package reviewme.security.aspect;

import static reviewme.security.aspect.ResourceAuthorizationUtils.getCurrentSession;
import static reviewme.security.aspect.ResourceAuthorizationUtils.getTarget;

import jakarta.servlet.http.HttpSession;
import lombok.RequiredArgsConstructor;
import org.aspectj.lang.ProceedingJoinPoint;
import org.aspectj.lang.annotation.Around;
import org.aspectj.lang.annotation.Aspect;
import org.springframework.stereotype.Component;
import reviewme.auth.domain.GitHubMember;
import reviewme.review.domain.Review;
import reviewme.review.repository.ReviewRepository;
import reviewme.review.service.exception.ReviewNotFoundException;
import reviewme.reviewgroup.repository.ReviewGroupRepository;
import reviewme.security.aspect.exception.ForbiddenReviewAccessException;
import reviewme.security.session.SessionManager;

@Aspect
@Component
@RequiredArgsConstructor
public class ReviewAuthorizationAspect {

private final SessionManager sessionManager;
private final ReviewRepository reviewRepository;
private final ReviewGroupRepository reviewGroupRepository;

@Around("@annotation(requireReviewAccess)")
public Object checkReviewAccess(ProceedingJoinPoint joinPoint,
RequireReviewAccess requireReviewAccess) throws Throwable {
HttpSession session = getCurrentSession();
if (session == null) {
throw new ForbiddenReviewAccessException();
}

long reviewId = getTarget(joinPoint, requireReviewAccess.target(), Long.class);
Review review = reviewRepository.findById(reviewId)
.orElseThrow(() -> new ReviewNotFoundException(reviewId));
if (!(canMemberAccess(review, session) || canGuestAccess(review, session))) {
throw new ForbiddenReviewAccessException();
}

return joinPoint.proceed();
}

private boolean canMemberAccess(Review review, HttpSession session) {
GitHubMember gitHubMember = sessionManager.getGitHubMember(session);
if (gitHubMember == null) {
return false;
}

boolean isReviewGroupCreator = reviewGroupRepository.existsByIdAndMemberId(
review.getReviewGroupId(), gitHubMember.getMemberId()
);
boolean isReviewAuthor = review.getMemberId() != null && review.getMemberId() == gitHubMember.getMemberId();

return isReviewGroupCreator || isReviewAuthor;
}

private boolean canGuestAccess(Review review, HttpSession session) {
String reviewRequestCode = sessionManager.getReviewRequestCode(session);
if (reviewRequestCode == null) {
return false;
}

return reviewGroupRepository.existsByIdAndReviewRequestCode(review.getReviewGroupId(), reviewRequestCode);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package reviewme.security.aspect;

import static reviewme.security.aspect.ResourceAuthorizationUtils.getCurrentSession;
import static reviewme.security.aspect.ResourceAuthorizationUtils.getTarget;

import jakarta.servlet.http.HttpSession;
import lombok.RequiredArgsConstructor;
import org.aspectj.lang.ProceedingJoinPoint;
import org.aspectj.lang.annotation.Around;
import org.aspectj.lang.annotation.Aspect;
import org.springframework.stereotype.Component;
import reviewme.auth.domain.GitHubMember;
import reviewme.security.aspect.exception.ForbiddenReviewGroupAccessException;
import reviewme.reviewgroup.domain.ReviewGroup;
import reviewme.reviewgroup.domain.exception.ReviewGroupNotFoundException;
import reviewme.reviewgroup.repository.ReviewGroupRepository;
import reviewme.security.session.SessionManager;

@Aspect
@Component
@RequiredArgsConstructor
public class ReviewGroupAuthorizationAspect {

private final SessionManager sessionManager;
private final ReviewGroupRepository reviewGroupRepository;

@Around("@annotation(requireReviewGroupAccess)")
public Object checkReviewGroupAccess(ProceedingJoinPoint joinPoint,
RequireReviewGroupAccess requireReviewGroupAccess) throws Throwable {
HttpSession session = getCurrentSession();
if (session == null) {
throw new ForbiddenReviewGroupAccessException();
}

long reviewGroupId = getTarget(joinPoint, requireReviewGroupAccess.target(), Long.class);
ReviewGroup reviewGroup = reviewGroupRepository.findById(reviewGroupId)
.orElseThrow(() -> new ReviewGroupNotFoundException(reviewGroupId));
if (!(canMemberAccess(reviewGroup, session) || canGuestAccess(reviewGroup, session))) {
throw new ForbiddenReviewGroupAccessException();
}

return joinPoint.proceed();
}

private boolean canMemberAccess(ReviewGroup reviewGroup, HttpSession session) {
GitHubMember gitHubMember = sessionManager.getGitHubMember(session);
return gitHubMember != null && reviewGroup.getMemberId() == gitHubMember.getMemberId();
}

private boolean canGuestAccess(ReviewGroup reviewGroup, HttpSession session) {
String reviewRequestCode = sessionManager.getReviewRequestCode(session);
return reviewGroup.getReviewRequestCode().equals(reviewRequestCode);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package reviewme.security.aspect.exception;

import lombok.extern.slf4j.Slf4j;
import reviewme.global.exception.ForbiddenException;

@Slf4j
public class ForbiddenReviewAccessException extends ForbiddenException {

public ForbiddenReviewAccessException() {
super("리뷰에 접근할 권한이 없어요.");
log.info("Forbidden review access occurred.");
}
}
Loading