[Spring 지하철 경로 조회 - 1,2단계] 다니(이다은) 미션 제출합니다. #57
Conversation
| return null; | ||
| // 유효한 로그인인 경우 LoginMember 만들어서 응답하기 | ||
| HttpServletRequest request = (HttpServletRequest) webRequest.getNativeRequest(); | ||
| String token = AuthorizationExtractor.extract(request); |
There was a problem hiding this comment.
중요한 건 아니지만, token 지역변수를 아래에서 한 번만 사용하기 때문에 굳이 선언해서 사용할 필요는 없을 것 같아요~
추가로, 이곳에서도 유효한 token인지 체크를 해주면 어떨까요?
jwtTokenProvider.validateToken(token);
There was a problem hiding this comment.
이 의견에서 이어져서 Interceptor에서 validateToken을 하면 Resolver에서는 validateToken을 안 해도 되지 않을까요...?!
게이츠의 의견이 궁금합니다 !
| import wooteco.subway.auth.infrastructure.JwtTokenProvider; | ||
| import wooteco.subway.exception.InvalidMemberException; | ||
| import wooteco.subway.member.dao.MemberDao; | ||
| import wooteco.subway.member.domain.Member; | ||
|
|
||
| @Service | ||
| public class AuthService { |
There was a problem hiding this comment.
서비스 layer를 사용한다면 트랜잭션도 적용해보면 좋을 것 같아요!
There was a problem hiding this comment.
질문이 하나 있는데요!
AuthService/getPayload()는 DAO를 사용하지 않으므로 클래스 단위가 아닌 메소드 단위에 @Transactional을 적용했습니다.
이렇게 일부 메소드에서만 DAO를 사용하면 지금처럼 메소드 단위로 어노테이션을 붙이면 되는 건가요??
아니면 이 경우에도 클래스 단위로 어노테이션을 붙이면 될까요?!
There was a problem hiding this comment.
사실 말씀하신 것처럼 하셔도 되요~ ^^
다만, 메소드 단위로 @ Transactional을 걸다보면 누락할 위험이 있지 않을까해서 말씀드렸어요!
| HttpServletRequest request = (HttpServletRequest) webRequest.getNativeRequest(); | ||
| String token = AuthorizationExtractor.extract(request); | ||
| Member member = authService.findMemberByToken(token); | ||
| return new LoginMember(member.getId(), member.getEmail()); |
| @Override | ||
| public void addInterceptors(InterceptorRegistry registry) { | ||
| registry.addInterceptor(loginInterceptor) | ||
| .addPathPatterns("/member/me"); |
There was a problem hiding this comment.
현재 해당 인터셉터가 적용되고 있는 걸까요? (/api/member/me)
| public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) throws Exception { | ||
| String token = AuthorizationExtractor.extract(request); | ||
| jwtTokenProvider.validateToken(token); | ||
| return super.preHandle(request, response, handler); |
There was a problem hiding this comment.
현재 LoginInterceptor는 /members/me URI에만 적용되고 있는 것 같은데요.
그런데 해당 컨트롤러 메소드는 @AuthenticationPrincipal LoginMember loginMember를 인자를 받고 있기 때문에, AuthenticationPrincipalArgumentResolver에서 validateToken을 처리하게 된다면, 해당 Interceptor가 필요없지 않을까요?
해당 인터셉터를 무조건 제거해야 돼라기 보다, 다니의 의견을 물어보는 거니 편히 답변 부탁드려요 ^^
There was a problem hiding this comment.
요청이 들어오면 Interceptor에서 먼저 validateToken을 하고,
유효하면 Resolver로 넘기고 아니면 예외를 발생하면 되겠다 생각해서 Interceptor에서 validateToken을 하도록 구현했습니다!
만약 Interceptor가 아닌 Resolver에서 토큰 유효성 검증을 하면 미리 잡을 수 있는 예외를 한 단계 더 뒤에서 잡아야 할 거 같아요.
그런데, Interceptor가 없으면 Resolver만 있게 되니까 게이츠의 의견도 맞는 것 같습니다 !
한편으로는 /api/members/me 외에 다른 URI의 권한도 확인해야 하면 Interceptor에서 토큰 검증을 하는 게 필요할 거 같습니다 !
지금 요구사항 내에서는 Resolver만 사용해도 괜찮을까요?!
|
안녕하세요, 게이츠! 다니입니다 🙌 빠른 피드백 감사합니다 👍 참고해서 리팩토링 진행했습니다 ! 리팩토링 하다가 궁금해진 점들은 피드백 아래에 👀 이모지와 함께 코멘트로 남겨뒀습니다! 이번에도 코드 리뷰 기다리고 있겠습니다! |
📚 다니의 학습 로그[Spring] HandlerMethodArgumentResolver - 4내용태그{Spring}, {HandlerMethodArgumentResolver}, {interface}, {annotation} |
| import wooteco.subway.auth.infrastructure.JwtTokenProvider; | ||
| import wooteco.subway.exception.InvalidMemberException; | ||
| import wooteco.subway.member.dao.MemberDao; | ||
| import wooteco.subway.member.domain.Member; | ||
|
|
||
| @Service | ||
| public class AuthService { |
There was a problem hiding this comment.
사실 말씀하신 것처럼 하셔도 되요~ ^^
다만, 메소드 단위로 @ Transactional을 걸다보면 누락할 위험이 있지 않을까해서 말씀드렸어요!
📚 다니의 학습 로그[Spring] Interceptor - 4내용Interceptor?
Configuration
@Override
public void addInterceptors(InterceptorRegistry registry) {
registry.addInterceptor(subwayInterceptor)
.addPathPatterns(
"/api/members/me/**",
"/api/stations/**",
"/api/lines/**",
"/api/paths"
);
}링크 |
안녕하세요, 게이츠! 다니입니다 🙌
페어 @perenok 와 함께 이번 미션 열심히 진행했습니다 !
인증, 인가에 대한 개념이 부족해서 테코톡과 학습 테스트로 먼저 공부하고 미션을 수행했어요 !
JWT, Vue.js도 처음이어서 초반에 많이 헤맸지만 페어 덕분에 잘 완료할 수 있었습니다 ~!
코드에서 부족한 부분 마구마구 지적해주세요!!!
피드백을 통해서 많이 성장하고 싶습니다 ㅎㅎㅎ
귀한 시간 내주셔서 감사합니다 🙇♀️❣️