refactor: Token VO 적용, 제공과 저장 관심사 분리, 환경변수 사용하도록 수정 #479
Conversation
- '토큰을 객체로 만들 때, 실제로 그 토큰이 가진 성질을 100% 반영해야 하는가?'에 대해서 고민이 되었다. 그런데 실제로 코드에서 토큰이 사용되는 것은 "문자열로 구성된 발급"이 대부분이므로, 토큰이 실제로 가지는 claim들을 100% 구현하는 것은 오히려 복잡도를 증가시킨다는 생각이 들었다.
- auth 하위의 테스트 코드에 'RedisTemplate' autowired 존재하지 않게 - io.jsonwebtoken.Claims 에 대한 참조는 반드시 필요한 곳에만 존재하게
- 토큰과 관련된 설정 값은 TokenType이 아니라 환경변수를 사용하도록 점진적으로 변경한다.
- 값 객체와 제네릭을 최대한 활용하여 코드에 의미를 전달하고 중복 코드를 줄인다.
nayonsoso
requested review from
Gyuhyeok99,
lsy1307,
whqtker and
wibaek
as code owners
Walkthrough
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Suggested reviewers
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. ✨ 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. 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: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/main/java/com/example/solidconnection/auth/client/AppleOAuthClient.java (1)
81-90: 중요: ID 토큰 검증 시 서명만 확인하고 핵심 클레임(aud/iss/exp 등)을 검증하지 않습니다.
서명 검증만으로는 다른 앱용 토큰, 만료 토큰, 위조 iss를 걸러내지 못합니다.
최소 aud(=client_id), iss(=https://appleid.apple.com), exp/iat, 필요 시 nonce를 검증하세요.
- 토큰 파싱 후 Claims에서 aud/iss/exp 등을 체크하세요.
- 검증 실패 시 INVALID_APPLE_ID_TOKEN으로 매핑하세요.
- 이메일이 누락될 수 있으니 null 처리도 고려하세요.
적용 예시(diff):
- private String parseEmailFromToken(PublicKey applePublicKey, String idToken) { + private String parseEmailFromToken(PublicKey applePublicKey, String idToken) { try { - return Jwts.parser() - .setSigningKey(applePublicKey) - .parseClaimsJws(idToken) - .getBody() - .get("email", String.class); + var claims = Jwts.parser() + .setSigningKey(applePublicKey) + .parseClaimsJws(idToken) + .getBody(); + + // 1) iss 검증 + if (!"https://appleid.apple.com".equals(claims.getIssuer())) { + throw new CustomException(INVALID_APPLE_ID_TOKEN); + } + // 2) aud(=client_id) 검증 + if (!properties.clientId().equals(claims.getAudience())) { + throw new CustomException(INVALID_APPLE_ID_TOKEN); + } + // 3) exp 검증 + if (claims.getExpiration() == null || claims.getExpiration().getTime() < System.currentTimeMillis()) { + throw new CustomException(INVALID_APPLE_ID_TOKEN); + } + // 4) 이메일은 없을 수 있으니 null 허용 또는 후속 처리 + return claims.get("email", String.class); } catch (Exception e) { - throw new CustomException(INVALID_APPLE_ID_TOKEN); + throw new CustomException(INVALID_APPLE_ID_TOKEN); } }src/main/java/com/example/solidconnection/auth/client/KakaoOAuthClient.java (1)
59-67: 에러 코드 판별을 문자열 포함으로 처리하면 취약합니다. 명시적 타입/본문 파싱으로 전환하세요.
e.getMessage().contains("KOE303"/"KOE320") 방식은 메시지 포맷 변화에 취약합니다.
RestClientResponseException으로 캐치 후 상태코드와 JSON body의 error_code를 읽는 편이 견고합니다.
- RestClientResponseException를 우선 캐치하세요.
- responseBodyAsString에서 error 또는 error_code를 파싱하세요.
- 해당 코드에 따라 CustomException 매핑을 수행하세요.
적용 예시(diff):
- } catch (Exception e) { - if (e.getMessage().contains("KOE303")) { - throw new CustomException(KAKAO_REDIRECT_URI_MISMATCH); - } - if (e.getMessage().contains("KOE320")) { - throw new CustomException(INVALID_OR_EXPIRED_KAKAO_AUTH_CODE); - } - throw new CustomException(INVALID_OR_EXPIRED_KAKAO_AUTH_CODE); + } catch (org.springframework.web.client.RestClientResponseException e) { + String body = e.getResponseBodyAsString(); + if (body != null && body.contains("KOE303")) { + throw new CustomException(KAKAO_REDIRECT_URI_MISMATCH); + } + if (body != null && body.contains("KOE320")) { + throw new CustomException(INVALID_OR_EXPIRED_KAKAO_AUTH_CODE); + } + throw new CustomException(INVALID_OR_EXPIRED_KAKAO_AUTH_CODE); + } catch (Exception e) { + throw new CustomException(INVALID_OR_EXPIRED_KAKAO_AUTH_CODE, e); }src/main/java/com/example/solidconnection/auth/service/signin/EmailSignInService.java (1)
24-29: readOnly 트랜잭션에서 쓰기 작업이 발생합니다.
검증 결과 resetQuitedAt 메서드가 엔티티의 quitedAt 필드를 변경하며, generateAndSaveRefreshToken 호출 시 토큰 저장소(Redis 등)에 값이 기록되는 것을 확인했습니다. 따라서 EmailSignInService.signIn에 적용된@Transactional(readOnly = true)는 부적절합니다.
- 트랜잭션 읽기 전용 설정 제거
EmailSignInService.signIn 메서드에서@Transactional(readOnly = true)를 삭제하고 기본@Transactional으로 변경해야 합니다.- 쓰기 작업 분리 (선택사항)
내부 쓰기 로직(resetQuitedAt, generateAndSaveRefreshToken 등)에@Transactional(propagation = REQUIRES_NEW)을 적용해 별도 트랜잭션으로 분리할 수도 있습니다.적용 예시(diff):
- @Transactional(readOnly = true) + @Transactional public SignInResponse signIn(EmailSignInRequest signInRequest) {src/main/java/com/example/solidconnection/auth/service/signup/EmailSignUpTokenProvider.java (1)
20-21: readOnly 트랜잭션에서 쓰기 작업 수행 — 즉시 수정 필요
- 메서드 내에서 passwordTemporaryStorage.save(...) 및 generateAndSaveSignUpToken(...)으로 쓰기 작업을 수행합니다.
- @transactional(readOnly = true)는 플러시 억제/쓰기 최적화로 인해 저장 누락, 예외, 2차 캐시 불일치 등 문제를 유발할 수 있습니다.
안전하게 읽기-쓰기 트랜잭션으로 전환하세요.
- @Transactional(readOnly = true) + @Transactional public String issueEmailSignUpToken(EmailSignUpTokenRequest request) {
🧹 Nitpick comments (58)
src/main/java/com/example/solidconnection/auth/client/AppleOAuthClient.java (2)
28-31: 문서 주석 포맷 정리 제안: Javadoc로 승격하고 참조 링크를 @see로 명시하세요.
현재는 블록 주석(/* ... */)이라 IDE/자바독 연동 이점이 적습니다.
클래스 상단 설명이므로 Javadoc(/** ... */)으로 전환하면 가독성과 탐색성이 좋아집니다.
- 제목을 한 줄 요약으로 두세요.
- 세부 링크는 @see 태그로 분리하세요.
- “전략 패턴” 구현 사실은 @implNote로 남기면 깔끔합니다.
적용 예시(diff):
-/* - * - 애플 인증을 위한 OAuth2 클라이언트 - * - https://developer.apple.com/documentation/signinwithapplerestapi/generate_and_validate_tokens - * - OAuthClient 인터페이스를 사용하는 전략 패턴으로 구현됨 - * */ +/** + * 애플 인증을 위한 OAuth2 클라이언트. + * + * @implNote OAuthClient 인터페이스를 사용하는 전략(Strategy) 패턴으로 구현되었습니다. + * @see <a href="https://developer.apple.com/documentation/signinwithapplerestapi/generate_and_validate_tokens">Generate and Validate Tokens</a> + */
58-69: 예외 래핑 시 원인 보존 및 상태 코드 체크를 권장합니다.
현재 e.getMessage()만 전달되어 스택트레이스가 유실됩니다.
HTTP 상태/본문 기반의 분기(권한/유효성/서버오류)로 에러를 구분하면 운영 이슈 추적이 수월합니다.
- CustomException에 cause를 함께 전달하세요.
- ResponseEntity의 status/body null 체크 후 명시적 예외를 던지세요.
- 필요 시 로그에 tokenUrl/redirectUri는 마스킹 처리하여 기록하세요.
적용 예시(diff):
- ResponseEntity<AppleTokenDto> response = restTemplate.exchange( + ResponseEntity<AppleTokenDto> response = restTemplate.exchange( properties.tokenUrl(), HttpMethod.POST, new HttpEntity<>(formData, headers), AppleTokenDto.class ); - return Objects.requireNonNull(response.getBody()).idToken(); + if (!response.getStatusCode().is2xxSuccessful() || response.getBody() == null) { + throw new CustomException(APPLE_AUTHORIZATION_FAILED, "Invalid response from Apple Token API"); + } + return response.getBody().idToken(); } catch (Exception e) { - throw new CustomException(APPLE_AUTHORIZATION_FAILED, e.getMessage()); + throw new CustomException(APPLE_AUTHORIZATION_FAILED, e); }src/main/java/com/example/solidconnection/auth/client/KakaoOAuthClient.java (2)
26-31: 문서 주석 포맷 정리 제안: 링크는 @see로, 패턴 설명은 @implNote로 이동하세요.
블록 주석 대신 Javadoc을 사용하면 IDE/자바독 노출이 좋아집니다.
다수 링크는 @see로 빼고, “전략 패턴”은 @implNote로 요약하면 더 읽기 쉽습니다.
- 한 줄 요약 제목.
- 링크 3건은 @see로.
- 패턴 설명은 @implNote로.
적용 예시(diff):
-/* - * - 카카오 인증을 위한 OAuth2 클라이언트 - * - https://developers.kakao.com/docs/latest/ko/kakaologin/rest-api#request-code - * - https://developers.kakao.com/docs/latest/ko/kakaologin/rest-api#request-token - * - https://developers.kakao.com/docs/latest/ko/kakaologin/rest-api#req-user-info - * - OAuthClient 인터페이스를 사용하는 전략 패턴으로 구현됨 - * */ +/** + * 카카오 인증을 위한 OAuth2 클라이언트. + * + * @implNote OAuthClient 인터페이스를 사용하는 전략(Strategy) 패턴으로 구현되었습니다. + * @see <a href="https://developers.kakao.com/docs/latest/ko/kakaologin/rest-api#request-code">Request Code</a> + * @see <a href="https://developers.kakao.com/docs/latest/ko/kakaologin/rest-api#request-token">Request Token</a> + * @see <a href="https://developers.kakao.com/docs/latest/ko/kakaologin/rest-api#req-user-info">Request User Info</a> + */
50-58: 토큰 발급 요청은 POST 바디(form-urlencoded)로 전송하는 형태가 더 안전하고 문서와 일치합니다.
현재는 POST + 쿼리스트링 조합으로 호출합니다.
카카오 문서는 application/x-www-form-urlencoded 바디 전송을 권장합니다.
- LinkedMultiValueMap으로 바디를 구성하세요.
- Content-Type을 명시하고 HttpEntity로 전달하세요.
- Uri에는 쿼리 파라미터를 제거하세요.
적용 예시(diff):
- private String getKakaoAccessToken(String code) { + private String getKakaoAccessToken(String code) { try { - ResponseEntity<KakaoTokenDto> response = restTemplate.exchange( - buildTokenUri(code), - HttpMethod.POST, - null, - KakaoTokenDto.class - ); + var headers = new HttpHeaders(); + headers.set("Content-Type", "application/x-www-form-urlencoded;charset=utf-8"); + var form = new org.springframework.util.LinkedMultiValueMap<String, String>(); + form.add("grant_type", "authorization_code"); + form.add("client_id", kakaoOAuthClientProperties.clientId()); + form.add("redirect_uri", kakaoOAuthClientProperties.redirectUrl()); + form.add("code", code); + + ResponseEntity<KakaoTokenDto> response = restTemplate.exchange( + kakaoOAuthClientProperties.tokenUrl(), + HttpMethod.POST, + new HttpEntity<>(form, headers), + KakaoTokenDto.class + ); return Objects.requireNonNull(response.getBody()).accessToken();Also applies to: 70-77
src/main/java/com/example/solidconnection/auth/service/signin/EmailSignInService.java (1)
31-34: 이메일 비교 전 전처리를 권장합니다. 공백·대소문자 이슈를 흡수하세요.
로그인 UX를 위해 trim + 소문자 통일 처리를 추가하면 흔한 입력 실수를 방지합니다. 저장소 인덱스도 소문자 기준이면 더 안전합니다.
1) email = email.trim().toLowerCase(Locale.ROOT)로 정규화.
2) 저장 시에도 동일 규칙을 적용해 일관성 확보.적용 예시(diff):
- private SiteUser getEmailMatchingUserOrThrow(String email) { - return siteUserRepository.findByEmailAndAuthType(email, AuthType.EMAIL) + private SiteUser getEmailMatchingUserOrThrow(String email) { + String normalized = email == null ? "" : email.trim().toLowerCase(java.util.Locale.ROOT); + return siteUserRepository.findByEmailAndAuthType(normalized, AuthType.EMAIL) .orElseThrow(() -> new CustomException(SIGN_IN_FAILED)); }src/main/java/com/example/solidconnection/auth/domain/Subject.java (1)
3-7: Subject 값 유효성 검사를 추가해 방어 코드를 완성하세요.
빈 문자열이나 공백만 있는 subject는 인증 체계에서 모호성을 만듭니다. 간단한 검증으로 조기에 차단하세요.
1) null/blank 거부.
2) 필요 시 길이 상한도 고려.적용 예시(diff):
public record Subject( String value ) { + + public Subject { + if (value == null || value.isBlank()) { + throw new IllegalArgumentException("subject value must not be blank"); + } + } }추가 팁.
3) Subject가 이메일을 담는다면, PII 로그 최소화 정책을 적용해 필요 시 마스킹을 고려하세요.src/test/java/com/example/solidconnection/auth/service/signin/EmailSignInServiceTest.java (2)
41-44: 성공 케이스에서 토큰의 "내용"까지 검증하면 리팩터링 안정성이 높아집니다.
null 체크만으로는 빈 문자열/형식 오류를 못 잡습니다.
1) 도메인 VO를 반환한다면 accessToken().token(), refreshToken().token()의 notBlank를 확인하세요.
2) 가능하다면 TokenProvider로 subject를 파싱해 요청 이메일과 일치하는지도 함께 검증하세요.
56-59: 예외 메시지 대신 에러 코드를 직접 검증하세요.
메시지는 번역·문구 변경에 취약합니다. 커스텀 예외의 ErrorCode를 꺼내 동등성 비교가 더 견고합니다.
1) assertThatThrownBy(...).isInstanceOf(CustomException.class).extracting("errorCode").isEqualTo(ErrorCode.SIGN_IN_FAILED).
2) 메시지 contains는 보조 검증으로만 유지하세요.Also applies to: 69-72
src/main/java/com/example/solidconnection/auth/service/TokenStorage.java (3)
11-13: 1) VO 도입 취지와 어긋나는 반환 타입(Optional) 정렬 제안현 설계는 저장/삭제는
T extends Token을 다루지만 조회만Optional<String>을 반환합니다. 이러면 VO를 도입한 이점(타입 안정성, 도메인 경계)을 조회 경로에서 잃습니다. 두 가지 방향을 제안합니다.
- A안. 시그니처를
Optional<T>로 변경합니다. 코드 전반의 타입 안정성이 즉시 올라갑니다.- B안. 호환성을 유지하려면 mapper 오버로드를 추가해 VO 복원을 명시적으로 위임합니다.
적용 예시(A안):
- <T extends Token> Optional<String> findToken(Subject subject, Class<T> tokenClass); + <T extends Token> Optional<T> findToken(Subject subject, Class<T> tokenClass);적용 예시(B안):
+import java.util.function.Function; + <T extends Token> T saveToken(Subject subject, T token); - <T extends Token> Optional<String> findToken(Subject subject, Class<T> tokenClass); + <T extends Token> Optional<String> findToken(Subject subject, Class<T> tokenClass); + <T extends Token> Optional<T> findToken(Subject subject, Class<T> tokenClass, Function<String, T> mapper); <T extends Token> void deleteToken(Subject subject, Class<T> tokenClass);
7-13: 2) 저장/조회/삭제의 행위 의미를 Javadoc로 명세해 주세요동일 Subject+토큰타입 저장 시 덮어쓰기인지, 멱등 보장인지, TTL 갱신 정책은 무엇인지, 조회 miss 시의 예외/Optional 정책 등 명세가 필요합니다. 운영 이슈를 줄입니다.
원하시면 간단한 Javadoc 초안 제공하겠습니다.
1-1: 3) 패키지 위치 재고(서비스가 아닌 SPI 성격)
TokenStorage는 인프라 SPI에 가깝습니다.auth.service보다는auth.token또는auth.spi같은 위치가 의도를 더 잘 드러냅니다. 큰 변경은 아니므로 후속 리팩터로 고려해 주세요.src/test/java/com/example/solidconnection/auth/service/signup/PasswordTemporaryStorageTest.java (1)
25-47: 2) TTL/만료 시나리오 테스트 추가 제안임시 저장소라면 만료 동작이 핵심입니다. 현재는 save/find/delete 해피패스만 검증합니다. TTL 기반 만료 검증을 추가하면 회귀를 잡을 수 있습니다.
원하시면 Awaitility 기반의 만료 테스트 스켈레톤을 드리겠습니다.
src/test/resources/application.yml (2)
90-98: 2) 만료 시간(expire-time) 전부 600000ms로 동일: 토큰별 TTL을 차등화하는 걸 권합니다.
테스트 목적이라면 OK지만, 실제 시나리오 검증을 위해 access(짧게), refresh(길게), sign-up(아주 짧게)로 구분하면 결함 포착력이 좋아집니다.원하시면 테스트 프로파일만 분리해 TTL을 다르게 가져가도록 yml 오버라이드 예시를 드리겠습니다.
91-91: 3) storage-key-prefix 네이밍 컨벤션을 한 가지로 통일해 두면 좋겠습니다.
ACCESS/REFRESH/BLACKLIST vs SIGN_UP처럼 혼재되어 있어요. 언더스코어 사용 여부만 팀 규칙으로 고정해 두면, 키 가독성과 Grep 정확도가 올라갑니다.합의만 주시면 컨벤션에 맞춘 일괄 변경 패치도 도와드릴게요.
Also applies to: 95-95, 98-98, 101-101
src/test/java/com/example/solidconnection/security/filter/SignOutCheckFilterTest.java (2)
64-65: 블랙리스트 키 생성의 하드코딩(구분자 ':')을 줄여서 드리프트를 예방하세요
- ":" 구분자를 테스트에 하드코딩하면 운영 코드의 키 조합 규칙이 바뀔 때 테스트가 조용히 어긋날 수 있습니다.
- 최소한 문자열 포맷을 사용해 가독성을 높이고, 가능하면 공용 키 빌더(예: TokenBlackListService의 공개 메서드 또는 별도 KeyBuilder 유틸)를 도입해 중복 규칙을 단일화하는 것을 권합니다.
다음과 같이 미세 개선도 가능합니다.
- String refreshTokenKey = tokenProperties.blackList().storageKeyPrefix() + ":" + token; + String refreshTokenKey = String.format("%s:%s", tokenProperties.blackList().storageKeyPrefix(), token);
103-105: 토큰 만료 1초는 CI에서 플레이키(간헐 실패) 리스크가 있습니다
- 현재 1초 만료는 머신/컨테이너 부하에 따라 테스트 실행 시점에 만료될 수 있습니다.
- 본 테스트는 "로그아웃 블랙리스트" 체크가 목적이므로 만료 시간은 여유 있게 30~60초로 두는 편이 안전합니다.
예시:
- .setExpiration(new Date(System.currentTimeMillis() + 1000)) + .setExpiration(new Date(System.currentTimeMillis() + 60_000))src/test/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManagerTest.java (2)
58-66: Max-Age 검증 표현을 약간 정리하면 가독성이 좋아집니다
- 기대값 계산을 한 번만 수행해 상수처럼 읽히게 하면 의도가 명확해집니다.
- divide-by-1000 로직이 반복되지 않도록 변수로 승격하세요.
예시:
- () -> assertThat(header).contains("Max-Age=" + tokenProperties.refresh().expireTime() / 1000), + () -> { + long expectedMaxAge = tokenProperties.refresh().expireTime() / 1000; + assertThat(header).contains("Max-Age=" + expectedMaxAge); + },
35-36: TTL 구성의 유효성도 한 번 체크하면 좋겠습니다
- tokenProperties.refresh().expireTime()이 0 이하로 잘못 설정되면 쿠키 설정이 비직관적일 수 있습니다.
- @ConfigurationProperties 바인딩 클래스에 Bean Validation(@min(1000) 등)을 적용하거나, 테스트에 "음수/0 TTL → 예외" 케이스를 추가하는 방안을 고려해보세요.
src/main/java/com/example/solidconnection/auth/service/signup/SignUpService.java (2)
66-73: 가입 후 정리(clean-up)를 finally 블록으로 감싸 실패에도 누수 없게 하세요
- signIn 과정에서 예외가 발생하면 임시 저장된 비번/토큰이 남을 수 있습니다.
- try/finally 로 정리 로직을 보장하면 운영 장애 시에도 데이터가 일관되게 정리됩니다.
예시:
- // 로그인 - SignInResponse response = signInService.signIn(siteUser); - - // 회원가입을 위해 저장한 데이터(SignUpToken, 비밀번호) 삭제 - clearSignUpData(email, authType); - - return response; + try { + // 로그인 + return signInService.signIn(siteUser); + } finally { + // 회원가입을 위해 저장한 데이터(SignUpToken, 비밀번호) 삭제 + clearSignUpData(email, authType); + }
75-85: 중복 체크는 사전검증 + DB 유니크 제약의 이중 안전망을 권장합니다
- existsBy... 사전검증은 경쟁 상태에서 동시 가입을 완전히 막지 못합니다.
- DB 레벨 유니크 인덱스/제약을 두고, save 시 DataIntegrityViolationException을 캐치하여 사용자 친화 메시지로 변환하는 패턴을 권장합니다.
Also applies to: 52-53
src/main/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManager.java (2)
29-32: TTL 유효성 가드 추가를 권장합니다
- 잘못된 구성(0 또는 음수) 시 Max-Age가 비정상 설정될 수 있어, 명시적으로 실패하게 하는 편이 안전합니다.
- 초기화 시점 검증(Bean Validation)과 함께 런타임 가드가 있으면 문제 위치 파악이 빨라집니다.
- long tokenExpireTime = tokenProperties.refresh().expireTime(); - long cookieMaxAge = convertExpireTimeToCookieMaxAge(tokenExpireTime); + long tokenExpireTime = tokenProperties.refresh().expireTime(); + if (tokenExpireTime <= 0) { + throw new IllegalStateException("token.refresh.expire-time must be > 0"); + } + long cookieMaxAge = convertExpireTimeToCookieMaxAge(tokenExpireTime); setRefreshTokenCookie(response, refreshToken, cookieMaxAge);
29-37: 만료시간은 Duration으로 모델링하면 변환 실수를 원천 차단할 수 있습니다
- 밀리초↔초 변환은 반복되기 쉬운 보일러플레이트이며, 단위 실수의 원인이 됩니다.
- TokenProperties.refresh().expireTime()을 long이 아닌 Duration으로 바꾸면 toSeconds() 등 타입 안전 API를 활용할 수 있습니다.
예시 아이디어(구성 클래스 변경 필요):
- TokenConfig.refresh.expireTime: Duration (예: "PT14D" 또는 "14d" 지원)
- 여기서는
long cookieMaxAge = tokenProperties.refresh().expire().toSeconds();형태로 사용src/test/java/com/example/solidconnection/auth/service/TokenBlackListServiceTest.java (2)
41-44: 2) 블랙리스트 키의 TTL까지 검증하면 회귀에 더 강해집니다.
키 존재만 확인하면 반영구 저장이나 TTL 미설정을 놓칠 수 있어요. 구성(TokenProperties.blackList.*) 기반 TTL이 설정되는지 함께 검증해요.다음 변경으로 TTL 양수를 확인해보세요.
- String foundBlackListToken = redisTemplate.opsForValue().get(blackListTokenKey); - assertThat(foundBlackListToken).isNotNull(); + String foundBlackListToken = redisTemplate.opsForValue().get(blackListTokenKey); + Long ttlSeconds = redisTemplate.getExpire(blackListTokenKey); + assertThat(foundBlackListToken).isNotNull(); + assertThat(ttlSeconds).as("블랙리스트 키 TTL(초)").isNotNull().isPositive();
55-56: 3) 동작 검증 중복을 줄이거나 파라미터화로 정리해요.
존재/부재 케이스가 구조가 같으니 ParameterizedTest로 합치면 가독성이 올라갑니다.원한다면 JUnit5 ParameterizedTest 변환 예시를 드릴게요.
Also applies to: 61-62
src/main/java/com/example/solidconnection/auth/service/AuthService.java (3)
42-48: 2) quit 파라미터 네이밍을 accessToken으로 맞추면 가독성이 좋아집니다.
동일한 의미를 공유하는 signOut과 네이밍을 통일하면 읽기가 쉬워집니다.아래처럼 이름만 바꾸면 호출부 영향이 없습니다.
- public void quit(long siteUserId, String token) { + public void quit(long siteUserId, String accessToken) { @@ - signOut(token); + signOut(accessToken);
45-47: 3) LocalDate.now 의존은 경계 시간이슈에 취약해요. Clock 주입을 고려해요.
자정 경계에서 테스트 플래키하거나 타임존 변경에 민감해질 수 있어요.
- 서비스에 java.time.Clock을 주입하고 LocalDate.now(clock)으로 계산하세요.
- 테스트에서는 Clock.fixed(...)로 고정하면 안정성이 좋아집니다.
예시(참고용, 파일 외부 변경 포함):
// 필드 private final Clock clock; // 생성자 주입 후 사용 LocalDate tomorrow = LocalDate.now(clock).plusDays(1);
30-33: signOut 멱등성 & 실패 허용 설계 보강 제안
- 멱등성 보장 명시
- JavaDoc에 signOut 메서드를 여러 번 호출해도 동일한 결과(블랙리스트 추가와 리프레시 토큰 삭제)가 보장된다고 적어주세요.
- deleteRefreshTokenByAccessToken 비존재 토큰 허용
- AuthTokenProvider.deleteRefreshTokenByAccessToken는 토큰이 없어도 tokenStorage.deleteToken 호출만 하고 예외를 던지지 않습니다.
- addToBlacklist 멱등성 검증
- TokenBlackListService.addToBlacklist는 redisTemplate.opsForValue().set으로 기존 키가 있어도 덮어쓰기를 수행해 멱등성을 만족합니다.
- 문서화로 API 사용성 강화
- 위 동작이 실패를 허용하고 멱등성을 갖는다는 점을 메서드 시그니처나 JavaDoc에 명시하면 개발자 경험이 한층 좋아집니다.
src/test/java/com/example/solidconnection/auth/service/AuthServiceTest.java (3)
67-71: 2) 로그아웃 멱등성 테스트를 하나 더 추가해 보세요.
두 번 호출해도 동일한 상태(리프레시 토큰 없음, 블랙리스트 true)가 유지되는지 점검하면 회귀 방어에 유용합니다.예시:
@Test void 로그아웃은_멱등적이다() { authService.signOut(accessToken.token()); authService.signOut(accessToken.token()); // 2회 호출 assertAll( () -> assertThat(tokenStorage.findToken(expectedSubject, RefreshToken.class)).isEmpty(), () -> assertThat(tokenBlackListService.isTokenBlacklisted(accessToken.token())).isTrue() ); }
79-86: 3) 날짜 검증의 경계 플래키니스 완화 제안입니다.
서비스가 Clock을 주입하도록 바뀐다면, 테스트도 동일 Clock을 주입받아 tomorrow 계산을 고정하면 안정적입니다. 지금도 대체로 괜찮지만, 자정 근처 실행 시 드물게 실패할 수 있어요.원하시면 Clock.fixed(...) 적용 예시를 드릴게요.
106-114: 4) 잘못된 리프레시 토큰 케이스는 랜덤 문자열을 쓰면 더 명확해요.
액세스 토큰 문자열을 재사용해도 동작하지만, 랜덤 문자열이면 의미가 더 분리되고 의도가 또렷합니다.간단히
String invalidRefreshToken = UUID.randomUUID().toString();으로 바꿔도 됩니다.src/test/java/com/example/solidconnection/auth/token/RedisTokenStorageTest.java (2)
39-50: TTL도 검증하면 회귀에 강해집니다.
현재 저장 성공만 확인합니다. TTL이 설정된다는 사실까지 검증하면 설정 변경 시 회귀를 조기에 잡을 수 있습니다.
- RedisTemplate 주입.
- getExpire(key)로 남은 TTL이 0보다 큼을 확인.
예시 코드:
@Autowired private org.springframework.data.redis.core.RedisTemplate<String, String> redisTemplate; @Test void 토큰_TTL이_설정된다() { // given redisTokenStorage.saveToken(subject, expectedToken); // when String key = com.example.solidconnection.auth.token.RedisTokenStorage.createKeyForTest(subject, tokenClass); // 또는 테스트용 키 생성 헬퍼 노출 Long ttl = redisTemplate.getExpire(key, java.util.concurrent.TimeUnit.MILLISECONDS); // then assertThat(ttl).isNotNull(); assertThat(ttl).isGreaterThan(0L); }키 생성이 외부에 노출되지 않았다면, 테스트 전용 헬퍼(패키지 프라이빗 정적 메서드) 도입을 고려해 주세요.
55-66: 오탈자 수정 제안: "반한다" → "반환한다".
메서드 이름의 한 글자 오탈자입니다. 가독성과 검색성을 위해 수정해 두는 편이 좋습니다.
- 명명 규칙 일관성.
다음 diff를 적용해 주세요:
- void 저장된_토큰이_있으면_Optional에_담아_반한다() { + void 저장된_토큰이_있으면_Optional에_담아_반환한다() {src/main/java/com/example/solidconnection/auth/service/signup/PasswordTemporaryStorage.java (3)
15-16: 키 프리픽스도 설정 기반으로 일원화해 주세요.
현재 TTL만 TokenProperties를 따르고 프리픽스는 하드코드입니다. 토큰·스토리지 키 정책을 한 곳에서 관리하면 혼선이 줄어듭니다.
- 운영환경별 프리픽스 제어.
- 키 스키마 일관성 확보.
다음 diff로 상수 제거를 권장합니다:
- private static final String KEY_PREFIX = "password:"; + // 키 프리픽스는 TokenProperties.signUp().storageKeyPrefix()를 따릅니다.
45-47: convertToKey에서 TokenProperties의 프리픽스를 사용하세요.
하드코드 대신 구성값을 사용해 키 스키마를 통일합니다.
- 설정 주도 키 생성.
적용 diff:
- private String convertToKey(String email) { - return KEY_PREFIX + email; - } + private String convertToKey(String email) { + return tokenProperties.signUp().storageKeyPrefix() + ":" + email; + }
21-30: 중복 저장 시 덮어쓰기 방지 고려.
동일 이메일로 연속 호출 시 기존 값을 덮어씁니다. 필요하다면 setIfAbsent(SETNX)로 1회성 비밀번호 저장을 보장할 수 있습니다.
- 경쟁 상태에서 의도치 않은 갱신 차단.
예시 diff(Spring Data Redis 2.1+ 지원 시):
- redisTemplate.opsForValue().set( - convertToKey(email), - encodedPassword, - expireTime, - TimeUnit.MILLISECONDS - ); + redisTemplate.opsForValue().setIfAbsent( + convertToKey(email), + encodedPassword, + expireTime, + TimeUnit.MILLISECONDS + );정책적으로 재발급을 허용한다면 현재 구현도 OK입니다.
src/main/java/com/example/solidconnection/auth/token/TokenBlackListService.java (2)
29-32: NPE 방지형 불리언 비교로 안전성을 높이세요.
RedisTemplate.hasKey는 Boolean를 반환합니다. 명시 비교가 더 안전합니다.
- 잠재적 NPE 회피.
적용 diff:
- String blackListKey = createKey(accessToken); - return redisTemplate.hasKey(blackListKey); + String blackListKey = createKey(accessToken); + return Boolean.TRUE.equals(redisTemplate.hasKey(blackListKey));
34-36: 토큰 원문 대신 해시를 키에 사용하는 방식을 검토해 주세요.
레디스 키에 토큰 원문을 그대로 쓰면 운영자·백업 등 보조 채널 노출 위험이 커집니다.
- 해시(SHA-256 등)로 식별성 유지.
- 개인정보/보안 노출면 축소.
예시 구현(호환성 고려해 마이그레이션 플래그와 함께 단계적 전환 권장):
private String createKey(String accessToken) { String prefix = tokenProperties.blackList().storageKeyPrefix(); return prefix + ":" + sha256(accessToken); } private String sha256(String value) { try { var md = java.security.MessageDigest.getInstance("SHA-256"); byte[] digest = md.digest(value.getBytes(java.nio.charset.StandardCharsets.UTF_8)); StringBuilder sb = new StringBuilder(); for (byte b : digest) sb.append(String.format("%02x", b)); return sb.toString(); } catch (java.security.NoSuchAlgorithmException e) { throw new IllegalStateException("SHA-256 not available", e); } }주: 기존 키와의 호환을 위해 일정 기간 double-write/dual-read 전략을 권합니다.
src/main/java/com/example/solidconnection/auth/token/config/TokenProperties.java (2)
7-8: 불필요해진 PostConstruct import를 정리해 주세요.
정적 초기화 제거 시 더 이상 사용되지 않습니다.
- 불필요 import 제거.
적용 diff:
-import jakarta.annotation.PostConstruct;
1-12: 호출부 업데이트 가이드(참고용).
정적 메서드 제거에 따라 사용처를 인스턴스 주입 방식으로 바꿔야 합니다.
- RedisTokenStorage 등에서 TokenProperties 필드 주입.
- tokenProperties.getExpireTime(...), getStorageKeyPrefix(...)로 변경.
예시(외부 파일 참고용):
// before long ttl = TokenProperties.getExpireTime(tokenClass); String prefix = TokenProperties.getStorageKeyPrefix(tokenClass); // after @RequiredArgsConstructor @Component class RedisTokenStorage { private final TokenProperties tokenProperties; ... long ttl = tokenProperties.getExpireTime(tokenClass); String prefix = tokenProperties.getStorageKeyPrefix(tokenClass); }원하시면 사용처 전체를 일괄 변환하는 패치도 만들어 드리겠습니다.
src/main/java/com/example/solidconnection/auth/token/RedisTokenStorage.java (3)
21-26: 정적 호출(TokenProperties.get…) 대신 DI로 주입해 일관성과 테스트 용이성을 높이세요.
본 파일은 TokenProperties를 정적 메서드로 사용하지만, 다른 컴포넌트는 빈 주입을 사용합니다.
- 구성 변경시 테스트 더블/오버라이드가 어렵습니다. 2) 설정 갱신(프로파일별 값) 시 정적 참조는 취약합니다.
제안:
private final TokenProperties tokenProperties;를 주입하고 인스턴스 메서드로 만료/프리픽스를 조회하도록 통일하세요.필요 시
TokenProperties에 클래스→프로퍼티 매핑 조회용 인스턴스 메서드를 추가하는 소규모 리팩터도 고려해 주세요.Also applies to: 46-48
21-26: 만료값 방어 로직을 추가해 잘못된 설정(0/음수)로 인한 무기한/즉시만료를 방지하세요.
- 구성 실수로 인해 토큰이 즉시 만료되거나 영구 키가 생성될 수 있습니다.
- TTL이 0 이하인 경우 예외 또는 최소 TTL로 보정하세요. 2) 로깅으로 조기 탐지하세요.
예시:
- redisTemplate.opsForValue().set( - createKey(subject, token.getClass()), - token.token(), - TokenProperties.getExpireTime(token.getClass()), - TimeUnit.MILLISECONDS - ); + long ttlMillis = TokenProperties.getExpireTime(token.getClass()); + if (ttlMillis <= 0) { + throw new IllegalStateException("ExpireTime must be > 0 ms for " + token.getClass().getSimpleName()); + } + redisTemplate.opsForValue().set( + createKey(subject, token.getClass()), + token.token(), + ttlMillis, + TimeUnit.MILLISECONDS + );
31-38: findToken이 문자열을 반환합니다 — 도메인 타입 Optional 반환을 검토하세요.
저장은 T를 받지만 조회는 String을 돌려 일관성이 약합니다.
- 팩토리/생성자를 통해
Optional<T>를 반환하면 타입 안정성이 높아집니다. 2) 호출부 캐스팅/래핑이 사라져 가독성이 개선됩니다.간단한 팩토리(예:
TokenFactory.from(Class<T>, String))를 도입하거나 각 Token이static T of(String)을 제공하도록 하는 방식을 권장합니다.원하시면 최소 침습 팩토리 초안까지 작성해드릴게요.
src/main/java/com/example/solidconnection/auth/service/signup/SignUpTokenProvider.java (1)
66-69: 누락된 claim에 대해 NPE 대신 도메인 예외를 반환하세요.
parseAuthType에서 claim이 없으면AuthType.valueOf(null)NPE가 날 수 있습니다.
- 일관되게
SIGN_UP_TOKEN_INVALID로 매핑하세요. 2) 호출부에서 별도 try-catch가 필요 없게 됩니다.public AuthType parseAuthType(String token) { - String serializedAuthType = tokenProvider.parseClaims(token, AUTH_TYPE_CLAIM_KEY, String.class); - return AuthType.valueOf(serializedAuthType); + String serializedAuthType = tokenProvider.parseClaims(token, AUTH_TYPE_CLAIM_KEY, String.class); + if (serializedAuthType == null || serializedAuthType.isBlank()) { + throw new CustomException(SIGN_UP_TOKEN_INVALID); + } + return AuthType.valueOf(serializedAuthType); }src/test/java/com/example/solidconnection/auth/service/JwtTokenProviderTest.java (1)
166-173: 서명 알고리즘을 운영 구현(HS512)과 동일하게 맞추면 혼선을 줄일 수 있습니다.
- 테스트는 HS256, 운영 구현은 HS512입니다. 만료 검증 목적에는 문제 없지만, 독자 입장에서 불필요한 인지부하가 생깁니다.
- 테스트 서명 알고리즘을 HS512로 통일하세요. 2) “운영과 동일한 조건하에 실패”를 보장합니다.
- .signWith(SignatureAlgorithm.HS256, jwtProperties.secret()) + .signWith(SignatureAlgorithm.HS512, jwtProperties.secret()) .compact(); @@ - .signWith(SignatureAlgorithm.HS256, jwtProperties.secret()) + .signWith(SignatureAlgorithm.HS512, jwtProperties.secret()) .compact();Also applies to: 175-182
src/main/java/com/example/solidconnection/auth/service/TokenProvider.java (1)
8-11: 토큰 만료 시간 단위 명시 및 파라미터 이름 개선 제안
Javadoc으로 단위 명시
메소드 파라미터expiration이 밀리초 단위임을 명확히 하기 위해 각 선언부 위에
@param expiration 토큰 만료 시간(밀리초)주석을 추가하세요.
예시:/** * @param expiration 토큰 만료 시간(밀리초) */ String generateToken(Subject subject, long expiration);파라미터 이름 변경 검토
long expiration을long expireTimeMillis로 바꾸면 호출부에서 단위 혼동을 완전히 제거할 수 있지만, 파급 범위가 크므로 변경 시점과 영향 범위를 꼼꼼히 검토해 주세요.src/test/java/com/example/solidconnection/auth/service/signin/SignInServiceTest.java (1)
46-59: 리팩터링 방향과 테스트 의도가 정확히 들어맞습니다.
Access/Refresh 둘 다 Subject를 검증하고, 저장소에서 저장된 RefreshToken 값까지 일치 검증합니다.
- 도메인 타입(Subject/RefreshToken) 기반의 계약 검증이 명확합니다. 2) TokenStorage 추상화 교체에도 테스트가 견고합니다.
추가 케이스 제안: 동일 사용자 재로그인 시 기존 리프레시 토큰이 교체되는지(rotate) 및 이전 토큰 불일치 시 무효화되는지까지 검증하면 더욱 탄탄해집니다.
src/main/java/com/example/solidconnection/auth/token/JwtTokenProvider.java (3)
33-44: [1] JWT 키 강도 보장 및 최신 파서 API 적용 제안.
이왕 HS512를 쓰는 만큼 키 길이 검증과 최신 jjwt 파서 사용으로 안전성과 유지보수성을 올려봅시다.
- 키 파생을 Base64 디코딩 + Keys.hmacShaKeyFor로 명시합니다.
이유는 충분한 키 길이 보장과 잘못된 시크릿 형식 조기 검출입니다.
- 파싱은 parserBuilder()를 사용해 가독성과 향후 마이그레이션 비용을 줄입니다.
실패 시점의 예외 메시지도 더 선명해집니다.
- 라이브러리 버전이 0.11.x 이상일 때 적용 가능합니다.
버전이 낮다면 유지해도 되지만, 업그레이드 계획이 있다면 지금 맞춰두는 편이 낫습니다.적용 예시(diff):
@@ - return Jwts.builder() - .setClaims(jwtClaims) - .setIssuedAt(now) - .setExpiration(expiredDate) - .signWith(SignatureAlgorithm.HS512, jwtProperties.secret()) - .compact(); + Key key = Keys.hmacShaKeyFor(Decoders.BASE64.decode(jwtProperties.secret())); + return Jwts.builder() + .setClaims(jwtClaims) + .setIssuedAt(now) + .setExpiration(expiredDate) + .signWith(key, SignatureAlgorithm.HS512) + .compact(); @@ - return Jwts.parser() - .setSigningKey(jwtProperties.secret()) - .parseClaimsJws(token) - .getBody(); + Key key = Keys.hmacShaKeyFor(Decoders.BASE64.decode(jwtProperties.secret())); + return Jwts.parserBuilder() + .setSigningKey(key) + .build() + .parseClaimsJws(token) + .getBody();추가 import:
import io.jsonwebtoken.io.Decoders; import io.jsonwebtoken.security.Keys; import java.security.Key;Also applies to: 56-65
29-31: [3] 커스텀 클레임 맵의 타입을 String → Object로 완화 검토.
확장성을 생각하면 숫자/불리언/배열 등 비문자형 클레임도 자연스럽게 담을 수 있게 열어두는 편이 좋습니다.
- 서명은 그대로 두고, 시그니처만
Map<String, Object>로 바꾸는 방안을 고려해주세요.
- 파서 쪽은 이미 제네릭 리턴을 지원하므로 조정 비용이 낮습니다.
- 단, 호출부 ripple이 있으니 이번 PR에서는 선택 사항입니다.
56-65: [4] 예외 원인 보존 또는 로깅으로 디버깅 가시성 개선.
원인 스택이 사라지면 운영 중 트러블슈팅이 어려워집니다.
- CustomException이 cause를 받는 생성자를 지원하면 cause를 전달하세요.
- 미지원이면
@Slf4j추가 후 debug 레벨 로깅을 권장합니다.가능한 경우의 변경 예:
- } catch (Exception e) { - throw new CustomException(INVALID_TOKEN); - } + } catch (Exception e) { + throw new CustomException(INVALID_TOKEN, e); + }src/main/java/com/example/solidconnection/auth/service/AuthTokenProvider.java (3)
67-72: [2] subject가 숫자가 아닐 때 NumberFormatException → 도메인 예외로 변환.
외부 입력에서 숫자가 아닌 subject가 들어오면 500으로 튈 수 있습니다.
- NumberFormatException을 INVALID_TOKEN으로 변환해 컨트롤러 레이어에서 일관 처리되도록 합시다.
- 보안 관점에서도 내부 스택이 노출되는 일을 줄일 수 있습니다.
- public SiteUser parseSiteUser(String token) { - Subject subject = tokenProvider.parseSubject(token); - long siteUserId = Long.parseLong(subject.value()); - return siteUserRepository.findById(siteUserId) - .orElseThrow(() -> new CustomException(USER_NOT_FOUND)); - } + public SiteUser parseSiteUser(String token) { + Subject subject = tokenProvider.parseSubject(token); + final long siteUserId; + try { + siteUserId = Long.parseLong(subject.value()); + } catch (NumberFormatException e) { + throw new CustomException(INVALID_TOKEN); + } + return siteUserRepository.findById(siteUserId) + .orElseThrow(() -> new CustomException(USER_NOT_FOUND)); + }추가 import:
import static com.example.solidconnection.common.exception.ErrorCode.INVALID_TOKEN;
32-36: [3] 토큰 카테고리(typ) 클레임 추가로 운영/디버깅 편의 향상.
접근/갱신/가입 등 토큰 종류를 클레임으로 명시하면 로그 분석과 정책 분기에 도움이 됩니다.
- access 토큰에
"typ":"access"를 추가합니다.
- 향후 미들웨어에서 토큰 라우팅이나 감사 로그에 유용합니다.
- String token = tokenProvider.generateToken( - subject, - Map.of(ROLE_CLAIM_KEY, role.name()), - tokenProperties.access().expireTime() - ); + Map<String, String> claims = Map.of( + ROLE_CLAIM_KEY, role.name(), + "typ", "access" + ); + String token = tokenProvider.generateToken( + subject, + claims, + tokenProperties.access().expireTime() + );
41-45: [4] 리프레시 토큰에도 typ 클레임을 맞춰 부여.
일관된 분류 체계는 운영 편의와 오탐 방지(작업자 실수)에도 의미가 큽니다.
- refresh 토큰에
"typ":"refresh"부여를 제안합니다.
- 이미 접근 토큰에 typ를 넣는다면 함께 맞추는 게 좋습니다.
- String token = tokenProvider.generateToken( - subject, - tokenProperties.refresh().expireTime() - ); + String token = tokenProvider.generateToken( + subject, + Map.of("typ", "refresh"), + tokenProperties.refresh().expireTime() + );src/test/java/com/example/solidconnection/auth/service/signup/SignUpTokenProviderTest.java (3)
52-66: [1] 저장·파싱 플로우를 통합 검증하는 좋은 테스트입니다.
생성→저장→파싱까지 한 번에 검증되어 회귀에 튼튼합니다.
- Subject/클레임/저장된 값 모두 일치 검증이 명확합니다.
- 필요 시 만료 시간까지 함께 검증하면 더 완결적입니다.
도우미로 exp-iat를 비교하는 헬퍼를 추가할 수 있습니다.
47-50: [2] 클레임 키 하드코딩 의존도 축소 제안.
테스트에서 "authType" 문자열을 직접 쓰면 운영 코드 변경에 민감해집니다.
- 가능하다면 프로덕션의 상수를 참조하거나,
- 이 테스트는 parseAuthType API를 통해 간접 검증하는 방식으로 단순화해도 충분합니다.
164-171: [4] 만료 토큰 생성 시 서명 알고리즘 일치성 소소한 제안.
프로덕션은 HS512, 여기서는 HS256입니다.
- 목적이 “만료” 검증이라 문제는 없지만,
- 동일 알고리즘을 쓰면 분석 시 혼선을 줄일 수 있습니다.
src/test/java/com/example/solidconnection/auth/service/AuthTokenProviderTest.java (3)
50-58: [1] access 토큰에 role 클레임도 직접 검증하면 더 탄탄합니다.
현재는 DB에서 유저를 다시 읽어 role을 확인합니다.
- 토큰 자체의 role 클레임을 파싱해 기대값과 비교하면 “발급 내용”까지 검증됩니다.
- 회귀 시 토큰 생성 로직 변경을 더 빨리 감지할 수 있습니다.
// then String accessTokenValue = accessToken.token(); Subject actualSubject = tokenProvider.parseSubject(accessTokenValue); - Role actualRole = authTokenProvider.parseSiteUser(accessTokenValue).getRole(); + Role actualRole = authTokenProvider.parseSiteUser(accessTokenValue).getRole(); + String actualRoleClaim = tokenProvider.parseClaims(accessTokenValue, "role", String.class); assertAll( () -> assertThat(accessTokenValue).isNotNull(), () -> assertThat(actualSubject).isEqualTo(expectedSubject), - () -> assertThat(actualRole).isEqualTo(siteUser.getRole()) + () -> assertThat(actualRole).isEqualTo(siteUser.getRole()), + () -> assertThat(actualRoleClaim).isEqualTo(siteUser.getRole().name()) );
81-88: [2] 변수 명칭 오해 소지 제거.
AccessToken을 fakeRefreshToken 변수에 담는 부분은 독자에게 혼란을 줄 수 있습니다.
- 의미가 드러나도록 이름을 바꾸면 가독성이 좋아집니다.
- 테스트 의도 파악 시간이 줄어듭니다.
- AccessToken fakeRefreshToken = authTokenProvider.generateAccessToken(siteUser); + AccessToken accessTokenAsFakeRefresh = authTokenProvider.generateAccessToken(siteUser); @@ - () -> assertThat(authTokenProvider.isValidRefreshToken(fakeRefreshToken.token())).isFalse() + () -> assertThat(authTokenProvider.isValidRefreshToken(accessTokenAsFakeRefresh.token())).isFalse()
105-115: [4] parseSiteUser 정상 플로우 확인 OK, 에러 플로우도 추가 제안.
정상 케이스는 충분합니다.
- subject가 숫자가 아닐 때 INVALID_TOKEN을 기대하는 에러 케이스를 하나 더 두면 견고합니다.
- 상위 코멘트의 예외 변환이 반영되면 함께 맞춰주세요.
원하시면 추가 테스트 케이스 초안을 드리겠습니다.
📜 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 (45)
src/main/java/com/example/solidconnection/auth/client/AppleOAuthClient.java(1 hunks)src/main/java/com/example/solidconnection/auth/client/KakaoOAuthClient.java(1 hunks)src/main/java/com/example/solidconnection/auth/controller/AuthController.java(1 hunks)src/main/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManager.java(2 hunks)src/main/java/com/example/solidconnection/auth/domain/AccessToken.java(1 hunks)src/main/java/com/example/solidconnection/auth/domain/RefreshToken.java(1 hunks)src/main/java/com/example/solidconnection/auth/domain/SignUpToken.java(1 hunks)src/main/java/com/example/solidconnection/auth/domain/Subject.java(1 hunks)src/main/java/com/example/solidconnection/auth/domain/Token.java(1 hunks)src/main/java/com/example/solidconnection/auth/domain/TokenType.java(0 hunks)src/main/java/com/example/solidconnection/auth/dto/ReissueResponse.java(1 hunks)src/main/java/com/example/solidconnection/auth/dto/SignInResponse.java(1 hunks)src/main/java/com/example/solidconnection/auth/service/AccessToken.java(0 hunks)src/main/java/com/example/solidconnection/auth/service/AuthService.java(2 hunks)src/main/java/com/example/solidconnection/auth/service/AuthTokenProvider.java(3 hunks)src/main/java/com/example/solidconnection/auth/service/RefreshToken.java(0 hunks)src/main/java/com/example/solidconnection/auth/service/SignUpTokenProvider.java(0 hunks)src/main/java/com/example/solidconnection/auth/service/TokenProvider.java(1 hunks)src/main/java/com/example/solidconnection/auth/service/TokenStorage.java(1 hunks)src/main/java/com/example/solidconnection/auth/service/oauth/OAuthService.java(2 hunks)src/main/java/com/example/solidconnection/auth/service/signin/EmailSignInService.java(1 hunks)src/main/java/com/example/solidconnection/auth/service/signin/SignInService.java(1 hunks)src/main/java/com/example/solidconnection/auth/service/signup/EmailSignUpTokenProvider.java(2 hunks)src/main/java/com/example/solidconnection/auth/service/signup/PasswordTemporaryStorage.java(2 hunks)src/main/java/com/example/solidconnection/auth/service/signup/SignUpService.java(1 hunks)src/main/java/com/example/solidconnection/auth/service/signup/SignUpTokenProvider.java(1 hunks)src/main/java/com/example/solidconnection/auth/token/JwtTokenProvider.java(3 hunks)src/main/java/com/example/solidconnection/auth/token/RedisTokenStorage.java(1 hunks)src/main/java/com/example/solidconnection/auth/token/TokenBlackListService.java(2 hunks)src/main/java/com/example/solidconnection/auth/token/config/TokenConfig.java(1 hunks)src/main/java/com/example/solidconnection/auth/token/config/TokenProperties.java(1 hunks)src/main/java/com/example/solidconnection/security/authentication/TokenAuthenticationProvider.java(1 hunks)src/test/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManagerTest.java(3 hunks)src/test/java/com/example/solidconnection/auth/service/AuthServiceTest.java(4 hunks)src/test/java/com/example/solidconnection/auth/service/AuthTokenProviderTest.java(5 hunks)src/test/java/com/example/solidconnection/auth/service/JwtTokenProviderTest.java(4 hunks)src/test/java/com/example/solidconnection/auth/service/TokenBlackListServiceTest.java(3 hunks)src/test/java/com/example/solidconnection/auth/service/signin/EmailSignInServiceTest.java(1 hunks)src/test/java/com/example/solidconnection/auth/service/signin/SignInServiceTest.java(3 hunks)src/test/java/com/example/solidconnection/auth/service/signup/PasswordTemporaryStorageTest.java(1 hunks)src/test/java/com/example/solidconnection/auth/service/signup/SignUpTokenProviderTest.java(7 hunks)src/test/java/com/example/solidconnection/auth/token/RedisTokenStorageTest.java(1 hunks)src/test/java/com/example/solidconnection/security/filter/SignOutCheckFilterTest.java(3 hunks)src/test/java/com/example/solidconnection/websocket/WebSocketStompIntegrationTest.java(1 hunks)src/test/resources/application.yml(1 hunks)
💤 Files with no reviewable changes (4)
- src/main/java/com/example/solidconnection/auth/service/SignUpTokenProvider.java
- src/main/java/com/example/solidconnection/auth/service/AccessToken.java
- src/main/java/com/example/solidconnection/auth/domain/TokenType.java
- src/main/java/com/example/solidconnection/auth/service/RefreshToken.java
🧰 Additional context used
🧬 Code graph analysis (6)
src/main/java/com/example/solidconnection/auth/service/signup/SignUpTokenProvider.java (4)
src/main/java/com/example/solidconnection/auth/token/RedisTokenStorage.java (1)
Component(13-49)src/main/java/com/example/solidconnection/auth/token/JwtTokenProvider.java (1)
Component(17-66)src/main/java/com/example/solidconnection/auth/service/signup/EmailSignUpTokenProvider.java (1)
Component(12-32)src/main/java/com/example/solidconnection/auth/service/SignUpTokenProvider.java (1)
Component(21-84)
src/test/java/com/example/solidconnection/auth/token/RedisTokenStorageTest.java (2)
src/test/java/com/example/solidconnection/auth/service/AuthTokenProviderTest.java (2)
TestContainerSpringBootTest(20-116)Nested(61-103)src/test/java/com/example/solidconnection/auth/service/JwtTokenProviderTest.java (4)
DisplayName(24-183)Nested(37-84)Nested(86-112)Nested(114-164)
src/main/java/com/example/solidconnection/auth/token/RedisTokenStorage.java (3)
src/main/java/com/example/solidconnection/auth/service/AuthTokenProvider.java (1)
Component(18-77)src/main/java/com/example/solidconnection/auth/service/signup/SignUpTokenProvider.java (1)
Component(17-70)src/main/java/com/example/solidconnection/auth/token/JwtTokenProvider.java (1)
Component(17-66)
src/test/java/com/example/solidconnection/auth/service/JwtTokenProviderTest.java (2)
src/test/java/com/example/solidconnection/auth/service/AuthTokenProviderTest.java (1)
Nested(61-103)src/test/java/com/example/solidconnection/auth/service/signup/SignUpTokenProviderTest.java (1)
Nested(80-138)
src/test/java/com/example/solidconnection/auth/service/signin/SignInServiceTest.java (2)
src/test/java/com/example/solidconnection/auth/service/AuthTokenProviderTest.java (1)
TestContainerSpringBootTest(20-116)src/test/java/com/example/solidconnection/auth/token/RedisTokenStorageTest.java (1)
TestContainerSpringBootTest(18-89)
src/test/java/com/example/solidconnection/auth/service/signup/SignUpTokenProviderTest.java (3)
src/test/java/com/example/solidconnection/auth/service/AuthTokenProviderTest.java (2)
TestContainerSpringBootTest(20-116)Nested(61-103)src/test/java/com/example/solidconnection/auth/token/RedisTokenStorageTest.java (2)
TestContainerSpringBootTest(18-89)Nested(52-75)src/test/java/com/example/solidconnection/auth/service/JwtTokenProviderTest.java (4)
DisplayName(24-183)Nested(37-84)Nested(86-112)Nested(114-164)
⏰ 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
🔇 Additional comments (28)
src/main/java/com/example/solidconnection/auth/service/signin/EmailSignInService.java (1)
1-1: 패키지 이동 자체는 깔끔합니다. 컴포넌트 스캔 범위만 확인해주세요.
Spring Boot의 @SpringBootApplication 스캔 루트가 상위 패키지인지 확인하면 빈 등록 이슈를 예방할 수 있습니다.
1) main 애플리케이션 클래스 위치가 com.example.solidconnection 이하라면 OK입니다.
2) 구성 클래스에 @componentscan으로 별도 범위를 제한한 경우, signin 하위가 포함되는지 점검하세요.src/main/java/com/example/solidconnection/auth/domain/Subject.java (1)
1-7: 도메인으로의 이동은 적절합니다. 의미가 더 명확해졌습니다.
토큰·인증 흐름의 공통 축인 Subject가 domain으로 승격되어 응집도가 올라갑니다.
1) 서비스 계층에서의 의존 역전에도 도움이 됩니다.
2) 다른 VO들과 함께 한 곳에서 관리하기 용이합니다.src/test/java/com/example/solidconnection/auth/service/signin/EmailSignInServiceTest.java (1)
1-1: 패키지 경로 정리는 테스트와 생산 코드를 잘 정렬해줍니다.
테스트 디스커버리와 컨텍스트 로딩에 긍정적 영향입니다.
1) 다른 signin 관련 테스트들도 동일 구조로 정렬되어 있는지 한 번에 점검하세요.src/test/java/com/example/solidconnection/auth/service/signup/PasswordTemporaryStorageTest.java (1)
1-1: 1) 패키지 정리 LGTM
signup하위로 이동이 컨텍스트 의존성과 일치합니다. 네이밍과 위치가 의도를 더 잘 담습니다.src/main/java/com/example/solidconnection/auth/domain/AccessToken.java (1)
3-7: 토큰 로깅 유출 점검 결과 및 수동 검토 안내
토큰 로깅 유출 여부를 점검하는 스크립트를 실행했습니다.
실행 결과,AccessToken,RefreshToken,SignUpToken또는token()호출이 포함된 로그 라인은 발견되지 않았습니다.
로그에서 마법처럼 토큰이 사라진 모습을 확인할 수 있었습니다.
이는 현재 코드베이스에 원문 토큰이 로그로 남지 않고 있다는 뜻으로 보입니다.
하지만 혹시 모를 누출 위험을 피하기 위해 수동 검토를 권장드립니다.
아래 절차를 따라 한 번 더 확인해 주세요:
- 점검 스크립트 실행
터미널에 아래 명령어를 복사해 붙여넣고 실행합니다.rg -nP -C2 --type=java '(logger|log)\.(trace|debug|info|warn|error)\s*\(.*(AccessToken|RefreshToken|SignUpToken|token\(\))'- 로그 마스킹 확인
출력된 로그에서 토큰이****처럼 암호화·마스킹되어 있는지 확인합니다.- 이상 발견 시 조치
토큰이 노출된 로그를 발견했다면, 해당 로그 레벨 및 메시지를 즉시 수정해 주세요.src/test/resources/application.yml (1)
89-101: 검증 완료: sign-up/black-list 매핑 일치
- 검증 결과
TokenProperties레코드의 컴포넌트 이름인signUp(15행)과blackList(16행)가 application.yml의sign-up/black-list설정과 Spring Boot의 relaxed binding 규칙에 따라 정확히 매핑됨을 확인했습니다.- 코드 내에서
tokenProperties.signUp().expireTime()및tokenProperties.blackList().storageKeyPrefix()호출이 정상적으로 이루어지는 것을 통해 바인딩이 올바르게 작동함을 검증했습니다.추가 수정이나 검토가 필요하지 않으므로 이 코멘트를 resolve합니다.
src/main/java/com/example/solidconnection/auth/dto/SignInResponse.java (2)
3-4: 1) 도메인 토큰 타입으로의 import 전환이 깔끔합니다.
DTO가 서비스 레이어 종속성을 끊고 도메인 모델(AccessToken/RefreshToken)만 의존하도록 정리된 점이 좋습니다.
11-13: 팩토리 메서드 계약 및 레거시 import 검증 완료
검토 요청하신 팩토리 메서드 계약이 그대로 유지됨을 확인했습니다.
legacy 패키지 경로 import 잔존 여부를 아래와 같이 검증했습니다.
- 레거시 import 검증
rg -nP 'com\.example\.solidconnection\.auth\.service\.(AccessToken|RefreshToken)\b' -C2명령 실행- 결과 없음 → legacy import 미존재 확인
추가 수정은 필요하지 않습니다. 😊
src/test/java/com/example/solidconnection/websocket/WebSocketStompIntegrationTest.java (1)
7-7: 1) AccessToken import의 도메인 전환이 테스트 의존성 방향과 일치합니다.
테스트가 도메인 타입에 직접 의존하도록 만든 흐름이 전체 리팩터링 목적과 잘 맞습니다.src/main/java/com/example/solidconnection/auth/dto/ReissueResponse.java (1)
3-3: 1) ReissueResponse도 일관되게 도메인 AccessToken으로 정리된 점 좋습니다.
Factory 메서드 계약이 유지되므로 소비자 측 영향도 최소입니다.src/test/java/com/example/solidconnection/security/filter/SignOutCheckFilterTest.java (1)
9-9: 구성 기반 전환 반영 OK
- TokenProperties 의존성 주입으로 키 프리픽스 설정을 환경설정에서 읽도록 바뀐 점 좋습니다.
- 테스트가 운영 코드의 구성값을 그대로 참조하니, 프리픽스 변경에도 테스트 유지보수 비용이 줄어듭니다.
Also applies to: 40-41
src/main/java/com/example/solidconnection/auth/service/signup/EmailSignUpTokenProvider.java (1)
30-31: API가 즉시 사용 가능한 토큰 문자열을 반환하도록 단순화된 점 좋습니다
- 컨트롤러/호출자 관점에서 VO → 문자열 변환 책임이 서비스 내부로 들어와 사용성이 향상되었습니다.
- 반환 타입 정리가 전체 흐름(발급→저장→문자열 전달)을 명확히 합니다.
src/test/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManagerTest.java (1)
35-36: TokenProperties 주입으로 TTL의 구성 기반 검증이 가능해졌습니다
- TokenType 의존 제거가 테스트에도 반영되어 일관성이 좋아졌습니다.
- 구성값 변경 시 테스트의 기대값도 자동으로 따라가므로 유지보수성이 향상됩니다.
src/main/java/com/example/solidconnection/auth/service/signup/SignUpService.java (2)
1-1: 패키지 정리와 의존성 경로 일치 OK
- signup/signin 하위 패키지 분리 원칙에 맞게 SignInService 임포트 경로가 정리되었습니다.
- 공개 API의 FQCN 변경이 컨트롤러/구성과 일관되게 적용된 것으로 보입니다.
Also applies to: 9-10
87-93: 검토 결과: 소셜 가입 시 password null 허용은 의도된 동작입니다.소셜 가입(AuthType != EMAIL) 시
getTemporarySavedPassword가 null을 반환하는 대신, SiteUser 생성자의password파라미터에 null이 전달하는 것은 설계상 허용됩니다.
SiteUser 엔티티의password필드는@Column(nullable = true)로 선언되어 있어 JPA 레벨에서 null을 허용합니다.
마이그레이션 스크립트에서도password VARCHAR(255) NULL로 정의되어 있어 DB 레벨 제약 위반이 발생하지 않습니다.
SiteUser 생성자(public SiteUser(..., String password))는 null 입력을 안전하게 처리하며 별도의 NPE 방지 로직이 없어도 문제가 없습니다.변경사항 워크스루:
- 엔티티 nullable 설정
- JPA 엔티티
SiteUser에서@Column(nullable = true)로 password 필드가 null 허용됨을 확인했습니다.- DB 마이그레이션(nullable)
- 스키마 마이그레이션 스크립트에서
password VARCHAR(255) NULL로 컬럼이 정의된 것을 확인했습니다.- 생성자 파라미터 null 허용
SiteUser생성자의String password파라미터가 null을 받아 필드에 할당하도록 구현되어 있습니다.따라서 NPE 발생이나 제약 위반 이슈는 없으며, 추가 수정은 필요하지 않습니다.
src/main/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManager.java (1)
26-27: 구성 주입으로 만료시간 상수 의존 제거, 방향 OK
- TokenProperties 기반으로 쿠키 Max-Age를 계산하는 방향이 전체 PR의 설정 일원화와 정합합니다.
- 테스트와 운영 코드가 동일한 구성 소스를 바라봐서 드리프트 가능성이 낮습니다.
src/main/java/com/example/solidconnection/auth/service/signin/SignInService.java (2)
1-4: 1) 패키지 이동·임포트 정리: 전반적으로 일관성 있게 반영되었어요.
패키지 경로 정리와 VO 기반 임포트 전환이 의도와 맞습니다. 컴파일 타임 의존성도 간결해졌습니다.Also applies to: 6-6
3-7: 변경된 VO 시그니처 사용처 전수 확인 완료
변경된 AccessToken/RefreshToken 반환 시그니처에 대한 전수 검사를 마쳤습니다.
SignInService.signIn에서
- authTokenProvider.generateAccessToken(siteUser)를 호출해 AccessToken을 생성합니다.
- authTokenProvider.generateAndSaveRefreshToken(siteUser)를 호출해 RefreshToken을 생성·저장합니다.
- SignInResponse.of(accessToken, refreshToken)를 호출해 DTO 변환을 수행합니다.
AuthService.reissue에서
- authTokenProvider.parseSiteUser(requestedRefreshToken)로 SiteUser를 파싱합니다.
- authTokenProvider.generateAccessToken(siteUser)로 새 AccessToken을 생성합니다.
- ReissueResponse.from(newAccessToken)로 DTO 변환을 수행합니다.
이상으로 전체 사용처에서 DTO 변환부와의 계약이 일치함을 확인했습니다.
src/test/java/com/example/solidconnection/auth/service/TokenBlackListServiceTest.java (1)
25-33: 검증 완료: TokenBlackListService에서 토큰 파싱/검증 코드가 전혀 없음을 확인했어요.
private final 필드로 TokenProperties와 RedisTemplate만 선언되어 있어요.
parse 또는 validate 메서드 호출과 TokenProvider 사용도 발견되지 않았어요.
따라서 테스트에서 더미 문자열 "accessToken"을 그대로 사용해도 예외나 NPE 위험이 없습니다.
검증 내역:
1) 의존성 필드 확인: TokenProperties, RedisTemplate
2) 파싱/검증 호출 여부: 없음src/main/java/com/example/solidconnection/auth/service/AuthService.java (1)
6-6: 4) AccessToken 임포트 추가는 재발급 경로와 일관됩니다.
Reissue에서 VO로 반환하기 때문에 임포트가 필요하고, 현재 사용과 합치합니다.src/test/java/com/example/solidconnection/auth/service/AuthServiceTest.java (1)
39-40: 1) TokenStorage/TokenProvider 기반으로 전환한 테스트가 의도에 잘 맞아요.
주체 파싱과 저장소 확인이 구현 변경(VO·저장소 분리)에 정확히 대응합니다.Also applies to: 47-49, 52-59
src/test/java/com/example/solidconnection/auth/token/RedisTokenStorageTest.java (1)
77-88: 삭제 플로우 검증 케이스는 명확하고 충분합니다.
저장 → 삭제 → 조회 순서가 명확하고 기대 결과도 정확합니다.
- 동작 검증 적합.
src/main/java/com/example/solidconnection/auth/service/oauth/OAuthService.java (1)
53-56: SignUpToken VO 도입 반영이 깔끔합니다.
도메인 객체로 감싸고 외부 노출 시에는 token()만 전달하는 경계 설계가 좋습니다.
- API 표면 최소화.
- 타입 안정성 향상.
src/main/java/com/example/solidconnection/auth/token/config/TokenProperties.java (1)
30-36: 미정의 토큰 클래스에 대한 NPE 가능성 제거.
기존 구현은 Map.get 결과가 null이면 NPE가 납니다. 위 refactor가 이를 해소합니다.
- 방어적 프로그래밍.
src/main/java/com/example/solidconnection/auth/service/AuthTokenProvider.java (2)
74-76: [6] toSubject 단일 책임 깔끔합니다.
SiteUser → Subject 변환이 한 곳에 모여 있어 호출부가 단순합니다.
40-48: 스토리지 TTL 설정, TokenProperties 기반으로 정상 작동 확인했습니다
- RedisTokenStorage.saveToken에서
TokenProperties.getExpireTime(token.getClass())를 사용해 TTL을TimeUnit.MILLISECONDS단위로 정확히 적용하고 있습니다.TokenConfig.expireTime이 JWT 만료와 쿠키 만료에서 모두 밀리초 단위로 일관되게 정의되어 있어, 설정 소스를 따로 분리할 필요 없이 통일성이 보장됩니다.- 따라서 현재 구현대로면 스토리지 자동 청소(TTL 만료)도 정상적으로 이루어질 것으로 판단되며, 추가 조치는 필요 없어 보입니다.
src/test/java/com/example/solidconnection/auth/service/signup/SignUpTokenProviderTest.java (1)
127-137: [3] 서버 발급 여부 검증 시나리오 깔끔합니다.
SpyBean을 이용해 저장소 조회를 empty로 강제하여 분기 검증이 잘 됩니다.
- 의도된 예외와 메시지까지 검증되어 신뢰도가 높습니다.
- 이 케이스는 운영 이슈 재현에 특히 유용합니다.
src/test/java/com/example/solidconnection/auth/service/AuthTokenProviderTest.java (1)
97-102: [3] 삭제 플로우 테스트 좋습니다.
발급→삭제→조회 빈 값까지 일관되게 검증되어 안정적입니다.
src/main/java/com/example/solidconnection/auth/controller/AuthController.java
Outdated
Show resolved
Hide resolved
src/main/java/com/example/solidconnection/auth/token/JwtTokenProvider.java
Show resolved
Hide resolved
src/main/java/com/example/solidconnection/auth/token/RedisTokenStorage.java
Show resolved
Hide resolved
src/main/java/com/example/solidconnection/auth/token/TokenBlackListService.java
Show resolved
Hide resolved
...in/java/com/example/solidconnection/security/authentication/TokenAuthenticationProvider.java
Show resolved
Hide resolved
src/test/java/com/example/solidconnection/auth/token/RedisTokenStorageTest.java
Show resolved
Hide resolved
nayonsoso
left a comment
nayonsoso
left a comment
There was a problem hiding this comment.
(이후에 예정된) 멘토의 회원가입을 구현할 때,
개발자 분들이 auth 패키지에서 이해되지 않는 지점이 없었으면 좋겠다
라는 목표로 작업했습니다 🥹
[패키지 변경 + 테스트 코드에 프로덕션 코드 변경사항 반영] 때문에 길어보이긴 하지만
프로덕션 코드 변경사항만 보면 그렇게 많지 않습니다 ㅎㅎ
읽어보시고 편하게 코멘트 남겨주세요~
There was a problem hiding this comment.
기존의 문제점 : TokenType enum 에서 [어떤 prefix로], [얼마동안] 토큰을 저장할 것인지 노출
보안상 이런 정보는 숨기는게 좋음
| public void signOut(String token) { | ||
| SiteUser siteUser = authTokenProvider.parseSiteUser(token); | ||
| AccessToken accessToken = authTokenProvider.generateAccessToken(siteUser); | ||
| authTokenProvider.deleteRefreshTokenByAccessToken(accessToken); | ||
| public void signOut(String accessToken) { | ||
| tokenBlackListService.addToBlacklist(accessToken); | ||
| authTokenProvider.deleteRefreshTokenByAccessToken(accessToken); |
There was a problem hiding this comment.
로그아웃 시 불필요하게 엑세스 토큰을 생성하는 부분이 있었습니다.🐒
🔽 지난번에 규혁님이 말씀하신 이슈도 이 때문이었습니다.
이번 PR에서 이 부분도 수정했습니다.
| private void validateIssuedByServer(String email) { | ||
| tokenStorage.findToken(new Subject(email), SignUpToken.class) | ||
| .orElseThrow(() -> new CustomException(SIGN_UP_TOKEN_NOT_ISSUED_BY_SERVER)); | ||
| } |
There was a problem hiding this comment.
기존에는 AuthTokenProvider 는 RedisTemplate를 사용하지 않고,
(그 당시 토큰 저장의 책임을 맡은) TokenProvider에 의존한 반면
SignUpTokenProvider는 RedisTemplate을 사용했었습니다.
토큰 저장의 책임을 TokenStorage 로 분리하면서 이와 같이 통일되지 않은 부분들도 수정했습니다 👍
- code rabbit 리뷰 반영: solid-connection#479 (comment)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/main/java/com/example/solidconnection/auth/service/signup/SignUpTokenProvider.java (2)
55-60: 2) 서버 발급 토큰 ‘동일성’ 검증 반영 — 아주 좋습니다.
- 저장된 토큰과 제출 토큰의 값 일치 비교가 들어가 탈취·구토큰 재사용을 효과적으로 차단합니다.
- 불일치 시 SIGN_UP_TOKEN_INVALID로 통일한 것도 사용자/로그 측면에서 메시지 일관성이 좋아졌습니다.
46-53: 3) subject 존재성/공백 검증을 함께 수행해 방어선을 한 겹 더 두는 걸 권장합니다.
- 현재는 AUTH_TYPE만 확인합니다. Subject가 null/blank여도 1차 검증을 통과할 수 있으므로, 여기서 바로 걸러두면 후속 로직 단순화와 오탐 감소에 도움이 됩니다.
- parseClaims 호출과 동일 맥락에서 parseSubject를 호출해 공백 여부를 검사하세요.
제안 diff:
private void validateFormatAndExpiration(String token) { // 파싱되는지, AuthType이 포함되어있는지 검증 try { String serializedAuthType = tokenProvider.parseClaims(token, AUTH_TYPE_CLAIM_KEY, String.class); AuthType.valueOf(serializedAuthType); + Subject subject = tokenProvider.parseSubject(token); + if (subject == null || subject.value() == null || subject.value().isBlank()) { + throw new CustomException(SIGN_UP_TOKEN_INVALID); + } } catch (Exception e) { throw new CustomException(SIGN_UP_TOKEN_INVALID); } }
- 변경 요약
- subject 파싱 및 null/blank 거부 로직 추가
🧹 Nitpick comments (4)
src/main/java/com/example/solidconnection/auth/service/signup/SignUpTokenProvider.java (4)
26-35: 1) 만료시간 단일 소스화와 Enum 직렬화 안정화, 이메일 정규화까지 한 번에 정리해 봅시다.
- 만료시간은 TokenStorage도 TokenProperties.getExpireTime(SignUpToken.class)를 참조합니다. 생성 쪽도 동일한 정적 진실소스 사용이 유지보수에 유리합니다.
- Enum은 toString() 대신 name()을 저장해 두면, 추후 toString 커스터마이징에 영향받지 않아 안전합니다.
- 키 일관성과 중복 계정 처리에 대비해 이메일을 trim + lower-case로 정규화해 저장/검증 모두 동일 기준을 쓰는 걸 권장합니다.
제안 diff:
- public SignUpToken generateAndSaveSignUpToken(String email, AuthType authType) { - Subject subject = new Subject(email); + public SignUpToken generateAndSaveSignUpToken(String email, AuthType authType) { + String normalizedEmail = email == null ? null : email.trim().toLowerCase(java.util.Locale.ROOT); + Subject subject = new Subject(normalizedEmail); String token = tokenProvider.generateToken( subject, - Map.of(AUTH_TYPE_CLAIM_KEY, authType.toString()), - tokenProperties.signUp().expireTime() + Map.of(AUTH_TYPE_CLAIM_KEY, authType.name()), + TokenProperties.getExpireTime(SignUpToken.class) ); SignUpToken signUpToken = new SignUpToken(token); return tokenStorage.saveToken(subject, signUpToken); }
- 변경 요약
- 만료시간: tokenProperties.signUp().expireTime() → TokenProperties.getExpireTime(SignUpToken.class)
- Enum 직렬화: authType.toString() → authType.name()
- 이메일 정규화: trim + lower-case 후 Subject 생성
62-64: 4) parseEmail에서도 정규화를 적용해 키 일관성을 보장하는 것을 권장합니다.
- 생성/저장 시 정규화했다면, 읽기/검증도 동일 정규화를 적용해야 불필요한 불일치가 생기지 않습니다.
제안 diff:
- public String parseEmail(String token) { - return tokenProvider.parseSubject(token).value(); - } + public String parseEmail(String token) { + String email = tokenProvider.parseSubject(token).value(); + return email == null ? null : email.trim().toLowerCase(java.util.Locale.ROOT); + }
- 변경 요약
- 이메일 trim + lower-case 정규화
22-25: 6) 선택: Redis 평문 저장 대신 해시(혹은 HMAC) 저장으로 토큰 노출 리스크를 낮춰보세요.
- Redis 유출 시 평문 토큰은 즉시 악용 가능합니다. 값 객체의 token() 대신 고정 솔트/pepper를 섞은 HMAC 또는 단방향 해시를 저장하고, 검증 시 입력 토큰을 동일 방식으로 해싱해 비교하면 노출 피해를 크게 줄일 수 있습니다.
- 저장/검증 경로에만 영향이 있어 외부 API에는 투명하게 적용 가능합니다.
1-70: 7) 테스트 케이스 보강 제안으로 신뢰도 올리기.
- 다음 시나리오를 추가하면 회귀에 강해집니다.
- 정상 토큰: 생성→저장→검증 성공.
- 불일치 토큰: 동일 이메일에 다른 토큰 제출 시 INVALID.
- 만료 토큰: exp 지난 토큰 INVALID.
- 잘못된 authType 클레임(unknown 값) INVALID.
- subject 누락/공백 토큰 INVALID.
- 이메일 정규화: 대문자/공백 포함 입력으로 생성한 토큰을 정규화된 키로 찾고 검증이 일치하는지.
- 필요하시면 테스트 스켈레톤까지 빠르게 드릴게요.
📜 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/example/solidconnection/auth/controller/AuthController.java(1 hunks)src/main/java/com/example/solidconnection/auth/service/signup/SignUpTokenProvider.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/example/solidconnection/auth/controller/AuthController.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-24T16:34:21.459Z
Learnt from: nayonsoso
PR: solid-connection/solid-connect-server#479
File: src/main/java/com/example/solidconnection/auth/token/config/TokenProperties.java:1-12
Timestamp: 2025-08-24T16:34:21.459Z
Learning: TokenProperties의 정적 메서드 getExpireTime(Class<? extends Token>)과 getStorageKeyPrefix(Class<? extends Token>)는 다형성을 위한 의도적인 설계입니다. 이를 통해 RedisTokenStorage가 토큰 타입별 분기 없이 제네릭하게 동작할 수 있습니다.
Applied to files:
src/main/java/com/example/solidconnection/auth/service/signup/SignUpTokenProvider.java
🧬 Code graph analysis (1)
src/main/java/com/example/solidconnection/auth/service/signup/SignUpTokenProvider.java (4)
src/main/java/com/example/solidconnection/auth/service/signup/PasswordTemporaryStorage.java (1)
Component(11-48)src/main/java/com/example/solidconnection/auth/token/RedisTokenStorage.java (1)
Component(13-49)src/main/java/com/example/solidconnection/auth/token/JwtTokenProvider.java (1)
Component(17-66)src/main/java/com/example/solidconnection/auth/service/signup/EmailSignUpTokenProvider.java (1)
Component(12-32)
⏰ 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
🔇 Additional comments (1)
src/main/java/com/example/solidconnection/auth/service/signup/SignUpTokenProvider.java (1)
41-44: 5) 검증 플로우 구성(1차: 형식/만료 → 2차: 서버발급 동일성) 깔끔합니다.
- 책임 분리가 명확하고, 에러 코드도 통일되어 운영·로깅에 유리합니다.
- code rabbit 리뷰 반영: solid-connection#479 (comment)
- code rabbit 리뷰 반영: solid-connection#479 (comment)
- code rabbit 리뷰 반영: solid-connection#479 (comment)
- code rabbit 리뷰 반영: solid-connection#479 (comment)
- application.yml을 보면 알다시피 더 명시적이게 바뀐다는 장점도 있다. - code rabbit 리뷰 반영: solid-connection#479 (comment)
nayonsoso
force-pushed
the
refactor/adjust-token-vo
branch
from
05bb088 to
21542c6
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/example/solidconnection/auth/token/JwtTokenProvider.java (1)
43-45: 3) JJWT 0.9.x 환경에서 시크릿 길이 검증 추가
- 현재 프로젝트의 JJWT 버전은
io.jsonwebtoken:jjwt:0.9.1(build.gradle 46행)로 확인되었습니다.- 따라서 0.11+용
Keys.hmacShaKeyFor적용 대신, 기존signWith(SignatureAlgorithm.HS512, jwtProperties.secret())방식을 유지하되, 부팅 시점에 시크릿 길이를 검증해 주세요.
src/main/java/com/example/solidconnection/auth/token/JwtTokenProvider.java클래스의 생성자 또는@PostConstruct메서드에 아래 로직을 추가합니다.byte[] keyBytes = jwtProperties.secret() .getBytes(StandardCharsets.UTF_8); if (keyBytes.length < 64) { throw new CustomException( ErrorCode.INVALID_JWT_SECRET, "JWT 비밀키는 최소 64바이트(512비트) 이상이어야 합니다." ); }- 길이 부족 시 애플리케이션 부팅을 실패 처리하여 보안 강도를 보장합니다.
- 추후 JJWT를 0.11 이상으로 업그레이드할 계획이 생기면, 해당 버전의
parserBuilder()및Keys.hmacShaKeyFor(...)적용을 별도 검토해주세요.
♻️ Duplicate comments (3)
src/main/java/com/example/solidconnection/auth/token/JwtTokenProvider.java (1)
48-54: 4) subject null/blank 가드가 추가되어 안전성이 크게 개선되었습니다.
- 이전 라운드 피드백을 정확히 반영하셨고, INVALID_TOKEN으로 일관된 에러 매핑도 좋습니다.
src/main/java/com/example/solidconnection/auth/token/TokenBlackListService.java (1)
23-30: 3) TTL 적용 반영 확인 — 이전 피드백이 잘 반영되었습니다.
TTL 미설정으로 무기한 누적되던 이슈가 해소되어 운영 리스크(메모리 증가) 감소가 기대됩니다.
opsForValue().set(key, value, Duration)시그니처를 쓰려면expireTime()이Duration이어야 합니다.
Duration이 0/음수일 때 즉시 만료될 수 있으니, 방어 로직(최소 TTL 보장)을 고려해도 좋습니다.적용 예시:
- redisTemplate.opsForValue().set( - blackListKey, - SIGN_OUT_VALUE, - tokenProperties.blackList().expireTime() - ); + var ttl = tokenProperties.blackList().expireTime(); + if (ttl == null || ttl.isZero() || ttl.isNegative()) { + // 최소 TTL 1분 보장(팀 규칙에 맞게 조정 가능) + ttl = java.time.Duration.ofMinutes(1); + } + redisTemplate.opsForValue().set(blackListKey, SIGN_OUT_VALUE, ttl);src/main/java/com/example/solidconnection/auth/service/signup/SignUpTokenProvider.java (1)
47-55: 형식/만료 검증 로직 정돈, 이전 피드백 반영 확인
- subject 파싱과 authType 클래스 변환을 try-catch로 감싸 일관된 에러로 매핑한 부분이 깔끔합니다.
- 앞선 논의(Subject 유효성 검증 추가)가 반영되어, 빈 subject가 걸러집니다. 좋습니다.
🧹 Nitpick comments (11)
src/main/java/com/example/solidconnection/auth/token/JwtTokenProvider.java (3)
34-39: 2) 만료시간(Duration) 입력값에 대한 사전 검증을 추가해 주세요.
- 음수/0/NULL Duration이 들어오면 과거 시점(exp < iat)의 토큰이 생성될 수 있습니다.
- 설정 오류를 조기에 드러내기 위해 precondition 체크를 권장합니다.
적용 제안(diff):
- private String generateJwtTokenValue(String subject, Map<String, String> claims, Duration expireTime) { + private String generateJwtTokenValue(String subject, Map<String, String> claims, Duration expireTime) { + if (expireTime == null || expireTime.isZero() || expireTime.isNegative()) { + throw new IllegalArgumentException("expireTime must be positive"); + } Claims jwtClaims = Jwts.claims().setSubject(subject); jwtClaims.putAll(claims); Date now = new Date(); Date expiredDate = new Date(now.getTime() + expireTime.toMillis());
57-59: 5) parseClaims 인자(claimName) 검증으로 방어적 코딩을 더해 주세요.
- null/blank 키 전달 시 NPE/불명확한 에러가 날 수 있습니다.
- 개발자 실수를 조기에 발견하도록 IllegalArgumentException 또는 도메인 예외로 선변별을 권장합니다.
적용 제안(diff):
- public <T> T parseClaims(String token, String claimName, Class<T> claimType) { - return parseJwtClaims(token).get(claimName, claimType); - } + public <T> T parseClaims(String token, String claimName, Class<T> claimType) { + if (claimName == null || claimName.isBlank()) { + throw new IllegalArgumentException("claimName must not be null/blank"); + } + return parseJwtClaims(token).get(claimName, claimType); + }
30-35: 6) 커스텀 클레임을 String으로 고정하면 향후 확장성이 제한됩니다.
- 숫자/불리언/배열 등 타입 보존이 필요한 경우 Map<String, ?> 또는 Map<String, Object>가 유연합니다.
- 다만 제네릭 변경은 호출부(SignUp/Auth 등) 전파가 필요해 후속 PR로의 점진적 전환을 권장합니다.
src/test/java/com/example/solidconnection/auth/service/JwtTokenProviderTest.java (2)
114-130: 3) ‘subject 미존재’ 시나리오는 만료와 분리해 검증하는 편이 더 정확합니다.
- 현재는 만료 토큰을 사용해 INVALID_TOKEN이 발생합니다.
- subject 미존재 자체를 검증하려면 유효기간 내 토큰(미만료)로 케이스를 구성하는 것이 좋습니다.
수정 제안(diff):
- // given - Claims claims = Jwts.claims(new HashMap<>()); - String subjectNotExistingToken = createExpiredToken(claims); - String subjectBlankToken = tokenProvider.generateToken(new Subject(" "), expectedExpireTime); + // given + Claims claims = Jwts.claims(new HashMap<>()); + String subjectNotExistingToken = Jwts.builder() + .setClaims(claims) // subject intentionally omitted + .setIssuedAt(new Date()) + .setExpiration(new Date(System.currentTimeMillis() + 60_000)) // non-expired + .signWith(SignatureAlgorithm.HS512, jwtProperties.secret()) + .compact(); + String subjectBlankToken = tokenProvider.generateToken(new Subject(" "), expectedExpireTime);
185-201: 4) 테스트에서 사용하는 서명 알고리즘을 프로덕션(HS512)과 일치시키면 혼선을 줄일 수 있습니다.
- 현재 헬퍼는 HS256을 사용합니다.
- 프로덕션은 HS512이므로 통일 시 추후 알고리즘 변경/검증 로직 상이로 인한 혼선을 예방할 수 있습니다.
- 기능상 큰 차이는 없지만(파서는 헤더 알고리즘을 따릅니다), 가독성과 유지보수성 측면에서 유리합니다.
수정 제안(diff):
- .signWith(SignatureAlgorithm.HS256, jwtProperties.secret()) + .signWith(SignatureAlgorithm.HS512, jwtProperties.secret()) .compact(); @@ - .signWith(SignatureAlgorithm.HS256, jwtProperties.secret()) + .signWith(SignatureAlgorithm.HS512, jwtProperties.secret()) .compact();src/main/java/com/example/solidconnection/auth/token/TokenBlackListService.java (1)
23-30: 5) 입력값 가드와 주석 최신화 제안.
운영 안전망과 문서 일치성을 위해 가벼운 손질을 권장드립니다.
null/공백 토큰이 들어오면 빈 키 생성(prefix:)을 막기 위해 입력값 검사 후IllegalArgumentException또는 no-op 처리.
- 메서드 상단 주석의 “key = BLACKLIST:{accessToken}”는 이제 프리픽스가 설정 기반이라 실제 동작과 다릅니다. 주석을
key = {storageKeyPrefix}:{tokenIdOrHash}형태로 업데이트하세요.public void addToBlacklist(String accessToken) { - String blackListKey = createKey(accessToken); + if (accessToken == null || accessToken.isBlank()) { + throw new IllegalArgumentException("accessToken must not be null/blank"); + } + String blackListKey = createKey(accessToken); redisTemplate.opsForValue().set( blackListKey, SIGN_OUT_VALUE, tokenProperties.blackList().expireTime() ); } @Override public boolean isTokenBlacklisted(String accessToken) { - String blackListKey = createKey(accessToken); + if (accessToken == null || accessToken.isBlank()) { + return false; // 혹은 IllegalArgumentException, 팀 컨벤션에 맞게 + } + String blackListKey = createKey(accessToken); return redisTemplate.hasKey(blackListKey); }Also applies to: 33-36
src/main/java/com/example/solidconnection/auth/service/signup/SignUpTokenProvider.java (1)
57-62: 서버 발급 검증의 오류 코드 의미 확인 및 ‘불일치’ 케이스 보강 제안
- 현재 저장 미존재 및 값 불일치를 모두 SIGN_UP_TOKEN_NOT_ISSUED_BY_SERVER로 처리합니다. 도메인 메시지로 충분하지만, 불일치 케이스를 INVALID로 구분할지 최종 의도를 확인해 주세요.
- 테스트에도 “저장값 존재하나 불일치” 시나리오 추가를 권장합니다.
- find 후 equals 비교 로직은 간결하고 의도 명확합니다. 유지하세요.
불일치 케이스 추가 테스트 예시는 다음과 같습니다:
@Test void 우리_서버_토큰과_값이_불일치하면_예외가_발생한다() { // given String issued = signUpTokenProvider.generateAndSaveSignUpToken(email, authType).token(); String different = issued.substring(0, issued.length() - 2) + "xx"; given(tokenStorage.findToken(subject, SignUpToken.class)).willReturn(Optional.of(different)); // when & then assertThatCode(() -> signUpTokenProvider.validateSignUpToken(issued)) .isInstanceOf(CustomException.class) .hasMessageContaining(SIGN_UP_TOKEN_NOT_ISSUED_BY_SERVER.getMessage()); }src/test/java/com/example/solidconnection/auth/service/signup/SignUpTokenProviderTest.java (4)
69-79: 삭제 후 후속 검증 추가 제안
- deleteByEmail 이후 validateSignUpToken이 NOT_ISSUED_BY_SERVER를 발생하는지까지 확인하면 시나리오가 완결됩니다.
추가 예시:
assertThatCode(() -> signUpTokenProvider.validateSignUpToken( signUpTokenProvider.generateAndSaveSignUpToken(email, authType).token())) .doesNotThrowAnyException(); signUpTokenProvider.deleteByEmail(email); assertThatCode(() -> signUpTokenProvider.validateSignUpToken( tokenProvider.generateToken(new Subject(email), Duration.ofMinutes(10)))) .isInstanceOf(CustomException.class) .hasMessageContaining(SIGN_UP_TOKEN_NOT_ISSUED_BY_SERVER.getMessage());
115-126: authType 타입 불일치 검증 OK, ‘누락’ 케이스도 추가 제안
- 잘못된 문자열 값에 대한 INVALID 매핑을 검증합니다.
- authType 클레임 ‘부재(null)’ 시나리오도 추가하면 parseClaims의 null 반환 경로가 커버됩니다.
추가 예시:
@Test void authType_클레임이_없으면_INVALID() { String missingClaim = tokenProvider.generateToken(subject, Duration.ofMinutes(10)); assertThatCode(() -> signUpTokenProvider.validateSignUpToken(missingClaim)) .isInstanceOf(CustomException.class) .hasMessageContaining(SIGN_UP_TOKEN_INVALID.getMessage()); }
128-138: 서버 미발급 예외 검증 OK, ‘값 불일치’ 별도 케이스 추가 권장
- 현재는 저장소 미존재(empty)만 검증합니다.
- 저장값 존재하나 다른 값인 경우도 추가하면 서버-발급검증 분기 커버리지가 완성됩니다.
예시 코드는 상단 코멘트(서버 발급 검증) 참고 바랍니다.
153-163: parseAuthType 정상 경로 테스트 OK, 실패 경로도 추가 제안
- 본 PR에서 parseAuthType이 CustomException으로 일관 매핑되도록 패치 예정이라면, 누락/blank/잘못된 값에 대한 예외 테스트를 추가해 주세요.
예시:
@Test void parseAuthType_클레임_누락_시_INVALID() { String token = tokenProvider.generateToken(subject, Duration.ofMinutes(10)); assertThatCode(() -> signUpTokenProvider.parseAuthType(token)) .isInstanceOf(CustomException.class) .hasMessageContaining(SIGN_UP_TOKEN_INVALID.getMessage()); }
📜 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 (16)
src/main/java/com/example/solidconnection/auth/controller/AuthController.java(1 hunks)src/main/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManager.java(2 hunks)src/main/java/com/example/solidconnection/auth/service/TokenProvider.java(1 hunks)src/main/java/com/example/solidconnection/auth/service/signup/PasswordTemporaryStorage.java(2 hunks)src/main/java/com/example/solidconnection/auth/service/signup/SignUpTokenProvider.java(1 hunks)src/main/java/com/example/solidconnection/auth/token/JwtTokenProvider.java(2 hunks)src/main/java/com/example/solidconnection/auth/token/RedisTokenStorage.java(1 hunks)src/main/java/com/example/solidconnection/auth/token/TokenBlackListService.java(2 hunks)src/main/java/com/example/solidconnection/auth/token/config/TokenConfig.java(1 hunks)src/main/java/com/example/solidconnection/auth/token/config/TokenProperties.java(1 hunks)src/main/resources/secret(1 hunks)src/test/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManagerTest.java(3 hunks)src/test/java/com/example/solidconnection/auth/service/JwtTokenProviderTest.java(5 hunks)src/test/java/com/example/solidconnection/auth/service/signup/SignUpTokenProviderTest.java(7 hunks)src/test/java/com/example/solidconnection/auth/token/RedisTokenStorageTest.java(1 hunks)src/test/resources/application.yml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/main/resources/secret
🚧 Files skipped from review as they are similar to previous changes (10)
- src/main/java/com/example/solidconnection/auth/token/config/TokenConfig.java
- src/main/java/com/example/solidconnection/auth/token/RedisTokenStorage.java
- src/main/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManager.java
- src/test/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManagerTest.java
- src/test/java/com/example/solidconnection/auth/token/RedisTokenStorageTest.java
- src/main/java/com/example/solidconnection/auth/service/signup/PasswordTemporaryStorage.java
- src/main/java/com/example/solidconnection/auth/controller/AuthController.java
- src/main/java/com/example/solidconnection/auth/token/config/TokenProperties.java
- src/test/resources/application.yml
- src/main/java/com/example/solidconnection/auth/service/TokenProvider.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-24T16:34:21.459Z
Learnt from: nayonsoso
PR: solid-connection/solid-connect-server#479
File: src/main/java/com/example/solidconnection/auth/token/config/TokenProperties.java:1-12
Timestamp: 2025-08-24T16:34:21.459Z
Learning: TokenProperties의 정적 메서드 getExpireTime(Class<? extends Token>)과 getStorageKeyPrefix(Class<? extends Token>)는 다형성을 위한 의도적인 설계입니다. 이를 통해 RedisTokenStorage가 토큰 타입별 분기 없이 제네릭하게 동작할 수 있습니다.
Applied to files:
src/main/java/com/example/solidconnection/auth/service/signup/SignUpTokenProvider.java
🧬 Code graph analysis (4)
src/main/java/com/example/solidconnection/auth/service/signup/SignUpTokenProvider.java (3)
src/main/java/com/example/solidconnection/auth/token/JwtTokenProvider.java (1)
Component(18-71)src/main/java/com/example/solidconnection/auth/token/RedisTokenStorage.java (1)
Component(12-47)src/main/java/com/example/solidconnection/auth/service/signup/EmailSignUpTokenProvider.java (1)
Component(12-32)
src/test/java/com/example/solidconnection/auth/service/signup/SignUpTokenProviderTest.java (2)
src/test/java/com/example/solidconnection/auth/token/RedisTokenStorageTest.java (2)
TestContainerSpringBootTest(19-90)Nested(53-76)src/test/java/com/example/solidconnection/auth/service/JwtTokenProviderTest.java (4)
DisplayName(25-202)Nested(38-85)Nested(87-131)Nested(133-183)
src/test/java/com/example/solidconnection/auth/service/JwtTokenProviderTest.java (3)
src/test/java/com/example/solidconnection/auth/service/signup/SignUpTokenProviderTest.java (1)
Nested(81-139)src/test/java/com/example/solidconnection/auth/token/RedisTokenStorageTest.java (1)
Nested(53-76)src/test/java/com/example/solidconnection/auth/service/AuthTokenProviderTest.java (1)
Nested(61-103)
src/main/java/com/example/solidconnection/auth/token/JwtTokenProvider.java (3)
src/main/java/com/example/solidconnection/auth/service/signup/SignUpTokenProvider.java (1)
Component(17-72)src/main/java/com/example/solidconnection/auth/service/AuthTokenProvider.java (1)
Component(18-77)src/main/java/com/example/solidconnection/security/authentication/TokenAuthenticationProvider.java (1)
Component(12-33)
⏰ 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
🔇 Additional comments (13)
src/main/java/com/example/solidconnection/auth/token/JwtTokenProvider.java (1)
25-32: 1) VO(Subject)·Duration 기반 시그니처 적용이 일관되고 명확합니다.
- Subject와 Duration을 전면 도입해 API 표면이 깔끔해졌습니다.
- SignUp/Auth 토큰 제공자 사용처와도 호환이 잘 맞습니다.
src/test/java/com/example/solidconnection/auth/service/JwtTokenProviderTest.java (4)
44-52: 1) subject·만료 검증 테스트가 의도와 일치합니다.
- iat/exp 차이를 Duration으로 검증하는 방식이 구현과 정확히 맞물립니다.
65-75: 2) 커스텀 클레임 검증 케이스 구성이 탄탄합니다.
- 복수 키/값 검증과 타입 지정 파싱이 명확합니다.
136-153: 5) 존재하는 클레임 파싱 테스트는 충분히 명확합니다.
- 타입 지정 파싱과 값 일치가 잘 드러납니다.
167-182: 6) 존재하지 않는 클레임에 대해 null을 기대하는 케이스가 안정성을 높여줍니다.
- 다운스트림에서 Optional 처리 여부 판단에도 도움이 됩니다.
src/main/java/com/example/solidconnection/auth/token/TokenBlackListService.java (3)
15-16: 2) 생성자 주입(@requiredargsconstructor)으로 의존성 관리, 일관성 좋아요.
불변 필드 + 생성자 주입으로 테스트 용이성과 명시성이 좋아졌습니다. 추가 액션은 없습니다.
33-36: 7) 조회 경로는 단순·일관되어 좋습니다.
hasKey로 존재성만 확인하는 현재 설계가 요구사항에 부합한다면 유지해도 무방합니다. 후속으로 키 해시화가 적용되면 이 부분은 그대로 동작합니다.
3-3: 토큰 블랙리스트 설정 점검 및 기본값 정의 요청
- application.yml에서
token.black-list프로퍼티 누락 확인
token.black-list.storage-key-prefix와token-black-list.expire-time이 정의되어 있지 않습니다.TokenProperties에 기본값 설정 또는 @ConstructorBinding 기본값 검토
- 프로퍼티 바인딩이 누락될 경우 NPE 위험이 있으므로
blackList필드에 기본값을 명시하거나 @ConstructorBinding의 default 값을 확인해주세요.확인 후 회신 부탁드립니다!
src/main/java/com/example/solidconnection/auth/service/signup/SignUpTokenProvider.java (2)
64-66: parseEmail 단순 위임 깔끔합니다
- TokenProvider로 위임되어 책임이 잘 분리되었습니다.
27-36: 토큰 처리 로직 개선 제안
- 만료시간 소스 통일
- 현재 토큰 발급(tokenProperties.signUp().expireTime())과 Redis 저장(TokenProperties.getExpireTime(SignUpToken.class))이 같은 설정 객체를 참조하지만, 호출 방식이 달라 가독성·유지보수성 측면에서 분리된 소스처럼 보일 수 있습니다.
-TokenProperties.getExpireTime(SignUpToken.class)로 통일해 발급과 저장이 항상 동일한 TTL을 사용하도록 권장합니다.- Enum 직렬화 명확화
-authType.toString()은 기본적으로name()과 같은 결과를 내지만, 향후toString()이 오버라이드될 경우 의도치 않은 값이 저장될 수 있습니다.
-authType.name()을 사용해 직렬화 값을 명확히 고정하는 편이 안전합니다.- 이메일 입력 방어
-EmailSignUpTokenRequestDTO에 이메일 필드가@NotBlank또는
- 만약 DTO에 해당 검증이 없다면, DTO에 애노테이션을 추가하거나generateAndSaveSignUpToken메서드에서 null/blank 검사 로직을 넣어 방어적으로 처리하시길 제안합니다.src/test/java/com/example/solidconnection/auth/service/signup/SignUpTokenProviderTest.java (3)
53-67: 생성/저장 플로우 검증이 충실합니다
- subject, claim, 저장소 저장 여부를 한 테스트에서 모두 검증하여 회귀 방지에 유용합니다.
- SpyBean으로 저장소 상호작용을 관찰하는 방식이 적절합니다.
84-92: 유효 토큰 검증 성공 케이스 OK
- 정상 경로를 빠르게 검증해 실패 테스트와 균형을 맞추고 있습니다.
104-113: 비-JWT 문자열 예외 검증 OK
- 포맷 위반 처리에 대한 회귀를 잘 막아줍니다.
src/main/java/com/example/solidconnection/auth/service/signup/SignUpTokenProvider.java
Show resolved
Hide resolved
src/main/java/com/example/solidconnection/auth/token/TokenBlackListService.java
Show resolved
Hide resolved
src/main/java/com/example/solidconnection/auth/token/TokenBlackListService.java
Show resolved
Hide resolved
Gyuhyeok99
left a comment
Gyuhyeok99
left a comment
There was a problem hiding this comment.
추상화가 진짜 엄청나네요... 앞으로 이런 작업은 누가 하죠 😭
whqtker
left a comment
whqtker
left a comment
There was a problem hiding this comment.
추상화가 정말 우아하네요 .. 확인했습니다 정말 고생 많으셨어요 ~!!
…ken-vo # Conflicts: # src/main/resources/secret
3 participants
작업 내용
크게 분류하자면 아래와 같은 작업을 했습니다.
(이것 때문인지 라인 변경이 많습니다 🫨)
특이 사항
두개의 PR로 나눌까도 싶었지만, 과도기에 있는 코드가 존재하는게 이상할 것 같아 하나의 PR로 올려봅니다🥲
리뷰 요구사항 (선택)
커밋 따라 읽으시면 쉽습니다!
코멘트로 설명도 달아둘게요~