hotfix: claims email삭제에 따른 오류 수정 #16
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- loadByUserName 수정
hotfix: claims email삭제에 따른 오류 수정
WalkthroughJWT 인증에서 사용자 식별 기준을 이메일에서 AUTH_ID 클레임으로 변경하고, 이에 따라 AuthDetailsService는 문자열 ID를 Long으로 파싱해 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as Client
participant Filter as Security Filter
participant JwtAuth as JwtAuth
participant ADS as AuthDetailsService
participant Repo as UserRepository
Client->>Filter: 요청 + Authorization 헤더
Filter->>JwtAuth: getAuthentication(token, expectedType)
JwtAuth->>JwtAuth: 토큰 검증 및 타입 확인
JwtAuth->>JwtAuth: 클레임 추출: AUTH_ID
JwtAuth->>ADS: loadUserByUsername(AUTH_ID as String)
ADS->>ADS: Long.parseLong(id)
ADS->>Repo: findById(id: Long)
Repo-->>ADS: User 또는 empty
alt User 존재
ADS-->>JwtAuth: UserDetails
JwtAuth-->>Filter: Authentication 반환
Filter-->>Client: 요청 계속 처리
else User 없음
ADS-->>JwtAuth: UserNotFoundException
JwtAuth-->>Filter: 인증 실패 / 예외
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 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. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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
🧹 Nitpick comments (2)
src/main/java/com/dnd/moddo/global/jwt/auth/JwtAuth.java (2)
3-7: Jws 캐싱 리팩토링 대비: io.jsonwebtoken.Jws import 추가 제안아래 리팩토링 제안을 적용하려면 Jws 타입 import가 필요합니다.
+import io.jsonwebtoken.Jws;
23-33: JwtAuth.getAuthentication 메서드 리팩터링 제안: JWS 파싱 중복 제거 및 예외 처리 정합성 확보
- JwtAuth.getAuthentication에서는 jwtUtil.getJwt(token)을 두 번 호출하여(Claims 조회 및 헤더 조회) 서명 검증이 중복 수행됩니다.
- 현재 AUTH_ID 클레임을 꺼낼 때
claims.get(...).toString()으로 처리해, 클레임 누락(null) 또는 타입 불일치 시 NPE 또는 ClassCastException 위험이 있습니다.- 토큰 타입 불일치 시 MissingTokenException을 던지는데, “토큰 미제공” 의미의 예외와 혼동될 수 있으므로 TokenInvalidException(“잘못된 토큰”) 계열로 변경하는 것이 더 직관적입니다.
- UsernamePasswordAuthenticationToken 생성 시 credentials로 빈 문자열("")이 전달되는데, Spring Security 권장대로
null을 사용하는 것이 바람직합니다.아래 예시처럼 한 번만 파싱한 Jws 인스턴스를 재사용하고, 타입 안전하게 처리해 주세요:
public Authentication getAuthentication(String token, String expectedTokenType) { - Claims claims = jwtUtil.getJwt(token).getBody(); - - if (isNotExpectedToken(token, expectedTokenType)) { - throw new MissingTokenException(); - } - - UserDetails userDetails = authDetailsService.loadUserByUsername( - claims.get(JwtConstants.AUTH_ID.message).toString()); - return new UsernamePasswordAuthenticationToken(userDetails, "", userDetails.getAuthorities()); + // 토큰 미제공/빈 문자열 검증 + if (token == null || token.isEmpty()) { + throw new MissingTokenException(); + } + // 서명 검증 포함, 파싱 1회만 수행 + final Jws<Claims> jws = jwtUtil.getJwt(token); + + // 토큰 타입 불일치 시 잘못된 토큰 예외로 처리 + if (isNotExpectedToken(jws, expectedTokenType)) { + throw new TokenInvalidException(); + } + + // AUTH_ID 클레임을 Long 타입으로 안전하게 꺼내고 null 검증 + final Long userId = jws.getBody().get(JwtConstants.AUTH_ID.message, Long.class); + if (userId == null) { + throw new TokenInvalidException(); + } + + UserDetails userDetails = authDetailsService.loadUserByUsername(userId.toString()); + // credentials는 null 권장 + return new UsernamePasswordAuthenticationToken(userDetails, null, userDetails.getAuthorities()); } private boolean isNotExpectedToken(Jws<Claims> jws, String expectedTokenType) { - String role = String.valueOf(jws.getHeader().get(JwtConstants.TYPE.message)); - return !role.equals(expectedTokenType); + // HEADER(TYPE) 값과 비교 (대소문자 포함 일치 여부 확인) + String tokenType = String.valueOf(jws.getHeader().get(JwtConstants.TYPE.message)); + return !expectedTokenType.equals(tokenType); }추가 검토 권장 사항:
expectedTokenType값이 JwtProvider에서 설정하는 HEADER(TYPE) 문자열과 정확히(대소문자 포함) 일치하는지 확인해 주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/main/java/com/dnd/moddo/global/jwt/auth/JwtAuth.java(1 hunks)src/main/java/com/dnd/moddo/global/security/auth/AuthDetailsService.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/main/java/com/dnd/moddo/global/security/auth/AuthDetailsService.java (3)
src/main/java/com/dnd/moddo/domain/user/exception/UserNotFoundException.java (1)
UserNotFoundException(6-12)src/main/java/com/dnd/moddo/global/jwt/utill/JwtProvider.java (1)
RequiredArgsConstructor(20-73)src/main/java/com/dnd/moddo/global/security/auth/AuthDetails.java (1)
RequiredArgsConstructor(13-55)
src/main/java/com/dnd/moddo/global/jwt/auth/JwtAuth.java (4)
src/main/java/com/dnd/moddo/global/jwt/utill/JwtUtil.java (1)
Component(14-49)src/main/java/com/dnd/moddo/global/security/auth/AuthDetailsService.java (1)
RequiredArgsConstructor(14-27)src/main/java/com/dnd/moddo/domain/auth/service/RefreshTokenService.java (1)
RequiredArgsConstructor(15-40)src/main/java/com/dnd/moddo/global/jwt/utill/JwtProvider.java (1)
RequiredArgsConstructor(20-73)
🔇 Additional comments (4)
src/main/java/com/dnd/moddo/global/jwt/auth/JwtAuth.java (2)
20-22: 의존성 주입 및 불변 필드 선언 — OK생성자 주입(@requiredargsconstructor)과 final 필드 사용 적절합니다.
23-33: RefreshTokenService: EMAIL 클레임 참조 제거 및 AUTH_ID 기반 로직으로 수정 필요RefreshTokenService가 여전히
JwtConstants.EMAIL을 사용하고 있어, JWT 발급자(JwtProvider)가 AUTH_ID만 클레임에 넣는 현재 구조와 불일치합니다. 이대로라면 리프레시 토큰 재발급 흐름이 실패하므로, 아래 파일들을 AUTH_ID 기반으로 수정해야 합니다.수정이 필요한 위치:
- src/main/java/com/dnd/moddo/domain/auth/service/RefreshTokenService.java (기존 EMAIL 클레임 참조 삭제, AUTH_ID(Long) 추출 로직 추가)
- src/test/java/com/dnd/moddo/domain/auth/service/RefreshTokenServiceTest.java (테스트 모킹 및 검증 부분에서 EMAIL → AUTH_ID 변경)
예시 수정안:
--- a/src/main/java/com/dnd/moddo/domain/auth/service/RefreshTokenService.java +++ b/src/main/java/com/dnd/moddo/domain/auth/service/RefreshTokenService.java @@ -25,10 +25,15 @@ public class RefreshTokenService { - String email; - try { - email = jwtUtil.getJwt(jwtUtil.parseToken(token)) - .getBody().get(JwtConstants.EMAIL.message).toString(); - } catch (Exception e) { - throw new TokenInvalidException(); - } - User user = userRepository.getByEmail(email); + Long userId; + try { + String rawToken = jwtUtil.parseToken(token); + userId = jwtUtil.getJwt(rawToken) + .getBody() + .get(JwtConstants.AUTH_ID.message, Long.class); + if (userId == null) throw new TokenInvalidException(); + } catch (Exception e) { + throw new TokenInvalidException(); + } + User user = userRepository.findById(userId) + .orElseThrow(() -> new UserNotFoundException(userId));테스트 코드에서도 다음과 같이 변경해야 합니다:
--- a/src/test/java/com/dnd/moddo/domain/auth/service/RefreshTokenServiceTest.java +++ b/src/test/java/com/dnd/moddo/domain/auth/service/RefreshTokenServiceTest.java @@ -56,7 +56,8 @@ class RefreshTokenServiceTest { // 기존 EMAIL 모킹 - when(mockClaims.get(JwtConstants.EMAIL.message)).thenReturn(email); + // AUTH_ID(Long) 모킹 + when(mockClaims.get(JwtConstants.AUTH_ID.message, Long.class)).thenReturn(userId); // 이후 userRepository 접근 로직 검증...위 변경을 통해 JwtProvider와 일관된 클레임 처리 및 안정적인 리프레시 토큰 재발급 흐름을 보장할 수 있습니다.
Likely an incorrect or invalid review comment.
src/main/java/com/dnd/moddo/global/security/auth/AuthDetailsService.java (2)
9-13: import 및 서비스 구성 — OK@requiredargsconstructor, @transactional(readOnly = true), @service 구성 적절합니다. UserRepository 의존성 주입도 문제 없습니다.
21-26: 결론: Authentication#getName()/Principal#getName() 사용처 없음 – 검토 불필요전체
src디렉터리 검색 결과,
Authentication#getName()또는Principal#getName()호출: 0건@AuthenticationPrincipal어노테이션 사용: 0건SecurityContextHolder.getContext().getAuthentication():JwtFilter.java한 곳에서만 사용따라서 현재 코드베이스에서는 인증 식별자(id) 대신
AuthDetails.getUsername()이 반환하는 이메일을 잘못 참조할 가능성이 없습니다. 본 리뷰 코멘트는 적용 대상이 아닌 것으로 판단되며, 무시해도 좋습니다.Likely an incorrect or invalid review comment.
Comment on lines
+21
to
+26
| @Override | ||
| public UserDetails loadUserByUsername(String id) throws UsernameNotFoundException { | ||
| return userRepository.findById(Long.parseLong(id)) | ||
| .map(AuthDetails::new) | ||
| .orElseThrow(() -> new UserNotFoundException(id)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
id 파싱 오류 처리 및 예외 메시지 오버로드 수정
- Long.parseLong(id)에서 NumberFormatException 발생 시 현재 흐름상 500 가능성이 큽니다. 보안/안정성 측면에서 인증 실패(UsernameNotFoundException 등)로 매핑하는 것이 안전합니다.
- not found 시 UserNotFoundException(Long) 오버로드를 써야 “아이디” 메시지가 노출됩니다. 현재 String 오버로드가 선택되어 “이메일” 메시지가 노출됩니다.
아래처럼 수정해 주세요.
- @Override
- public UserDetails loadUserByUsername(String id) throws UsernameNotFoundException {
- return userRepository.findById(Long.parseLong(id))
- .map(AuthDetails::new)
- .orElseThrow(() -> new UserNotFoundException(id));
- }
+ @Override
+ public UserDetails loadUserByUsername(String id) throws UsernameNotFoundException {
+ if (id == null) {
+ throw new UsernameNotFoundException("id is null");
+ }
+ final Long userId;
+ try {
+ userId = Long.parseLong(id);
+ } catch (NumberFormatException e) {
+ // 인증 실패로 취급하여 401/404로 매핑되도록 UsernameNotFoundException 사용
+ throw new UsernameNotFoundException("invalid id: " + id);
+ }
+ return userRepository.findById(userId)
+ .map(AuthDetails::new)
+ .orElseThrow(() -> new UserNotFoundException(userId));
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @Override | |
| public UserDetails loadUserByUsername(String id) throws UsernameNotFoundException { | |
| return userRepository.findById(Long.parseLong(id)) | |
| .map(AuthDetails::new) | |
| .orElseThrow(() -> new UserNotFoundException(id)); | |
| } | |
| @Override | |
| public UserDetails loadUserByUsername(String id) throws UsernameNotFoundException { | |
| if (id == null) { | |
| throw new UsernameNotFoundException("id is null"); | |
| } | |
| final Long userId; | |
| try { | |
| userId = Long.parseLong(id); | |
| } catch (NumberFormatException e) { | |
| // 인증 실패로 취급하여 401/404로 매핑되도록 UsernameNotFoundException 사용 | |
| throw new UsernameNotFoundException("invalid id: " + id); | |
| } | |
| return userRepository.findById(userId) | |
| .map(AuthDetails::new) | |
| .orElseThrow(() -> new UserNotFoundException(userId)); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/dnd/moddo/global/security/auth/AuthDetailsService.java
around lines 21 to 26, handle NumberFormatException from Long.parseLong by
catching it and rethrowing a UsernameNotFoundException (so malformed id maps to
authentication failure), and when looking up the user use the Long overload of
UserNotFoundException for the not-found case (i.e., parse id to long first, then
call userRepository.findById(parsedId).map(AuthDetails::new).orElseThrow(() ->
new UserNotFoundException(parsedId))). Ensure no NumberFormatException can
escape and that the correct UserNotFoundException(Long) constructor is used.
fix: 도메인 변경에 따른 Docs url 변경
choyeseol
approved these changes
Aug 25, 2025
📝 테스트 커버리지 리포트입니다!
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/test/java/com/dnd/moddo/global/util/RestDocsTestSupport.java (1)
39-44: 테스트 설정의 하드코딩된 호스트를 설정화(환경변수/시스템 프로퍼티)로 전환 제안환경(스테이징/프리뷰)별로 다른 도메인을 사용하는 경우, 하드코딩된 문자열 대신 설정 주입이 유연합니다.
아래와 같이 변경을 고려해 주세요.
적용 diff(해당 라인 범위 내):
- .host("api.moddo.kr") + .host(getRestDocsHost())클래스 내부(아무 위치)에 보조 메서드 추가:
private static String getRestDocsHost() { String host = System.getProperty("REST_DOCS_HOST"); if (host == null || host.isBlank()) { host = System.getenv("REST_DOCS_HOST"); } return (host == null || host.isBlank()) ? "api.moddo.kr" : host; }대안: 이미
RestDocsConfig를 사용 중이므로, 해당 설정으로 이동/통합해 단일 구성 지점에서 관리하는 것도 좋습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
README.md(1 hunks)src/test/java/com/dnd/moddo/global/util/RestDocsTestSupport.java(1 hunks)
⏰ 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)
- GitHub Check: build-and-push-docker
🔇 Additional comments (3)
README.md (2)
58-58: API 문서 링크 도메인 교체 적절함운영 API 도메인으로 보이는
api.moddo.kr로의 링크 업데이트가 합리적입니다. 아래 테스트 RestDocs 설정 변경(api.moddo.kr)과도 일관성이 맞습니다.
58-58: 레거시 도메인(moddo.kro.kr) 참조 제거 완료 확인스크립트 실행 결과 다음과 같습니다.
moddo.kro.kr관련 참조는 전혀 발견되지 않았습니다.api.moddo.kr참조는 아래 위치에서만 확인되었습니다.
- README.md:58
- src/test/java/com/dnd/moddo/global/util/RestDocsTestSupport.java:41
추가 작업 없이 변경 사항을 머지하셔도 무방합니다.
src/test/java/com/dnd/moddo/global/util/RestDocsTestSupport.java (1)
39-44: RestDocs 호스트를api.moddo.kr로 변경한 점 LGTM문서화 요청 URL의 스킴/호스트(
https/api.moddo.kr) 및removePort()조합이 운영 도메인 노출 목적에 부합합니다. README의 API 문서 링크 변경과도 일관됩니다.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#️⃣연관된 이슈
x
🔀반영 브랜치
main -> develop
🔧변경 사항
💬리뷰 요구사항(선택)
Summary by CodeRabbit