feat: oauth #16
feat: oauth #16
Conversation
|
Warning Rate limit exceeded@mkSpace has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 16 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (37)
WalkthroughOAuth 소셜 로그인 기능이 Google 전용에서 Google과 Apple을 모두 지원하는 범용 구조로 리팩토링되었습니다. 각 소셜 제공자별 클라이언트, DTO, 설정, 저장소, 도메인 모델이 추가 또는 변경되었으며, API 엔드포인트와 서비스 계층도 다중 제공자 지원 방식으로 확장되었습니다. DB 스키마와 엔티티도 이에 맞게 수정되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthController
participant AuthApplicationService
participant OAuthClient(Google/Apple)
participant ProviderAPI(Google/Apple)
participant AuthService
participant UserRepository
Client->>AuthController: POST /api/v1/auth/login/{providerType} (authorizationCode, name)
AuthController->>AuthApplicationService: signInForSocial(params, name)
AuthApplicationService->>OAuthClient: requestOAuthInfo(params)
OAuthClient->>ProviderAPI: (토큰 교환 및 사용자 정보 요청)
ProviderAPI-->>OAuthClient: OAuthInfo 반환
OAuthClient-->>AuthApplicationService: OAuthInfo(providerType, providerId, revocableToken)
AuthApplicationService->>AuthService: socialLogin(providerType, providerId, name, revocableToken)
AuthService->>UserRepository: findUserByProviderAndProviderId(providerType, providerId)
alt 신규 회원
AuthService->>UserRepository: 회원가입 및 저장
end
AuthService-->>AuthApplicationService: AuthResult(exists, accessToken)
AuthApplicationService-->>AuthController: OAuthLoginResponse
AuthController-->>Client: ApiResponse<OAuthLoginResponse>
Suggested labels
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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: 13
🔭 Outside diff range comments (1)
noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/config/OAuthFeignConfig.kt (1)
12-14: 🛠️ Refactor suggestion클래스 가시성 및 네이밍 일관성
클래스명이OAuthFeignConfig로 변경되었으나internal으로 선언되어 모듈 외부 접근이 제한됩니다. Spring 설정 클래스는public(default)으로 선언하는 것이 안전하므로internal제거를 고려하세요.
🧹 Nitpick comments (21)
noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/google/GoogleTokens.kt (1)
1-20: Jackson 매핑 확장성 및 강건성 확보
필요 없는 필드를 무시할 수 있도록@JsonIgnoreProperties(ignoreUnknown = true)를 클래스 레벨에 추가하는 것을 권장합니다. 또한refreshToken처럼 nullable 속성에 대해 기본값을 제공하면 역직렬화 실패 시 안정성이 높아집니다.+@JsonIgnoreProperties(ignoreUnknown = true) data class GoogleTokens( @JsonProperty("access_token") val accessToken: String, @JsonProperty("expires_in") val expiresIn: Long, @JsonProperty("refresh_token") - val refreshToken: String?, + val refreshToken: String? = null, @JsonProperty("scope") val scope: String, @JsonProperty("token_type") val tokenType: String, )noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/common/Revocable.kt (1)
1-5: Revocable 인터페이스 정의 적절성
revokeToken(token: String): Boolean메서드 서명이 명확하며 확장성도 확보되어 있습니다. 다만 KDoc 주석을 추가하면 구현체 사용 시 가독성이 향상됩니다.noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/common/OAuthClient.kt (1)
5-8: OAuth 클라이언트 인터페이스가 잘 설계되었습니다.전략 패턴을 활용한 깔끔한 추상화로 다양한 OAuth 제공자를 지원할 수 있습니다. 구현체에서는 예외 처리와 오류 상황에 대한 적절한 처리를 고려하세요.
구현체에서 OAuth 플로우 실패 시의 예외 처리 전략을 문서화하는 것을 권장합니다.
noweekend-core/core-api/src/main/kotlin/noweekend/core/domain/auth/AuthApplicationService.kt (1)
7-7: 메서드 문서화 고려사항.메서드에서 발생할 수 있는 예외나 에러 처리에 대한 문서화를 추가하는 것을 고려해보세요. 특히 OAuth 인증 실패, 네트워크 오류 등의 케이스에 대한 가이드라인이 있으면 좋겠습니다.
noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/google/GoogleLoginParams.kt (1)
6-6: 입력 검증 고려사항.authorization code에 대한 기본적인 검증(null/empty 체크, 형식 검증)을 추가하는 것을 고려해보세요.
-class GoogleLoginParams(private val code: String) : OAuthLoginParams { +class GoogleLoginParams(private val code: String) : OAuthLoginParams { + init { + require(code.isNotBlank()) { "Authorization code cannot be blank" } + }noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/common/OAuthInfo.kt (2)
5-9: 메서드 대신 프로퍼티 사용 권장
Kotlin 관례에 따라fun getProviderType(): ProviderType대신val providerType: ProviderType형태의 프로퍼티 선언이 더 idiomatic합니다.
5-9: KDoc 주석 추가 권장
공개 인터페이스인 만큼 각 메서드에 KDoc을 추가하여 함수 목적과 반환 값을 명확히 문서화하면 유지보수성 향상에 도움이 됩니다.noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/google/GoogleOAuthInfo.kt (2)
7-17: 예상치 못한 JSON 필드 무시 설정 권장
Feign을 통한 JSON 매핑 시 새로운 필드가 추가될 경우 로직 에러를 방지하려면@JsonIgnoreProperties(ignoreUnknown = true)를 클래스에 선언하세요.
7-17: 이메일 노출 유무 검토
GoogleOAuthInfo에서는OAuthInfo인터페이스를 통해 노출되지 않습니다. 이후 서비스 로직에서 이메일이 필요하다면 인터페이스 확장 또는 별도 DTO를 고려하세요.noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/apple/ApplePublicKeyResponse.kt (2)
3-4: 추가 설정: JSON 필드 무시
예상치 못한 JSON 필드를 무시하도록@JsonIgnoreProperties(ignoreUnknown = true)를ApplePublicKeyResponse와ApplePublicKey클래스에 선언해 매핑 안정성을 확보하세요.
5-12: KDoc 주석 추가 고려
데이터 클래스에 KDoc을 추가해 API 응답 필드 의미를 문서화하면 유지보수성 향상에 도움이 됩니다.noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/apple/AppleLoginParams.kt (2)
6-9: 생성자 파라미터에 val 사용 검토
private val code: String대신val code: String으로 선언하면 별도 getter 구현 없이code프로퍼티에 바로 접근할 수 있습니다.
6-9: KDoc 주석 추가 권장
클래스와 메서드에 KDoc을 추가해code가 어떤 인증 흐름에 사용되는지 문서화하면 협업 시 이해도를 높일 수 있습니다.noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/apple/AppleTokens.kt (2)
9-10: expiresIn 필드의 데이터 타입 검토 필요OAuth 표준에 따르면
expires_in은 일반적으로 초 단위 숫자값이므로Int타입이 더 적절할 수 있습니다.Apple OAuth 문서를 확인 후 다음과 같이 변경을 고려해보세요:
@JsonProperty("expires_in") -val expiresIn: String, +val expiresIn: Int,
21-22: 에러 처리 방식 개선 제안성공과 실패 응답을 같은 객체로 처리하면 클라이언트 코드에서 에러 처리가 복잡해질 수 있습니다.
error필드가 있을 때 다른 필드들의 처리 방식을 명확히 하거나, 별도의 에러 응답 타입을 고려해보세요.noweekend-core/core-api/src/main/kotlin/noweekend/core/api/controller/v1/request/LoginRequest.kt (2)
12-14: Authorization Code 검증 개선 제안OAuth Authorization Code는 일반적으로 특정 형식을 가지므로, 단순한
@NotBlank검증보다는 패턴 검증을 추가하는 것을 고려해보세요.+import jakarta.validation.constraints.Pattern -@field:NotBlank(message = "authorizationCode는 필수입니다.") +@field:NotBlank(message = "authorizationCode는 필수입니다.") +@field:Pattern(regexp = "^[A-Za-z0-9._-]+$", message = "유효하지 않은 authorizationCode 형식입니다.") val authorizationCode: String,
20-25: 확장 함수의 완전성 및 확장성 검토현재 when 표현식이 모든
ProviderType값을 처리하지만, 새로운 제공자가 추가될 때 컴파일 시점에 누락을 감지할 수 있도록 개선할 수 있습니다.fun LoginRequest.toOAuthLoginParams(providerType: ProviderType): OAuthLoginParams { return when (providerType) { ProviderType.GOOGLE -> GoogleLoginParams(authorizationCode) ProviderType.APPLE -> AppleLoginParams(authorizationCode) + // 새로운 제공자 추가 시 여기에 case를 추가하세요 } }또는 sealed class를 사용한 타입 안전성 강화를 고려해보세요.
noweekend-core/core-api/src/main/kotlin/noweekend/core/domain/auth/AuthApplicationServiceImpl.kt (1)
8-8: 트랜잭션 관리가 누락되어 있습니다.인증 서비스에서 여러 테이블에 걸친 작업을 수행하므로 트랜잭션 관리가 필수적입니다.
트랜잭션 애노테이션을 추가해야 합니다:
// TODO: Transactional 넣어야 함 +@Transactional @Servicenoweekend-clients/client-oauth/src/main/kotlin/noweekend/client/google/GoogleClient.kt (1)
40-43: redirect URI 상수에 대한 설명 추가 권장
postmessage는 Google OAuth의 특수한 redirect URI입니다. 코드 가독성을 위해 주석을 추가하는 것이 좋겠습니다.companion object { private const val GOOGLE_AUTHORIZATION_TYPE = "authorization_code" + // Google OAuth의 웹 애플리케이션용 특수 redirect URI private const val DEFAULT_REDIRECT_URI = "postmessage" }noweekend-core/core-domain/src/main/kotlin/noweekend/core/domain/auth/AuthService.kt (2)
42-42: 에러 메시지 외부화 권장하드코딩된 한국어 에러 메시지는 국제화를 어렵게 만듭니다. MessageSource를 사용하여 외부화하는 것을 고려해보세요.
-require(!name.isNullOrBlank()) { "신규 회원가입 시 이름(name) 정보가 필요합니다." } +require(!name.isNullOrBlank()) { messageSource.getMessage("auth.registration.name.required", null, locale) }
27-28: findUserProviderAndProviderId 메서드명 개선 제안메서드명이 다소 길고 "Provider"가 중복됩니다. 더 간결한 이름을 고려해보세요.
-val user = userReader.findUserProviderAndProviderId(providerType, providerId) +val user = userReader.findByProviderTypeAndId(providerType, providerId)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (37)
noweekend-clients/client-oauth/build.gradle.kts(1 hunks)noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/apple/AppleApi.kt(1 hunks)noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/apple/AppleClient.kt(1 hunks)noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/apple/AppleLoginParams.kt(1 hunks)noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/apple/AppleOAuthInfo.kt(1 hunks)noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/apple/ApplePublicKeyResponse.kt(1 hunks)noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/apple/AppleTokens.kt(1 hunks)noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/common/OAuthClient.kt(1 hunks)noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/common/OAuthInfo.kt(1 hunks)noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/common/OAuthLoginParams.kt(1 hunks)noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/common/Revocable.kt(1 hunks)noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/config/ComponentScanConfig.kt(1 hunks)noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/config/OAuthConfig.kt(1 hunks)noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/config/OAuthFeignConfig.kt(1 hunks)noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/google/GoogleClient.kt(1 hunks)noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/google/GoogleLoginParams.kt(1 hunks)noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/google/GoogleOAuthInfo.kt(1 hunks)noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/google/GoogleResourceApi.kt(1 hunks)noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/google/GoogleTokenApi.kt(1 hunks)noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/google/GoogleTokens.kt(1 hunks)noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/google/GoogleUserInfoRequest.kt(0 hunks)noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/google/GoogleUserInfoResponse.kt(0 hunks)noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/properties/AppleAuthProperties.kt(1 hunks)noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/properties/GoogleAuthProperties.kt(1 hunks)noweekend-clients/client-oauth/src/main/resources/client-oauth.yml(1 hunks)noweekend-core/core-api/src/main/kotlin/noweekend/core/api/controller/v1/AuthController.kt(3 hunks)noweekend-core/core-api/src/main/kotlin/noweekend/core/api/controller/v1/request/LoginRequest.kt(1 hunks)noweekend-core/core-api/src/main/kotlin/noweekend/core/api/controller/v1/response/OAuthLoginResponse.kt(1 hunks)noweekend-core/core-api/src/main/kotlin/noweekend/core/domain/auth/AuthApplicationService.kt(1 hunks)noweekend-core/core-api/src/main/kotlin/noweekend/core/domain/auth/AuthApplicationServiceImpl.kt(1 hunks)noweekend-core/core-domain/src/main/kotlin/noweekend/core/domain/auth/AuthService.kt(1 hunks)noweekend-core/core-domain/src/main/kotlin/noweekend/core/domain/user/User.kt(1 hunks)noweekend-core/core-domain/src/main/kotlin/noweekend/core/domain/user/UserReader.kt(1 hunks)noweekend-core/core-domain/src/main/kotlin/noweekend/core/domain/user/UserRepository.kt(1 hunks)noweekend-storage/db-core/src/main/kotlin/noweekend/storage/db/core/user/UserCoreRepository.kt(1 hunks)noweekend-storage/db-core/src/main/kotlin/noweekend/storage/db/core/user/UserEntity.kt(4 hunks)noweekend-storage/db-core/src/main/kotlin/noweekend/storage/db/core/user/UserQueryDslRepository.kt(1 hunks)
💤 Files with no reviewable changes (2)
- noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/google/GoogleUserInfoResponse.kt
- noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/google/GoogleUserInfoRequest.kt
🔇 Additional comments (26)
noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/config/ComponentScanConfig.kt (1)
8-11: 컴포넌트 스캔 대상 확장 확인
새로 추가된noweekend.client.apple및noweekend.client.common패키지가 올바르게 스캔되어 OAuthClient, Revocable 등 신규 빈이 등록됩니다. 기존 Google 클라이언트와 충돌 없이 잘 작동하는지 통합 테스트로 검증해 주세요.noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/config/OAuthFeignConfig.kt (1)
6-10: Feign 클라이언트 스캔에 Apple 패키지 추가
"noweekend.client.apple"가@EnableFeignClients에 포함되어 Apple API 인터페이스가 Feign 클라이언트로 등록됩니다. Google/Apple 모두 정상 등록되는지 확인하세요.noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/common/OAuthLoginParams.kt (1)
5-8: OAuth 로그인 파라미터 인터페이스가 잘 설계되었습니다.인터페이스가 간결하고 명확하며, 다양한 OAuth 제공자를 지원하기 위한 적절한 추상화를 제공합니다.
noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/config/OAuthConfig.kt (1)
8-13: OAuth 설정 클래스가 올바르게 구현되었습니다.Spring Boot의 설정 속성 바인딩을 위한 적절한 구성이며, 애플과 구글 OAuth 속성을 모두 활성화합니다.
noweekend-storage/db-core/src/main/kotlin/noweekend/storage/db/core/user/UserQueryDslRepository.kt (1)
12-18:❓ Verification inconclusive
메서드 시그니처 변경으로 인한 호환성 문제를 확인하세요.
findUserByProviderAndEmail에서findUserByProviderAndProviderId로의 변경은 중요한 변경사항입니다. 다음을 확인해야 합니다:
- 모든 호출 코드가 새로운 메서드 시그니처에 맞게 업데이트되었는지
- 데이터베이스 스키마가
providerId필드를 지원하는지- 기존 데이터의 마이그레이션 전략
- 사용자 고유성 제약조건에 미치는 영향
다음 스크립트를 실행하여 이전 메서드명의 사용을 확인하세요:
🏁 Script executed:
#!/bin/bash # 이전 메서드명의 사용을 검색합니다 rg -A 3 "findUserByProviderAndEmail" # providerId 관련 데이터베이스 변경사항을 확인합니다 rg -A 5 "providerId"Length of output: 11350
메서드 시그니처 변경: 이메일→providerId 전환 후속 검토
코드 상 모든 호출부(
findUserByProviderAndEmail)가findUserByProviderAndProviderId로 정상 교체되었으며,UserEntity에도providerId필드가 정의되어 있습니다. 아래 사항을 추가 확인해주세요:• DB 스키마에
provider_id컬럼이 실제로 존재하는지 (마이그레이션 파일 점검)
• 기존 사용자 데이터를providerId기준으로 마이그레이션하는 전략 수립 여부
•provider_id컬럼에 고유성 제약(unique constraint)이 적용되어 있는지마이그레이션 파일(예:
src/main/resources/db/migration/)에서 관련 변경 내역을 검토하고, 필요 시 스키마 및 제약조건을 업데이트해 주세요.noweekend-core/core-api/src/main/kotlin/noweekend/core/domain/auth/AuthApplicationService.kt (1)
6-8: 인터페이스 설계가 깔끔합니다.애플리케이션 서비스 레이어의 좋은 추상화를 제공하고 있습니다.
name파라미터가 nullable로 설계된 것은 OAuth 제공자가 이름 정보를 제공하지 않는 경우를 고려한 것으로 보입니다.noweekend-clients/client-oauth/src/main/resources/client-oauth.yml (2)
1-12: 멀티 프로바이더 OAuth 설정 구조가 잘 설계되었습니다.Google과 Apple OAuth 설정이 체계적으로 구성되어 있어 확장성이 좋습니다.
3-4: 민감 정보 암호화 및 보안 강화 필요.현재 클라이언트 시크릿과 키 정보가 평문으로 되어 있습니다. PR 목표에서 언급된 대로 Jasypt를 사용한 암호화가 필요합니다.
또한 Apple의
secret-key-file-path가 유효한 파일 경로인지 검증하는 로직을 추가하는 것을 권장합니다.Also applies to: 8-12
noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/google/GoogleLoginParams.kt (1)
6-9: 깔끔한 구현입니다.
OAuthLoginParams인터페이스를 올바르게 구현하고 있으며, immutable한 설계가 좋습니다.noweekend-core/core-domain/src/main/kotlin/noweekend/core/domain/user/UserReader.kt (2)
15-17: 메서드 시그니처 변경이 적절합니다.이메일 기반에서 provider ID 기반으로의 전환이 OAuth 멀티 프로바이더 지원에 적합합니다. 이메일을 제공하지 않는 소셜 로그인 케이스를 잘 고려한 설계입니다.
15-16:✅ Verification successful
호출부 업데이트 확인 필요.
메서드 시그니처가 변경되었으므로 이 메서드를 호출하는 모든 코드가 올바르게 업데이트되었는지 확인이 필요합니다.
다음 스크립트로 호출부를 확인해보세요:
🏁 Script executed:
#!/bin/bash # findUserProviderAndEmail 메서드 호출부를 찾아 업데이트 여부 확인 rg -A 3 "findUserProviderAndEmail" # 새로운 메서드 findUserProviderAndProviderId 사용 확인 rg -A 3 "findUserProviderAndProviderId"Length of output: 1180
호출부 업데이트 완료
findUserProviderAndEmail호출부가 코드에 더 이상 존재하지 않으며, AuthService를 포함한 모든 호출부가findUserProviderAndProviderId로 올바르게 변경되었습니다.noweekend-core/core-domain/src/main/kotlin/noweekend/core/domain/user/UserRepository.kt (2)
5-5: 리포지토리 인터페이스 변경이 일관성 있게 적용되었습니다.provider ID 기반 사용자 검색으로의 전환이 OAuth 멀티 프로바이더 아키텍처와 잘 맞습니다.
5-5:❓ Verification inconclusive
구현체 업데이트 및 데이터베이스 스키마 확인 필요.
인터페이스 변경에 따라 다음 사항들을 확인해주세요:
- 모든 리포지토리 구현체가 새로운 시그니처에 맞게 업데이트되었는지
- 데이터베이스 스키마에
provider_id컬럼이 추가되고 적절한 인덱스가 설정되었는지- 기존 이메일 기반 데이터의 마이그레이션 전략이 수립되었는지
다음 스크립트로 구현체 업데이트 상황을 확인해보세요:
🏁 Script executed:
#!/bin/bash # UserRepository 구현체들을 찾아 메서드 시그니처 확인 ast-grep --pattern 'class $_ : UserRepository { $$$ fun findUserByProvider$_($$$) { $$$ } $$$ }' # 데이터베이스 엔티티에서 providerId 필드 확인 rg -A 5 "providerId.*String"Length of output: 6867
구현체 시그니처 업데이트 확인 완료, DB 스키마·데이터 마이그레이션 추가 검증 필요
모든 저장소 구현체와 도메인 레이어에서 새 인터페이스
findUserByProviderAndProviderId(providerType: ProviderType, providerId: String): User
메서드 호출이 정상 적용된 것을 확인했습니다.
다만, 다음 항목은 수작업 검증이 필요합니다:
- 데이터베이스 마이그레이션 스크립트에
provider_id컬럼 추가 및 적절한 인덱스 설정 여부
(예:src/main/resources/db/migration또는 Liquibase/ Flyway 파일 확인)- 기존 이메일 기반 사용자 데이터의
provider_id마이그레이션 전략 수립 여부noweekend-core/core-api/src/main/kotlin/noweekend/core/api/controller/v1/response/OAuthLoginResponse.kt (1)
5-11: 변경된 DTO 로직 적절
@Schema어노테이션을 사용해 API 문서화를 잘 적용했습니다. 특별한 이슈가 발견되지 않습니다.noweekend-storage/db-core/src/main/kotlin/noweekend/storage/db/core/user/UserCoreRepository.kt (1)
20-27:✅ Verification successful
아키텍처 변경 승인 및 의존성 검증 요청
이메일 기반에서 Provider ID 기반 사용자 조회로의 변경이 OAuth 다중 제공자 지원 목표와 일치합니다. 예외 메시지도 디버깅에 유용한 정보를 포함하고 있어 좋습니다.
다음 스크립트로 이 메서드를 호출하는 모든 코드가 업데이트되었는지 확인해주세요:
🏁 Script executed:
#!/bin/bash # Description: 이전 메서드명 사용 여부 및 새 메서드 구현 확인 # Expected: 이전 메서드 호출이 없고, queryDslRepository에 새 메서드가 구현되어 있어야 함 echo "=== 이전 메서드명 사용 여부 확인 ===" rg "findUserByProviderAndEmail" --type kotlin echo "=== queryDslRepository에 새 메서드 구현 확인 ===" rg "findUserByProviderAndProviderId" --type kotlin -A 3Length of output: 3022
OAuth 공급자 ID 기반 조회 변경 검증 완료
findUserByProviderAndEmail호출이 코드베이스에 더 이상 존재하지 않습니다.findUserByProviderAndProviderId메서드는 아래 파일들에 정상적으로 반영되어 있습니다.
- UserCoreRepository.kt
- UserQueryDslRepository.kt
- core-domain의 UserReader.kt, UserRepository.kt
- 예외 메시지도 일관되게 “User not found: $providerType, $providerId” 형식으로 잘 적용되었습니다.
위 검증 결과, 모든 호출부와 구현체가 올바르게 업데이트되었으므로 추가 수정은 필요하지 않습니다.
noweekend-storage/db-core/src/main/kotlin/noweekend/storage/db/core/user/UserEntity.kt (2)
53-76: 변환 메서드 구현 승인도메인 객체와 엔티티 간의 변환 메서드가 새로운 필드들을 모두 포함하여 올바르게 구현되었습니다. 양방향 매핑이 일관성 있게 처리되어 있습니다.
29-30:❓ Verification inconclusive
nullable email의 unique 제약조건 주의사항
다음 스크립트로 현재 사용 중인 데이터베이스의 NULL unique 제약조건 동작을 확인해주세요:
🏁 Script executed:
#!/bin/bash # Description: 프로젝트에서 사용하는 데이터베이스 확인 # Expected: 데이터베이스별 NULL unique 제약조건 동작 이해 echo "=== 데이터베이스 설정 확인 ===" fd "application" --extension yml --extension yaml --extension properties | xargs grep -l "datasource\|database" | head -3 | xargs catLength of output: 184
사용 DB 엔진 확인 및 nullable email의 unique 제약 검증 필요
UserEntity.kt에서email: String?필드에@Column(nullable = true, unique = true)를 적용하셨는데,
DB마다 NULL 값에 대한 unique 제약 처리 방식이 다릅니다:
- 대부분의 DB(PostgreSQL, MySQL 등)는 NULL끼리 중복을 허용하여 여러 개의 NULL 레코드를 생성
- 일부 DB(SQL Server 등)는 NULL 처리 방식이 다를 수 있음
의도대로 “NULL일 경우에도 중복을 허용하지 않길 원하시는지”를 확실히 확인하시고, 필요 시 다음을 고려하세요:
- 실제 사용 중인 DB 엔진 (application.yml 또는 application.properties 내
spring.datasource.url등) 확인- NULL을 제외한 유니크 인덱스(Partial Index) 생성
- 애플리케이션 레벨에서 NULL 중복 방지 로직 추가
검토 대상 위치:
- src/main/kotlin/noweekend/storage/db/core/user/UserEntity.kt –
@Column(nullable=true, unique=true)설정noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/apple/AppleOAuthInfo.kt (1)
6-15: 깔끔한 구현입니다.OAuthInfo 인터페이스를 올바르게 구현하고 있으며, private 필드를 사용한 캡슐화가 잘 되어 있습니다.
noweekend-core/core-domain/src/main/kotlin/noweekend/core/domain/user/User.kt (3)
8-8: 이메일 필드를 nullable로 변경한 것이 적절합니다.OAuth 제공자가 이메일을 제공하지 않을 수 있으므로 이 변경사항은 올바른 접근법입니다.
11-13: providerId와 revocableToken 필드 추가가 OAuth 구현에 적합합니다.제공자 기반 사용자 식별과 토큰 해지 기능을 지원하는 좋은 설계입니다.
19-38: 팩토리 메서드 업데이트가 일관성 있게 이루어졌습니다.새로운 필드들이 newUser 메서드에 올바르게 반영되어 있습니다.
noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/apple/AppleApi.kt (2)
15-19: Public keys 엔드포인트 구현이 올바릅니다.Apple JWT 토큰 검증을 위한 공개 키 조회 API가 적절히 구현되었습니다.
32-41: 토큰 해지 엔드포인트가 올바르게 구현되었습니다.POST 메서드와 form 데이터 사용이 OAuth 표준에 부합합니다.
noweekend-core/core-domain/src/main/kotlin/noweekend/core/domain/auth/AuthService.kt (1)
12-20: 트랜잭션 설정이 적절합니다클래스 레벨에서 읽기 전용 트랜잭션을 기본으로 설정하고, 쓰기가 필요한 메서드에만 @transactional을 적용한 것은 좋은 패턴입니다.
noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/google/GoogleResourceApi.kt (1)
9-19: Feign 클라이언트 리팩토링이 잘 되었습니다토큰과 리소스 API를 분리하고, 적절한 명명 규칙을 따른 것이 좋습니다. HttpHeaders 상수 사용도 적절합니다.
noweekend-core/core-api/src/main/kotlin/noweekend/core/api/controller/v1/AuthController.kt (1)
65-65:❌ Incorrect review comment
provider 타입 검증 필요
지원하지 않는 provider type이 들어올 경우에 대한 처리가 필요합니다.
🏁 Script executed:
#!/bin/bash # ProviderType enum의 정의를 확인하여 어떤 provider들이 지원되는지 확인 ast-grep --pattern 'enum class ProviderType { $$$ }'Length of output: 459
providerType 검증 로직 확인 필요:
toOAuthLoginParams메서드에서 지원하지 않는 providerType에 대한 예외 처리 여부를 살펴봅시다.
🏁 Script executed:
#!/bin/bash # toOAuthLoginParams 정의 위치와 내부 로직 확인 rg -n "toOAuthLoginParams"Length of output: 572
providerType 바인딩 확인을 위해 컨트롤러 메서드 시그니처를 살펴봅시다.
🏁 Script executed:
#!/bin/bash rg -n "@PostMapping" -A5 -B5 noweekend-core/core-api/src/main/kotlin/noweekend/core/api/controller/v1/AuthController.ktLength of output: 521
toOAuthLoginParams 내부 로직을 확인해 봅시다.
🏁 Script executed:
#!/bin/bash rg -n "fun LoginRequest.toOAuthLoginParams" -A20 -B5 noweekend-core/core-api/src/main/kotlin/noweekend/core/api/controller/v1/request/LoginRequest.ktLength of output: 560
검증 불필요 — enum 바인딩 및 exhaustive when으로 이미 처리됨
AuthController의
@PathVariable("providerType") providerType: ProviderType는 Spring이 자동으로 문자열을ProviderTypeenum으로 변환하며, 지원되지 않는 값이 들어오면 400 Bad Request로 응답합니다.
또한LoginRequest.toOAuthLoginParams의 when 문은 현재 enum에 정의된APPLE두 케이스를 모두 다루고 있어 별도의 “지원하지 않는 providerType” 검증 로직이 필요 없습니다.Likely an incorrect or invalid review comment.
| @ConfigurationProperties(prefix = "oauth.google") | ||
| data class GoogleAuthProperties( | ||
| val clientId: String, | ||
| val clientSecret: String, | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
설정 속성 클래스에 검증과 보안 고려사항을 추가하세요.
현재 구현은 올바르지만 다음 개선사항을 고려해보세요:
@NotBlank또는@NotEmpty검증 어노테이션 추가- 민감한 자격 증명 정보의 암호화 처리 확인
- 설정 값이 누락된 경우의 오류 처리
+import jakarta.validation.constraints.NotBlank
@ConfigurationProperties(prefix = "oauth.google")
data class GoogleAuthProperties(
+ @NotBlank
val clientId: String,
+ @NotBlank
val clientSecret: String,
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @ConfigurationProperties(prefix = "oauth.google") | |
| data class GoogleAuthProperties( | |
| val clientId: String, | |
| val clientSecret: String, | |
| ) | |
| import jakarta.validation.constraints.NotBlank | |
| @ConfigurationProperties(prefix = "oauth.google") | |
| data class GoogleAuthProperties( | |
| @NotBlank | |
| val clientId: String, | |
| @NotBlank | |
| val clientSecret: String, | |
| ) |
🤖 Prompt for AI Agents
In
noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/properties/GoogleAuthProperties.kt
around lines 5 to 9, add validation annotations such as @NotBlank or @NotEmpty
to the clientId and clientSecret properties to ensure they are not empty. Also,
verify that sensitive credentials like clientSecret are handled securely,
possibly by encrypting or masking them. Finally, implement error handling for
cases where these configuration values are missing to prevent runtime issues.
| @ConfigurationProperties(prefix = "oauth.apple") | ||
| data class AppleAuthProperties( | ||
| val appId: String, | ||
| val keyId: String, | ||
| val teamId: String, | ||
| val secretKeyFilePath: String, | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
설정 파일 경로 검증 및 보안 고려사항 추가를 제안합니다.
secretKeyFilePath 속성에 대해 파일 존재 여부를 검증하는 어노테이션을 추가하고, 민감한 정보를 포함하므로 로깅 시 노출되지 않도록 주의가 필요합니다.
다음과 같이 개선할 수 있습니다:
+import jakarta.validation.constraints.NotBlank
+import org.springframework.validation.annotation.Validated
+@Validated
@ConfigurationProperties(prefix = "oauth.apple")
data class AppleAuthProperties(
+ @field:NotBlank
val appId: String,
+ @field:NotBlank
val keyId: String,
+ @field:NotBlank
val teamId: String,
+ @field:NotBlank
val secretKeyFilePath: String,
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @ConfigurationProperties(prefix = "oauth.apple") | |
| data class AppleAuthProperties( | |
| val appId: String, | |
| val keyId: String, | |
| val teamId: String, | |
| val secretKeyFilePath: String, | |
| ) | |
| import jakarta.validation.constraints.NotBlank | |
| import org.springframework.validation.annotation.Validated | |
| @Validated | |
| @ConfigurationProperties(prefix = "oauth.apple") | |
| data class AppleAuthProperties( | |
| @field:NotBlank | |
| val appId: String, | |
| @field:NotBlank | |
| val keyId: String, | |
| @field:NotBlank | |
| val teamId: String, | |
| @field:NotBlank | |
| val secretKeyFilePath: String, | |
| ) |
🤖 Prompt for AI Agents
In
noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/properties/AppleAuthProperties.kt
around lines 5 to 11, add validation to ensure the file at secretKeyFilePath
exists by using an appropriate annotation or custom validator. Also, update any
logging or toString methods to exclude or mask secretKeyFilePath to prevent
sensitive information exposure.
| @Column(name = "provider_id", length = 100, nullable = false) | ||
| val providerId: String, | ||
|
|
||
| @Column(name = "revocable_token", length = 100, nullable = true) | ||
| val revocableToken: String?, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
필드 길이 검증 필요
providerId와 revocableToken의 길이 제한(100자)이 실제 OAuth 제공자들의 값 범위에 적합한지 확인이 필요합니다.
🌐 Web query:
Apple OAuth provider ID와 refresh token의 일반적인 길이는 얼마인가요?
💡 Result:
Apple OAuth 시스템에서 provider ID와 refresh token의 길이는 서비스 구현과 보안 요구 사항에 따라 달라집니다.
Provider ID 길이
Apple OAuth에서 provider ID는 일반적으로 다음과 같은 특성을 가집니다:
- Apple의 OAuth 프로세스에서는 client-id로 앱의 Bundle ID를 사용합니다[3]
- Bundle ID는 앱의 information property list 리소스 파일(info.plist)의 CFBundleIdentifier 키 값입니다[4]
- 일반적으로 Bundle ID는 역방향 도메인 이름 형식(예: com.company.appname)으로 구성되어 있습니다
- 정확한 길이 제한은 명시적으로 언급되어 있지 않지만, 실용적인 길이 범위 내에서 사용됩니다
Refresh Token 길이
Apple의 refresh token에 관한 특성은 다음과 같습니다:
- Apple refresh token은 명확한 길이 제한이 공식 문서에 명시되어 있지 않습니다
- 비교적 긴 유효기간을 가지며, Access Token이 만료됐을 때 새로 발급해주는 역할을 합니다[6]
- 카카오와 같은 다른 OAuth 제공자의 경우, refresh token의 최대 길이는 약 60자이지만 변경될 수 있으며, 안전하게 100자 이상으로 설정하는 것이 권장됩니다[8]
Apple의 refresh token에는 특별한 특성이 있습니다:
- 시간 기반 만료가 없지만, Apple ID 계정의 특정 이벤트(예: 비밀번호 변경)에 의해 무효화될 수 있습니다[10]
- 한 번 받으면 재발급이 불가능하며, 새로운 refresh token은 재로그인 없이는 얻을 수 없습니다[10]
- 애플의 access_token 만료 기간은 10분인 반면, refresh_token은 만료 기간이 없습니다[3]
개발 시에는 refresh token을 저장할 때 충분한 공간을 확보하는 것이 중요하며, 보안상 민감한 정보이므로 적절히 암호화하여 저장해야 합니다.
Citations:
- 1: https://developer.apple.com/documentation/devicemanagement/implementing-the-oauth2-authentication-user-enrollment-flow
- 2: https://firebase.google.com/docs/auth/ios/apple
- 3: https://velog.io/@numerical43/java-spring-apple-oauth-login
- 4: https://developers.google.com/identity/protocols/oauth2/native-app
- 5: https://firebase.google.com/docs/auth/web/apple
- 6: https://weekoding.tistory.com/27
- 7: https://mingg123.tistory.com/262
- 8: https://devtalk.kakao.com/t/android-ios-login-api-access-token/112681
- 9: https://stytch.com/docs/guides/oauth/idp-overview
- 10: https://developer.apple.com/forums/thread/651237
🌐 Web query:
Google OAuth sub claim과 refresh token의 일반적인 길이는 얼마인가요?
💡 Result:
Google OAuth에서의 sub claim과 refresh token 길이에 대해 설명드리겠습니다.
-
sub claim:
- sub는 "subject"의 약자로, Google OAuth ID 토큰의 페이로드에 포함되는 고유 사용자 식별자입니다.
- 길이는 고정되어 있지 않으나, 일반적으로 21자 내외의 문자열이며, 주로 영숫자와 일부 특수문자가 섞여 있습니다. 예: "10769150350006150715113082367"
-
refresh token:
- refresh token은 액세스 토큰이 만료되었을 때 새로운 액세스 토큰을 발급받기 위해 사용됩니다.
- 길이는 토큰마다 다르지만 일반적으로 90자 이상이며, Base64 인코딩된 문자열 형태로 복잡하고 긴 문자열입니다.
정리하면,
- sub claim: 약 20~22자 내외
- refresh token: 보통 90자 이상, 길이에 일정한 제한은 없지만 매우 긴 문자열
추가로 정확한 길이가 궁금하시거나 특수한 상황에서 변동 가능하니, 애플리케이션에서 이를 저장할 때 충분한 길이를 넉넉하게 잡는 것이 좋습니다. 예를 들어 refresh token은 최소 256자 정도 필드를 확보하는 편이 안전합니다.
필드 길이 검증 및 revocableToken 길이 확장 검토
providerId는 일반적으로 번들 ID(예: com.company.appname) 형태로 100자면 충분하므로 현 상태 유지 가능revocableToken(OAuth 리프레시/중단 토큰)은 공급자별로 90자 이상, 경우에 따라 매우 길어질 수 있으므로 최소 256자 이상으로 확장 검토 필요
요약된 변경 제안:
- providerId:
length = 100유지 - revocableToken:
length = 256(또는 여유 있는 크기) 로 수정
🤖 Prompt for AI Agents
In
noweekend-storage/db-core/src/main/kotlin/noweekend/storage/db/core/user/UserEntity.kt
around lines 43 to 47, the revocableToken field's length is currently set to
100, which may be insufficient for some OAuth providers' refresh or revocable
tokens that can exceed 90 characters. Keep the providerId length at 100 as it is
adequate, but increase the revocableToken column length to at least 256 to
safely accommodate longer tokens. Update the @Column annotation for
revocableToken to reflect this new length.
| @GetMapping( | ||
| value = ["/token"], | ||
| consumes = [MediaType.APPLICATION_JSON_VALUE], | ||
| ) | ||
| fun getGoogleToken( | ||
| @RequestParam("code") code: String, | ||
| @RequestParam("redirect_uri") redirectUri: String?, | ||
| @RequestParam("client_id") clientId: String, | ||
| @RequestParam("client_secret") clientSecret: String, | ||
| @RequestParam("grant_type") grantType: String, | ||
| ): GoogleTokens |
There was a problem hiding this comment.
OAuth 토큰 엔드포인트의 HTTP 메서드와 컨텐츠 타입이 올바르지 않습니다.
OAuth 2.0 사양(RFC 6749)에 따르면 토큰 엔드포인트는 POST 메서드와 application/x-www-form-urlencoded 컨텐츠 타입을 사용해야 합니다.
다음과 같이 수정하세요:
- @GetMapping(
+ @PostMapping(
value = ["/token"],
- consumes = [MediaType.APPLICATION_JSON_VALUE],
+ consumes = [MediaType.APPLICATION_FORM_URLENCODED_VALUE],
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @GetMapping( | |
| value = ["/token"], | |
| consumes = [MediaType.APPLICATION_JSON_VALUE], | |
| ) | |
| fun getGoogleToken( | |
| @RequestParam("code") code: String, | |
| @RequestParam("redirect_uri") redirectUri: String?, | |
| @RequestParam("client_id") clientId: String, | |
| @RequestParam("client_secret") clientSecret: String, | |
| @RequestParam("grant_type") grantType: String, | |
| ): GoogleTokens | |
| @PostMapping( | |
| value = ["/token"], | |
| consumes = [MediaType.APPLICATION_FORM_URLENCODED_VALUE], | |
| ) | |
| fun getGoogleToken( | |
| @RequestParam("code") code: String, | |
| @RequestParam("redirect_uri") redirectUri: String?, | |
| @RequestParam("client_id") clientId: String, | |
| @RequestParam("client_secret") clientSecret: String, | |
| @RequestParam("grant_type") grantType: String, | |
| ): GoogleTokens |
🤖 Prompt for AI Agents
In
noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/google/GoogleTokenApi.kt
lines 14 to 24, the OAuth token endpoint incorrectly uses a GET method and JSON
content type. According to OAuth 2.0 spec, change the HTTP method to POST and
update the consumes attribute to "application/x-www-form-urlencoded" to comply
with the standard.
| override fun requestOAuthInfo(params: OAuthLoginParams): OAuthInfo { | ||
| val appleTokens = requestAccessToken(params) | ||
| if (appleTokens?.idToken == null) throw IllegalStateException("cannot find idToken from apple") | ||
| val claims = getClaims(appleTokens.idToken) | ||
| return AppleOAuthInfo(claims.subject, appleTokens.refreshToken) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
refreshToken null 체크 필요
refreshToken이 null일 경우에 대한 처리가 필요합니다. Apple OAuth는 refreshToken을 항상 반환하지 않을 수 있습니다.
override fun requestOAuthInfo(params: OAuthLoginParams): OAuthInfo {
val appleTokens = requestAccessToken(params)
if (appleTokens?.idToken == null) throw IllegalStateException("cannot find idToken from apple")
val claims = getClaims(appleTokens.idToken)
- return AppleOAuthInfo(claims.subject, appleTokens.refreshToken)
+ return AppleOAuthInfo(claims.subject, appleTokens.refreshToken ?: "")
}🤖 Prompt for AI Agents
In
noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/apple/AppleClient.kt
around lines 38 to 43, the code does not check if refreshToken is null before
using it. Since Apple OAuth may not always return a refreshToken, add a null
check for appleTokens.refreshToken and handle the null case appropriately, such
as throwing an exception or providing a fallback, to prevent potential null
pointer issues.
| private fun getClaims(identityToken: String): Claims { | ||
| val header = identityToken.substring(0, identityToken.indexOf(".")) | ||
| val headerMap: Map<String, String> = objectMapper.readValue(String(Base64.getDecoder().decode(header))) | ||
| val key = getPublicKey(headerMap[KEY_KID], headerMap[KEY_ALGORITHM]) | ||
|
|
||
| val nBytes = Base64.getUrlDecoder().decode(key.n) | ||
| val eBytes = Base64.getUrlDecoder().decode(key.e) | ||
|
|
||
| val n = BigInteger(1, nBytes) | ||
| val e = BigInteger(1, eBytes) | ||
|
|
||
| val publicKeySpec = RSAPublicKeySpec(n, e) | ||
| val keyFactory = KeyFactory.getInstance(key.kty) | ||
| val publicKey = keyFactory.generatePublic(publicKeySpec) | ||
|
|
||
| return Jwts.parserBuilder().setSigningKey(publicKey).build().parseClaimsJws(identityToken).body | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
JWT 검증 로직 강화 필요
JWT 파싱 시 만료 시간 검증이 누락되어 있습니다. 또한 더 구체적인 에러 메시지가 필요합니다.
private fun getClaims(identityToken: String): Claims {
val header = identityToken.substring(0, identityToken.indexOf("."))
val headerMap: Map<String, String> = objectMapper.readValue(String(Base64.getDecoder().decode(header)))
val key = getPublicKey(headerMap[KEY_KID], headerMap[KEY_ALGORITHM])
val nBytes = Base64.getUrlDecoder().decode(key.n)
val eBytes = Base64.getUrlDecoder().decode(key.e)
val n = BigInteger(1, nBytes)
val e = BigInteger(1, eBytes)
val publicKeySpec = RSAPublicKeySpec(n, e)
val keyFactory = KeyFactory.getInstance(key.kty)
val publicKey = keyFactory.generatePublic(publicKeySpec)
- return Jwts.parserBuilder().setSigningKey(publicKey).build().parseClaimsJws(identityToken).body
+ return try {
+ Jwts.parserBuilder()
+ .setSigningKey(publicKey)
+ .build()
+ .parseClaimsJws(identityToken)
+ .body
+ } catch (e: Exception) {
+ throw IllegalStateException("Failed to verify Apple ID token: ${e.message}", e)
+ }
}🤖 Prompt for AI Agents
In
noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/apple/AppleClient.kt
between lines 93 and 109, the JWT parsing logic lacks expiration time validation
and detailed error handling. Enhance the getClaims function by adding a check to
verify the token's expiration time after parsing the claims. Also, wrap the
parsing logic in a try-catch block to catch exceptions and provide more specific
error messages for debugging and security purposes.
| private fun getPrivateKey(secretKeyPath: String): PrivateKey { | ||
| val resource = FileSystemResource(secretKeyPath) | ||
| val keyReader = FileReader(resource.uri.path) | ||
| PemReader(keyReader).use { reader -> | ||
| val content = reader.readPemObject().content | ||
| return KeyFactory.getInstance("EC") | ||
| .generatePrivate(PKCS8EncodedKeySpec(content)) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
private key 보안 강화 필요
파일 시스템에서 직접 private key를 읽는 것은 보안상 위험할 수 있습니다. 또한 파일 존재 여부를 먼저 확인해야 합니다.
private fun getPrivateKey(secretKeyPath: String): PrivateKey {
val resource = FileSystemResource(secretKeyPath)
+ if (!resource.exists()) {
+ throw IllegalStateException("Private key file not found: $secretKeyPath")
+ }
val keyReader = FileReader(resource.uri.path)
PemReader(keyReader).use { reader ->
val content = reader.readPemObject().content
return KeyFactory.getInstance("EC")
.generatePrivate(PKCS8EncodedKeySpec(content))
}
}추가로 AWS KMS나 HashiCorp Vault 같은 key management 서비스 사용을 고려해보세요.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private fun getPrivateKey(secretKeyPath: String): PrivateKey { | |
| val resource = FileSystemResource(secretKeyPath) | |
| val keyReader = FileReader(resource.uri.path) | |
| PemReader(keyReader).use { reader -> | |
| val content = reader.readPemObject().content | |
| return KeyFactory.getInstance("EC") | |
| .generatePrivate(PKCS8EncodedKeySpec(content)) | |
| } | |
| } | |
| private fun getPrivateKey(secretKeyPath: String): PrivateKey { | |
| val resource = FileSystemResource(secretKeyPath) | |
| if (!resource.exists()) { | |
| throw IllegalStateException("Private key file not found: $secretKeyPath") | |
| } | |
| val keyReader = FileReader(resource.uri.path) | |
| PemReader(keyReader).use { reader -> | |
| val content = reader.readPemObject().content | |
| return KeyFactory.getInstance("EC") | |
| .generatePrivate(PKCS8EncodedKeySpec(content)) | |
| } | |
| } |
🤖 Prompt for AI Agents
In
noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/apple/AppleClient.kt
around lines 83 to 91, improve private key security by first checking if the
private key file exists before reading it to avoid runtime errors. Replace
direct file system access with a more secure key management approach, such as
integrating AWS KMS or HashiCorp Vault, to handle private keys securely instead
of reading them directly from the file system.
| private fun requestAccessToken(params: OAuthLoginParams): String { | ||
| return googleTokenApi.getGoogleToken( | ||
| code = params.getCode(), | ||
| redirectUri = DEFAULT_REDIRECT_URI, | ||
| clientId = googleAuthProperties.clientId, | ||
| clientSecret = googleAuthProperties.clientSecret, | ||
| grantType = GOOGLE_AUTHORIZATION_TYPE, | ||
| ).accessToken | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
액세스 토큰 획득 시 에러 처리 필요
토큰 API 호출이 실패하거나 빈 토큰을 반환할 경우에 대한 처리가 없습니다.
private fun requestAccessToken(params: OAuthLoginParams): String {
- return googleTokenApi.getGoogleToken(
+ val tokenResponse = googleTokenApi.getGoogleToken(
code = params.getCode(),
redirectUri = DEFAULT_REDIRECT_URI,
clientId = googleAuthProperties.clientId,
clientSecret = googleAuthProperties.clientSecret,
grantType = GOOGLE_AUTHORIZATION_TYPE,
- ).accessToken
+ )
+
+ if (tokenResponse?.accessToken.isNullOrBlank()) {
+ throw IllegalStateException("Failed to obtain access token from Google")
+ }
+
+ return tokenResponse.accessToken
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private fun requestAccessToken(params: OAuthLoginParams): String { | |
| return googleTokenApi.getGoogleToken( | |
| code = params.getCode(), | |
| redirectUri = DEFAULT_REDIRECT_URI, | |
| clientId = googleAuthProperties.clientId, | |
| clientSecret = googleAuthProperties.clientSecret, | |
| grantType = GOOGLE_AUTHORIZATION_TYPE, | |
| ).accessToken | |
| } | |
| private fun requestAccessToken(params: OAuthLoginParams): String { | |
| val tokenResponse = googleTokenApi.getGoogleToken( | |
| code = params.getCode(), | |
| redirectUri = DEFAULT_REDIRECT_URI, | |
| clientId = googleAuthProperties.clientId, | |
| clientSecret = googleAuthProperties.clientSecret, | |
| grantType = GOOGLE_AUTHORIZATION_TYPE, | |
| ) | |
| if (tokenResponse?.accessToken.isNullOrBlank()) { | |
| throw IllegalStateException("Failed to obtain access token from Google") | |
| } | |
| return tokenResponse.accessToken | |
| } |
🤖 Prompt for AI Agents
In
noweekend-clients/client-oauth/src/main/kotlin/noweekend/client/google/GoogleClient.kt
between lines 26 and 34, the requestAccessToken function lacks error handling
for cases when the token API call fails or returns an empty token. Modify the
function to catch exceptions from googleTokenApi.getGoogleToken and check if the
returned accessToken is null or empty. If an error occurs or the token is empty,
throw a meaningful exception or handle the error appropriately to prevent silent
failures.
noweekend-core/core-api/src/main/kotlin/noweekend/core/api/controller/v1/AuthController.kt
Show resolved
Hide resolved
| fun getGoogleToken( | ||
| @RequestParam("code") code: String, | ||
| @RequestParam("redirect_uri") redirectUri: String?, | ||
| @RequestParam("client_id") clientId: String, | ||
| @RequestParam("client_secret") clientSecret: String, | ||
| @RequestParam("grant_type") grantType: String, | ||
| ): GoogleTokens |
There was a problem hiding this comment.
제가 기존에 알던 모바일과 협업하는 로그인 방식하고 좀 다르네요. FrontEnd와 협업할때 서버쪽에서 직접 Oauth 구현하는 것과 유사해보여요. Grant 방식의 로그인은 모바일과 협업할때도 요렇게 구현하나요??
| // TODO: Transactional 넣어야 함 | ||
| @Service | ||
| class AuthApplicationServiceImpl( |
There was a problem hiding this comment.
뭔가.. 파사드같은 느낌? 그리고 @Transactional 을 사용하지 않아도 될거같아요. client 요청과 로그인 요청을 하나의 큰 트랜잭션 범위일 필요는 없을 것 같아요.
| // TODO: Transactional 넣어야 함 | ||
| @Service | ||
| class AuthApplicationServiceImpl( | ||
| private val oauthClients: List<OAuthClient>, |
There was a problem hiding this comment.
이렇게 의존성주입하는건 처음 보네요. 요건 잘 몰라서 그러는데 GoogleOauthClient, AppleOauthClient를 캡슐화한 일급컬렉션으로 주입하는거랑 리스트로 주입하는거랑 뭐가 더 좋을지 고민되네요.
| @Transactional(readOnly = true) | ||
| class AuthService( | ||
| private val userWriter: UserWriter, | ||
| private val userReader: UserReader, | ||
| private val jwtProvider: JwtProvider, | ||
| ) { | ||
| fun googleLogin(email: String, name: String?): AuthResult { | ||
| val user = userReader.findUserProviderAndEmail(ProviderType.GOOGLE, email) | ||
| ?: return registerAndRespond(email, name) | ||
|
|
||
| @Transactional |
There was a problem hiding this comment.
Class단과 Method단에서 모두 @Transactional 을 사용하지 않아도 될거같아요. DB에 존재하는 데이터인지는 Reader가 책임지고 갖고오고, DB에 새로운 값을 insert하는 건 Writer가 담당하니까요. 객체 생성 자체는 도메인에서 담당하니까 해당 코드에서만 보면 AuthService.class , socialLogin() 둘 다 @Transactional은 불필요해보이는데 어떻게 생각하세요? 물론 나중에 여러 Service가 결합된 하나의 확실한 트랜잭션을 보장해야한다면 그때 추가하는게 더 좋아보여요.
mkSpace
force-pushed
the
feature/oauth
branch
2 times, most recently
from
500bcaa to
7b961db
Compare
요약(개요)
소셜 로그인 구현
우선 새벽에 급하게 작업하느라고 기존에 있던 아키텍처에 벗어난 코드들이 있을 수 있는데 우선 임시로 올려둡니다.
작업 내용
집중해서 리뷰해야 하는 부분
구조를 조금 많이 변경했어요. 준형씨가 열심히 개발했는데 기존 코드를 너무 많이 바꾼것 같아서 미안 ㅋㅋ. (이 부분은 리뷰 하고 같이 얘기해보자)
우선 주안점으로 email을 뺀 이유는 소셜 로그인에서 email과 같은 개인정보는 사용자가 임의로 제공하지 않을 수 있기에 더 명확하게 구분할 수 있는 provider id를 사용할 수 있게 했어요.
OAuth 관련해서 기능 구현했는데 구조가 너무 파편화되어서 아키텍처를 깊게 이해하지 않고 우선 기능만 구현했어요. 이 PR 리뷰한 이후에 같이 얘기해보면 좋겠네요.
우선 제가 생각했을 때 Service가 domain layer에서 Repository에 접근하는건 알겠는데 이러면 다른 테이블과의 트랜잭션이 필요한 경우 트랜잭션으로 묶기 어려워 보이며 이러한 이유 등으로 기존에 Controller 단에서 OAuth Client를 호출하는 등의 문제점이 생긴걸로 보여서 presentation 단에 application service라고 새로운 레이어를 추가했어요. 추후에 Domain 간의 트랜잭션 지원을 위해 Transaction AOP를 Application Service 단에서도 사용해야 할 것으로 보입니다.
우선 이 정도만 전달하고 궁금한 점은 충분히 Spring Data JPA 로 구현 가능한 부분들이 Query DSL로 되어있는데 이유가 있을까요?
기타 전달 사항 및 참고 자료(선택)
아직 완성된 PR은 아니에요. 애플 revoke token api까지 만들어야 하며, apple config, google config 받아서 jasypt 암호화 한 이후에 배포할 예정입니다.
Summary by CodeRabbit
신규 기능
기능 개선
버그 수정
설정
DB 변경