[BE] refactor: 리프레시 토큰 요청 핸들러 수정 #520
Conversation
| class AuthServiceTest { | ||
| @InjectMocks | ||
| private AuthService authService; | ||
|
|
| @@ -54,28 +66,28 @@ private Member initializeMember(final UserInfo userInfo) { | |||
| return member; | |||
| } | |||
|
|
|||
| public TokenResponse reissueAccessTokenAndRefreshToken(final Long memberId) { | |||
| public TokenResponse reissueAccessTokenAndRefreshToken(final String refreshToken) { | |||
| final Long memberId = jwtTokenProvider.getPayload(refreshToken); | |||
| final Member member = memberRepository.findById(memberId) | |||
There was a problem hiding this comment.
이부분도 커버링 인덱스 적용하면좋을거같아요!
memberId만 사용하는거같아서요
There was a problem hiding this comment.
where 절의 조건에 id만 주고, select절에 id값만 가져오는 상황이겠네요! 이 부분은 existsById()메서드를 만들어서 발생하는 쿼리를 확인해보니 아래와 같았습니다. 이 쿼리가 커버링 인덱스를 탄다고는 하는데, 실제 데이터베이스에서도 실행 계획을 살펴보면 좋을 것 같네요.
select
count(*)
from
member m1_0
where
and m1_0.id=?혹시 이와 같은 방식 말고 다른 개선된 방식이 있으면 공유해주세요 👍
| public TokenResponse login(final UserInfo userInfo, final SocialType type) { | ||
| final Member loginMember = memberRepository.findBySocialIdAndSocialType(userInfo.socialId(), type) | ||
| final Member member = memberRepository.findBySocialIdAndSocialType(userInfo.socialId(), type) |
There was a problem hiding this comment.
밑에 부분에서 RefreshToken을 저장할 때에 member 객체가 필요한 상황입니다. 커버링 인덱스로 찾아온 후에 findById()로 실제 객체를 불러오는 것이 더 좋은지 궁금합니다!ㅎㅎㅎ
There was a problem hiding this comment.
https://github.com/woowacourse-teams/2023-dong-gle/pull/424/files
이 pr참고해서 만들면될거같아요!
member의 id와 socialId를 가져와서 member를 다시 만들어서 RefreshToken을 저장한면 findBySocialIdAndSocialType 한번의 쿼리로 로직을 끝낼 수 있을거같습니다!
There was a problem hiding this comment.
오잉 저거 한참 전에 머지된 것인데, 제꺼에는 Member에서 id를 매개변수로 가지는 생성자가 public으로 안열려있네요?! 흠 일단 저도 정팩메로 하나 열어놓고 구현했습니다.
MemberRepository의 Optional<MemberInfo> findBySocialIdAndSocialType 쿼리는 다음과 같이 나가는 것을 확인했습니다.
select
m1_0.id,
m1_0.social_id
from
member m1_0
where
and m1_0.social_id=?
and m1_0.social_type=?
| public TokenResponse login(final UserInfo userInfo, final SocialType type) { | ||
| final Member loginMember = memberRepository.findBySocialIdAndSocialType(userInfo.socialId(), type) | ||
| final Member member = memberRepository.findBySocialIdAndSocialType(userInfo.socialId(), type) |
There was a problem hiding this comment.
https://github.com/woowacourse-teams/2023-dong-gle/pull/424/files
이 pr참고해서 만들면될거같아요!
member의 id와 socialId를 가져와서 member를 다시 만들어서 RefreshToken을 저장한면 findBySocialIdAndSocialType 한번의 쿼리로 로직을 끝낼 수 있을거같습니다!
|
|
||
| @ExtendWith(MockitoExtension.class) | ||
| class AuthServiceTest { | ||
| @InjectMocks |
There was a problem hiding this comment.
오 InejctMock에 대해 처음 알게됐네요. authService는 생성하는데 그 내부 필드들을 Mock으로 주입하는 거죠?
|
|
||
| @ParameterizedTest | ||
| @DisplayName("refreshToken에 대한 유효성을 검사한다.") | ||
| @MethodSource("provideValidateRefreshToken") |
There was a problem hiding this comment.
오 이 방식에 대해서도 처음 알았네요! 맛있군요..
| } catch (final ExpiredJwtException e) { | ||
| throw new ExpiredAccessTokenException(); | ||
| } catch (final JwtException | IllegalArgumentException e) { | ||
| } catch (final Exception e) { |
There was a problem hiding this comment.
궁금한게 예외를 잡고 난 뒤에 새로운 Exception을 띄어주지 않고 바로 return true;를 할꺼라면 getClaims에서 예외를 따로 구분해서 발생시킬 필요가 없지 않을까 싶어서요! 아니면 getClaims에서 예외를 따로 발생시켰을때 log에 남아서 그런걸까요?! 궁금해서 남깁니다~!
There was a problem hiding this comment.
invalidTokenUsage() 메서드에서는 원래 payload를 추출하여 리프레시 토큰의 유효기간이 지났는가에 대한 검증을 담당하는 메서드로 기억합니다. 근데 이번에 알게된게 getClaims()를 하는 와중에 리프레시 토큰의 유효기간이 터지면 알아서 예외가 터지는 것을 확인했습니다. 기존에 코드를 최대한 안고치려고 임시 방편으로 예외를 잡아 정상 흐름으로 반환 했던 것 같네요 ㅠㅠㅠ
확실하게 RefreshToken이 만료되면 라이브러리에서 정의한 ExpiredJwtException이 터지는지 확인하고 로직을 바꾸도록 하겠습니다..!!
| member = memberRepository.save(member); | ||
| memberRepository.save(member); |
There was a problem hiding this comment.
어... 한 트랜잭션 내에서의 동일한 엔티티에 대해서는 하이버네이트가 ID값을 온전하게 유지해준다고 알고 있어서 없앴었네요ㅎㅎㅎ 아래와 같은 상황입니다.
Member member = Member.of(null, "member1", ...); // null 자리가 id 라고 가정
Member savedMember = memberRepository.save(member);
assertThat(member.getId()).isEqualTo(savedMember.getId());일단 인식적으로는 이전의 코드가 더 나은 듯 하여 원복 시켜놓겠습니다.
| } catch (final ExpiredJwtException e) { | ||
| throw new ExpiredAccessTokenException(); | ||
| } catch (final JwtException | IllegalArgumentException e) { | ||
| } catch (final Exception e) { |
| public static Member of(final Long id, final MemberName memberName, final Long socialId, final SocialType socialType) { | ||
| return new Member(id, memberName, socialId, socialType); |
There was a problem hiding this comment.
위에 생성자에서 private으로 잠가주었어서, public한 정팩메를 하나 더 만들어주었습니다. 보통 정팩메를 사용할 때에는 기본 생성자는 외부에서 사용하지 못하도록 닫아 놓는 것이 관례인 것 같고, 이전에 닫아 놓은 겸 겸사겸사 정팩메를 사용하였습니다!!
There was a problem hiding this comment.
그러게용 다른 엔티티들은 대부분 생성자로 열려있는거 같은데 Member만 정팩메를 쓰고 있는 것 같아서 궁금했습니다 !
| } | ||
|
|
||
| private Jws<Claims> getClaims(final String token) { | ||
| public Jws<Claims> getClaims(final String token) { |
There was a problem hiding this comment.
여기서 토큰이 만료되면 자체적으로 ExpiredJwtException이 발생하더라구요! 위의 inValidTokenUsage()를 없애고 inValidTokenUsage()를 사용하는 곳에서는 getPayload()를 호출하도록 수정하였습니다.
HubCreator
force-pushed
the
feature/refresh-token-handling-507
branch
from
5bd2244 to
0a2ef41
Compare
* refactor: `RefreshTokenAuthInterceptor` 제거 * refactor: 'AuthController'의 ArgumentResolver 제거 및 예외 핸들링 * test: 테스트 케이스 작성 * test: 테스트 케이스 추가 * refactor: 쿼리 개선 및 테스트 케이스 추가 * refactor: `JwtTokenProvider`의 `inValidTokenUsage()` 제거 * chore: 서브모듈 업데이트 반영
🛠️ Issue
✅ Tasks
AuthController에서accessToken에 대한ArgumentResolver가 동작하지 않도록 수정AuthController에서refreshToken을 쿠키에서 추출해 사용하도록 수정RefreshTokenInterceptor삭제JwtTokenProvider예외 처리 로직 추가login과reissueAccessTokenAndRefreshToken메서드에서 함께 사용했던createToken메서드 제거⏰ Time Difference
📝 Note
login에서는 데이터베이스에refreshToken이 이미 있든 없든 새로accessToken과refreshToken을 발급해주도록 기존의 로직을 남겨 두었습니다.reissueAccessTokenAndRefreshToken에서는 기존에RefreshToken이 데이터베이스에 없으면 이는 올바르지 않은 접근이므로 예외를 날리도록 로직을 수정하였습니다.