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

feat(#45): redis 모듈 추가 #49

Merged
merged 20 commits into from
Sep 26, 2023
Merged

Conversation

jusung-c
Copy link
Collaborator

@jusung-c jusung-c commented Sep 13, 2023




  • redis 모듈 추가
  • auth-api에 redis 모듈 의존성 추가
  • database.yml 포트 설정 (일단 test, local 서버만 설정)
  • redisConfig




  • Refresh Token 발급 구현
  • Refresh Token 관리 with Redis
  • Refresh Token 재발급 구현




  • Refreash Token CRUD 테스트
  • logout 테스트
  • Refresh Token 재발급 테스트




관련 이슈
#45

@jusung-c jusung-c added the chore 빌드 과정 또는 보조 기능 (문서 생성 기능 등) 수정 label Sep 13, 2023
@ghdcksgml1
Copy link
Owner

redisConfig 파일까지 작성되면 Merge 하겠습니다~

@jusung-c jusung-c requested a review from ghdcksgml1 September 14, 2023 00:33
Copy link
Owner

@ghdcksgml1 ghdcksgml1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다! ㅎㅎ 피드백 드린 부분과 accessToken 만료 시 refreshToken을 활용해서 accessToken을 재발급하는 로직만 추가하면 완벽할 것 같네요 😁

@jusung-c jusung-c force-pushed the feat(#45)-heachi-domain-redis branch from 03a4b2d to 0ac592c Compare September 20, 2023 15:34
Copy link
Owner

@ghdcksgml1 ghdcksgml1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

점점 실력이 느는 것 같아 아주 뿌듯하군요!

리뷰 남겼습니다. 추가로 기능 구현 되시면, 리뷰 요청해주세요 😁

@@ -72,6 +73,13 @@ public JsonResult<UserSimpleInfoResponse> userInfo(@AuthenticationPrincipal User
return JsonResult.successOf(UserSimpleInfoResponse.of(user));
}

@GetMapping("/logout")
public JsonResult<String> logout(@RequestHeader(name = "Authorization") String token) {
String[] tokens = token.split(" ");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arrays.asList(token.split(" " ))으로 List로 받아 사용하는 건 어떨까요?
그후 List의 size 체크 후 인덱스를 접근하는게 OutOfBounds 에러를 방지할 수 있을 것 같습니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수정했어요! 이런 부분까지 예외날 상황을 고려해야 하는군여..!
테스트까지 완료했슴다

image


// refreshToken 유효성 검사
if (!jwtService.isTokenValid(refreshToken, email)) {
throw new RefreshTokenException(ExceptionMessage.JWT_INVALID_RTK);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JWT 관련 예외인데 RefreshTokenException을 만들어 사용하신 이유가 있을까요??

Copy link
Collaborator Author

@jusung-c jusung-c Sep 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RedisException을 구분해야 할 것 같아서 만들었는데 JWT로 통합하는게 더 맞겠네요! 수정했습니다!

image image

@jusung-c
Copy link
Collaborator Author

재발급 구현 부분 수정 & 테스트 해봤어요
맞게 했는지 모르겠네요.. 리뷰 부탁드립니다..!

@jusung-c jusung-c requested a review from ghdcksgml1 September 23, 2023 04:46
@ghdcksgml1
Copy link
Owner

공부를 해보니 refreshToken은 API로 재발급하는게 맞는 것 같네요. JwtAuthenticationFilter에서 수정하신 코드 삭제하셔야할 것 같습니다.
저의 무지로... 죄송합니다. 🥲


스크린샷 2023-09-23 오후 5 19 29

@jusung-c
Copy link
Collaborator Author

jusung-c commented Sep 25, 2023

늦어서 죄송하빈다..!
리뷰 부탁드릴게요 🙏😢

Copy link
Owner

@ghdcksgml1 ghdcksgml1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다~ 필요없는 부분 주석 제거와 추가로 에러, 로직 성공 부분들에 로그를 달아주셨으면 좋겠습니다 ㅎㅎ.

되도록 log.info(">>>> {} : {}") 형식으로 통일시켜주세요~

if (!jwtService.isTokenValid(refreshToken, claims.getSubject())) {
throw new JwtException(ExceptionMessage.JWT_INVALID_RTK);
// 레디스에 존재하는지 확인
if (refreshTokenRepository.findById(refreshToken).isEmpty()) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

기존에 모든 Optional이 Null일때 orElseThrow를 썼는데 통일하는게 좋아보입니다. ㅎ.ㅎ

- 성공, 실패 로그 추가
- findById() 예외처리 시 orElseThrow() 사용
@jusung-c jusung-c requested a review from ghdcksgml1 September 26, 2023 06:24
Copy link
Owner

@ghdcksgml1 ghdcksgml1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

확인했습니다! 고생하셨어용

@ghdcksgml1 ghdcksgml1 merged commit c168cc4 into dev Sep 26, 2023
1 check passed
@ghdcksgml1 ghdcksgml1 deleted the feat(#45)-heachi-domain-redis branch September 26, 2023 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore 빌드 과정 또는 보조 기능 (문서 생성 기능 등) 수정
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants