refactor: 로그아웃, 탈퇴 시 refresh 토큰을 삭제하도록 #303
Conversation
|
""" Walkthrough
Suggested reviewers
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/com/example/solidconnection/auth/controller/AuthController.java (2)
97-103: 좋은 리팩토링이지만 공백 토큰에 대한 검증이 제거되었어요.
리팩토링의 장점:
- 중복 코드를
getAccessToken메서드로 추출하여 코드 가독성 향상- 토큰 추출 로직을 한 곳에서 관리하여 유지보수성 개선
주의 사항:
- 이전 코드에서는 토큰이 null인지와 공백인지 모두 확인했을 가능성이 있습니다
- 새로운
getAccessToken메서드는 null 체크만 수행하고 있어요- 공백 토큰도 유효하지 않은 입력으로 처리하는 것이 좋을 수 있습니다
private String getAccessToken(Authentication authentication) { String accessToken = (String) authentication.getCredentials(); if (accessToken == null) { throw new CustomException(ErrorCode.AUTHENTICATION_FAILED, "엑세스 토큰이 없습니다."); } + if (accessToken.isBlank()) { + throw new CustomException(ErrorCode.AUTHENTICATION_FAILED, "엑세스 토큰이 비어있습니다."); + } return accessToken; }
122-129: 토큰 추출 및 검증 로직을 분리한 것은 매우 좋은 리팩토링입니다.
잘 구현된 점:
- 중복 코드 제거로 DRY 원칙 준수
- 명확한 예외 메시지로 디버깅 용이성 향상
- 프라이빗 메서드로 구현하여 캡슐화 적용
추가 개선 가능성:
- 앞서 언급한 공백 토큰 검증을 추가하면 더 견고해질 것
- 에러 메시지의 오타 수정: "엑세스" → "액세스"
private String getAccessToken(Authentication authentication) { String accessToken = (String) authentication.getCredentials(); if (accessToken == null) { - throw new CustomException(ErrorCode.AUTHENTICATION_FAILED, "엑세스 토큰이 없습니다."); + throw new CustomException(ErrorCode.AUTHENTICATION_FAILED, "액세스 토큰이 없습니다."); } return accessToken; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/com/example/solidconnection/auth/controller/AuthController.java(2 hunks)src/main/java/com/example/solidconnection/auth/service/AuthService.java(1 hunks)src/main/java/com/example/solidconnection/auth/service/AuthTokenProvider.java(1 hunks)src/test/java/com/example/solidconnection/auth/service/AuthServiceTest.java(3 hunks)src/test/java/com/example/solidconnection/auth/service/AuthTokenProviderTest.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (14)
src/main/java/com/example/solidconnection/auth/service/AuthTokenProvider.java (1)
52-56: 리프레시 토큰 삭제 기능이 잘 구현되었습니다.새로운
deleteRefreshTokenByAccessToken메서드가 깔끔하게 구현되었습니다. 이 구현은:
- 액세스 토큰에서 subject 값을 추출하고
- 해당 subject로 리프레시 토큰 키를 생성한 후
- Redis에서 리프레시 토큰을 삭제합니다
구현이 간결하고 목적에 부합합니다.
src/test/java/com/example/solidconnection/auth/service/AuthTokenProviderTest.java (1)
73-85: 테스트 케이스가 명확하게 작성되었습니다.새로 추가된
액세서_토큰에_해당하는_리프레시_토큰을_삭제한다테스트는 다음과 같이 잘 구성되어 있습니다:
- 주어진 subject에 대한 리프레시 토큰 생성 및 저장
- 동일한 subject에 대한 액세스 토큰 생성
- 액세스 토큰을 사용하여
deleteRefreshTokenByAccessToken메서드 호출- Redis에서 리프레시 토큰이 삭제되었는지 검증
given-when-then 구조가 명확하고, 검증 로직이 적절합니다.
src/main/java/com/example/solidconnection/auth/service/AuthService.java (5)
22-24: 문서 업데이트가 적절히 이루어졌습니다.로그아웃 메서드의 javadoc이 새로운 기능(리프레시 토큰 삭제)을 반영하도록 적절히 업데이트되었습니다.
28-28: 리프레시 토큰 삭제 로직이 적절히 추가되었습니다.로그아웃 시 리프레시 토큰을 삭제하는 로직이 성공적으로 구현되었습니다. 이는 보안 측면에서 중요한 개선사항입니다.
33-37: 문서 업데이트가 적절히 이루어졌습니다.탈퇴 메서드의 javadoc이 새로운 기능(로그아웃 처리)을 반영하도록 적절히 업데이트되었습니다.
39-39: TODO 코멘트가 명확하게 작성되었습니다.TODO 코멘트는 향후 개선 방향을 명확하게 제시하고 있습니다. 이슈 번호(#299)를 포함하여 추적이 용이하도록 작성되었습니다.
미래에
SiteUser파라미터를 제거하고token만 받도록 수정하는 것은 결합도를 낮추는 좋은 방향입니다.
42-42: 로그아웃 로직이 적절히 통합되었습니다.탈퇴 과정에서 로그아웃 처리가 적절히 추가되었습니다. 이렇게 함으로써 사용자 탈퇴 시 인증 토큰이 깔끔하게 정리됩니다.
src/test/java/com/example/solidconnection/auth/service/AuthServiceTest.java (6)
38-40: RedisTemplate 주입이 적절히 추가되었습니다.RedisTemplate을 주입받아 Redis 상태 변화를 검증할 수 있도록 테스트가 개선되었습니다.
44-46: 테스트 준비 단계가 적절히 개선되었습니다.테스트에 필요한 Subject 객체와 AccessToken 객체를 생성하는 코드가 명확하게 추가되었습니다.
51-55: 검증 로직이 적절히 강화되었습니다.로그아웃 후 다음 사항을 검증하도록 테스트가 개선되었습니다:
- Redis에서 리프레시 토큰이 삭제되었는지 확인
- 액세스 토큰이 블랙리스트에 추가되었는지 확인
assertAll을 사용하여 여러 검증을 한 번에 실행하는 것이 좋은 접근법입니다.
62-64: 테스트 준비 단계가 적절히 개선되었습니다.사용자 객체에서 Subject를 추출하고 AccessToken을 생성하는 코드가 명확하게 추가되었습니다.
66-66: 서비스 호출이 새로운 메서드 시그니처에 맞게 업데이트되었습니다.
quit메서드 호출이 새로운 시그니처(토큰 파라미터 추가)에 맞게 올바르게 업데이트되었습니다.
70-75: 검증 로직이 적절히 강화되었습니다.탈퇴 후 다음 사항을 검증하도록 테스트가 개선되었습니다:
- 사용자의 탈퇴일이 내일로 설정되었는지 확인
- Redis에서 리프레시 토큰이 삭제되었는지 확인
- 액세스 토큰이 블랙리스트에 추가되었는지 확인
assertAll을 사용하여 여러 검증을 한 번에 실행하는 것이 좋은 접근법입니다.src/main/java/com/example/solidconnection/auth/controller/AuthController.java (1)
105-113: 매개변수 추가와 토큰 전달 로직 개선이 잘 이루어졌습니다.
개선된 점:
Authentication매개변수가 추가되어 토큰 추출 가능- 추출된 액세스 토큰이 서비스 계층으로 전달되어 요구사항 구현
- 코드가 간결해지고 책임이 명확하게 분리됨
기대 효과:
- PR 목표대로 회원 탈퇴 시 리프레시 토큰 삭제 기능 지원
- 인증 관련 작업이 일관된 방식으로 처리됨
nayonsoso
force-pushed
the
refactor/298-invalidate-refresh-token
branch
from
d837cd9 to
9af3c3f
Compare
- Authentication 자체가 null 인 경우도 예외처리
nayonsoso
force-pushed
the
refactor/298-invalidate-refresh-token
branch
from
9af3c3f to
8deddff
Compare
Gyuhyeok99
left a comment
Gyuhyeok99
left a comment
There was a problem hiding this comment.
확인했습니다! 함수명 가독성이 참 좋은 거 같네요 ㅎㅎ
보다가 갑자기 의문이 든 건 로그아웃 api HTTP 메서드가 PATCH인가?라는 생각이 들었습니다!
부분 수정의 느낌보다는 상태 변경이라고 해야할까요..? 그래서 POST가 더 맞는 거 같다는 생각이 드는데 어떻게 생각하시나요?
3 participants
관련 이슈
작업 내용
특이 사항
#297 (review) 에서 이야기된 것처럼,
Authentication 으로부터 String accessToken을 추출하는것은 중복 코드가 2곳 뿐이며,
auth 패키지 이외에는 이 로직이 사용되는 곳이 없을 것이기에 ArgumentResolver로 만드는것은 과하다 생각했습니다.
그래서 우선은 private 메서드를 만들어주었습니다.
+) TODO 부분도 빠르게 작업해보겠습니다 🏃♀️🏃♀️
리뷰 요구사항 (선택)