Skip to content

Conversation

@jsoonworld
Copy link
Member

📄 Work Description

이번 PR은 Spring Security 의존성을 점진적으로 제거하기 위한 첫 단계로, 기존에 여러 곳에 분산되어 있던 JWT 관련 로직을 auth 도메인 내의 커스텀 JwtProvider로 통합하고 리팩토링하는 작업을 진행했습니다. 이를 통해 인증/인가 코드의 응집도를 높이고 향후 유지보수성을 개선하고자 합니다.

  • auth 도메인에 특화된 커스텀 JwtProvider를 구현하여 JWT 생성, 검증 및 사용자 ID 추출 로직을 통합했습니다.
  • Access Token과 Refresh Token을 함께 반환하기 위한 Token DTO를 record 형식으로 추가했습니다.
  • JWT 관련 예외 처리 클래스(JwtException, JwtErrorCode)의 패키지 위치를 common.security에서 auth.jwt로 이동하여 응집도를 높였습니다.
  • JwtProvider 변경에 따라, 영향을 받는 다른 클래스들의 의존성 및 import 경로를 수정했습니다.

⚙️ ISSUE

@jsoonworld jsoonworld requested a review from Copilot July 13, 2025 12:28
@jsoonworld jsoonworld self-assigned this Jul 13, 2025
@jsoonworld jsoonworld added ✨ feat 새로운 기능 추가 🦊장순🦊 labels Jul 13, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR centralizes JWT handling by introducing a custom JwtProvider in the auth domain, moves JWT exception types to a more cohesive package, and updates all related imports.

  • Implemented JwtProvider to handle JWT creation, validation, and user ID extraction.
  • Relocated JwtException and JwtErrorCode from common.security.jwt.exception to auth.jwt.exception and updated imports in dependent classes.
  • Added a Token record for bundling access and refresh tokens, and updated related classes to use the new provider and exception paths.

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
external/pushNotification/user/domain/UserSyncEvent.java Specified @Enumerated(EnumType.STRING) for eventType
common/security/jwt/filter/CustomJwtAuthenticationEntryPoint.java Updated imports to use relocated auth.jwt.exception classes
common/security/jwt/auth/UserIdConverter.java Updated imports to use relocated auth.jwt.exception classes
common/security/jwt/auth/JwtClaimsParser.java Updated imports to use relocated auth.jwt.exception classes
common/security/jwt/application/JwtUserIdExtractor.java Updated imports to use relocated auth.jwt.exception classes
common/security/jwt/application/JwtClaimsGenerator.java Updated imports to use relocated auth.jwt.exception classes
common/exception/GlobalExceptionHandler.java Updated imports to use relocated auth.jwt.exception classes
auth/jwt/exception/JwtException.java Moved package from common.security.jwt.exception
auth/jwt/exception/JwtErrorCode.java Moved package and added two new error codes
auth/jwt/JwtProvider.java Introduced new custom provider for JWT generation and parsing
auth/dto/Token.java Added Token record for access and refresh tokens
Comments suppressed due to low confidence (2)

src/main/java/org/terning/terningserver/auth/jwt/JwtProvider.java:30

  • The new JwtProvider methods (generateTokens, getUserIdFrom, resolveToken, parseClaims) currently lack unit tests. Consider adding tests to cover successful flows and error scenarios like missing or expired tokens.
    public Token generateTokens(Long userId) {

src/main/java/org/terning/terningserver/common/security/jwt/filter/CustomJwtAuthenticationEntryPoint.java:1

  • [nitpick] After moving JWT exception classes into auth.jwt.exception, you may want to consolidate all JWT-related code (filters, converters, parsers) under the auth.jwt package to keep package structure consistent.
package org.terning.terningserver.common.security.jwt.filter;


@PostConstruct
protected void init() {
secretKey = Keys.hmacShaKeyFor(valueConfig.getSecretKey().getBytes());
Copy link

Copilot AI Jul 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specify a charset when converting the secret key string to bytes, e.g., getSecretKey().getBytes(StandardCharsets.UTF_8), to avoid platform-default encoding differences.

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

피드백 감사합니다!

getSecretKey().getBytes()처럼 문자열을 바이트로 변환할 때 Charset을 명시하지 않으면, OS에 따라 기본 인코딩이 달라져 예상치 못한 인증 오류를 발생시킬 수 있는 중요한 부분을 잘 짚어주신 것 같아요!

마침 ValueConfig의 init() 메소드에서는 이미 StandardCharsets.UTF_8을 사용해 Base64 인코딩을 하고 있었는데, JwtProvider에서는 이 부분을 놓치고 있었네요.

두 클래스 간의 역할을 명확히 하고 일관성을 유지하기 위해, ValueConfig에서는 Base64 인코딩 로직을 제거하고, JwtProvider의 init() 메소드에서 제안해주신 대로 StandardCharsets.UTF_8을 명시하여 SecretKey를 생성하도록 수정하겠습니다. 이렇게 하면 SecretKey 생성 책임이 JwtProvider로 일원화되어 코드가 더 명확해질 것 같네요!

꼼꼼한 리뷰 감사합니다!

@jsoonworld jsoonworld merged commit 7fabe93 into develop Jul 14, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feat : 커스텀 JWT Provider 구현

2 participants