-
Notifications
You must be signed in to change notification settings - Fork 74
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
인가(Authorization) 구현하기 #79
Conversation
- 간략한 패키지 구조 |-- common | |-- config | |-- exception | |-- interceptors | `-- utils |-- product (모든 도메인 패키지 구조 동일) |-- application | `-- exception |-- domain |-- infra `-- presentation `-- dto
이번 주 작업 관련해서 한가지 궁금한 점이 있습니다! 따라서 실 객체 테스트를 진행할 경우 테스트 환경이 무거워지지만, 검증은 확실하게 보장받을 수 있을 것 같아서 Controller에서 실제 spring 환경을 띄우는 통합 테스트를 진행하면 좋을 것 같다는 생각을 하였는데 해당 의견에 대해 멘토님은 어떻게 생각하시는지 궁금합니다! |
- JsonUtil: Jackson 라이브러리를 사용한 객체 매핑 유틸 - MockMvcCharacterEncodingCustomizer: mockMvc 테스트 결과 값 인코딩 UTF-8로 설정
@BeforeEach | ||
void setUp() { | ||
given(authenticationService.parseToken(eq(INVALID_VALUE_TOKEN_1.토큰_값()))) | ||
.willThrow(new InvalidTokenException(INVALID_VALUE_TOKEN_1.토큰_값())); | ||
} |
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.
한가지 궁금한 점이 있습니다
컨트롤러 전체 테스트를 실행할 때 위 코드의 given절 매개변수에 eq()를 빼면 하나 빼고 모든 테스트가 다 터지는 현상이 발생하는데, 이해가 잘 가지 않습니다..!
@BeforeEach
void setUp() {
given(authenticationService.parseToken(INVALID_VALUE_TOKEN_1.토큰_값()))
.willThrow(new InvalidTokenException(INVALID_VALUE_TOKEN_1.토큰_값()));
}
근데 또 단일로 하나씩 실행되면 실패했던 테스트들이 모두 성공합니다....
그래서 더 혼란스럽습니다
추가로 모든 테스트로 들어오기도 전에 given절에서 아래와 같은 동일한 에러로 다 터져버립니다
디버깅을 걸어봐도 테스트 자체로 들어오지도 않습니다
하루 종일 찾아보아도 오류의 원인을 모르겠습니다 😂 (Nested 문제인가...)
Invalid token: eyJhbGciOiJIUzI1NiJ9.eyJ1c2VySWQiOjF9.ZZ3CUxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
com.codesoom.assignment.session.application.exception.InvalidTokenException: Invalid token: eyJhbGciOiJIUzI1NiJ9.eyJ1c2VySWQiOjF9.ZZ3CUxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
at com.codesoom.assignment.product.presentation.ProductControllerMockTest.setUp(ProductControllerMockTest.java:68)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:566)
at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:686)
at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
at org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131)
at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:149)
at org.junit.jupiter.engine.extension.TimeoutExtension.interceptLifecycleMethod(TimeoutExtension.java:126)
at org.junit.jupiter.engine.extension.TimeoutExtension.interceptBeforeEachMethod(TimeoutExtension.java:76)
at org.junit.jupiter.engine.execution.ExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(ExecutableInvoker.java:115)
at org.junit.jupiter.engine.execution.ExecutableInvoker.lambda$invoke$0(ExecutableInvoker.java:105)
at org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:106)
at org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:64)
at org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:45)
at org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:37)
at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:104)
at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:98)
at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.invokeMethodInExtensionContext(ClassBasedTestDescriptor.java:481)
at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.lambda$synthesizeBeforeEachMethodAdapter$18(ClassBasedTestDescriptor.java:466)
at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeBeforeEachMethods$2(TestMethodTestDescriptor.java:169)
at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeBeforeMethodsOrCallbacksUntilExceptionOccurs$5(TestMethodTestDescriptor.java:197)
at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeBeforeMethodsOrCallbacksUntilExceptionOccurs(TestMethodTestDescriptor.java:197)
at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeBeforeEachMethods(TestMethodTestDescriptor.java:166)
at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:133)
at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:71)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:135)
at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
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.
이 문제 혹시 해결하셨나요? 저한테는 재현이 되질 않네요
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.
ProductControllerMockTest
의 맨 상단(62 Line)에 있는 @BeforeEach
부분에서 eq()
를 빼고 테스트 진행 시에 위에 같은 에러가 터집니다!
@BeforeEach
void setUp() {
given(authenticationService.parseToken(INVALID_VALUE_TOKEN_1.토큰_값()))
.willThrow(new InvalidTokenException(INVALID_VALUE_TOKEN_1.토큰_값()));
}
근데 단순 문자열인데 eq()가 있고 없고의 차이점을 아무리 찾아봐도 잘 모르겠습니다..! 😂
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.
흠... 정확하게는 모르겠지만, 일단 eq()
없이 실행해보니 parseToken
이 실행되서 InvalidTokenException
예외가 발생하는 것이 아니라, 저 코드 자체에서 예외가 던져지더라고요. 아직 파악은 다 못했습니다.
doThrow(new InvalidTokenException(INVALID_VALUE_TOKEN_1.토큰_값()))
.when(authenticationService).parseToken(INVALID_VALUE_TOKEN_1.토큰_값());
이렇게 하면 eq
에 상관없이 테스트가 통과하는데, 어떤 차이가 있는지 봐야겠습니다
See also
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.
아 이 부분은 이유를 정말 모르겠습니다...
그냥 무조건 given을 사용할 때는 eq()
나 any()
를 붙여주는게 좋을까요..?
app/src/main/java/com/codesoom/assignment/user/presentation/UserController.java
Show resolved
Hide resolved
app/src/test/java/com/codesoom/assignment/product/application/ProductServiceTest.java
Show resolved
Hide resolved
app/src/test/java/com/codesoom/assignment/product/application/ProductServiceTest.java
Show resolved
Hide resolved
테스트는 얼마나 믿고 사용할 수 있는지가 중요합니다. 충분히 테스트가 안된다고 생각한다면, 둘다 진행해야 될 것 같습니다 |
7주차 과제 목표 중 아래의 문구가 잘 이해가 되지 않습니다!
Security를 통해 Auth 토큰을 디코딩하면 나오는 userId와 |
- 기존 URI를 직접 체크하여 인증 여부를 확인했던 방식을 `@PreAuthorize("isAuthenticated()")` 방식으로 변경 - `@PreAuthorize`가 선언되어 있는 컨트롤러만 인증(Auth) 진행
#79 (comment) 에서 해결된 문제이므로 토글처리 하겠습니다 :) 토글 접기/펼치기(db9bc7b, b88ccbe) 해당 수정 내용에서도 동작의 이해가 되지 않는 부분이 있어서 주석으로 관련 내용을 작성해보았습니다 이 부분도 위의 질문이랑 비슷한 느낌입니다..!
주석으로 남겨논 코드의 예시입니다. (verify 구문을 주석하면 정상적으로 테스트가 통과되니 더욱 이해가 되지 않습니다..) @Nested
@DisplayName("유효하지 않은 인증 토큰이 주어지면")
class Context_with_invalid_token {
@Test
@DisplayName("401 코드로 응답한다")
void it_responses_401() throws Exception {
ResultActions perform = 회원_수정_API_요청(
INVALID_VALUE_TOKEN_1.인증_헤더값(),
ID_MIN.value(),
USER_1
);
perform.andExpect(status().isUnauthorized());
// 아니 왜 해당 테스트를 단일로 실행하면 통과 되는데, 전체 실행을 하면 invoke가 됐다고 실패가 되지...?
// 해당 구문은 isAuthenticated()에서 실패돼서 컨트롤러로 못들어오는 로직이여야 하는데..
// 심지어 위에 status().isUnauthorized() 코드는 검증 성공으로 뜸.... (verfiy 구문 주석하면 테스트 올패스)
verify(userService, never()).updateUser(eq(ID_MIN.value()), any(UserModificationData.class));
}
} 이게 제가 코드를 잘 못 짜거나 그런거면 에러문구를 통해서 찾으면 되는데, 상식적으로는 정상인 상황인데도 안되는 것 같아서 더 미치겠습니다... |
저는 처음에 debug모드로 실행해서 그래서 첫 번째 제 가설은 그래서 그래서 찾아보니 @BeforeEach
void setUp() {
Mockito.reset(userService);
given(authenticationService.parseToken(eq(VALID_TOKEN_1.토큰_값())))
.willReturn(VALID_TOKEN_1.아이디());
given(authenticationService.parseToken(eq(INVALID_VALUE_TOKEN_1.토큰_값())))
.willThrow(new InvalidTokenException(INVALID_VALUE_TOKEN_1.토큰_값()));
} See also |
관련 피드백에서 궁금한 점이 있습니다. 따라서 아래처럼 각 테스트 별로 검증에 집중해야 할 요소들을 정리해보았는데, 해당 내용에 대한 피드백을 부탁드립니다! Controller MockMvc Test
Service Mock Test
Controller 실 객체 통합 테스트
Service 실 객체 테스트
|
추가로 패키지 구조 관련 질문입니다. 현재 패키지 구조를 아래와 같이 구성해보았는데, 더 좋은 구조가 있을지 궁금합니다! |-- common
| |-- authentication
| | |-- config
| | |-- filters
| | `-- security
| |-- exception (도메인 별 ExceptionHandler 모아두는 곳)
| `-- utils
|-- product
|-- application
| `-- exception (도메인 별 Exception: 예외를 던지는 주체가 application이므로)
|-- domain
|-- infra
`-- presentation
`-- dto |
해당 오류 관련하여 분석 후 블로그에 정리해보았습니다! 요약하자면 |
- 기존에는 각 필드별로 세세히 검증했지만, Controller 테스트의 목적은 반환 값 검증이 메인이 아님 - 응답 상태 코드 검증 및 내부 메서드 호출 검증에 집중
어떤 것을 테스트해야 할 지 많이 고민한 흔적이 보이네요. 그러다보니 테스트 코드를 작성하다가 어떤 패턴이 보이셨나봐요. 검증도 중요하지만 저는 테스트코드가 내가 작성하는 코드의 예제라고 생각해요. 내가 작성한 코드는 이렇게 사용하는거야 라고 보여주는 거죠. |
패키지 구조에 대해서는 일관성만 있으면 저는 어떤 구조도 상관없다고 생각해요. 다만 하나 제가 신경쓰는 부분은 다른 패키지들과 어떤 관계를 맺는지를 많이 봅니다. 예를들어서 상품 도메인에서 자꾸 다른 사용자 도메인에 어떤 패키지를 사용한다던가 하면 이상한 징조로 바라보고 구조를 변경해야 하는 신호로 받아들입니다. |
저도 처음 알았습니다 ㅎㅎ 공유해 주셔서 감사해요. 코드숨 채널에도 한 번 공유해 주세요! |
과제 중에 아래와 같이 수정 API Controller 메서드 3번째 매개변수에 @PatchMapping("{id}")
@PreAuthorize("isAuthenticated()")
UserResultData update(@PathVariable final Long id,
@RequestBody @Valid final UserModificationData modificationData,
@IdVerification final UserAuthentication userAuthentication) {
User user = userService.updateUser(id, modificationData);
return getUserResultData(user);
} 하지만 아래의 AOP 코드에서 분명 매개변수에 @Before("execution(* *..UserController*.*(..)))")
public void validateUserId(JoinPoint joinPoint) {
boolean isVerified = Arrays.stream(joinPoint.getArgs())
.filter(this::isIdVerification)
.findAny()
.isPresent();
if (isVerified) {
// 토큰 userId와 요청 데이터 userId 비교 검증
}
}
private boolean isIdVerification(final Object parameter) {
// 여기에서 `@IdVerification `을 찾지 못함....
return parameter.getClass().isAnnotationPresent(IdVerification.class);
} 아래는 @Target(ElementType.METHOD)
@Retention(RetentionPolicy.RUNTIME)
public @interface IdVerification {
} 혹시 도움을 좀 주실 수 있으신가요? |
userAuthentication에 그래서 위 사실을 검증하기 위해서 @UserController.IdVerification
public class UserAuthentication extends AbstractAuthenticationToken {
// ... 생략
} 그랬더니 다음과 같이 true를 반환하는 것을 확인했습니다. 근데 생각해보니 |
/** | ||
* Access Token을 통해 전달되는 id와 @PathVariable을 통해 전달되는 id를 서로 비교합니다. | ||
* | ||
* @param joinPoint 메서드 관련 정보 | ||
* @throws AccessDeniedException id가 존재하지 않거나 서로 다를 때 던집니다. | ||
*/ | ||
@Before("idVerificationMethod()") | ||
private void validateId(final JoinPoint joinPoint) throws AccessDeniedException { | ||
Long requestId = getIdByPathVariable(joinPoint); | ||
Long tokenId = getIdByUserAuthentication(joinPoint); | ||
|
||
if (!requestId.equals(tokenId)) { | ||
throw new AccessDeniedException(""); | ||
} | ||
} | ||
|
||
|
||
// TODO: Controller 매개변수에 Long 타입이 두개 이상일 경우를 고려해야함 | ||
// @PathVariable 값을 가져오는 방법으로 수정하기 | ||
private static Long getIdByPathVariable(final JoinPoint joinPoint) throws AccessDeniedException { | ||
return Arrays.stream(joinPoint.getArgs()) | ||
.filter(Long.class::isInstance) | ||
.map(Long.class::cast) | ||
.findFirst() | ||
.orElseThrow(() -> new AccessDeniedException("")); | ||
} |
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.
만약 UserController 메서드의 매개변수 중 userId를 제외한 Long 타입이 추가로 선언될 경우 해당 방식은 위험해보입니다.
@PathVariable
의 id값을 가져오려고 하루종일 시도해봤는데 실패하였습니다ㅠㅠ
조언을 좀 구해도 될까요?
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.
@PathVariable
이 붙은 id만 찾으려고 하시는군요 한 번 저도 고민해 보겠습니다
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.
첫 번째로 든 생각은 PathVariable이 꼭 Long이어야 하는가? 입니다. Long타입이 아니라, Long을 감싼 다른 객체를 이용하면, 이 객체가 PathVariable인지 알 것 같아요.
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.
아하 그렇다면 @PathVariable
로 받아온 id를 특정 객체로 집어넣는 방법을 찾아야겠군요
@PatchMapping("{id}") | ||
@PreAuthorize("isAuthenticated()") | ||
@IdVerification | ||
UserResultData update(@PathVariable final Long id, | ||
@RequestBody @Valid final UserModificationData modificationData, | ||
final Authentication authentication) { | ||
final UserAuthentication userAuthentication) { | ||
User user = userService.updateUser(id, modificationData); | ||
return getUserResultData(user); | ||
} |
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.
update 메서드의 3번째 매개변수인 final UserAuthentication userAuthentication
는 사용하지 않는데 AOP를 위해 선언하는것은 바람직하지 않다고 생각합니다.
AOP에서 UserAuthentication 값을 갖고올만한 방법이 있을까요?
DI로 주입받으려고 시도해봤는데 UserAuthentication
객체에 @Component
를 선언하면 userId 멤버에서 오류가 났습니다..
@Component
public class UserAuthentication extends AbstractAuthenticationToken {
private final Long userId;
// 생성자 매개변수 userId에서 컴파일 에러 (Could not autowire. No beans of 'Long' type found.)
public UserAuthentication(Long userId) {
super(authorities());
this.userId = userId;
}
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.
@Component
가 의미하는 것이 무엇인지 먼저 알아야 할 것 같아요
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.
UserAuthentication
객체를 Spring bean으로 등록하기 위해 설정하였는데 좀 더 자세히 알아봐야겠네요!
안녕하세요 :)
이번 주는 아래와 같이 작업을 진행하면서 연습해볼 생각입니다!
AccessToken
의 userId와@PathVariable
의 userId 비교 검증이번 주도 잘 부탁드립니다!! 🙇♂️🙇♂️