feat: 리프레시 토큰을 쿠키에 저장 #417
Conversation
nayonsoso
requested review from
Gyuhyeok99,
lsy1307,
whqtker and
wibaek
as code owners
|
""" Walkthrough
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8분 Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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: 1
🔭 Outside diff range comments (1)
src/main/java/com/example/solidconnection/auth/controller/AuthController.java (1)
96-107: 회원가입 컨트롤러에 쿠키 설정을 놓친 것 같아요!
회원가입 처리 후 SignInResponse에 담긴 refreshToken이 클라이언트에 전달되지 않습니다.변경 사항 안내:
1. 메서드 시그니처 수정
- signUp 메서드에 HttpServletResponse httpServletResponse 파라미터 추가
2. 쿠키 설정 로직 추가
-refreshTokenCookieManager.setCookie(httpServletResponse, signInResponse.refreshToken());호출
3. 테스트 및 문서 검증
- 변경 후 회원가입 플로우에서 브라우저에 쿠키가 정상 설정되는지 확인해주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/example/solidconnection/auth/controller/AuthController.java(4 hunks)src/main/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManager.java(1 hunks)src/test/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManagerTest.java(1 hunks)
🔇 Additional comments (11)
src/main/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManager.java (2)
11-13: 쿠키 보안 설정 상수들이 적절합니다.
COOKIE_NAME: "refreshToken"으로 명확하게 명명PATH: "/"로 설정하여 애플리케이션 전체에서 접근 가능SAME_SITE: "Strict"로 설정하여 CSRF 공격 방지보안 측면에서 잘 구성되었습니다.
26-35: 쿠키 삭제 로직이 올바르게 구현되었습니다.
- 빈 값("")으로 설정
maxAge(0)으로 즉시 만료 처리- 동일한 보안 속성 유지로 일관성 보장
쿠키 삭제를 위한 표준적인 접근 방식을 잘 따르고 있습니다.
src/test/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManagerTest.java (3)
17-20: 테스트 설정이 간단하고 명확합니다.
@BeforeEach에서RefreshTokenCookieManager인스턴스를 직접 생성하는 방식이 단위 테스트에 적합합니다.
22-42: 리프레시 토큰 쿠키 설정 테스트가 포괄적입니다.
- 쿠키 이름과 값 검증
HttpOnly속성 확인Secure속성 확인Path=/설정 확인Max-Age만료 시간 확인SameSite=Strict설정 확인모든 중요한 쿠키 보안 속성들을 빠짐없이 검증하고 있어 훌륭합니다.
44-63: 쿠키 삭제 테스트도 필요한 모든 검증을 수행합니다.
- 빈 값으로 쿠키 설정 확인 (
refreshToken=)- 보안 속성들 유지 확인
Max-Age=0으로 즉시 만료 설정 확인쿠키 삭제 시에도 보안 속성들이 일관성 있게 유지되는지 검증하는 점이 좋습니다.
src/main/java/com/example/solidconnection/auth/controller/AuthController.java (6)
50-50: 의존성 주입이 적절하게 추가되었습니다.
RefreshTokenCookieManager를 필드로 주입받아 컨트롤러에서 쿠키 관리 기능을 사용할 수 있도록 구성했습니다.
52-62: Apple OAuth 로그인에 쿠키 설정 로직이 잘 통합되었습니다.
HttpServletResponse파라미터 추가OAuthSignInResponse타입 체크 후 쿠키 설정- 기존 로직 변경 없이 쿠키 기능만 추가
instanceof패턴 매칭을 사용한 타입 체크가 깔끔하고, 로그인 성공 시에만 쿠키를 설정하는 조건부 로직이 적절합니다.
64-74: Kakao OAuth 로그인도 Apple과 동일한 패턴으로 구현되었습니다.일관성 있는 구현으로 코드 가독성과 유지보수성이 좋습니다.
76-84: 이메일 로그인에서 쿠키 설정이 직관적으로 구현되었습니다.OAuth와 달리 이메일 로그인은 항상
SignInResponse를 반환하므로 타입 체크 없이 바로 쿠키를 설정하는 것이 맞습니다.
109-118: 로그아웃에서 쿠키 삭제 로직이 적절히 추가되었습니다.
HttpServletResponse파라미터 추가- 기존 토큰 무효화 후 쿠키 삭제
- 순서가 논리적으로 올바름
사용자 경험 측면에서 완전한 로그아웃 처리가 이루어집니다.
120-130: 회원탈퇴에서도 쿠키 삭제가 일관성 있게 처리되었습니다.로그아웃과 동일한 패턴으로 쿠키 삭제가 구현되어 일관성이 좋습니다. 회원탈퇴 시에도 모든 인증 정보가 깔끔하게 정리됩니다.
src/main/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManager.java
Show resolved
Hide resolved
nayonsoso
force-pushed
the
feat/416-add-refresh-token-to-cookie
branch
from
1de897e to
af8aba7
Compare
whqtker
left a comment
whqtker
left a comment
There was a problem hiding this comment.
웹 쪽 지식이 많이 부족해서 리뷰 & 질문 퀄리티가 낮을 수 있습니다. 미리 양해 부탁드립니다 ㅠㅠ 🥲
- 토큰을 재발급할 때는
refreshTokenCookieManager를 통해 쿠키를 세팅하는 로직이 없는데, 의도하신 건가요 ? 토큰 재발급 시에는 따로 쿠키를 설정하지 않아도 되는지 궁금합니다.
토큰 재발급은 '리프레시 토큰은 그대로, 액세스 토큰만 새로운 것으로'하는 방식을 사용하고 있습니다! (인터넷의 어떤 기술블로그들은 리프레시 토큰과 엑세스 토큰을 모두 새롭게 만들어주는 방법도 있다고 하더라고요. |
Gyuhyeok99
left a comment
Gyuhyeok99
left a comment
There was a problem hiding this comment.
이해했습니다! 고생하셨습니다 ㅎㅎ
그런데 controller에 있는건 좀 어색한 거 같기도 하네요.. 그냥 이건 넘어갈까요 아니면 auth 내 util같은 패키지에 있는 게 나을까요..
| private long changeMicroSecondToSecond(long microSeconds) { | ||
| return microSeconds / 1000; | ||
| } |
There was a problem hiding this comment.
마이크로초를 초로 바꾸려면 1,000,000으로 나눠야 하는 거 아닌가요!?
There was a problem hiding this comment.
음~~ 좋은 지적 감사드립니다.
저는 바보입니다. 💀
jwt의 expiration 단위는 "밀리 세컨드" 이고, 쿠키의 expiration 단위는 "세컨드" 입니다.
ms를 마이크로 세컨드로 해석했나봅니다 ㅜ
함수명과 인자명의 micro 표현을 milli 로 변경했습니다~ 7a922ce
| public void deleteCookie(HttpServletResponse response) { | ||
| ResponseCookie cookie = ResponseCookie.from(COOKIE_NAME, "") | ||
| .httpOnly(true) | ||
| .secure(true) | ||
| .path(PATH) | ||
| .maxAge(0) // 쿠키 삭제를 위해 maxAge를 0으로 설정 | ||
| .sameSite(SAME_SITE) | ||
| .build(); | ||
| response.addHeader("Set-Cookie", cookie.toString()); | ||
| } |
There was a problem hiding this comment.
private ResponseCookieBuilder createCookieBuilder(String value) {
return ResponseCookie.from(COOKIE_NAME, value)
.httpOnly(true)
.secure(true)
.path(PATH)
.sameSite(SAME_SITE);
}
이런식으로 공통부분 뺄 수 있을 거 같긴 한데 꼭 반영은 안해도 될 거 같습니다!
"Set-Cookie"만 따로 상수로 안빼신 이유가 있나요?
whqtker
left a comment
whqtker
left a comment
There was a problem hiding this comment.
코멘트 확인했습니다 ~! 바로 이해했습니다.
개발하면서 쿠키를 직접적으로 다룬 적은 없었는데, 작성하신 코드 보면서 도움 많이 되었습니다 ! 감사합니다
제가 더 찾은 추가적인 개선점은 없어서, approve 하겠습니다 !
이 자체가 컨트롤러는 아니지만, '레이어드 아티텍처' 관점에서 보면 controller에 있어도 괜찮다 생각합니다~ |
nayonsoso
force-pushed
the
feat/416-add-refresh-token-to-cookie
branch
from
12eae52 to
6bb998b
Compare
|
확인했습니다! 넵 controller에 있어도 괜찮을 거 같습니다! |
3 participants
관련 이슈
작업 내용
회의 때 나온 내용을 구현합니다~
특이 사항
중복 코드를 줄이려면 줄일 수는 있겠지만 (e.g. SignUpService.signUp() 이 호출될 때 쿠키 세팅 등..)
"쿠키"는 웹의 영역이라 생각하여, Controller 의 영역을 벗어나지 않으려 했습니다.
그래서
RefreshTokenCookieManager도 auth.controller 패키지 하위에 위치했습니다.