Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 55 minutes and 25 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough카카오 전용 액세스 토큰 검증을 OIDC 기반 다중 소셜 로그인(카카오·애플)으로 전환하며, 요청/응답 DTO·명세·엔티티·레포지토리·에러코드 및 JWKS/토큰 검증 컴포넌트를 추가/변경했습니다. 컨트롤러 경로가 Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthController
participant AuthService
participant SocialAuthServiceManager
participant OidcTokenValidator
participant OidcJwksClient
participant UserRepository
participant Database
Client->>AuthController: POST /api/v1/auth/login (idToken, provider)
AuthController->>AuthService: signIn(SignInRequestCommand)
AuthService->>SocialAuthServiceManager: getSocialId(command)
SocialAuthServiceManager->>OidcTokenValidator: extractSocialId(idToken, provider)
OidcTokenValidator->>OidcJwksClient: getKeys(jwksUri)
OidcJwksClient->>OidcJwksClient: (캐시 확인/HTTP GET)
OidcJwksClient-->>OidcTokenValidator: JWKS JsonNode
OidcTokenValidator->>OidcTokenValidator: JWK → RSA PublicKey, JWT 파싱, iss/aud 검증
OidcTokenValidator-->>SocialAuthServiceManager: socialId
SocialAuthServiceManager-->>AuthService: SocialType(provider, socialId)
AuthService->>UserRepository: findBySocialIdAndProvider(socialId, provider)
UserRepository->>Database: SELECT ...
Database-->>UserRepository: 사용자
UserRepository-->>AuthService: User 또는 null
alt 신규 사용자
AuthService-->>AuthController: SignInResponse (incomplete signup with socialId)
else 기존 사용자
AuthService-->>AuthController: SignInResponse (issue accessToken)
end
AuthController-->>Client: HTTP 200 (SignInResponse)
sequenceDiagram
participant Client
participant AuthController
participant AuthService
participant OidcTokenValidator
participant OidcJwksClient
participant UserRepository
Client->>AuthController: POST /api/v1/auth/signup (idToken, provider, profile...)
AuthController->>AuthService: signUp(SignUpRequestCommand)
AuthService->>OidcTokenValidator: extractSocialId(idToken, provider)
OidcTokenValidator->>OidcJwksClient: getKeys(jwksUri)
OidcJwksClient-->>OidcTokenValidator: JWKS
OidcTokenValidator-->>AuthService: socialId
AuthService->>UserRepository: existsBySocialId(socialId)
alt 이미 존재
AuthService-->>AuthController: throw CustomException(DUPLICATE_SOCIAL_ID)
else 신규
AuthService->>UserRepository: save(User(socialId, provider, ...))
AuthService-->>AuthController: SignUpResponse
end
AuthController-->>Client: HTTP 200/409
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/main/kotlin/org/appjam/smashing/domain/auth/service/AuthService.kt (1)
66-90:⚠️ Potential issue | 🔴 Critical회원가입에서 클라이언트가 보낸
socialId/provider를 그대로 신뢰하면 계정 선점이 가능합니다.현재
/signup는 요청 본문의socialId/provider를 그대로 저장합니다. 그러면 공격자가 임의의 소셜 식별자로 먼저 가입해 두고, 실제 사용자의 첫 로그인보다 앞서 계정을 선점할 수 있습니다. 회원가입 시점에도 서버가idToken으로 다시 식별자를 추출하거나,/login에서 발급한 서명된 임시 signup token으로 검증해 동일 주체임을 묶어야 합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/org/appjam/smashing/domain/auth/service/AuthService.kt` around lines 66 - 90, In signUp (AuthService.signUp) do not trust socialId/provider from the request body; instead verify the caller's identity by validating an idToken (or the signed temporary signup token issued by /login) with the relevant provider before using socialId/provider to create the User; ensure the validated subject from the token matches the request's socialId/provider and reject or overwrite on mismatch, perform this verification before calling userRepository.save or validateUser/validateCommand so attackers cannot preempt accounts by supplying arbitrary socialId/provider.src/main/kotlin/org/appjam/smashing/domain/auth/controller/AuthController.kt (1)
26-45:⚠️ Potential issue | 🟡 MinorOpenAPI 스펙에 202 응답을 추가로 문서화해주세요.
signIn()엔드포인트는 회원가입 완료 여부에 따라 HTTP 200 또는 202를 반환하지만, 현재@Operation애노테이션은 응답 상태를 문서화하지 않고 있습니다.@ApiResponses를 통해 202 응답(미완료 회원)을 명시하면 클라이언트가 정상 플로우임을 명확히 인지할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/org/appjam/smashing/domain/auth/controller/AuthController.kt` around lines 26 - 45, Add OpenAPI response documentation to the signIn endpoint in AuthController: update the `@Operation` for signIn() to include `@ApiResponses` with at least two `@ApiResponse` entries — one for responseCode = "200" (completed signup) and one for responseCode = "202" (incomplete signup/accepted) — both referencing the SignInResponse schema; ensure the 202 description aligns with the logic that uses SuccessCode.ACCEPTED.httpStatus and keep the existing summary/description unchanged.src/main/kotlin/org/appjam/smashing/domain/auth/dto/request/SignUpRequest.kt (1)
14-39:⚠️ Potential issue | 🔴 Critical회원가입 요청도
provider누락 시 500으로 터집니다.
provider필드가@ValidEnum만 있고@NotBlank가 없습니다.EnumValidator의isValid()메서드는 null/blank 값에 대해 true를 반환하므로, null이나 빈 문자열은 유효성 검사를 통과합니다. 그러나toCommand()메서드의provider!!에서 NullPointerException이 발생합니다. 다른 필수 필드(socialId, nickname, gender 등)과 동일하게@NotBlank검증을 추가해야 합니다.예시 수정
+ `@field`:NotBlank(message = "provider를 입력해주세요.") `@field`:ValidEnum(message = "잘못된 provider 값입니다.", enumClass = ProviderType::class) val provider: String?,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/org/appjam/smashing/domain/auth/dto/request/SignUpRequest.kt` around lines 14 - 39, The provider property in SignUpRequest lacks `@NotBlank` so enum-only validation lets null/blank pass and causes NPE at provider!! in toCommand(); add `@field`:NotBlank(message = "provider를 입력해주세요.") to the provider declaration (same style as nickname/gender) so validation fails for missing provider before toCommand() is called, and ensure toCommand() continues to use provider!! and ofIgnoreCase<ProviderType>(provider!!) to build the SignUpRequestCommand.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/kotlin/org/appjam/smashing/domain/auth/dto/request/SignInRequest.kt`:
- Around line 12-18: The provider field in SignInRequest is allowed through
validation (ValidEnum) when null/blank and then causes a NullPointerException in
toCommand() via provider!!; add `@field`:NotBlank (or equivalent javax/validation
NotBlank annotation) to the provider property so missing/empty provider values
fail request validation instead of reaching toCommand() and causing a 500;
verify ValidEnum (EnumValidator) still runs after NotBlank to map provider via
ofIgnoreCase<ProviderType> in toCommand().
In `@src/main/kotlin/org/appjam/smashing/domain/auth/service/AuthService.kt`:
- Around line 176-179: validateUser currently checks only socialId and can block
different-provider accounts with identical socialId; update validateUser to
check both socialId and provider (use repository method like
existsBySocialIdAndProvider or add a query that accepts requestCommand.provider)
so the duplicate check matches the same criteria used by signIn (which looks up
by socialId and provider). Ensure you reference requestCommand.socialId and
requestCommand.provider and throw CustomException(ErrorCode.DUPLICATE_SOCIAL_ID)
when that combined-existence check returns true.
In
`@src/main/kotlin/org/appjam/smashing/domain/auth/social/OidcTokenValidator.kt`:
- Around line 28-36: The extractSocialId function is missing audience
validation; update extractSocialId to accept an aud parameter and after parsing
claims verify claims.audience (or claims.get("aud")) matches the provided aud,
throwing the appropriate CustomException (e.g., ErrorCode.INVALID_ID_TOKEN) on
mismatch; then update KakaoOidcValidator and AppleOidcValidator to pass their
provider-specific client_id into oidcTokenValidator.extractSocialId(...) so each
token's aud is validated against the intended client.
- Around line 34-39: The thrown CustomException(ErrorCode.INVALID_ISS) inside
the try block is being swallowed by the broad catch (e: Exception) and remapped
to INVALID_ID_TOKEN; update the error handling in OidcTokenValidator (the method
that reads claims.issuer and returns claims.subject) so you do not override
existing CustomException: either narrow the catch to only expected parsing
exceptions (e.g., JwtException/whatever specific token parse exception your JWT
library throws) or rethrow CustomException by checking if (e is CustomException)
throw e else throw CustomException(ErrorCode.INVALID_ID_TOKEN); reference
symbols: claims.issuer, claims.subject, CustomException, ErrorCode.INVALID_ISS,
ErrorCode.INVALID_ID_TOKEN.
- Around line 46-49: The code in OidcTokenValidator uses a newly created
RestTemplate() to fetch jwksUri which bypasses shared timeout/connection
policies; replace the direct instantiation by injecting a RestTemplate bean
configured with sensible connect/read timeouts (define it in a `@Configuration`
and expose as `@Bean`), use that injected RestTemplate instance to call
getForObject(jwksUri, JsonNode::class.java) in the method that reads jwks, and
wrap network errors/timeouts to throw the existing
CustomException(ErrorCode.INVALID_ID_TOKEN) so slow/unresponsive JWKS endpoints
don't block request threads indefinitely.
In `@src/main/kotlin/org/appjam/smashing/domain/user/entity/User.kt`:
- Around line 14-18: The table currently defines a non-unique index for the
social key which allows duplicate (social_id, provider) rows; update the User
entity's `@Table` declaration to enforce a DB-level unique constraint on
(social_id, provider) so duplicates cannot be persisted. Specifically, add a
UniqueConstraint for the columns "social_id" and "provider" (or replace the
existing idx_social Index with a unique constraint) in the `@Table` annotation for
the User class to ensure uniqueness relied on by findBySocialIdAndProvider().
---
Outside diff comments:
In
`@src/main/kotlin/org/appjam/smashing/domain/auth/controller/AuthController.kt`:
- Around line 26-45: Add OpenAPI response documentation to the signIn endpoint
in AuthController: update the `@Operation` for signIn() to include `@ApiResponses`
with at least two `@ApiResponse` entries — one for responseCode = "200" (completed
signup) and one for responseCode = "202" (incomplete signup/accepted) — both
referencing the SignInResponse schema; ensure the 202 description aligns with
the logic that uses SuccessCode.ACCEPTED.httpStatus and keep the existing
summary/description unchanged.
In
`@src/main/kotlin/org/appjam/smashing/domain/auth/dto/request/SignUpRequest.kt`:
- Around line 14-39: The provider property in SignUpRequest lacks `@NotBlank` so
enum-only validation lets null/blank pass and causes NPE at provider!! in
toCommand(); add `@field`:NotBlank(message = "provider를 입력해주세요.") to the provider
declaration (same style as nickname/gender) so validation fails for missing
provider before toCommand() is called, and ensure toCommand() continues to use
provider!! and ofIgnoreCase<ProviderType>(provider!!) to build the
SignUpRequestCommand.
In `@src/main/kotlin/org/appjam/smashing/domain/auth/service/AuthService.kt`:
- Around line 66-90: In signUp (AuthService.signUp) do not trust
socialId/provider from the request body; instead verify the caller's identity by
validating an idToken (or the signed temporary signup token issued by /login)
with the relevant provider before using socialId/provider to create the User;
ensure the validated subject from the token matches the request's
socialId/provider and reject or overwrite on mismatch, perform this verification
before calling userRepository.save or validateUser/validateCommand so attackers
cannot preempt accounts by supplying arbitrary socialId/provider.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a486138d-e8cd-443a-9c56-8f4937e31291
📒 Files selected for processing (19)
src/main/kotlin/org/appjam/smashing/domain/auth/controller/AuthController.ktsrc/main/kotlin/org/appjam/smashing/domain/auth/dto/command/SignInRequestCommand.ktsrc/main/kotlin/org/appjam/smashing/domain/auth/dto/command/SignUpRequestCommand.ktsrc/main/kotlin/org/appjam/smashing/domain/auth/dto/request/SignInRequest.ktsrc/main/kotlin/org/appjam/smashing/domain/auth/dto/request/SignUpRequest.ktsrc/main/kotlin/org/appjam/smashing/domain/auth/dto/response/SignInResponse.ktsrc/main/kotlin/org/appjam/smashing/domain/auth/dto/response/SocialType.ktsrc/main/kotlin/org/appjam/smashing/domain/auth/enums/ProviderType.ktsrc/main/kotlin/org/appjam/smashing/domain/auth/service/AuthService.ktsrc/main/kotlin/org/appjam/smashing/domain/auth/social/OidcTokenValidator.ktsrc/main/kotlin/org/appjam/smashing/domain/auth/social/SocialAuthServiceManager.ktsrc/main/kotlin/org/appjam/smashing/domain/auth/social/apple/AppleOidcValidator.ktsrc/main/kotlin/org/appjam/smashing/domain/auth/social/kakao/KakaoApiClient.ktsrc/main/kotlin/org/appjam/smashing/domain/auth/social/kakao/KakaoAuthTokenValidator.ktsrc/main/kotlin/org/appjam/smashing/domain/auth/social/kakao/KakaoOidcValidator.ktsrc/main/kotlin/org/appjam/smashing/domain/auth/social/kakao/dto/response/KakaoUserResponse.ktsrc/main/kotlin/org/appjam/smashing/domain/user/entity/User.ktsrc/main/kotlin/org/appjam/smashing/domain/user/repository/UserRepository.ktsrc/main/kotlin/org/appjam/smashing/global/exception/ErrorCode.kt
💤 Files with no reviewable changes (3)
- src/main/kotlin/org/appjam/smashing/domain/auth/social/kakao/dto/response/KakaoUserResponse.kt
- src/main/kotlin/org/appjam/smashing/domain/auth/social/kakao/KakaoApiClient.kt
- src/main/kotlin/org/appjam/smashing/domain/auth/social/kakao/KakaoAuthTokenValidator.kt
Comment on lines
176
to
179
| private fun validateUser(requestCommand: SignUpRequestCommand) { | ||
| if (userRepository.existsByKakaoId(requestCommand.kakaoId)) { | ||
| throw CustomException(ErrorCode.DUPLICATE_KAKAO_ID) | ||
| if (userRepository.existsBySocialId(requestCommand.socialId)) { | ||
| throw CustomException(ErrorCode.DUPLICATE_SOCIAL_ID) | ||
| } |
There was a problem hiding this comment.
중복 체크는 provider까지 포함해야 합니다.
signIn()은 바로 위에서 (socialId, provider)로 조회하는데, 가입 검증만 existsBySocialId()를 쓰면 Apple/Kakao 간에 우연히 같은 문자열이 나온 경우 정상 가입까지 막아버립니다. 중복 판단 기준을 로그인 조회 기준과 동일하게 맞춰주세요.
예시 수정
- if (userRepository.existsBySocialId(requestCommand.socialId)) {
+ if (userRepository.existsBySocialIdAndProvider(
+ socialId = requestCommand.socialId,
+ provider = requestCommand.provider,
+ )) {
throw CustomException(ErrorCode.DUPLICATE_SOCIAL_ID)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/kotlin/org/appjam/smashing/domain/auth/service/AuthService.kt`
around lines 176 - 179, validateUser currently checks only socialId and can
block different-provider accounts with identical socialId; update validateUser
to check both socialId and provider (use repository method like
existsBySocialIdAndProvider or add a query that accepts requestCommand.provider)
so the duplicate check matches the same criteria used by signIn (which looks up
by socialId and provider). Ensure you reference requestCommand.socialId and
requestCommand.provider and throw CustomException(ErrorCode.DUPLICATE_SOCIAL_ID)
when that combined-existence check returns true.
Comment on lines
14
to
18
| @Table( | ||
| indexes = [ | ||
| Index(name = "idx_nickname", columnList = "nickname") | ||
| Index(name = "idx_nickname", columnList = "nickname"), | ||
| Index(name = "idx_social", columnList = "social_id, provider") | ||
| ] |
There was a problem hiding this comment.
소셜 식별자는 복합 유니크 제약으로 보호해야 합니다.
지금은 일반 인덱스만 있어서 동시 가입 두 건이 모두 애플리케이션 검증을 통과하면 같은 (social_id, provider) 레코드가 중복 저장될 수 있습니다. 이후 findBySocialIdAndProvider()가 단일 사용자를 전제로 동작하므로, 이 키는 DB 레벨에서 유니크하게 막아야 합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/kotlin/org/appjam/smashing/domain/user/entity/User.kt` around lines
14 - 18, The table currently defines a non-unique index for the social key which
allows duplicate (social_id, provider) rows; update the User entity's `@Table`
declaration to enforce a DB-level unique constraint on (social_id, provider) so
duplicates cannot be persisted. Specifically, add a UniqueConstraint for the
columns "social_id" and "provider" (or replace the existing idx_social Index
with a unique constraint) in the `@Table` annotation for the User class to ensure
uniqueness relied on by findBySocialIdAndProvider().
kyoooooong
reviewed
Apr 14, 2026
kyoooooong
left a comment
kyoooooong
left a comment
Contributor
There was a problem hiding this comment.
유짱으로 이행시하겠습니다.
유빈이는
짱이다....😻
정말 힘들었을텐데 수고 너무너무 많았습니다!! 👍
| .parseClaimsJws(idToken) | ||
| .body | ||
|
|
||
| if (claims.issuer != iss) throw CustomException(ErrorCode.INVALID_ISS) |
Contributor
There was a problem hiding this comment.
OIDC 공통화한 방향 너무너무 좋습니다!
다만 현재 iss만 검증하고 aud 검증이 빠져 있는데, ID Token은 우리 client를 대상으로 발급된 토큰인지도 함께 확인해야 해서 provider별 clientId 기준으로 aud까지 검증해주면 더 안전할 것 같아요.
또 지금 구조상 Apple/Kakao가 각각 따로 처리하기보다, provider별 설정값만 넘기고 OidcTokenValidator에서 공통 검증하도록 두는 쪽이 더 자연스러울 수 있을 것같은데, 혹시 유빈님은 어떠실까요?
Member
Author
There was a problem hiding this comment.
좋습니다!
해당 변경사항 반영했는데 의도하신 부분이랑 일치하는지 확인 부탁드립니다! 5886099
| val accessToken: String?, | ||
| @field:NotBlank(message = "idToken을 입력해주세요.") | ||
| val idToken: String?, | ||
| @field:ValidEnum(message = "잘못된 provider 값입니다.", enumClass = ProviderType::class) |
Contributor
There was a problem hiding this comment.
provider는 현재 enum validation만 하고 toCommand()에서 바로 provider!!로 사용하고 있어서, null/blank가 validation에서 걸러지지 않으면 500으로 떨어질 수 있을 것 같아요.
@notblank까지 같이 두어서 요청 검증 단계에서 400으로 막아주면 더 안전할 수 있을 것같습니다!
| private fun validateUser(requestCommand: SignUpRequestCommand) { | ||
| if (userRepository.existsByKakaoId(requestCommand.kakaoId)) { | ||
| throw CustomException(ErrorCode.DUPLICATE_KAKAO_ID) | ||
| if (userRepository.existsBySocialId(requestCommand.socialId)) { |
Contributor
There was a problem hiding this comment.
로그인 조회는 (socialId, provider) 조합으로 하고 있는데, 가입 시 중복 체크는 existsBySocialId()만 사용하고 있어서 기준이 조금 달라질 수 있을 것같아요.
현재 구조라면 중복 판단도 provider까지 포함해서 맞춰주는 게 더 자연스러울 수 있을 것같습니다!
Member
Author
There was a problem hiding this comment.
좋은 의견 감사합니다 :)
제가 생각을 해보았는데요! provider는 KAKAO 소셜 로그인인지, APPLE 소셜 로그인인지 기록하는 용이기 때문에 서로 다른 유저간 중복이 될 수밖에 없다고 판단이 들었습니다!
| val header = String(Base64.getUrlDecoder().decode(idToken.split(".")[0])) | ||
| val kid = ObjectMapper().readTree(header).get(KID).asText() | ||
|
|
||
| val jwks = RestTemplate().getForObject(jwksUri, JsonNode::class.java) |
Contributor
There was a problem hiding this comment.
여기 JWKS 조회가 인증 요청마다 외부 호출로 이어질 수 있어서 운영 측면에서는 조금 걱정되는 부분이 있을 것같아요.
RestTemplate()를 직접 생성하기보다는 timeout이 설정된 클라이언트를 주입받고, 가능하면 provider 단위 캐싱도 같이 두면 더 안정적일 수 있을 것같다는 생각이 들어요!
Member
Author
There was a problem hiding this comment.
오호 생각치 못한 방향이네요!
캐싱 작업은 처음 해봐서 제대로 했는지 모르겠네요.. 확인 부탁드립니다! 11dd186
| if (claims.issuer != iss) throw CustomException(ErrorCode.INVALID_ISS) | ||
|
|
||
| claims.subject | ||
| } catch (e: Exception) { |
Contributor
There was a problem hiding this comment.
여기서 내부에서 던진 CustomException도 마지막 catch (e: Exception)에 같이 잡히면서 INVALID_ID_TOKEN으로 덮일 수 있을 것같아요.
의도적인 부분이 아니었다면 CustomException은 그대로 던지고, try catch문을 지양하고 예상하지 못한 예외만 공통 에러로 변환해도 좋을 것 같습니다!
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/kotlin/org/appjam/smashing/global/exception/ErrorCode.kt`:
- Around line 45-49: The validator currently catches all exceptions and converts
them to INVALID_ID_TOKEN, which overwrites specific CustomException error codes
like INVALID_ISS and INVALID_AUD; update the OidcTokenValidator (refer to
OidcTokenValidator.kt and its try/catch around token validation) so that if the
caught exception is an instance of CustomException it is rethrown (or
propagated) unchanged, and only non-CustomException errors are
wrapped/translated into the INVALID_ID_TOKEN CustomException; ensure references
to CustomException, INVALID_ISS, INVALID_AUD, and INVALID_ID_TOKEN are used to
locate and adjust the handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8ed3f975-4106-4852-a572-f4fa35e87a0b
📒 Files selected for processing (6)
src/main/kotlin/org/appjam/smashing/domain/auth/dto/request/SignInRequest.ktsrc/main/kotlin/org/appjam/smashing/domain/auth/social/OidcProperties.ktsrc/main/kotlin/org/appjam/smashing/domain/auth/social/OidcTokenValidator.ktsrc/main/kotlin/org/appjam/smashing/domain/auth/social/SocialAuthServiceManager.ktsrc/main/kotlin/org/appjam/smashing/global/exception/ErrorCode.ktsrc/main/resources/application.yml
✅ Files skipped from review due to trivial changes (1)
- src/main/resources/application.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/kotlin/org/appjam/smashing/domain/auth/social/OidcTokenValidator.kt
Comment on lines
+45
to
+49
| INVALID_ISS(HttpStatus.UNAUTHORIZED, "AUTH-020", "유효하지 않은 ID 토큰 발급자입니다."), | ||
| INVALID_AUD(HttpStatus.UNAUTHORIZED, "AUTH-021", "유효하지 않은 audience입니다."), | ||
| DUPLICATE_SOCIAL_ID(HttpStatus.CONFLICT, "AUTH-022", "이미 존재하는 유저입니다."), | ||
| DUPLICATE_NICKNAME(HttpStatus.CONFLICT, "AUTH-023", "이미 사용 중인 닉네임입니다."), | ||
| INVALID_ID_TOKEN(HttpStatus.UNAUTHORIZED, "AUTH-024", "유효하지 않은 ID 토큰입니다."), |
There was a problem hiding this comment.
OIDC 세부 오류 코드가 호출부에서 덮어써질 수 있습니다.
INVALID_ISS/INVALID_AUD를 추가했지만, 현재 제공된 OidcTokenValidator.kt:44-50 흐름에서는 해당 CustomException도 catch (e: Exception)에 잡혀 최종적으로 INVALID_ID_TOKEN으로 바뀝니다. 이 enum 자체는 괜찮지만, 세부 에러 응답을 의도했다면 validator에서 CustomException은 그대로 재던지도록 조정해 주세요.
🐛 제안 수정 방향
try {
// issuer/audience 검증
if (claims.issuer != iss) throw CustomException(ErrorCode.INVALID_ISS)
if (claims.audience != clientId) throw CustomException(ErrorCode.INVALID_AUD)
claims.subject
+} catch (e: CustomException) {
+ throw e
} catch (e: Exception) {
throw CustomException(ErrorCode.INVALID_ID_TOKEN)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/kotlin/org/appjam/smashing/global/exception/ErrorCode.kt` around
lines 45 - 49, The validator currently catches all exceptions and converts them
to INVALID_ID_TOKEN, which overwrites specific CustomException error codes like
INVALID_ISS and INVALID_AUD; update the OidcTokenValidator (refer to
OidcTokenValidator.kt and its try/catch around token validation) so that if the
caught exception is an instance of CustomException it is rethrown (or
propagated) unchanged, and only non-CustomException errors are
wrapped/translated into the INVALID_ID_TOKEN CustomException; ensure references
to CustomException, INVALID_ISS, INVALID_AUD, and INVALID_ID_TOKEN are used to
locate and adjust the handling.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/kotlin/org/appjam/smashing/global/config/RestTemplateConfig.kt (1)
10-17: RestTemplate 범위 제한 검토 권장.해당
RestTemplate빈은 JWKS 조회 용도(3초 타임아웃)로 설계되었지만, 애플리케이션 전역@Bean으로 등록되어 있어 추후 다른 용도(외부 결제, 파일 업로드 등 장시간 호출)에서 주입받아 사용할 경우 3초 타임아웃이 부적절할 수 있습니다. JWKS 전용 한정 빈(@Qualifier또는 전용 이름)으로 분리하거나, 범용 빈이라면 타임아웃을 용도별로 구분하는 것을 고려해 주세요.♻️ 예시: 이름/Qualifier로 용도 분리
`@Configuration` class RestTemplateConfig { - `@Bean` - fun restTemplate(): RestTemplate { + `@Bean`("oidcJwksRestTemplate") + fun oidcJwksRestTemplate(): RestTemplate { val factory = SimpleClientHttpRequestFactory().apply { setConnectTimeout(3_000) setReadTimeout(3_000) } return RestTemplate(factory) } }그리고
OidcJwksClient에서@Qualifier("oidcJwksRestTemplate")로 주입받으면 됩니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/org/appjam/smashing/global/config/RestTemplateConfig.kt` around lines 10 - 17, 현재의 restTemplate() 빈은 전역으로 등록되어 모든 주입 지점에서 3초 타임아웃을 갖게 되므로 JWKS 전용으로 범위를 제한해야 합니다; restTemplate()을 JWKS 전용 빈으로 분리하여 `@Bean`(name = "oidcJwksRestTemplate") 또는 `@Qualifier`("oidcJwksRestTemplate")로 명명하고 기존 RestTemplate 빈을 전역 용도로 계속 사용하려면 별도의 범용 RestTemplate(더 긴 타임아웃)도 추가 생성하세요; 그러면 OidcJwksClient에서 해당 JWKS 전용 빈을 `@Qualifier`("oidcJwksRestTemplate")로 주입받도록 변경합니다.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/kotlin/org/appjam/smashing/domain/auth/social/OidcJwksClient.kt`:
- Around line 21-26: The getKeys loader currently treats any failure as
INVALID_ID_TOKEN and caches JWKS for 1 hour, causing long failures on key
rollover and conflating network errors with invalid tokens; add a
refresh(jwksUri: String) method on OidcJwksClient that invalidates the Caffeine
cache (cache.invalidate(jwksUri)) and re-fetches keys, and modify getKeys to
catch network-related exceptions from restTemplate.getForObject (e.g.,
ResourceAccessException, HttpServerErrorException) and throw a new
CustomException(ErrorCode.JWKS_FETCH_FAILED) so callers can distinguish
transient fetch errors; also update OidcTokenValidator.getPublicKey to call
jwksClient.refresh(jwksUri) once when a kid is not found before ultimately
throwing CustomException(ErrorCode.INVALID_ID_TOKEN).
---
Nitpick comments:
In `@src/main/kotlin/org/appjam/smashing/global/config/RestTemplateConfig.kt`:
- Around line 10-17: 현재의 restTemplate() 빈은 전역으로 등록되어 모든 주입 지점에서 3초 타임아웃을 갖게 되므로
JWKS 전용으로 범위를 제한해야 합니다; restTemplate()을 JWKS 전용 빈으로 분리하여 `@Bean`(name =
"oidcJwksRestTemplate") 또는 `@Qualifier`("oidcJwksRestTemplate")로 명명하고 기존
RestTemplate 빈을 전역 용도로 계속 사용하려면 별도의 범용 RestTemplate(더 긴 타임아웃)도 추가 생성하세요; 그러면
OidcJwksClient에서 해당 JWKS 전용 빈을 `@Qualifier`("oidcJwksRestTemplate")로 주입받도록 변경합니다.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7b8aa920-1a16-46af-9d84-0989f6a83887
📒 Files selected for processing (4)
build.gradle.ktssrc/main/kotlin/org/appjam/smashing/domain/auth/social/OidcJwksClient.ktsrc/main/kotlin/org/appjam/smashing/domain/auth/social/OidcTokenValidator.ktsrc/main/kotlin/org/appjam/smashing/global/config/RestTemplateConfig.kt
✅ Files skipped from review due to trivial changes (1)
- build.gradle.kts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/kotlin/org/appjam/smashing/domain/auth/social/OidcTokenValidator.kt
Comment on lines
+21
to
+26
| fun getKeys( | ||
| jwksUri: String, | ||
| ): JsonNode = cache.get(jwksUri) { | ||
| restTemplate.getForObject(jwksUri, JsonNode::class.java) | ||
| ?: throw CustomException(ErrorCode.INVALID_ID_TOKEN) | ||
| } ?: throw CustomException(ErrorCode.INVALID_ID_TOKEN) |
There was a problem hiding this comment.
JWKS 키 롤오버 및 네트워크 오류 처리 보완 필요.
두 가지 우려가 있습니다.
-
키 롤오버 시 복구 지연 (major): JWKS를
expireAfterWrite(1, HOURS)로 캐시하는데, 애플/카카오가 예정보다 일찍 서명 키를 롤링하면 캐시된 JWKS에 토큰의kid가 존재하지 않아OidcTokenValidator.getPublicKey()에서INVALID_ID_TOKEN이 발생하고, 이 상태가 최대 1시간 지속됩니다. 호출자(OidcTokenValidator)가kid를 못 찾았을 때 캐시를 무효화(cache.invalidate(jwksUri))하고 1회 재조회하도록 협조하거나, 본 클라이언트에refresh(jwksUri)메서드를 제공하는 것이 안전합니다. -
네트워크 오류가 토큰 오류로 둔갑 (major):
restTemplate.getForObject가 타임아웃/5xx로 예외(ResourceAccessException,HttpServerErrorException등)를 던지면 캐시 로더에서 그대로 전파되고, 상위OidcTokenValidator.extractSocialId의 포괄적 try-catch에서INVALID_ID_TOKEN(401)으로 변환됩니다. 실제로는 서버/네트워크 장애이므로INVALID_ID_TOKEN과 구분되는 에러코드(예:JWKS_FETCH_FAILED5xx 계열)로 매핑하는 것이 관측성과 클라이언트 재시도 판단에 유리합니다.
또한 Line 26의 바깥쪽 ?: throw CustomException(ErrorCode.INVALID_ID_TOKEN)은 로더가 이미 null일 때 throw하므로 실질적으로 도달하지 않습니다(Caffeine의 get(key, loader)는 로더가 null 반환 시 null 반환). 중복이지만 방어적 코드로 유지해도 무방합니다.
🛠️ 제안: 강제 갱신 메서드 추가 + 네트워크 예외 분리
`@Component`
class OidcJwksClient(
private val restTemplate: RestTemplate,
) {
private val cache: Cache<String, JsonNode> = Caffeine.newBuilder()
.expireAfterWrite(1, TimeUnit.HOURS)
.maximumSize(10)
.build()
fun getKeys(
jwksUri: String,
): JsonNode = cache.get(jwksUri) {
- restTemplate.getForObject(jwksUri, JsonNode::class.java)
- ?: throw CustomException(ErrorCode.INVALID_ID_TOKEN)
+ fetch(jwksUri)
} ?: throw CustomException(ErrorCode.INVALID_ID_TOKEN)
+
+ fun refresh(jwksUri: String): JsonNode {
+ cache.invalidate(jwksUri)
+ return getKeys(jwksUri)
+ }
+
+ private fun fetch(jwksUri: String): JsonNode = try {
+ restTemplate.getForObject(jwksUri, JsonNode::class.java)
+ ?: throw CustomException(ErrorCode.INVALID_ID_TOKEN)
+ } catch (e: RestClientException) {
+ throw CustomException(ErrorCode.JWKS_FETCH_FAILED) // 신규 에러코드 권장
+ }
}그리고 OidcTokenValidator.getPublicKey()에서 kid 미일치 시 jwksClient.refresh(jwksUri)로 1회 재시도 후에도 없으면 INVALID_ID_TOKEN을 던지는 흐름을 권장합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/kotlin/org/appjam/smashing/domain/auth/social/OidcJwksClient.kt`
around lines 21 - 26, The getKeys loader currently treats any failure as
INVALID_ID_TOKEN and caches JWKS for 1 hour, causing long failures on key
rollover and conflating network errors with invalid tokens; add a
refresh(jwksUri: String) method on OidcJwksClient that invalidates the Caffeine
cache (cache.invalidate(jwksUri)) and re-fetches keys, and modify getKeys to
catch network-related exceptions from restTemplate.getForObject (e.g.,
ResourceAccessException, HttpServerErrorException) and throw a new
CustomException(ErrorCode.JWKS_FETCH_FAILED) so callers can distinguish
transient fetch errors; also update OidcTokenValidator.getPublicKey to call
jwksClient.refresh(jwksUri) once when a kid is not found before ultimately
throwing CustomException(ErrorCode.INVALID_ID_TOKEN).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 Related Issue
#️⃣ 요약 설명
KAKAO: 기존 AccessToken으로 카카오 API를 직접 호출하는 로직에서 ID Token으로 검증하도록 수정했습니다.APPLE: 애플 로그인도 동일하게 처리했습니다.OidcTokenValidator에서 JWKS 공개키 조회 및 JWT 검증을 공통 처리하도록 했습니다.📝 작업 내용
👍 동작 확인
💬 리뷰 요구사항(선택)