[FIX] 토큰 발행 방 법 변경(NEW: 개인/공개키 방식(비대칭 키))#53
Merged
Merged
Conversation
Walkthrough대칭 HS256 비밀키 검증에서 RSA 공개키(Base64) 검증으로 전환하고 공개키 로드/유효성 검사, 관련 에러 상태 통합·추가 및 일부 엔티티 메서드명/공백 수정이 적용되었습니다. JwtAuthFilter의 토큰 빈값 에러 상태가 Changes
Sequence Diagram(s)sequenceDiagram
participant App as 애플리케이션
participant JwtProvider as JwtProvider
participant KeyFactory as KeyFactory
participant JWTParser as JWT 파서
rect rgb(235, 245, 255)
note right of JwtProvider: 초기화
App->>JwtProvider: new JwtProvider(publicKeyBase64)
JwtProvider->>JwtProvider: null/blank 검사
JwtProvider->>KeyFactory: Base64 디코딩 → X509EncodedKeySpec → generatePublic
KeyFactory-->>JwtProvider: PublicKey (RSAPublicKey)
JwtProvider->>JwtProvider: 공개키 길이 검사 (>=2048bit)
end
rect rgb(235, 255, 235)
note right of JWTParser: 토큰 검증
App->>JwtProvider: parse(token)
JwtProvider->>JWTParser: setSigningKey(publicKey)
JWTParser->>JWTParser: 서명 검증 (RSA) / 만료 검사
alt valid
JWTParser-->>JwtProvider: Claims
JwtProvider-->>App: Claims
else expired
JWTParser-->>JwtProvider: ExpiredJwtException
JwtProvider-->>App: EXPIRED_TOKEN 예외
else invalid
JWTParser-->>JwtProvider: Exception
JwtProvider-->>App: INVALID_TOKEN / INVALID_PUBLIC_KEY 예외
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
33Auto-Bot
requested review from
CHOOSLA,
Sangyoon98,
taemin3,
vivivim and
yangjiseonn
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/main/java/com/sampoom/user/common/jwt/JwtProvider.java (2)
13-15: 사용하지 않는 임포트를 제거하세요.비대칭 키 방식으로 전환하면서
SecretKeySpec과Key임포트는 더 이상 필요하지 않습니다.-import javax.crypto.spec.SecretKeySpec; import java.nio.charset.StandardCharsets; -import java.security.Key;
36-43: 주석 처리된 레거시 코드를 제거하세요.더 이상 사용하지 않는 대칭키 관련 코드는 제거하는 것이 코드 가독성에 좋습니다. Git 히스토리를 통해 언제든 이전 코드를 확인할 수 있습니다.
- // Auth에서 들어오는 Key의 초기화 (캐싱용 객체) - public JwtProvider(@Value("${jwt.secret}") String secret) { - if (secret.getBytes(StandardCharsets.UTF_8).length < 32) { - throw new BadRequestException(ErrorStatus.TOO_SHORT_SECRET_KEY); - } - this.key = new SecretKeySpec(secret.getBytes(StandardCharsets.UTF_8), - SignatureAlgorithm.HS256.getJcaName()); - } -
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/sampoom/user/common/jwt/JwtProvider.java(2 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: Analyze (java-kotlin)
🔇 Additional comments (3)
src/main/java/com/sampoom/user/common/jwt/JwtProvider.java (3)
16-19: 새로운 임포트가 적절합니다.RSA 공개키 처리를 위해 필요한 클래스들이 올바르게 임포트되었습니다.
23-24: 필드 변경이 적절합니다.대칭키(
Key)에서 공개키(PublicKey)로의 전환이 올바르게 적용되었습니다.
53-53: 토큰 검증 로직이 올바르게 구현되었습니다.검증 결과, 이 서비스는 토큰 검증 전용이며 토큰 생성은 별도의 auth-service에서 처리하는 올바른 마이크로서비스 아키텍처입니다. 공개키를 사용한 JWT 서명 검증 구현이 적절하게 완료되었습니다.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/sampoom/user/common/jwt/JwtAuthFilter.java(1 hunks)src/main/java/com/sampoom/user/common/jwt/JwtProvider.java(1 hunks)src/main/java/com/sampoom/user/common/response/ErrorStatus.java(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-16T10:32:56.782Z
Learnt from: Lee-Jong-Jin
Repo: 33-Auto/Sampoom-Management-Backend-User PR: 7
File: src/main/java/com/sampoom/backend/user/jwt/JwtAuthFilter.java:34-37
Timestamp: 2025-10-16T10:32:56.782Z
Learning: In JWT authentication filters for the Sampoom Management Backend User service, when userId or role is null in the token claims, log a warning with both values using log.warn and call chain.doFilter(req, res) before returning, rather than throwing an exception. This allows the request to continue without authentication.
Applied to files:
src/main/java/com/sampoom/user/common/jwt/JwtAuthFilter.java
📚 Learning: 2025-10-16T10:22:30.658Z
Learnt from: Lee-Jong-Jin
Repo: 33-Auto/Sampoom-Management-Backend-User PR: 7
File: src/main/java/com/sampoom/backend/user/common/exception/ForbiddenException.java:15-17
Timestamp: 2025-10-16T10:22:30.658Z
Learning: In the com.sampoom.backend.user.common.exception package, exception class constructors accepting ErrorStatus parameter (e.g., ForbiddenException, BadRequestException, UnauthorizedException) do not perform null checks on the errorStatus parameter before calling its getters.
Applied to files:
src/main/java/com/sampoom/user/common/jwt/JwtAuthFilter.javasrc/main/java/com/sampoom/user/common/response/ErrorStatus.javasrc/main/java/com/sampoom/user/common/jwt/JwtProvider.java
⏰ 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: Analyze (java-kotlin)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/com/sampoom/user/common/jwt/JwtProvider.java (1)
24-32:BadRequestException을 다시 감싸면서 상세 에러 코드가 사라집니다.
loadPublicKey가SHORT_PUBLIC_KEY같은BadRequestException을 던져도, 생성자와 메소드의catch (Exception)이 모두INVALID_PUBLIC_KEY로 덮어써 버려 신규 에러 코드가 절대 노출되지 않습니다.BadRequestException은 그대로 재전파하고, 그 외 예외만INVALID_PUBLIC_KEY로 매핑해 주세요.try { this.publicKey = loadPublicKey(publicKeyBase64); - } catch (Exception e) { - throw new BadRequestException(ErrorStatus.INVALID_PUBLIC_KEY); + } catch (BadRequestException e) { + throw e; + } catch (Exception e) { + throw new BadRequestException(ErrorStatus.INVALID_PUBLIC_KEY); } @@ - } catch (Exception e) { - throw new BadRequestException(ErrorStatus.INVALID_PUBLIC_KEY); + } catch (BadRequestException e) { + throw e; + } catch (Exception e) { + throw new BadRequestException(ErrorStatus.INVALID_PUBLIC_KEY); }Also applies to: 47-49
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/sampoom/user/common/jwt/JwtProvider.java(1 hunks)src/main/java/com/sampoom/user/common/response/ErrorStatus.java(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-16T10:22:30.658Z
Learnt from: Lee-Jong-Jin
Repo: 33-Auto/Sampoom-Management-Backend-User PR: 7
File: src/main/java/com/sampoom/backend/user/common/exception/ForbiddenException.java:15-17
Timestamp: 2025-10-16T10:22:30.658Z
Learning: In the com.sampoom.backend.user.common.exception package, exception class constructors accepting ErrorStatus parameter (e.g., ForbiddenException, BadRequestException, UnauthorizedException) do not perform null checks on the errorStatus parameter before calling its getters.
Applied to files:
src/main/java/com/sampoom/user/common/response/ErrorStatus.javasrc/main/java/com/sampoom/user/common/jwt/JwtProvider.java
⏰ 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: Analyze (java-kotlin)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/main/java/com/sampoom/user/common/response/ErrorStatus.java (1)
13-13: 과거 리뷰 코멘트가 이미 반영되었습니다.이전 리뷰에서 지적된 "2048바이트" → "2048비트" 수정이 현재 코드에 정확히 적용되어 있습니다.
🧹 Nitpick comments (3)
src/main/java/com/sampoom/user/common/entity/SoftDeleteEntity.java (1)
23-27: 메서드 이름을 동사형으로 변경하는 것을 권장합니다.
reactivation()은 명사형이지만, Java 메서드 명명 규칙에 따르면 동사나 동사구를 사용하는 것이 더 관례적입니다. 같은 클래스의softDelete()(Line 17)와 일관성을 맞추기 위해reactivate()로 변경하는 것을 고려해보세요.- public void reactivation(){ + public void reactivate(){ if(!this.deleted) return; this.deleted = false; this.deletedAt = null; }src/main/java/com/sampoom/user/common/jwt/JwtProvider.java (2)
21-21: 주석 처리된 레거시 코드를 제거하세요.대칭키 방식에서 사용하던 필드가 주석으로 남아 있습니다. 마이그레이션이 완료되었으므로 코드 가독성을 위해 제거하는 것이 좋습니다.
다음 diff를 적용하세요:
-// private final Key key; private final PublicKey publicKey;
38-54: RSA 키 타입 검증을 명시적으로 수행하는 것을 고려하세요.현재 구현에서는
instanceof RSAPublicKey체크가 실패하면 키 길이 검증을 건너뜁니다 (lines 43-47).KeyFactory.getInstance("RSA")는 유효한 키에 대해 항상RSAPublicKey를 반환해야 하지만, 명시적으로 타입을 강제하면 보안성이 더 강화됩니다.다음과 같이 개선할 수 있습니다:
private PublicKey loadPublicKey(String base64) throws Exception { try { byte[] keyBytes = Base64.getDecoder().decode(base64); PublicKey key = KeyFactory.getInstance("RSA") .generatePublic(new X509EncodedKeySpec(keyBytes)); - if (key instanceof RSAPublicKey rsaKey) { - if (rsaKey.getModulus().bitLength() < 2048) { - throw new BadRequestException(ErrorStatus.SHORT_PUBLIC_KEY); - } + if (!(key instanceof RSAPublicKey rsaKey)) { + throw new BadRequestException(ErrorStatus.INVALID_PUBLIC_KEY); + } + if (rsaKey.getModulus().bitLength() < 2048) { + throw new BadRequestException(ErrorStatus.SHORT_PUBLIC_KEY); } return key; } catch (BadRequestException e) {참고: 예외 처리 로직 (lines 49-52)은 이전 리뷰 피드백을 잘 반영하여
BadRequestException을 올바르게 전파하고 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/main/java/com/sampoom/user/common/config/security/.DS_Storeis excluded by!**/.DS_Storesrc/main/resources/.DS_Storeis excluded by!**/.DS_Store
📒 Files selected for processing (5)
src/main/java/com/sampoom/user/common/entity/BaseEmployeeEntity.java(1 hunks)src/main/java/com/sampoom/user/common/entity/BaseTimeEntity.java(1 hunks)src/main/java/com/sampoom/user/common/entity/SoftDeleteEntity.java(1 hunks)src/main/java/com/sampoom/user/common/jwt/JwtProvider.java(1 hunks)src/main/java/com/sampoom/user/common/response/ErrorStatus.java(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/main/java/com/sampoom/user/common/entity/BaseTimeEntity.java
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-16T10:22:30.658Z
Learnt from: Lee-Jong-Jin
Repo: 33-Auto/Sampoom-Management-Backend-User PR: 7
File: src/main/java/com/sampoom/backend/user/common/exception/ForbiddenException.java:15-17
Timestamp: 2025-10-16T10:22:30.658Z
Learning: In the com.sampoom.backend.user.common.exception package, exception class constructors accepting ErrorStatus parameter (e.g., ForbiddenException, BadRequestException, UnauthorizedException) do not perform null checks on the errorStatus parameter before calling its getters.
Applied to files:
src/main/java/com/sampoom/user/common/response/ErrorStatus.javasrc/main/java/com/sampoom/user/common/jwt/JwtProvider.java
📚 Learning: 2025-10-14T11:21:55.975Z
Learnt from: vivivim
Repo: 33-Auto/Sampoom-Management-Backend-User PR: 6
File: src/main/java/com/sampoom/backend/user/service/UserService.java:24-84
Timestamp: 2025-10-14T11:21:55.975Z
Learning: In the com.sampoom.backend.user project, use custom exceptions from com.sampoom.backend.user.common.exception package (BadRequestException, NotFoundException, etc.) instead of Java built-in exceptions (IllegalArgumentException, EntityNotFoundException) for consistent error handling and proper HTTP status code mapping in service layers.
Applied to files:
src/main/java/com/sampoom/user/common/response/ErrorStatus.java
⏰ 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: Analyze (java-kotlin)
🔇 Additional comments (3)
src/main/java/com/sampoom/user/common/entity/BaseEmployeeEntity.java (1)
55-55: 변경사항이 일관성 있게 적용되었습니다.
SoftDeleteEntity의 메서드명 변경에 맞춰 호출 코드가 올바르게 업데이트되었습니다. 만약SoftDeleteEntity에서reactivate()로 메서드명을 변경한다면, 이 부분도 함께 업데이트해야 합니다.src/main/java/com/sampoom/user/common/jwt/JwtProvider.java (2)
24-36: 예외 처리 개선이 잘 반영되었습니다.생성자의 예외 처리가 이전 리뷰 피드백을 반영하여 개선되었습니다:
BadRequestException을 별도로 catch하여 그대로 전파 (lines 30-31)- 기타 예외만
INVALID_PUBLIC_KEY로 래핑 (lines 32-33)이를 통해
SHORT_PUBLIC_KEY같은 구체적인 에러 상태가 올바르게 전달됩니다.
56-71: 토큰 파싱 로직이 올바르게 구현되었습니다.공개키 기반 검증으로의 전환이 정확하게 적용되었습니다:
- 토큰 null/blank 검증 추가 (lines 57-59)
publicKey를 사용한 서명 검증 (line 61)- 적절한 예외 처리로 만료/유효하지 않은 토큰 구분
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
📣 Related Issue
📝 Summary
🙏 Question & PR point
📬 Reference
Summary by CodeRabbit
보안 개선
설정 변경
오류 처리 개선
리팩터