-
Notifications
You must be signed in to change notification settings - Fork 8
feat: access token이 role을 cliam 으로 갖도록 #407
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: access token이 role을 cliam 으로 갖도록 #407
Conversation
|
""" Walkthrough
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20분
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 (11)
🚧 Files skipped from review as they are similar to previous changes (11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/main/java/com/example/solidconnection/auth/controller/AuthController.java(1 hunks)src/main/java/com/example/solidconnection/auth/service/AccessToken.java(1 hunks)src/main/java/com/example/solidconnection/auth/service/AuthService.java(2 hunks)src/main/java/com/example/solidconnection/auth/service/AuthTokenProvider.java(2 hunks)src/main/java/com/example/solidconnection/auth/service/SignInService.java(1 hunks)src/main/java/com/example/solidconnection/auth/service/TokenProvider.java(1 hunks)src/main/java/com/example/solidconnection/auth/token/JwtTokenProvider.java(2 hunks)src/test/java/com/example/solidconnection/auth/service/AuthServiceTest.java(5 hunks)src/test/java/com/example/solidconnection/auth/service/AuthTokenProviderTest.java(5 hunks)src/test/java/com/example/solidconnection/auth/service/JwtTokenProviderTest.java(1 hunks)src/test/java/com/example/solidconnection/auth/service/TokenBlackListServiceTest.java(2 hunks)
🔇 Additional comments (27)
src/main/java/com/example/solidconnection/auth/service/TokenProvider.java (1)
11-11: 인터페이스 확장이 깔끔하게 구현되었습니다!커스텀 클레임을 지원하는 오버로드 메서드 추가로 역할 정보를 토큰에 포함할 수 있게 되었네요. 기존 메서드와의 호환성을 유지하면서 기능을 확장한 좋은 설계입니다.
src/main/java/com/example/solidconnection/auth/service/SignInService.java (1)
19-19: 사용자 역할 정보가 액세스 토큰에 성공적으로 포함되었습니다!
siteUser.getRole()을 추가하여 토큰 생성 시 역할 정보를 전달하도록 수정된 부분이 PR 목표와 정확히 일치합니다. 프론트엔드에서 멘토/멘티 탭 구현이 훨씬 편리해질 것 같네요.src/main/java/com/example/solidconnection/auth/service/AccessToken.java (1)
3-3: AccessToken 레코드 구조가 체계적으로 개선되었습니다!다음과 같이 변경사항들이 잘 구성되어 있네요:
- Role 임포트 추가 - 필요한 의존성을 명확히 추가
- Role 필드 추가 - 레코드에 역할 정보를 포함하여 토큰 구조 확장
- 편의 생성자 제공 - String과 Role을 받아 Subject 객체를 자동 생성하는 깔끔한 구현
레코드의 불변성을 유지하면서 기능을 확장한 좋은 설계입니다.
Also applies to: 7-7, 11-12
src/main/java/com/example/solidconnection/auth/controller/AuthController.java (1)
116-116: 토큰 재발급에서 사용자 역할 정보 처리가 올바르게 구현되었습니다!다음 변경사항들이 잘 연결되어 있네요:
- 사용자 ID 파라미터 추가 -
@AuthorizedUser long siteUserId로 인증된 사용자 식별- 서비스 호출 업데이트 -
authService.reissue(siteUserId, reissueRequest)로 사용자 ID 전달이를 통해 토큰 재발급 시에도 사용자의 역할 정보가 포함된 액세스 토큰을 생성할 수 있게 되었습니다.
Also applies to: 119-119
src/test/java/com/example/solidconnection/auth/service/AuthTokenProviderTest.java (5)
7-7: Role 임포트 추가가 적절합니다.토큰에 역할 정보를 포함하는 새로운 기능을 위해 필요한 임포트입니다.
36-46: 역할 정보를 포함한 액세스 토큰 생성 테스트가 잘 구현되었습니다.
- 역할 매개변수 추가로 새로운 토큰 생성 방식을 정확히 테스트
- assertAll을 사용한 다중 검증으로 테스트 가독성 향상
- 토큰의 주체, 역할, 유효성을 모두 검증하여 완전한 테스트 커버리지 제공
70-70: 일관된 Role 매개변수 추가가 적절합니다.모든 테스트 메서드에서 Role.MENTEE를 일관되게 사용하여 새로운 메서드 시그니처와 호환성을 확보했습니다.
83-83: 테스트 시나리오에 맞는 Role 매개변수 추가입니다.리프레시 토큰 삭제 테스트에서도 일관되게 역할 정보를 포함하여 실제 사용 패턴을 반영합니다.
97-97: Subject 추출 테스트의 Role 매개변수 추가가 적절합니다.토큰에서 Subject를 추출하는 테스트에서도 새로운 메서드 시그니처를 정확히 반영했습니다.
src/test/java/com/example/solidconnection/auth/service/JwtTokenProviderTest.java (1)
38-78: 토큰 생성 테스트의 구조화가 우수합니다.
- 중첩 클래스로 관련 테스트들을 논리적으로 그룹화
- Subject만 있는 토큰과 커스텀 클레임이 있는 토큰을 각각 테스트
- JWT 클레임 파싱과 만료 시간 검증을 통한 완전한 토큰 유효성 확인
- 새로운 역할 기반 토큰 기능을 지원하는 테스트 커버리지 제공
src/main/java/com/example/solidconnection/auth/service/AuthTokenProvider.java (4)
4-4: Role 임포트 추가가 적절합니다.역할 기반 토큰 생성을 위한 필수 의존성입니다.
6-6: Map 임포트로 클레임 처리를 지원합니다.커스텀 클레임 맵 처리를 위해 필요한 임포트입니다.
16-16: ROLE_CLAIM_KEY 상수 정의가 우수한 설계입니다.역할 클레임 키를 상수로 정의하여 유지보수성과 일관성을 확보했습니다.
21-26: 역할 정보를 포함한 액세스 토큰 생성이 완벽하게 구현되었습니다.
- 메서드 시그니처에 Role 매개변수 추가로 역할 기반 토큰 생성 지원
- 상수를 사용한 역할 클레임 키로 일관성 확보
- 커스텀 클레임 맵을 통해 역할 정보를 토큰에 포함
- AccessToken 레코드에 주체, 역할, 토큰 문자열을 모두 포함하여 완전한 토큰 정보 제공
src/test/java/com/example/solidconnection/auth/service/AuthServiceTest.java (5)
47-56: @beforeeach 설정으로 테스트 구조가 크게 개선되었습니다.
- 공통 테스트 픽스처를 한 곳에서 초기화하여 중복 코드 제거
- siteUser, subject, accessToken을 재사용 가능한 필드로 정의
- 역할 정보를 포함한 AccessToken 생성으로 새로운 토큰 구조 반영
- 테스트 유지보수성과 가독성 향상
74-74: 탈퇴 메서드 호출에 사용자 ID 매개변수 추가가 적절합니다.새로운 AuthService.quit 메서드 시그니처에 맞춰 정확히 업데이트되었습니다.
97-97: 토큰 재발급 메서드 호출에 사용자 ID 매개변수 추가가 정확합니다.AuthService.reissue 메서드의 새로운 시그니처를 올바르게 반영했습니다.
112-112: 예외 시나리오 테스트에서도 일관된 매개변수 사용입니다.잘못된 리프레시 토큰 테스트에서도 새로운 메서드 시그니처를 정확히 적용했습니다.
108-108: 공유 accessToken 사용으로 테스트 일관성이 향상되었습니다.잘못된 리프레시 토큰 테스트에서 공유 픽스처를 사용하여 테스트 구조의 일관성을 유지했습니다.
src/main/java/com/example/solidconnection/auth/token/JwtTokenProvider.java (4)
13-13: Map 임포트로 커스텀 클레임 지원을 추가했습니다.커스텀 클레임 맵 처리를 위한 필수 의존성입니다.
27-29: 기존 generateToken 메서드의 위임 패턴이 우수합니다.빈 클레임 맵으로 헬퍼 메서드에 위임하여 기존 API 호환성을 유지하면서 새로운 기능을 지원합니다.
31-34: 커스텀 클레임을 지원하는 오버로드 메서드가 완벽합니다.
- Map<String, String> customClaims 매개변수로 역할 정보 등 커스텀 클레임 지원
- 동일한 헬퍼 메서드에 위임하여 코드 중복 방지
- TokenProvider 인터페이스 구현으로 계약 준수
36-47: JWT 토큰 생성 헬퍼 메서드가 훌륭하게 설계되었습니다.
- subject, claims, expireTime을 매개변수로 받아 유연한 토큰 생성 지원
- JWT Claims 객체에 커스텀 클레임을 추가하여 역할 정보 포함
- 현재 시간과 만료 시간을 정확히 설정
- HS512 알고리즘으로 안전한 토큰 서명
- 코드 중복을 제거하고 재사용성을 높인 우수한 리팩토링
src/main/java/com/example/solidconnection/auth/service/AuthService.java (1)
61-61: reissue 호출부 일관성 검증 완료변경된 reissue 메서드 호출부의 일관성을 모두 검증했습니다. AuthController와 테스트 코드에서 siteUserId가 올바르게 전달되고 있습니다. 다음과 같이 호출부를 확인했습니다:
- AuthController.reissueToken
- siteUserId와 ReissueRequest 파라미터가 모두 전달됨
- AuthServiceTest.reissue
- siteUser.getId()를 활용해 재발급 로직 테스트 통과
위 검증 결과, 모든 호출부가 새로운 시그니처와 일치합니다. 변경 사항을 승인합니다.
src/test/java/com/example/solidconnection/auth/service/TokenBlackListServiceTest.java (3)
7-7: 테스트 가독성과 새로운 역할 기반 토큰 지원이 잘 구현되었습니다.다음과 같은 개선사항들이 적절히 적용되었습니다:
1. Role 임포트 추가로 새로운 AccessToken 구조 지원 2. @DisplayName 어노테이션으로 테스트 클래스 설명 개선 3. @BeforeEach 임포트로 테스트 설정 메서드 지원Also applies to: 9-10, 16-16
26-26: 테스트 객체 생성을 중앙화한 리팩토링이 훌륭합니다.다음과 같은 개선사항들이 잘 적용되었습니다:
1. 공통 AccessToken 필드 선언으로 테스트 간 일관성 확보 2. @BeforeEach를 활용한 테스트 설정 메서드로 중복 제거 3. Role.MENTEE를 포함한 새로운 AccessToken 생성자 사용이러한 변경으로 테스트 유지보수성이 크게 향상되었고, 각 테스트마다 새로운 토큰 인스턴스를 보장합니다.
Also applies to: 28-31
37-37: 기존 테스트 로직과의 매끄러운 통합이 완료되었습니다.중앙화된 accessToken 필드 사용으로 다음과 같은 이점들을 얻었습니다:
1. 모든 테스트 메서드에서 일관된 토큰 인스턴스 사용 2. 기존 테스트 로직의 변경 없이 리팩토링 완료 3. 테스트의 가독성과 유지보수성 향상테스트 기능은 그대로 유지하면서 코드 구조만 개선된 훌륭한 리팩토링입니다.
Also applies to: 51-51, 54-54, 60-60
Gyuhyeok99
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다!
그런데 문제가 될 거 같진 않지만 혹시 권한 변경 시 기존 토큰들 무효화를 시켜야할까요? 아니면 이거에 대해선 권한 변경이 딱히 저희 서비스에서 없으니 무시해도 괜찮을까요?
whqtker
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다 ~ 토큰 생성 로직을 하나로 합쳐는 것도 좋은 변경이라고 생각합니다 !
1c861d0 to
e48e8e7
Compare
관련 이슈
작업 내용
프론트 개발자님이 멘토/멘티 탭 구현할 때 더 편하다 하셔서 추가합니다 🏃♀️
특이 사항
디스코드에 말씀드린 "auth 패키지 대변경🧹" 관점에서 추가설명하자면,
이번에 AccessToken 생성 방법 변경으로 '토큰 재발급 함수'에 아래의 변경이 있었습니다.
이 변경의 문제
방안
리뷰 요구사항 (선택)