feat: auth 관련 redis로 변경 + artist parsing 로직 변경 - #101 #125
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughSpring Cache 기반 RT 관리를 제거하고 Redis 기반으로 전환: RTCacheManager/CacheConfig/캐시 의존성 삭제, RefreshToken 도메인·리포지토리·RefreshTokenManager 추가. AuthService가 Redis로 RT 저장/검증/삭제 수행하도록 변경. JwtGenerator에서 RT 캐시 연동 제거 및 secretKey getter 추가. 이벤트 라인업 파싱 로직 수정. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant AuthService
participant JwtGenerator
participant RefreshTokenManager
participant Redis as Redis(RefreshToken)
Client->>AuthService: signup/login(credentials)
AuthService->>JwtGenerator: generateAccessToken(user)
JwtGenerator-->>AuthService: accessToken
AuthService->>JwtGenerator: generateRefreshToken(user)
JwtGenerator-->>AuthService: refreshToken
note over AuthService: TTL = jwtProperties.refreshTokenExpirationTime()
AuthService->>RefreshTokenManager: saveRefreshTokenInRedis(userId, RT, TTL)
RefreshTokenManager->>Redis: save RefreshToken(userId, RT, ttlSeconds)
Redis-->>RefreshTokenManager: ok
RefreshTokenManager-->>AuthService: ok
AuthService-->>Client: Token(AT, RT)
rect rgba(255,230,200,0.3)
note right of AuthService: 오류 시\nAuthRedisException / AuthRTException
end
sequenceDiagram
autonumber
actor Client
participant AuthService
participant RefreshTokenManager
participant Redis as Redis(RefreshToken)
participant JwtGenerator
Client->>AuthService: reissue(AT, RT)
AuthService->>RefreshTokenManager: validateSameWithOriginalRefreshToken(userId, RT)
RefreshTokenManager->>Redis: findById(userId)
Redis-->>RefreshTokenManager: stored RT or not found
alt not found
RefreshTokenManager-->>AuthService: throw AuthRTNotFoundException
AuthService-->>Client: UNAUTHORIZED
else mismatch
RefreshTokenManager-->>AuthService: throw AuthRTException
AuthService-->>Client: UNAUTHORIZED
else match
AuthService->>JwtGenerator: generate new AT/RT
JwtGenerator-->>AuthService: AT, RT'
AuthService->>RefreshTokenManager: saveRefreshTokenInRedis(userId, RT', TTL)
RefreshTokenManager->>Redis: upsert
AuthService-->>Client: Token(AT, RT')
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/com/permitseoul/permitserver/domain/auth/core/jwt/JwtGenerator.java (2)
26-29: 플랫폼 기본 문자셋 사용 지양 + 약한 키 사전 검증기본 문자셋 의존은 환경별 상이한 키를 만들 수 있습니다. UTF-8을 명시하고 HS256 최소 길이(32바이트) 미만이면 즉시 실패하도록 하세요.
- this.secretKey = Keys.hmacShaKeyFor(jwtProperties.secret().getBytes()); + byte[] keyBytes = jwtProperties.secret().getBytes(java.nio.charset.StandardCharsets.UTF_8); + if (keyBytes.length < 32) { + throw new IllegalArgumentException("JWT secret must be at least 256 bits (32 bytes) for HS256"); + } + this.secretKey = Keys.hmacShaKeyFor(keyBytes);추가 import:
import java.nio.charset.StandardCharsets;
53-55: AT/RT 사용자 역할 claim 타입 불일치 — 문자열로 통일 권장AT는
userRole.name()을 넣고, RT는 enum 자체를 넣고 있습니다. 파서/검증 로직에서 타입 불일치로 오류가 날 수 있으니 통일하세요.- claims.put(Constants.USER_ROLE, userRole); + claims.put(Constants.USER_ROLE, userRole.name());
🧹 Nitpick comments (12)
src/main/java/com/permitseoul/permitserver/domain/event/api/service/EventService.java (2)
76-78: 빈 입력 시 불필요한 리스트 할당 제거 제안초기에 리스트를 만들지 말고, 빈 입력이면 즉시 불변 빈 리스트를 반환하면 됩니다.
- final List<EventDetailResponse.LineupCategory> result = new ArrayList<>(); - if (rawLineupText == null || rawLineupText.isBlank()) return result; + if (rawLineupText == null || rawLineupText.isBlank()) return List.of(); + final List<EventDetailResponse.LineupCategory> result = new ArrayList<>();
93-98: 콤마 분할 시 ‘이름에 콤마 포함’ 케이스 검증 필요예: "Earth, Wind & Fire"가 잘리며 데이터 손실 위험. 또한 전각 콤마(，)·일본어 구분자(、)도 실데이터에 섞여 들어올 수 있습니다. 최소한 여러 콤마 변형을 허용하도록 확장 권장합니다.
- Arrays.stream(artistPart.split("\\s*,\\s*")) // 쉼표 기준 split, 주변 공백 무시 + Arrays.stream(artistPart.split("\\s*[,，、]\\s*")) // ASCII 콤마, 전각 콤마(，), 일본어 '、' 지원원한다면 따옴표로 감싼 토큰(예: "Earth, Wind & Fire")을 보존하는 CSV 파싱(Commons CSV 등)으로의 전환도 제안드립니다. 테스트 케이스(일반/연속 구분자/전각 콤마/따옴표 포함) 작성도 도와드릴게요.
src/main/java/com/permitseoul/permitserver/global/response/code/ErrorCode.java (1)
118-118: 메시지 톤/표기 일관화 제안메시지를 기존 스타일에 맞춰 한글/영문 표기를 정리하면 더 읽기 좋습니다.
- INTERNAL_RT_REDIS_ERROR(HttpStatus.INTERNAL_SERVER_ERROR, 50013, "refreshToken redis 에러입니다."), + INTERNAL_RT_REDIS_ERROR(HttpStatus.INTERNAL_SERVER_ERROR, 50013, "리프레시 토큰 Redis 오류입니다."),src/main/java/com/permitseoul/permitserver/domain/auth/core/domain/RefreshToken.java (1)
15-18: 단일 세션(사용자당 1키) 정책 검토
userId를 key로 단일 세션 모델을 강제합니다. 멀티 디바이스(동시 로그인) 요구가 있다면 deviceId/세션키를 키 스키마에 포함하거나 Set 구조로 전환이 필요합니다.src/main/java/com/permitseoul/permitserver/domain/auth/core/domain/Token.java (1)
13-16: 불변 값 객체로 강화(필드 final)토큰은 값 객체 성격이 강하므로 필드를
final로 두면 불변성이 보장되고 안전합니다.- private String accessToken; + private final String accessToken; - private String refreshToken; + private final String refreshToken;src/test/java/com/permitseoul/permitserver/auth/jwt/JwtProviderTest.java (1)
9-9: 미사용 import 제거
RefreshTokenManager를 사용하지 않으므로 import 제거를 권장합니다(경고 제거).-import com.permitseoul.permitserver.domain.auth.core.jwt.RefreshTokenManager;src/main/java/com/permitseoul/permitserver/domain/auth/core/exception/AuthRTException.java (1)
3-4: 에러코드 전달 생성자 추가 권장핸들러에서 정확한
ErrorCode매핑을 위해 생성자 오버로드를 두는 편이 안전합니다(다른 예외들과 일관).package com.permitseoul.permitserver.domain.auth.core.exception; +import com.permitseoul.permitserver.global.response.code.ErrorCode; + public class AuthRTException extends AuthCoreException { + public AuthRTException(final ErrorCode errorCode) { + super(errorCode); + } + public AuthRTException(final ErrorCode errorCode, final Throwable cause) { + super(errorCode, cause); + } }src/main/java/com/permitseoul/permitserver/domain/auth/core/exception/AuthRTNotFoundException.java (1)
3-4: 예외 매핑은 이미 서비스 레이어에서 처리됨 — 생성자 추가는 선택적 권장사항입니다RefreshTokenManager에서 AuthRTNotFoundException을 발생시키고(src/main/java/com/permitseoul/permitserver/domain/auth/core/jwt/RefreshTokenManager.java:33), AuthService에서 해당 예외를 잡아 ErrorCode.UNAUTHORIZED_WRONG_RT로 재매핑하고 있습니다(src/main/java/com/permitseoul/permitserver/domain/auth/api/service/AuthService.java:112-113,127-128). 전역 ExceptionHandler로 직접 매핑되도록 통일하려면 ErrorCode를 받는 생성자를 추가하면 되지만, 현재 흐름에서는 필수는 아닙니다.
src/main/java/com/permitseoul/permitserver/domain/auth/api/exception/AuthRedisException.java (1)
6-9: cause 체인 포함 오버로드 추가원인 예외(DataAccessException 등) 보존을 위해 cause를 받는 생성자도 두면 트러블슈팅에 유리합니다.
public class AuthRedisException extends AuthApiException { public AuthRedisException(ErrorCode errorCode) { super(errorCode); } + public AuthRedisException(ErrorCode errorCode, Throwable cause) { + super(errorCode, cause); + } }src/main/java/com/permitseoul/permitserver/domain/user/core/component/UserSaver.java (1)
13-15: 얇은 래퍼 유지 또는 단순화 고려현재는 Repository에 대한 단순 위임입니다. 도메인 규칙/감사 로직을 추가할 계획이 없다면 서비스에서 Repository를 직접 주입하는 단순화도 고려해 볼 수 있습니다. 유지한다면 Javadoc 등으로 컴포넌트 경계의 의도를 짧게 명시해 주세요.
src/main/java/com/permitseoul/permitserver/domain/auth/core/jwt/RefreshTokenManager.java (1)
32-37: 예외 정보 보강(선택)불일치 시 AuthRTException을 던지는데, 메시지/컨텍스트가 없어 운영 디버깅이 어렵습니다. 민감정보를 제외한 최소한의 컨텍스트(예: userId)만 포함하는 생성자 추가를 고려해 주세요.
src/main/java/com/permitseoul/permitserver/domain/auth/api/service/AuthService.java (1)
85-90: 세션 정책 명확화(단일 RT vs 다중 디바이스)userId를 키로 RT를 1개만 저장/검증/삭제하는 구조입니다. 다중 디바이스 동시 로그인 요구가 있으면 키 스키마(예: userId:deviceId)와 로테이션/폐기 정책을 정의해야 합니다.
Also applies to: 109-111, 131-132
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
build.gradle(0 hunks)src/main/java/com/permitseoul/permitserver/domain/auth/api/exception/AuthRedisException.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/auth/api/service/AuthService.java(7 hunks)src/main/java/com/permitseoul/permitserver/domain/auth/core/domain/RefreshToken.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/auth/core/domain/Token.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/auth/core/exception/AuthRTException.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/auth/core/exception/AuthRTNotFoundException.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/auth/core/jwt/JwtGenerator.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/auth/core/jwt/RTCacheManager.java(0 hunks)src/main/java/com/permitseoul/permitserver/domain/auth/core/jwt/RefreshTokenManager.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/auth/core/repository/RefreshTokenRepository.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/event/api/service/EventService.java(2 hunks)src/main/java/com/permitseoul/permitserver/domain/user/core/component/UserSaver.java(1 hunks)src/main/java/com/permitseoul/permitserver/global/config/CacheConfig.java(0 hunks)src/main/java/com/permitseoul/permitserver/global/filter/ExceptionHandlerFilter.java(0 hunks)src/main/java/com/permitseoul/permitserver/global/response/code/ErrorCode.java(1 hunks)src/test/java/com/permitseoul/permitserver/auth/jwt/JwtGeneratorCacheTest.java(0 hunks)src/test/java/com/permitseoul/permitserver/auth/jwt/JwtProviderTest.java(1 hunks)src/test/java/com/permitseoul/permitserver/domain/reservation/api/service/ReservationServiceTest.java(0 hunks)
💤 Files with no reviewable changes (6)
- build.gradle
- src/test/java/com/permitseoul/permitserver/auth/jwt/JwtGeneratorCacheTest.java
- src/test/java/com/permitseoul/permitserver/domain/reservation/api/service/ReservationServiceTest.java
- src/main/java/com/permitseoul/permitserver/domain/auth/core/jwt/RTCacheManager.java
- src/main/java/com/permitseoul/permitserver/global/config/CacheConfig.java
- src/main/java/com/permitseoul/permitserver/global/filter/ExceptionHandlerFilter.java
🧰 Additional context used
🧬 Code graph analysis (4)
src/main/java/com/permitseoul/permitserver/domain/auth/core/domain/Token.java (1)
src/main/java/com/permitseoul/permitserver/domain/auth/core/domain/RefreshToken.java (1)
Getter(8-33)
src/main/java/com/permitseoul/permitserver/domain/auth/core/jwt/RefreshTokenManager.java (2)
src/main/java/com/permitseoul/permitserver/domain/auth/core/exception/AuthRTException.java (1)
AuthRTException(3-4)src/main/java/com/permitseoul/permitserver/domain/auth/core/exception/AuthRTNotFoundException.java (1)
AuthRTNotFoundException(3-4)
src/main/java/com/permitseoul/permitserver/domain/auth/core/domain/RefreshToken.java (1)
src/main/java/com/permitseoul/permitserver/domain/auth/core/domain/Token.java (1)
AllArgsConstructor(8-26)
src/main/java/com/permitseoul/permitserver/domain/auth/api/service/AuthService.java (3)
src/main/java/com/permitseoul/permitserver/domain/auth/api/exception/AuthRedisException.java (1)
AuthRedisException(5-9)src/main/java/com/permitseoul/permitserver/domain/auth/core/exception/AuthRTNotFoundException.java (1)
AuthRTNotFoundException(3-4)src/main/java/com/permitseoul/permitserver/domain/auth/core/exception/AuthRTException.java (1)
AuthRTException(3-4)
🔇 Additional comments (11)
src/main/java/com/permitseoul/permitserver/domain/event/api/service/EventService.java (1)
100-102: 빈 아티스트 목록 스킵 처리 적절함빈 카테고리 누적 방지되어 출력 품질에 유리합니다. LGTM.
src/main/java/com/permitseoul/permitserver/domain/auth/core/jwt/JwtGenerator.java (2)
48-63: LGTM: RT 캐시 사이드이펙트 제거 방향 타당Redis 전환 맥락에서 메서드 내 캐시 연동 제거는 일관됩니다. 토큰 생성 자체는 문제 없어 보입니다.
65-69: 만료 시간 단위는 밀리초(ms)로 사용됩니다 — 설정값을 밀리초로 제공하거나 코드에서 단위를 변환하세요.JwtProperties의 accessTokenExpirationTime/refreshTokenExpirationTime은 long이고 JwtGenerator는 now.getTime()에 더하며, RefreshTokenManager.saveRefreshTokenInRedis는 ttlMillis를 받습니다. 리소스 설정(auth.jwt.*)에 ms 단위 값이 들어가 있는지 확인하거나 설정명/타입(Duration)으로 명확히 변경하세요.
참고 파일: src/main/java/com/permitseoul/permitserver/domain/auth/core/jwt/JwtProperties.java, JwtGenerator.java; src/main/java/com/permitseoul/permitserver/domain/auth/api/service/AuthService.java; src/main/java/com/permitseoul/permitserver/domain/auth/core/jwt/RefreshTokenManager.java.
src/main/java/com/permitseoul/permitserver/global/response/code/ErrorCode.java (1)
118-118: RT Cache 관련 에러코드 사용 여부 확인 — 수동 검증 필요
PR로 캐시 기반 RT 사용이 제거되었다면 INTERNAL_RT_CACHE_ERROR(50001) 제거 또는 @deprecated 처리 검토. 자동 검색이 실패해 사용처 확인이 불가하므로 로컬에서 아래 명령으로 확인해 주세요:
rg -n --hidden --no-ignore -S 'INTERNAL_RT_CACHE_ERROR|RTCache|RT_CACHE' || grep -RIn --exclude-dir=build -E 'INTERNAL_RT_CACHE_ERROR|RTCache|RT_CACHE' srcsrc/main/java/com/permitseoul/permitserver/domain/auth/core/domain/RefreshToken.java (1)
8-13: Redis TTL 인스턴스 단위 적용 설계는 적절합니다
@TimeToLive로 인스턴스별 만료를 주는 접근은 Redis 기반 RT 관리에 잘 맞습니다. 👍src/main/java/com/permitseoul/permitserver/domain/auth/core/domain/Token.java (1)
8-11: 생성/빌더 접근 제어 일원화 좋습니다외부에서 직접 생성 불가하도록 제한하고
of(...)만 노출하는 구조 적절합니다.src/main/java/com/permitseoul/permitserver/domain/auth/core/repository/RefreshTokenRepository.java (1)
6-7: 간결하고 충분합니다Spring Data Redis CrudRepository 확장으로 현재 요구사항 충족에 적합합니다.
src/main/java/com/permitseoul/permitserver/domain/user/core/component/UserSaver.java (1)
10-10: 리네임 적절/LGTMUserCreator → UserSaver, createUser → saveUser로의 변경이 역할과 일치합니다. 빈 주입 변경도 무리 없어 보입니다.
src/main/java/com/permitseoul/permitserver/domain/auth/core/jwt/RefreshTokenManager.java (1)
28-30: 삭제 실패 시 예외 전파 전략 확인deleteById에서 발생하는 DataAccessException은 현재 상위(Service)로 그대로 전파됩니다. signUp/login/reissue와 동일하게 Service 레이어에서 AuthRedisException으로 매핑하도록 유지하면 일관적입니다. 그대로 두되 Service에서 catch가 있는지 확인해 주세요.
src/main/java/com/permitseoul/permitserver/domain/auth/api/service/AuthService.java (2)
79-85: 소셜 액세스 토큰 검증 로직 깔끔/LGTMOptional + filter로 공백 토큰 방지 처리 명확합니다.
182-184: RT TTL 단위 확인 — JwtProperties의 값(밀리초) 사용 확인됨
- JwtProperties.refreshTokenExpirationTime()는 long 타입으로 JwtGenerator에서 Date(now.getTime() + jwtProperties.refreshTokenExpirationTime())에 그대로 더해져 있어 밀리초 단위로 사용됩니다.
- RefreshTokenManager.saveRefreshTokenInRedis의 시그니처는 ttlMillis로 명명되어 있으며(요청 출력에서 확인됨) 매개변수 이름과 JwtGenerator 사용 방식이 일관되어 있어, 현재 코드에서는 밀리초 단위 값(ms)을 전달하는 것이 맞습니다.
조치: AuthService의 호출(현재 jwtProperties.refreshTokenExpirationTime() 전달)은 올바른 단위(ms)로 보입니다. 단위 혼동 우려가 있으므로 설정 파일(application.yml/properties)에서 auth.jwt.refresh-token-expiration-time 값이 의도한 기간(예: 7일 → 604800000)으로 설정돼 있는지 확인하세요.
| saveRefreshTokenInRedis(newUserEntity.getUserId(), newToken.getRefreshToken()); | ||
|
|
||
| return TokenDto.of(newToken.getAccessToken(), newToken.getRefreshToken()); | ||
| } catch (AuthPlatformFeignException e) { | ||
| throw new AuthUnAuthorizedFeignException(ErrorCode.UNAUTHORIZED_FEIGN, e.getPlatformErrorCode()); | ||
| } catch (UserDuplicateException e ) { | ||
| throw new AuthUnAuthorizedException(ErrorCode.CONFLICT); | ||
| } catch (DataAccessException e) { | ||
| throw new AuthRedisException(ErrorCode.INTERNAL_RT_REDIS_ERROR); | ||
| } |
There was a problem hiding this comment.
Redis 저장 실패 예외 매핑 누락(현재 코드 기준으로 전파됨)
RefreshTokenManager.saveRefreshTokenInRedis가 DataAccessException을 내부에서 AuthRTException으로 래핑하고 있어, 여기서의 catch (DataAccessException)는 동작하지 않습니다. 결과적으로 signUp에서 Redis 장애가 발생하면 의도한 AuthRedisException(INTERNAL_RT_REDIS_ERROR)로 변환되지 않을 수 있습니다.
두 가지 중 하나로 일관화하세요.
- 권장: RefreshTokenManager에서 래핑 제거(제안한 diff 적용) → 현재 블록 유지.
- 대안: 여기서 AuthRTException도 함께 매핑.
대안 적용 diff:
- } catch (DataAccessException e) {
+ } catch (AuthRTException | DataAccessException e) {
throw new AuthRedisException(ErrorCode.INTERNAL_RT_REDIS_ERROR);
}📝 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.
| saveRefreshTokenInRedis(newUserEntity.getUserId(), newToken.getRefreshToken()); | |
| return TokenDto.of(newToken.getAccessToken(), newToken.getRefreshToken()); | |
| } catch (AuthPlatformFeignException e) { | |
| throw new AuthUnAuthorizedFeignException(ErrorCode.UNAUTHORIZED_FEIGN, e.getPlatformErrorCode()); | |
| } catch (UserDuplicateException e ) { | |
| throw new AuthUnAuthorizedException(ErrorCode.CONFLICT); | |
| } catch (DataAccessException e) { | |
| throw new AuthRedisException(ErrorCode.INTERNAL_RT_REDIS_ERROR); | |
| } | |
| saveRefreshTokenInRedis(newUserEntity.getUserId(), newToken.getRefreshToken()); | |
| return TokenDto.of(newToken.getAccessToken(), newToken.getRefreshToken()); | |
| } catch (AuthPlatformFeignException e) { | |
| throw new AuthUnAuthorizedFeignException(ErrorCode.UNAUTHORIZED_FEIGN, e.getPlatformErrorCode()); | |
| } catch (UserDuplicateException e ) { | |
| throw new AuthUnAuthorizedException(ErrorCode.CONFLICT); | |
| } catch (AuthRTException | DataAccessException e) { | |
| throw new AuthRedisException(ErrorCode.INTERNAL_RT_REDIS_ERROR); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/com/permitseoul/permitserver/domain/auth/api/service/AuthService.java
around lines 62 to 71, the catch(DataAccessException) won't catch the
AuthRTException that RefreshTokenManager currently wraps DataAccessException
into, so Redis failures escape the intended AuthRedisException mapping; either
remove the internal wrapping in RefreshTokenManager so DataAccessException is
thrown (preferred) and keep the existing catch block, or update this method to
also catch AuthRTException and rethrow new
AuthRedisException(ErrorCode.INTERNAL_RT_REDIS_ERROR) to ensure Redis errors are
consistently mapped.
src/main/java/com/permitseoul/permitserver/domain/auth/api/service/AuthService.java
Show resolved
Hide resolved
src/main/java/com/permitseoul/permitserver/domain/auth/api/service/AuthService.java
Show resolved
Hide resolved
src/main/java/com/permitseoul/permitserver/domain/auth/api/service/AuthService.java
Show resolved
Hide resolved
| /** 토큰 해시(SHA-256 등). 평문 저장 금지 */ | ||
| private String refreshToken; | ||
|
|
||
| /** 만료 시간(초 단위). @RedisHash 클래스 단위 TTL 대신 인스턴스별 TTL 추천 */ | ||
| @TimeToLive | ||
| private long ttlSeconds; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
평문 저장 혼동 방지: 필드명 명확화 권장
주석상 해시 저장 정책이라면 필드명을 refreshTokenHash로 바꿔 혼동을 줄이는 게 안전합니다. 이후 비교 시에도 반드시 해시값 기반 constant-time 비교를 사용해야 합니다.
- /** 토큰 해시(SHA-256 등). 평문 저장 금지 */
- private String refreshToken;
+ /** 토큰 해시(SHA-256 등). 평문 저장 금지 */
+ private String refreshTokenHash;팩토리도 함께 정리:
- public static RefreshToken of(final long userId, final String refreshToken, final long ttlSeconds) {
+ public static RefreshToken of(final long userId, final String refreshTokenHash, final long ttlSeconds) {
return RefreshToken.builder()
.userId(userId)
- .refreshToken(refreshToken)
+ .refreshTokenHash(refreshTokenHash)
.ttlSeconds(ttlSeconds)
.build();
}🤖 Prompt for AI Agents
In
src/main/java/com/permitseoul/permitserver/domain/auth/core/domain/RefreshToken
around lines 19 to 24, the field name refreshToken is ambiguous given the
comment that a hash (SHA-256 etc.) is stored; rename the field to
refreshTokenHash, update its Javadoc to state it stores the hashed token (never
plaintext), and refactor all constructors/factory methods, getters/setters,
equals/hashCode/usages and any persistence mappings to use the new name; when
comparing tokens ensure callers perform hash(input) and use a constant-time
comparison utility for the stored hash; also update any tests and serialization
mappings that reference the old field name.
| public static RefreshToken of(final long userId, final String refreshToken, final long ttlSeconds) { | ||
| return RefreshToken.builder() | ||
| .userId(userId) | ||
| .refreshToken(refreshToken) | ||
| .ttlSeconds(ttlSeconds) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
해시 강제 및 안전 비교 사용 여부 점검
실제 저장 전 해시가 강제되는지, 검증 시 MessageDigest.isEqual(...) 등 constant-time 비교가 적용되는지 확인이 필요합니다. 미적용 시 토큰 유출/타이밍 공격 위험이 있습니다.
점검 스크립트:
필요하면 RefreshTokenManager 내 해싱/비교 로직 보강 PR 패치 제안 드리겠습니다.
🏁 Script executed:
#!/bin/bash
# RefreshToken 생성/검증 경로에서 해시 및 안전 비교 사용 여부 탐색
rg -n -C3 'RefreshToken\.of\(|hash|SHA-256|MessageDigest\.isEqual|equals\(' src/main/java/com/permitseoul/permitserver/domain/authLength of output: 3672
토큰 해싱 강제화 및 상수시간 비교 적용 — 긴급 수정 필요
저장 시 토큰 해시 적용이 보이지 않고(평문 저장 가능), 검증에 Objects.equals가 사용되어 타이밍 공격/토큰 유출 위험이 확인됨.
- 위치(필요한 수정 지점):
- src/main/java/com/permitseoul/permitserver/domain/auth/core/jwt/RefreshTokenManager.java — 저장 호출( refreshTokenRepository.save(RefreshToken.of(...)) , ~line 22) 및 validateSameWithOriginalRefreshToken의 Objects.equals(...), ~line 34.
- src/main/java/com/permitseoul/permitserver/domain/auth/core/domain/RefreshToken.java — of(...)가 평문 값을 그대로 설정(예: lines 26–32).
- 권고 조치(즉시 적용):
- 저장 전에 들어온 refreshToken을 안전하게 해시(예: SHA‑256 또는 HMAC‑SHA256)하여 저장(필드명은 refreshTokenHash 등으로 명확화). 해싱을 RefreshToken.of 내부에서 강제하거나, RefreshTokenManager에서 해시 후 of에 전달하도록 변경.
- 검증 시에는 수신 토큰을 동일 방식으로 해시한 뒤 MessageDigest.isEqual(byte[] stored, byte[] computed)로 상수시간 비교(문자열 equals 사용 금지).
- 추가 권고: 단순 해시만으로는 탈취 시 재사용 위험이 있으므로 per‑token salt 또는 HMAC(비밀키) 사용 검토.
- 긴급성: 보안 취약점이므로 배포 전 반드시 패치 적용 필요. 원하면 수정 PR 초안 제공.
🤖 Prompt for AI Agents
In
src/main/java/com/permitseoul/permitserver/domain/auth/core/domain/RefreshToken.java
around lines 26–32 and in
src/main/java/com/permitseoul/permitserver/domain/auth/core/jwt/RefreshTokenManager.java
around lines ~22 and ~34, the plain refresh token is being stored and validated
using Objects.equals making it vulnerable to leakage and timing attacks; fix by
ensuring the token is hashed before persistence (rename stored field to
refreshTokenHash or similar) — either compute a secure hash (e.g., SHA-256) or
HMAC-SHA256 of the incoming token in RefreshToken.of (or compute in
RefreshTokenManager and pass the hash into of), persist only the hash, and when
validating compute the same hash for the presented token and compare using
MessageDigest.isEqual(byte[] storedHash, byte[] computedHash) for constant-time
comparison; optionally document/prepare hooks for per-token salt or HMAC secret
rotation to mitigate replay if needed.
| @Getter | ||
| private final Key secretKey; |
There was a problem hiding this comment.
비밀키 Getter 공개는 보안 리스크 — 접근 범위 축소 또는 제거 권장
서명키를 public으로 노출하면 오남용/로그 유출 등 치명적 사고로 이어질 수 있습니다. 최소한 패키지 범위로 축소하거나 게터를 제거하고 필요한 작업은 메서드로 위임하세요.
적용 예시(diff):
- @Getter
+ @Getter(lombok.AccessLevel.PACKAGE)
private final Key secretKey;추가: import 문에 lombok.AccessLevel이 필요합니다.
📝 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.
| @Getter | |
| private final Key secretKey; | |
| @Getter(lombok.AccessLevel.PACKAGE) | |
| private final Key secretKey; |
🤖 Prompt for AI Agents
In
src/main/java/com/permitseoul/permitserver/domain/auth/core/jwt/JwtGenerator.java
around lines 23-24 the secretKey field is annotated with a public getter which
exposes the signing key; reduce its visibility or remove the getter to mitigate
security risk. Replace the public getter with a package-private getter (e.g.,
use Lombok's @Getter(AccessLevel.PACKAGE)) or remove the getter entirely and
expose only specific methods that perform signing/verification using secretKey
internally; also add the import lombok.AccessLevel if applying the AccessLevel
change.
src/main/java/com/permitseoul/permitserver/domain/auth/core/jwt/RefreshTokenManager.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/main/java/com/permitseoul/permitserver/domain/auth/api/service/AuthService.java (2)
95-97: login의 Redis 예외 매핑도 일관화 필요 (이전 코멘트와 동일)
DataAccessException도 함께 매핑해 누락을 방지하세요.- } catch (AuthRTException e) { + } catch (AuthRTException | DataAccessException e) { throw new AuthRedisException(ErrorCode.INTERNAL_RT_REDIS_ERROR); }
116-118: reissue에서 Redis 오류를 401로 매핑하는 것은 부적절합니다Redis 저장 실패는 인증 자격 문제(401)가 아니라 서버/인프라 오류로 분류(5xx)되어야 합니다.
AuthRedisException으로 통일하고DataAccessException도 포착하세요.- } catch (AuthRTException e) { - throw new AuthUnAuthorizedException(ErrorCode.INTERNAL_RT_REDIS_ERROR); + } catch (AuthRTException | DataAccessException e) { + throw new AuthRedisException(ErrorCode.INTERNAL_RT_REDIS_ERROR); }
🧹 Nitpick comments (3)
src/main/java/com/permitseoul/permitserver/domain/auth/api/service/AuthService.java (3)
104-110: 재발급 플로우: 저장 실패 시 롤백/일관성 고려RT 검증 통과 후 새 토큰 발급 → Redis 저장 실패 시 기존 RT가 그대로 남습니다. 재시도/보상 전략(예: 저장 실패 시 기존 RT 유효기간 단축 또는 재시도) 검토를 권장합니다.
62-63: 회원가입 시 RT 저장 실패 재시도/폴백 고려초기 온보딩 단계에서 일시적 Redis 장애로 가입 전체가 실패하는 것은 UX 저하입니다. 단기 재시도(지수 백오프 1~2회) 후 실패 시 가입은 유지하되 RT 발급만 재시도 안내하는 선택지도 검토하세요.
182-184: RT TTL 단위 일치 확인 — 수정 불필요jwtProperties.refreshTokenExpirationTime()는 밀리초(ms)로 사용되며 JwtGenerator는 now.getTime()에 더해(ms) 만료일을 계산합니다. RefreshTokenManager.saveRefreshTokenInRedis의 파라미터명(ttlMillis)도 ms를 기대하고, 내부에서 TimeUnit.MILLISECONDS.toSeconds(...)로 초 단위로 변환해 Redis에 저장하므로 단위 불일치로 인한 문제는 없습니다.
사소한 스타일 제안:
- refreshTokenManager.saveRefreshTokenInRedis (userId, refreshToken, jwtProperties.refreshTokenExpirationTime()); + refreshTokenManager.saveRefreshTokenInRedis(userId, refreshToken, jwtProperties.refreshTokenExpirationTime());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/permitseoul/permitserver/domain/auth/api/service/AuthService.java(7 hunks)src/main/java/com/permitseoul/permitserver/domain/auth/core/jwt/RefreshTokenManager.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/permitseoul/permitserver/domain/auth/core/jwt/RefreshTokenManager.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/permitseoul/permitserver/domain/auth/api/service/AuthService.java (6)
src/main/java/com/permitseoul/permitserver/domain/auth/api/exception/AuthRedisException.java (1)
AuthRedisException(5-9)src/main/java/com/permitseoul/permitserver/domain/auth/api/exception/AuthUnAuthorizedException.java (1)
AuthUnAuthorizedException(5-9)src/main/java/com/permitseoul/permitserver/domain/auth/core/exception/AuthRTNotFoundException.java (1)
AuthRTNotFoundException(3-4)src/main/java/com/permitseoul/permitserver/domain/user/core/exception/UserDuplicateException.java (1)
UserDuplicateException(5-6)src/main/java/com/permitseoul/permitserver/domain/auth/core/exception/AuthRTException.java (1)
AuthRTException(3-4)src/main/java/com/permitseoul/permitserver/domain/auth/core/exception/AuthWrongJwtException.java (1)
AuthWrongJwtException(3-4)
🔇 Additional comments (3)
src/main/java/com/permitseoul/permitserver/domain/auth/api/service/AuthService.java (3)
123-131: 로그아웃 Redis 삭제 실패 매핑 추가 LGTM삭제 시
DataAccessException을AuthRedisException으로 변환하는 처리가 일관되고 적절합니다.
157-157: User 저장 책임 분리 변경 LGTM
UserSaver로 위임하는 방향 적절합니다.
69-71: Redis 예외 매핑 보강: DataAccessException도 함께 처리하세요RefreshTokenManager.saveRefreshTokenInRedis가 refreshTokenRepository.save를 호출하므로 DataAccessException이 전파될 수 있습니다. AuthService의 관련 catch(회원가입/로그인/토큰 재발급 경로)에서 AuthRTException만 잡고 있다면 DataAccessException도 함께 매핑하거나 별도 처리하세요. (참고: logout()에서는 이미 DataAccessException을 별도 처리하고 있습니다.)
- } catch (AuthRTException e) { + } catch (AuthRTException | DataAccessException e) { throw new AuthRedisException(ErrorCode.INTERNAL_RT_REDIS_ERROR); }위치: src/main/java/com/permitseoul/permitserver/domain/auth/api/service/AuthService.java — saveRefreshTokenInRedis 호출부(회원가입/로그인/재발급).
| } catch (UserNotFoundException e ) { | ||
| throw new AuthSocialNotFoundApiException(ErrorCode.NOT_FOUND_USER, socialAccessToken); | ||
| } catch (AuthRTException e) { |
There was a problem hiding this comment.
민감정보(소셜 액세스 토큰) 노출 위험
예외 생성자에 원문 토큰을 넘기면 로깅/응답으로 노출될 수 있습니다. 마스킹하거나 제거하세요.
- } catch (UserNotFoundException e ) {
- throw new AuthSocialNotFoundApiException(ErrorCode.NOT_FOUND_USER, socialAccessToken);
+ } catch (UserNotFoundException e ) {
+ // 민감정보 노출 방지
+ throw new AuthSocialNotFoundApiException(ErrorCode.NOT_FOUND_USER, "REDACTED");
}필요 시 서버 내부 로깅에만 마스킹된 토큰(예: 앞 6자+말줄임) 사용을 권장합니다.
📝 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.
| } catch (UserNotFoundException e ) { | |
| throw new AuthSocialNotFoundApiException(ErrorCode.NOT_FOUND_USER, socialAccessToken); | |
| } catch (AuthRTException e) { | |
| } catch (UserNotFoundException e ) { | |
| // 민감정보 노출 방지 | |
| throw new AuthSocialNotFoundApiException(ErrorCode.NOT_FOUND_USER, "REDACTED"); | |
| } catch (AuthRTException e) { |
🤖 Prompt for AI Agents
In
src/main/java/com/permitseoul/permitserver/domain/auth/api/service/AuthService.java
around lines 93 to 95, the code currently passes the raw socialAccessToken into
AuthSocialNotFoundApiException which may expose sensitive tokens in
logs/responses; instead, stop supplying the original token to the exception
(pass no token or a non-sensitive placeholder) or pass a masked version (e.g.,
first 6 chars + "...") only for internal logs; update the exception call to
remove the sensitive argument and, if you need traceability, generate and pass a
maskedToken variable computed before the catch for internal logging while
ensuring the public exception payload contains no raw token.
🔥Pull requests
⛳️ 작업한 브랜치
👷 작업한 내용
🚨 참고 사항