feat: apple login #503
feat: apple login #503
Conversation
WalkthroughAdded end-to-end Apple OAuth support: new JJWT/BouncyCastle deps, Apple client-secret JWT generator, Apple ID token decoder, DTOs and OAuth flows in service/controller, User provider field and Apple-specific user creation, session/CORS/auth config updates, and test wiring/resources. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant UC as UserController
participant OLS as OauthLoginService
participant AppleAPI as Apple OAuth API
participant AID as AppleIdTokenDecoder
participant US as UserService
participant DB as Database
Note over Client,UC: Apple OAuth authorization code flow -> token -> id_token decode -> session
Client->>UC: GET /auth/apple/callback?code=CODE
UC->>OLS: getOauthToken(CODE, appleOauthInfo)
OLS->>AppleAPI: POST /auth/token (code, client_id, client_secret, redirect_uri)
AppleAPI-->>OLS: OauthToken { access_token, id_token, ... }
OLS->>AID: processAppleLogin(id_token)
AID-->>OLS: AppleLoginResponse { sub, email, email_verified }
OLS-->>UC: AppleLoginResponse
UC->>UC: store AppleLoginResponse in session (appleUser)
UC-->>Client: 302 redirect to frontend with success/failure
Note over Client,UC: Later user action -> login/join uses session-stored AppleLoginResponse
Client->>UC: POST /users/login (session contains appleUser)
UC->>US: loginApple(sub)
US->>DB: find user by apple-sub-hash / create if joining
DB-->>US: SessionUser
US-->>UC: SessionUser
UC->>UC: clear session attrs
UC-->>Client: Login success response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/kotlin/upbrella/be/user/service/OauthLoginService.kt (1)
27-52: Misleading parameter name:redirectUriis being used as the token endpoint URL.The parameter
redirectUrion line 49 is used as the POST target for token exchange, butredirectUritypically refers to where the OAuth provider redirects after authorization. This is semantically confusing and likely incorrect.Consider renaming to
tokenUrior passing a separate token endpoint:-private fun getOauthTokenInternal(code: String, clientId: String, clientSecret: String, redirectUri: String): OauthToken? { +private fun getOauthTokenInternal(code: String, clientId: String, clientSecret: String, tokenUri: String): OauthToken? { // ... - val response = restTemplate.postForEntity(redirectUri, request, OauthToken::class.java) + val response = restTemplate.postForEntity(tokenUri, request, OauthToken::class.java)Also, Apple's token endpoint (
https://appleid.apple.com/auth/token) requiresredirect_uriin the request body, which is currently missing fromrequestPayloads.
🧹 Nitpick comments (8)
src/main/kotlin/upbrella/be/user/entity/User.kt (1)
29-59: Consider extracting common user creation logic.The
createNewUserandcreateNewAppleUsermethods share nearly identical logic with only the social ID extraction and provider value differing. Consider refactoring to reduce duplication.Example refactor:
companion object { + private fun createUser( + socialId: Long, + email: String, + provider: String, + joinRequest: JoinRequest, + aesEncryptor: AesEncryptor + ): User { + return User( + socialId = socialId, + name = joinRequest.name, + phoneNumber = joinRequest.phoneNumber, + email = email, + provider = provider, + bank = aesEncryptor.encrypt(joinRequest.bank), + accountNumber = aesEncryptor.encrypt(joinRequest.accountNumber) + ) + } + fun createNewUser( kakaoUser: KakaoLoginResponse, joinRequest: JoinRequest, aesEncryptor: AesEncryptor ): User { - return User( - socialId = kakaoUser.id.hashCode().toLong(), - name = joinRequest.name, - phoneNumber = joinRequest.phoneNumber, - email = kakaoUser.kakaoAccount?.email ?: "", - provider = "KAKAO", - bank = aesEncryptor.encrypt(joinRequest.bank), - accountNumber = aesEncryptor.encrypt(joinRequest.accountNumber) - ) + return createUser( + socialId = kakaoUser.id.hashCode().toLong(), + email = kakaoUser.kakaoAccount?.email ?: "", + provider = "KAKAO", + joinRequest = joinRequest, + aesEncryptor = aesEncryptor + ) } fun createNewAppleUser( appleUser: AppleLoginResponse, joinRequest: JoinRequest, aesEncryptor: AesEncryptor ): User { - return User( - socialId = appleUser.sub.hashCode().toLong(), - name = joinRequest.name, - phoneNumber = joinRequest.phoneNumber, - email = appleUser.email ?: "", - provider = "APPLE", - bank = aesEncryptor.encrypt(joinRequest.bank), - accountNumber = aesEncryptor.encrypt(joinRequest.accountNumber) - ) + return createUser( + socialId = appleUser.sub.hashCode().toLong(), + email = appleUser.email ?: "", + provider = "APPLE", + joinRequest = joinRequest, + aesEncryptor = aesEncryptor + ) } }src/main/kotlin/upbrella/be/user/service/UserService.kt (1)
64-80: Consider extracting common validation logic to reduce duplication.The
joinApplemethod duplicates the validation logic (existing user check, blacklist check) fromjoin. While functional, this could be consolidated into a private helper method to improve maintainability.private fun validateNewUser(socialIdHash: Long) { if (userReader.existsBySocialId(socialIdHash)) { throw ExistingMemberException("[ERROR] 이미 가입된 회원입니다. 로그인 폼으로 이동합니다.") } if (blackListReader.existsBySocialId(socialIdHash)) { throw BlackListUserException("[ERROR] 정지된 회원입니다. 정지된 회원은 재가입이 불가능합니다.") } }src/main/kotlin/upbrella/be/user/dto/token/AppleOauthInfo.kt (1)
8-9: Consider using a regular class or marking asfinalif CGLIB proxying isn't needed.The
openmodifier is unusual in Kotlin Spring components. If this is required for Spring's CGLIB proxying (e.g., for@Transactional), consider adding a comment explaining why. Otherwise, it could be removed.src/test/kotlin/upbrella/be/user/service/UserServiceDynamicTest.kt (1)
38-48: LGTM - Test fixture updated correctly. Consider adding Apple login/join tests.The User construction correctly uses the new entity signature with the
providerfield. The existing Kakao tests continue to work.Consider adding similar dynamic tests for
loginAppleandjoinApplemethods to ensure Apple OAuth flows are covered.src/main/kotlin/upbrella/be/user/controller/UserController.kt (2)
123-135: Unreachable else branch - consider simplifying.The
elsebranch on line 134 is unreachable because the conditionkakaoUser == null && appleUser == nullis already checked on line 119. Thewhenexpression can be simplified:val loggedInUser = when { kakaoUser != null -> { val user = userService.login(kakaoUser.id!!) session.removeAttribute("kakaoUser") user } - appleUser != null -> { + else -> { + // appleUser is guaranteed non-null here val user = userService.loginApple(appleUser.sub!!) session.removeAttribute("appleUser") user } - else -> throw NotSocialLoginedException("[ERROR] 소셜 로그인을 먼저 해주세요.") }
176-188: Same unreachable else branch pattern in join flow.The
elsebranch on line 187 is also unreachable for the same reason. Consider applying the same simplification as suggested for the login flow.src/main/kotlin/upbrella/be/util/AppleJwtGenerator.kt (2)
26-47: Check JJWTsignWithsignature vs library version; consider minor hardening of token constructionFunctionally this looks correct for Apple client secret generation (claims/headers/audience/ES256 are aligned), but there are a couple of details worth verifying/tuning:
- The call
.signWith(privateKey, SignatureAlgorithm.ES256)matches the newer JJWT API (0.10.x/0.11.x). If your project is still on an older 0.9.x JJWT, the expected signature issignWith(SignatureAlgorithm, Key)and this line will not compile. Please confirm the version and adjust if needed:- .signWith(privateKey, SignatureAlgorithm.ES256) + // For JJWT 0.10+ (new API) + .signWith(privateKey, SignatureAlgorithm.ES256) + // For JJWT 0.9.x (old API), you would instead use: + // .signWith(SignatureAlgorithm.ES256, privateKey)
Minor: the magic number
15777000000Lis correct for “~6 months” but opaque. Extracting it to a named constant (e.g.PRIVATE_KEY_MAX_AGE_MS) or using aDurationcalculation (Duration.ofDays(180).toMillis()) would make intent clearer, especially for future maintainers.Operationally, if
generateClientSecretis called often, consider caching the generated secret untilexpirationTimeinstead of regenerating each time, to avoid repeated disk I/O and parsing. (From the broader PR context, it looks like you may already only call this at startup, in which case you’re fine.)
52-61: MakeloadPrivateKeymore robust and ensure resources are closed viauseCurrent implementation assumes
PEMParser.readObject()always returnsPrivateKeyInfoand manually closes the parser. Two small improvements:
- Guard against unexpected PEM contents to avoid an unhelpful
ClassCastException.- Wrap
PEMParserinuseso it’s always closed, even if parsing fails.For example:
- private fun loadPrivateKey(p8KeyPath: String): PrivateKey { - val resource = ClassPathResource(p8KeyPath) - val p8Content = resource.inputStream.bufferedReader().use { it.readText() } - - val pemParser = PEMParser(StringReader(p8Content)) - val privateKeyInfo = pemParser.readObject() as PrivateKeyInfo - pemParser.close() - - return JcaPEMKeyConverter().getPrivateKey(privateKeyInfo) - } + private fun loadPrivateKey(p8KeyPath: String): PrivateKey { + val resource = ClassPathResource(p8KeyPath) + val p8Content = resource.inputStream.bufferedReader().use { it.readText() } + + return PEMParser(StringReader(p8Content)).use { pemParser -> + val obj = pemParser.readObject() + val privateKeyInfo = obj as? PrivateKeyInfo + ?: throw IllegalArgumentException( + "Unsupported key format in '$p8KeyPath': ${obj?.javaClass?.name ?: "null"}" + ) + + JcaPEMKeyConverter().getPrivateKey(privateKeyInfo) + } + }Also, note that the KDoc and
ClassPathResourceencourage placing the.p8key on the application classpath. For production, it’s generally safer to load this from an external secret store or mounted file path (env‑driven configuration) rather than bundling the private key into the application artifact.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
build.gradle(1 hunks)src/main/kotlin/upbrella/be/user/controller/UserController.kt(5 hunks)src/main/kotlin/upbrella/be/user/dto/response/AppleLoginResponse.kt(1 hunks)src/main/kotlin/upbrella/be/user/dto/token/AppleOauthInfo.kt(1 hunks)src/main/kotlin/upbrella/be/user/entity/User.kt(3 hunks)src/main/kotlin/upbrella/be/user/service/OauthLoginService.kt(4 hunks)src/main/kotlin/upbrella/be/user/service/UserService.kt(3 hunks)src/main/kotlin/upbrella/be/util/AppleJwtGenerator.kt(1 hunks)src/test/kotlin/upbrella/be/rent/controller/RentControllerTest.kt(2 hunks)src/test/kotlin/upbrella/be/rent/entity/HistoryTest.kt(1 hunks)src/test/kotlin/upbrella/be/rent/service/ConditionReportServiceTest.kt(1 hunks)src/test/kotlin/upbrella/be/rent/service/RentServiceTest.kt(1 hunks)src/test/kotlin/upbrella/be/slack/service/SlackAlarmServiceTest.kt(1 hunks)src/test/kotlin/upbrella/be/user/controller/UserControllerTest.kt(3 hunks)src/test/kotlin/upbrella/be/user/integration/UserIntegrationTest.kt(3 hunks)src/test/kotlin/upbrella/be/user/service/UserServiceDynamicTest.kt(1 hunks)
🧰 Additional context used
🪛 detekt (1.23.8)
src/main/kotlin/upbrella/be/user/controller/UserController.kt
[warning] 97-97: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
⏰ 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: test
🔇 Additional comments (11)
src/test/kotlin/upbrella/be/slack/service/SlackAlarmServiceTest.kt (1)
72-82: LGTM!The User construction has been properly updated to use named parameters including the new
providerfield.src/test/kotlin/upbrella/be/rent/service/ConditionReportServiceTest.kt (1)
62-72: LGTM!The User construction has been properly updated to use named parameters including the new
providerfield.src/main/kotlin/upbrella/be/user/entity/User.kt (1)
19-19: LGTM!The
providerfield properly distinguishes between KAKAO and APPLE login types with a sensible default.src/test/kotlin/upbrella/be/rent/entity/HistoryTest.kt (1)
49-59: LGTM!The User construction has been properly updated to use named parameters including the new
providerfield.src/test/kotlin/upbrella/be/rent/controller/RentControllerTest.kt (2)
135-145: LGTM!The User construction has been properly updated to use named parameters including the new
providerfield.
220-231: LGTM!The User construction has been properly updated to use named parameters including the new
providerfield.src/test/kotlin/upbrella/be/user/controller/UserControllerTest.kt (1)
56-56: LGTM!The
AppleOauthInfohas been properly added to the test setup, mirroring the existingKakaoOauthInfopattern.Also applies to: 70-70
src/test/kotlin/upbrella/be/user/integration/UserIntegrationTest.kt (1)
59-60: LGTM!The
AppleOauthInfohas been properly autowired and integrated into the controller setup for integration testing.Also applies to: 85-85
src/main/kotlin/upbrella/be/user/service/UserService.kt (1)
40-44: LGTM - loginApple follows the established pattern.The implementation correctly mirrors the Kakao login flow, using the Apple
subidentifier hashed to a Long for user lookup.src/test/kotlin/upbrella/be/rent/service/RentServiceTest.kt (1)
120-130: LGTM - Test fixture correctly updated for new User entity.The User construction now uses named parameters and includes the new
providerfield, properly reflecting the entity changes for Apple OAuth support.src/main/kotlin/upbrella/be/user/controller/UserController.kt (1)
91-112: Apple OAuth endpoint structure looks good, but verify the OAuth flow implementation.The endpoint structure correctly mirrors the Kakao flow. However, as noted in the
OauthLoginServicereview, the Apple OAuth implementation may not work correctly because Apple doesn't have a user info endpoint. Theid_tokenfrom the token response should be decoded directly instead of making a separate API call.
| // JWT for Apple Sign In | ||
| implementation 'io.jsonwebtoken:jjwt-api:0.11.5' | ||
| runtimeOnly 'io.jsonwebtoken:jjwt-impl:0.11.5' | ||
| runtimeOnly 'io.jsonwebtoken:jjwt-jackson:0.11.5' | ||
| implementation 'org.bouncycastle:bcpkix-jdk15on:1.70' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for security advisories using GitHub CLI for Maven packages
# and search for current version information
echo "=== Checking for JJWT (io.jsonwebtoken) CVEs ==="
# Using GitHub API to check advisories - note: may need alternative approach if API limits hit
curl -s "https://api.github.com/repos/jwtk/jjwt/issues?state=all&labels=security" | head -20
echo -e "\n=== Checking for BouncyCastle CVEs ==="
curl -s "https://api.github.com/repos/bcgit/bc-java/issues?state=all&labels=security" | head -20
echo -e "\n=== Checking Maven Central for JJWT latest version ==="
curl -s "https://repo1.maven.org/maven2/io/jsonwebtoken/jjwt-api/maven-metadata.xml" | grep -oP '<release>\K[^<]+' | tail -1
echo -e "\n=== Checking Maven Central for BouncyCastle latest version ==="
curl -s "https://repo1.maven.org/maven2/org/bouncycastle/bcpkix-jdk15on/maven-metadata.xml" | grep -oP '<release>\K[^<]+' | tail -1Repository: UPbrella/UPbrella_back
Length of output: 296
🌐 Web query:
JJWT library io.jsonwebtoken jjwt-api version 0.11.5 security vulnerabilities CVE
💡 Result:
Short answer — yes, there are public security advisories affecting JJWT variants around version 0.11.5:
-
CVE-2024-31033 (Weak/incorrect key handling in JJWT’s setSigningKey()/signWith()). This CVE/GHSA describes JJWT ignoring certain characters which can make keys appear stronger than they are; it is reported against jjwt-impl up through 0.12.5 (vendor disputes/notes). See NVD/Snyk/other advisories. [1][2][3]
-
Additional CVEs shown on package pages for jjwt artifacts (0.11.5) are usually from transitive dependencies (notably Jackson-related deserialization and other library CVEs). Maven/MvnRepository listings for jjwt 0.11.5 show dependency CVEs (examples vary by artifact: jjwt-jackson, jjwt-orgjson, jjwt-root). Check the specific artifact(s) you use to see which dependency CVEs apply. [4][5]
What to do
- If you use jjwt-impl (or any jjwt artifact), treat CVE-2024-31033 as relevant and plan to upgrade to a fixed/newer version (project recommends moving to newer 0.12.x/0.13.x lines or follow maintainer guidance). Verify exact affected artifact (jjwt-impl vs jjwt-api) in your dependency tree. [2][4]
References
- NVD / CVE-2024-31033 entry. [NVD]. [turn1search4].
- Snyk advisory for io.jsonwebtoken:jjwt-impl / CVE-2024-31033. [turn1search8].
- GitHub issue on jwtk/jjwt discussing the CVE report. [turn0search0].
- mvnrepository pages for jjwt artifacts (shows dependency CVEs for 0.11.5). [turn0search7][turn0search1].
If you want, I can (a) run a dependency-tree scan of your build file (pom/gradle) to list which jjwt artifact(s) and transitive CVEs affect you, or (b) recommend exact upgrade versions and mitigation steps for your setup.
🌐 Web query:
BouncyCastle bcpkix-jdk15on version 1.70 security vulnerabilities CVE 2021 2022 2023 2024
💡 Result:
Short answer — yes: bcpkix-jdk15on 1.70 is affected by multiple publicly disclosed Bouncy Castle CVEs from 2023–2024. Key items:
- CVE-2023-33201 (LDAP injection — affects Bouncy Castle Java < 1.74). 1.70 is vulnerable. [1][2]
- CVE-2023-33202 (PEMParser crafted ASN.1 → OutOfMemoryError DoS — affects Bouncy Castle Java < 1.73). 1.70 is vulnerable. [3][4]
- CVE-2024-29857 (ECCurve/F2m certificate parsing → excessive CPU / DoS — affects BC Java < 1.78). 1.70 is vulnerable. [5][6]
- CVE-2024-34447 (BCJSSE hostname verification / DNS poisoning — affects BC Java < 1.78). 1.70 is vulnerable. [7][8]
Notes:
- CVE-2022-45146 targets the BC‑FJA (FIPS Java API) modules (BC‑FJA) and not bcpkix-jdk15on; it does not apply to plain bcpkix builds. [9]
- Older CVEs (e.g., timing issue CVE-2020-15522 / GHSA-2021) affect versions < 1.66 and therefore do not affect 1.70. [10]
References
- GitHub advisory / CVE-2023-33201 (Bouncy Castle LDAP injection).
- NVD / CVE-2023-33201 summaries (vendor/GitHub pages).
- GitHub advisory / CVE-2023-33202 (PEMParser OOM DoS).
- GitLab / dependency-scanning entries for bcpkix-jdk15on (CVE-2023-33202).
- NVD / CVE-2024-29857 and Bouncy Castle advisory (ECCurve high CPU).
- GitHub Advisory Database / GHSA-8xfc-gm6g-vgpv (CVE-2024-29857).
- NVD / CVE-2024-34447 (BC TLS / hostname verification DNS issue).
- GitHub Advisory Database / GHSA-4h8f-2wvx-gg5w (CVE-2024-34447).
- CVE-2022-45146 (BC‑FJA FIPS Java API) — CVE details / cvedetails.
- GHSA/GitHub advisory for timing attack (CVE-2020-15522) showing affected versions <1.66.
If you want, I can: (a) produce a one-line remediation plan (recommended upgrade versions), or (b) fetch the exact vendor advisory pages/patch commits and provide direct links. Which would you prefer?
🌐 Web query:
latest stable versions io.jsonwebtoken jjwt and org.bouncycastle bcpkix 2025
💡 Result:
-
io.jsonwebtoken (JJWT): latest stable release 0.13.0 — released Aug 20, 2025. [1]
-
org.bouncycastle (BCP KIX): latest stable bcpkix (bc-java) release 1.82 — released Sep 17, 2025. (There is a 1.83-SNAPSHOT beta available.) [2][3]
Sources:
[1] Maven / JJWT 0.13.0 release details.
[2] Bouncy Castle Java releases — 1.82 release notes (Sep 17, 2025).
[3] Bouncy Castle betas / 1.83-SNAPSHOT listing.
Update dependency versions to address multiple confirmed security vulnerabilities.
Both libraries in lines 91-95 contain publicly disclosed CVEs that require immediate attention:
JJWT 0.11.5:
- CVE-2024-31033: Weak/incorrect key handling in jjwt-impl affecting version 0.11.5 (fixed in 0.12.x/0.13.x)
- Upgrade to: 0.13.0 (latest stable, Aug 2025)
BouncyCastle 1.70:
- CVE-2023-33201: LDAP injection vulnerability
- CVE-2023-33202: PEMParser OOM DoS vulnerability
- CVE-2024-29857: ECCurve certificate parsing DoS vulnerability
- CVE-2024-34447: BCJSSE hostname verification / DNS poisoning vulnerability
- Upgrade to: 1.82 (latest stable, Sep 2025)
Update the snippet to:
// JWT for Apple Sign In
implementation 'io.jsonwebtoken:jjwt-api:0.13.0'
runtimeOnly 'io.jsonwebtoken:jjwt-impl:0.13.0'
runtimeOnly 'io.jsonwebtoken:jjwt-jackson:0.13.0'
implementation 'org.bouncycastle:bcpkix-jdk15on:1.82'
🤖 Prompt for AI Agents
In build.gradle around lines 91 to 95 the project pins vulnerable versions of
JJWT and BouncyCastle; update the dependency coordinates to the secure versions
recommended in the review: change io.jsonwebtoken:jjwt-api, jjwt-impl and
jjwt-jackson to 0.13.0 and org.bouncycastle:bcpkix-jdk15on to 1.82, then refresh
Gradle dependencies (./gradlew --refresh-dependencies), rebuild and run your
test suite to ensure no API breakages; if any compile errors occur, adjust
imports or code to match the 0.13.0 jjwt API or resolve BouncyCastle API changes
accordingly.
| try { | ||
| appleAccessToken = oauthLoginService.getOauthToken(code.code, appleOauthInfo)!! | ||
| } catch (e: HttpClientErrorException) { | ||
| throw InvalidLoginCodeException("[ERROR] 로그인 코드가 유효하지 않습니다.") | ||
| } |
There was a problem hiding this comment.
Exception is swallowed - original exception context is lost.
Per static analysis: the HttpClientErrorException is caught and replaced with InvalidLoginCodeException, losing the original error details which could be valuable for debugging. Consider including the original exception as the cause:
try {
appleAccessToken = oauthLoginService.getOauthToken(code.code, appleOauthInfo)!!
} catch (e: HttpClientErrorException) {
- throw InvalidLoginCodeException("[ERROR] 로그인 코드가 유효하지 않습니다.")
+ throw InvalidLoginCodeException("[ERROR] 로그인 코드가 유효하지 않습니다.", e)
}This requires InvalidLoginCodeException to accept a cause parameter. The same issue exists in kakaoLogin (lines 74-76).
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 detekt (1.23.8)
[warning] 97-97: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🤖 Prompt for AI Agents
In src/main/kotlin/upbrella/be/user/controller/UserController.kt around lines
74-76 and 95-99, the catch blocks for HttpClientErrorException discard the
original exception which loses debugging context; update the catch to rethrow
InvalidLoginCodeException including the caught exception as its cause (e.g.,
pass the caught exception into the InvalidLoginCodeException constructor) and,
if necessary, modify InvalidLoginCodeException to provide a constructor that
accepts a message and a cause so the original stack trace and message are
preserved.
| data class AppleLoginResponse( | ||
| @JsonProperty("sub") | ||
| val sub: String?, // Apple user identifier | ||
|
|
||
| @JsonProperty("email") | ||
| val email: String?, | ||
|
|
||
| @JsonProperty("email_verified") | ||
| val emailVerified: Boolean? | ||
| ) |
There was a problem hiding this comment.
Consider making sub non-nullable or adding null-safety checks in callers.
Apple's sub claim is always present in valid ID tokens. Making it nullable here can lead to NPE issues. In UserService.kt, appleUser.sub.hashCode() is called without null checks (lines 65 and 130), which will throw NPE if sub is null.
Either make sub non-null:
data class AppleLoginResponse(
@JsonProperty("sub")
- val sub: String?, // Apple user identifier
+ val sub: String, // Apple user identifier (always present)Or ensure callers validate before use:
val socialIdHash = appleUser.sub?.hashCode()?.toLong()
?: throw IllegalArgumentException("Apple sub is required")🤖 Prompt for AI Agents
In src/main/kotlin/upbrella/be/user/dto/response/AppleLoginResponse.kt around
lines 5 to 14, the AppleLoginResponse.sub field is declared nullable but callers
in UserService.kt call sub.hashCode() without null checks; make sub non-nullable
(remove nullable marker) so the type reflects Apple's guarantee and forces
callers to treat it as required, or alternatively keep it nullable and update
UserService to defensively check and throw a clear exception or handle the
missing value before calling hashCode(); ensure all callsites (lines ~65 and
~130 in UserService.kt) are updated to match the chosen approach.
| @PostConstruct | ||
| fun init() { | ||
| // Apple JWT Client Secret을 동적으로 생성 | ||
| _clientSecret = appleJwtGenerator.generateClientSecret( | ||
| teamId = teamId, | ||
| keyId = keyId, | ||
| clientId = clientId, | ||
| p8KeyPath = p8KeyPath | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Apple Sign In client secret expiration maximum validity duration
💡 Result:
Maximum validity is 6 months (must not exceed 15,777,000 seconds from iat). [1][2]
Sources:
[1] Apple — Generate and validate tokens (Sign in with Apple).
[2] Pomerium docs — Apple integration (quoting Apple's exp limit).
🌐 Web query:
javax.annotation.PostConstruct deprecated Java version
💡 Result:
javax.annotation.PostConstruct (part of the JSR‑250 "common annotations" in module java.xml.ws.annotation) was deprecated for removal in Java 9 and the Java EE / common‑annotations modules were removed in JDK 11 — so PostConstruct is no longer provided by the JDK starting with Java 11. Add the javax.annotation-api (or jakarta.annotation-api) dependency to restore it. [1][2][3]
Sources:
- Oracle JDK removed Java EE / Common Annotations in JDK 11. [1]
- OpenJDK JIRA describing deprecation for removal in Java 9. [2]
- Practical guidance to add javax.annotation-api when using Java 11. [3]
🏁 Script executed:
# First, let's examine the full AppleOauthInfo.kt file to understand the implementation
fd AppleOauthInfo.ktRepository: UPbrella/UPbrella_back
Length of output: 126
🏁 Script executed:
cat -n src/main/kotlin/upbrella/be/user/dto/token/AppleOauthInfo.ktRepository: UPbrella/UPbrella_back
Length of output: 1500
🏁 Script executed:
# Search for any secret rotation, refresh, or regeneration logic
rg -i "regenerate|rotation|refresh|expir" src/main/kotlin/upbrella/be/user/dto/token/Repository: UPbrella/UPbrella_back
Length of output: 242
🏁 Script executed:
# Check the project's Java/Kotlin version configuration
fd -e properties -e gradle -e maven -e toml | head -20Repository: UPbrella/UPbrella_back
Length of output: 135
🏁 Script executed:
cat -n build.gradle | head -100Repository: UPbrella/UPbrella_back
Length of output: 4018
🏁 Script executed:
# Find and examine AppleJwtGenerator
fd AppleJwtGenerator.ktRepository: UPbrella/UPbrella_back
Length of output: 119
🏁 Script executed:
# Search for scheduled tasks, timers, or refresh mechanisms
rg -i "scheduled|timer|interval|refreshtoken" --type=kt | head -20Repository: UPbrella/UPbrella_back
Length of output: 92
🏁 Script executed:
cat -n src/main/kotlin/upbrella/be/util/AppleJwtGenerator.ktRepository: UPbrella/UPbrella_back
Length of output: 2444
🏁 Script executed:
# Search for scheduled tasks with correct syntax
rg -i "scheduled|@scheduled" --type=kotlinRepository: UPbrella/UPbrella_back
Length of output: 48
🏁 Script executed:
# Verify if spring-boot-starter transitively provides javax.annotation-api in Spring Boot 2.7.13
cat -n build.gradle | grep -A 2 "spring-boot-starter"Repository: UPbrella/UPbrella_back
Length of output: 826
Apple client secrets expire after 6 months — implement regeneration before expiration.
The Apple JWT client secret is generated at application startup with a 6-month expiration (line 33 in AppleJwtGenerator.kt: Date(now.time + 15777000000L)). However, AppleOauthInfo generates this secret only once via @PostConstruct (lines 35-44), with no mechanism to regenerate it before expiration. For long-running applications, this will cause authentication failures once the token expires.
Implement one of:
- Schedule periodic secret regeneration (e.g., every 5 months via
@Scheduled) - Generate secrets on-demand per request with caching
- Ensure application restarts before secret expiration
Additionally, upgrade from the deprecated javax.annotation.PostConstruct to jakarta.annotation.PostConstruct (add jakarta.annotation:jakarta.annotation-api dependency) or use Spring's @EventListener(ApplicationReadyEvent::class) for Java 17+ compatibility.
🤖 Prompt for AI Agents
In src/main/kotlin/upbrella/be/user/dto/token/AppleOauthInfo.kt around lines 35
to 44, the client secret is generated only once in a @PostConstruct method which
will expire after 6 months; replace the one-time init with a regeneration
strategy and update the lifecycle annotation: (1) switch from
javax.annotation.PostConstruct to jakarta.annotation.PostConstruct or use
Spring's @EventListener(ApplicationReadyEvent::class) to avoid deprecated javax,
and (2) implement periodic regeneration by adding a scheduled task (e.g.,
@Scheduled with a fixedRate or cron to run every ~5 months) that re-calls
appleJwtGenerator.generateClientSecret(...) and updates _clientSecret
atomically, or alternatively implement on-demand lazy generation with a cached
value and expiry check that regenerates before returning if expired; ensure
thread-safety when updating the shared secret and add necessary dependency
jakarta.annotation:jakarta.annotation-api if choosing jakarta.
| -----BEGIN PRIVATE KEY----- | ||
| MIGTAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBHkwdwIBAQQgPGJKUUO/Pk7S7u5c | ||
| VLfvBLGWRfHR3Ja6mZjKJg8Q0c2gCgYIKoZIzj0DAQehRANCAARLYCMOVGDALwBK | ||
| bKD6QdqFv8i5dX3LVxHiCcBPdlQPqFHpkOLQPF8eHvKuqTmVNjjZ9L5q6pDKxIUH | ||
| Qr0qJKqC | ||
| -----END PRIVATE KEY----- |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/kotlin/upbrella/be/user/service/OauthLoginService.kt (1)
29-54: Fix Kakao OAuth token endpoint configuration—redirectUrishould not be used as the POST URL.The current implementation incorrectly uses
redirectUrias the token endpoint (line 51), when it should be the OAuth callback URL. This violates the OAuth 2.0 specification.Compare with Apple's correct implementation (lines 73, 80):
- Token endpoint is hard-coded:
https://appleid.apple.com/auth/tokenredirect_uriis passed as a request parameterKakao should follow the same pattern:
- Add a
tokenUriconfiguration property (e.g.,https://kauth.kakao.com/oauth/token)- Add
"redirect_uri" to redirectUrito the request payload (line 45, like Apple)- Update line 51 to use the token endpoint URL, not
redirectUriThe test value
redirectUri = "kakao.com"(line 49 in OauthLoginServiceTest.kt) further confirms this is incorrect—it's neither a valid token endpoint nor a proper callback URL.
♻️ Duplicate comments (2)
src/main/kotlin/upbrella/be/user/dto/token/AppleOauthInfo.kt (2)
7-7: Usejakarta.annotation.PostConstructinstead of deprecatedjavax.annotation.PostConstruct.This annotation was deprecated in Java 9 and removed in Java 11. Since the project uses Java 17, add the
jakarta.annotation:jakarta.annotation-apidependency and update the import.-import javax.annotation.PostConstruct +import jakarta.annotation.PostConstruct
44-52: Client secret expires after 6 months with no regeneration mechanism.The Apple JWT client secret is generated once at startup. For long-running applications, this will cause authentication failures after 6 months. Consider implementing periodic regeneration via
@Scheduledor lazy generation with expiry checking.
🧹 Nitpick comments (7)
src/main/kotlin/upbrella/be/config/ProductionCorsConfig.kt (1)
15-22: Re‑check need to allowhttps://appleid.apple.comas a CORS origin with credentialsBecause
allowCredentials(true)is enabled, addinghttps://appleid.apple.commeans any JS running on that domain can call your API with user cookies. If the backend never receives browser XHR/fetch calls directly from Apple (only redirects and server‑to‑server token calls), you can safely drop this origin to keep the CORS surface minimal.src/main/kotlin/upbrella/be/user/controller/UserController.kt (2)
134-153: Tighten null‑safety and session cleanup in multi‑provider loginThe combined Kakao/Apple login flow is mostly straightforward, but a couple of small robustness points:
- You dereference
kakaoUser.id!!andappleUser.sub!!. If the upstream DTOs allow these fields to be null, this will 500. Either make those fields non‑nullable in the DTOs or add an explicit null check with a controlled error.- When both
kakaoUserandappleUserare present (edge case), thewhenprefers Kakao and leaves the Apple session attribute untouched. Consider clearing both social attributes after a successful login to avoid stale data hanging around in the session.
183-207: Broadened join flow is good; consider renaming and symmetrical cleanupExtending
/users/jointo handle both Kakao and Apple, and updating the message to"소셜 회원가입 성공", makes the behavior clearer. Now that it’s no longer Kakao‑specific:
- The method name
kakaoJoinis slightly misleading; consider renaming to something likesocialJoinfor clarity.- Similar to login, you might want to defensively clear both
"kakaoUser"and"appleUser"session attributes after a successful join, regardless of which provider was used, to prevent leftover social state.Also applies to: 216-216
src/main/kotlin/upbrella/be/util/AppleIdTokenDecoder.kt (3)
73-80: Consider caching Apple public keys to reduce latency.Fetching public keys from Apple's JWKS endpoint on every token decode adds unnecessary latency. Apple's keys rotate infrequently (typically months apart).
Consider caching the keys with a TTL (e.g., 24 hours) or implementing a cache-invalidation strategy that refetches keys only when a
kidis not found in the cache.
82-86: Add defensive validation for malformed tokens.If
idTokendoesn't contain a.separator,split(".")[0]won't fail butsplit(".")on a completely invalid string could produce unexpected results. Consider adding a format check.private fun parseHeader(idToken: String): Map<String, Any> { + val parts = idToken.split(".") + if (parts.size != 3) { + throw IllegalArgumentException("Invalid JWT format") + } - val headerEncoded = idToken.split(".")[0] + val headerEncoded = parts[0] val headerDecoded = String(Base64.getUrlDecoder().decode(headerEncoded)) return objectMapper.readValue(headerDecoded, Map::class.java) as Map<String, Any> }
39-43: Handleemail_verifiedas String or Boolean.Apple may return
email_verifiedas a String"true"/"false"in some cases rather than a Boolean. The current safe castas? Booleanwould returnnullif it's a String.return AppleLoginResponse( sub = claims["sub"] as? String, email = claims["email"] as? String, - emailVerified = claims["email_verified"] as? Boolean + emailVerified = when (val ev = claims["email_verified"]) { + is Boolean -> ev + is String -> ev.toBoolean() + else -> null + } )src/main/kotlin/upbrella/be/user/dto/token/AppleOauthInfo.kt (1)
16-92: Consider reducing duplication between Dev and Prod implementations.
DevAppleOauthInfoandProdAppleOauthInfoare nearly identical except for property key suffixes. Consider consolidating to a single implementation using profile-specific property files (e.g.,application-dev.ymlandapplication-prod.yml) with common property names.@Component class AppleOauthInfoImpl( @Value("\${apple.client-id}") override val clientId: String, @Value("\${apple.team-id}") private val teamId: String, // ... other properties with common names private val appleJwtGenerator: AppleJwtGenerator ) : AppleOauthInfo { // single implementation }Then define profile-specific values in
application-dev.yml:apple: client-id: ${APPLE_CLIENT_ID_DEV} team-id: ${APPLE_TEAM_ID_DEV}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/main/kotlin/upbrella/be/config/AuthConfig.kt(1 hunks)src/main/kotlin/upbrella/be/config/ProductionCorsConfig.kt(1 hunks)src/main/kotlin/upbrella/be/config/SessionCookieConfig.kt(1 hunks)src/main/kotlin/upbrella/be/user/controller/UserController.kt(5 hunks)src/main/kotlin/upbrella/be/user/dto/token/AppleOauthInfo.kt(1 hunks)src/main/kotlin/upbrella/be/user/dto/token/OauthToken.kt(1 hunks)src/main/kotlin/upbrella/be/user/service/OauthLoginService.kt(3 hunks)src/main/kotlin/upbrella/be/util/AppleIdTokenDecoder.kt(1 hunks)src/main/resources/application-dev.yml(2 hunks)src/main/resources/application-prod.yml(1 hunks)src/test/kotlin/upbrella/be/user/service/OauthLoginServiceTest.kt(1 hunks)
⏰ 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: test
🔇 Additional comments (9)
src/main/kotlin/upbrella/be/config/AuthConfig.kt (1)
20-35: Apple callback exclusion fromLoginInterceptorlooks appropriateExcluding
/auth/apple/**fromloginInterceptorso the OAuth callback can be reached without a prior login is consistent with the existing Kakao login exclusions.src/main/resources/application-dev.yml (1)
5-7: Env‑driven DB config and dev cookie settings look reasonableSwitching dev DB config to
${DATABASE_URL}/${DATABASE_USERNAME}/${DATABASE_PASSWORD}is a good move away from hard‑coded secrets. The dev session cookie configuration (same-site: lax,secure: false) is also appropriate for local/non‑TLS dev, just be aware behavior will differ slightly from prod for cross‑site OAuth flows.Also applies to: 19-24
src/main/kotlin/upbrella/be/config/SessionCookieConfig.kt (1)
1-23: Prod session cookie hardening is good; verify domain & config consistencySetting
secure = true,httpOnly = true, andpath = "/"underprodis solid. Please double‑check:
domain = "upbrella.co.kr"matches your actual public base domain for all API/front‑end hosts that must share the session (e.g.api.upbrella.co.kr,www.upbrella.co.kr).- This programmatic config stays consistent with
server.servlet.session.cookie.*properties inapplication-prod.yml(notablysame-site: none).src/main/resources/application-prod.yml (1)
20-24:SameSite=Nonematches cross‑site OAuth needs; keep it aligned withsecureSetting
server.servlet.session.cookie.same-site: noneis appropriate for cross‑site flows like Apple OAuth. SinceSessionCookieConfigsetssecure = true, this satisfies modern browser requirements forSameSite=Nonecookies; just ensuresecureis never disabled in prod, or cookies may be rejected.src/main/kotlin/upbrella/be/user/dto/token/OauthToken.kt (1)
11-13: AddingidTokentoOauthTokenis backwards‑compatible and aligns with AppleIntroducing
val idToken: String? = nullwith a default keeps existing positional constructor usages valid and, with the snake_case strategy, correctly binds to theid_tokenfield returned by providers like Apple.src/test/kotlin/upbrella/be/user/service/OauthLoginServiceTest.kt (1)
28-30: AppleIdTokenDecoder mock wiring is correctAdding the
appleIdTokenDecodermock keeps@InjectMocks OauthLoginServiceconstructible with the new dependency. Once you add Apple‑specific service tests, you can reuse this mock for those scenarios.src/main/kotlin/upbrella/be/user/controller/UserController.kt (1)
7-14: Apple OAuth wiring into UserController looks consistentImporting
RedirectViewandAppleOauthInfo, and injectingAppleOauthInfoalongsideKakaoOauthInfo, aligns with the new Apple OAuth flow used in the controller and services. As long as you have matchingAppleOauthInfobeans for all active profiles (dev/prod/test), this wiring is sound.Also applies to: 27-34
src/main/kotlin/upbrella/be/user/service/OauthLoginService.kt (2)
56-83: LGTM!The Apple token retrieval correctly uses Apple's token endpoint and includes all required parameters including
redirect_uriin the request payload.
107-110: LGTM!Clean delegation to
AppleIdTokenDecoderfor ID token processing.
| @PostMapping("/auth/apple") | ||
| fun appleLoginCallback( | ||
| session: HttpSession, | ||
| @RequestParam code: String, | ||
| @RequestParam(required = false) state: String?, | ||
| @RequestParam(required = false) id_token: String?, | ||
| @RequestParam(required = false) user: String? | ||
| ): RedirectView { | ||
| log.info("Apple login callback received - code: ${code.take(10)}...") | ||
|
|
||
| val appleOauthToken: OauthToken | ||
|
|
||
| try { | ||
| appleOauthToken = oauthLoginService.getOauthToken(code, appleOauthInfo)!! | ||
| } catch (e: HttpClientErrorException) { | ||
| log.error("Apple login failed", e) | ||
| return RedirectView("https://upbrella.co.kr/login?error=apple_login_failed") | ||
| } catch (e: Exception) { | ||
| log.error("Unexpected error during Apple login", e) | ||
| return RedirectView("https://upbrella.co.kr/login?error=server_error") | ||
| } | ||
|
|
||
| // id_token이 없으면 에러 | ||
| if (appleOauthToken.idToken.isNullOrEmpty()) { | ||
| log.error("Apple ID token is null or empty") | ||
| return RedirectView("https://upbrella.co.kr/login?error=no_id_token") | ||
| } | ||
|
|
||
| try { | ||
| val appleLoggedInUser = oauthLoginService.processAppleLogin(appleOauthToken.idToken!!) | ||
| session.setAttribute("appleUser", appleLoggedInUser) | ||
|
|
||
| log.info("Apple social login success - redirecting to frontend") | ||
| return RedirectView("https://upbrella.co.kr/login?apple=success") | ||
| } catch (e: Exception) { | ||
| log.error("Apple ID token validation failed", e) | ||
| return RedirectView("https://upbrella.co.kr/login?error=token_validation_failed") | ||
| } | ||
| } |
There was a problem hiding this comment.
I'll verify the three main concerns in this Apple OAuth callback review. Let me start by exploring the codebase to understand the full OAuth flow and check the specific issues raised.
<function_calls>
find . -type f -name "*.kt" | grep -E "(oauth|user|controller)" | head -20
</function_calls>
Let me search more broadly and also check the structure:
<function_calls>
ls -la src/main/kotlin/upbrella/be/user/controller/ 2>/dev/null || find . -type f -name "UserController.kt" | head -5
</function_calls>
I need to run shell scripts to examine the codebase. Let me start the investigation:
<function_calls>
#!/bin/bash
Find UserController and related files
echo "=== Finding UserController ==="
find . -type f -name "UserController.kt" 2>/dev/null
echo ""
echo "=== Finding OAuth-related files ==="
find . -type f -name "OAuth" -o -name "oauth" 2>/dev/null | head -20
echo ""
echo "=== Finding service files ==="
find . -type f -name "*Service.kt" | grep -i oauth 2>/dev/null
</function_calls>
Let me use a more targeted approach:
<function_calls>
#!/bin/bash
Search for the UserController file
rg -l "appleLoginCallback" --type kt 2>/dev/null | head -5
</function_calls>
Let me try a broader search:
<function_calls>
#!/bin/bash
List directory structure
find . -type d -name "user" | head -10
find . -type d -name "controller" | head -10
</function_calls>
Let me search for files containing the code snippet:
<function_calls>
#!/bin/bash
rg "@PostMapping("/auth/apple")" -t kt --max-count 5
</function_calls>
Let me try a different approach and look at git repository structure:
<function_calls>
#!/bin/bash
git ls-files | grep -E "(UserController|oauth)" | head -20
</function_calls>
Now let me search for Apple login related code:
<function_calls>
#!/bin/bash
rg "appleLoginCallback|processAppleLogin|getOauthToken" -t kt -A 2 -B 2 | head -100
</function_calls>
Let me search for state parameter handling:
<function_calls>
#!/bin/bash
rg "state" -t kt -i | grep -E "(session|state|csrf)" | head -30
</function_calls>
Let me search for configuration of redirect URLs:
<function_calls>
#!/bin/bash
rg "upbrella.co.kr|redirectUri|frontendUrl|frontend" -t kt -A 1 -B 1 | head -50
</function_calls>
Let me check for application properties or configuration files:
<function_calls>
#!/bin/bash
find . -type f ( -name "application*.yml" -o -name "application*.yaml" -o -name "application*.properties" ) | head -10
</function_calls>
Let me search for other OAuth providers to see the pattern:
<function_calls>
#!/bin/bash
rg "@PostMapping.*auth.*callback" -t kt -A 5 | head -100
</function_calls>
Let me check for OauthLoginService implementation:
<function_calls>
#!/bin/bash
rg "class OauthLoginService|fun processAppleLogin|fun getOauthToken" -t kt -A 3 | head -100
</function_calls>
Let me try to get all Kotlin files and examine the structure:
<function_calls>
#!/bin/bash
Get all Kotlin files
find . -type f -name "*.kt" 2>/dev/null | wc -l
echo "---"
Look for controller files
find . -type f -name "*.kt" 2>/dev/null | grep -i controller
</function_calls>
Let me search for the specific error messages from the code:
<function_calls>
#!/bin/bash
rg "Apple login callback received" -t kt -B 5 -A 30
</function_calls>
Let me try searching with the GitHub endpoint:
<function_calls>
#!/bin/bash
rg "Apple ID token is null" -t kt -B 10 -A 20
</function_calls>
Let me search broadly for all file extensions to understand the project better:
<function_calls>
#!/bin/bash
find . -type f -name "*.kt" | head -1 | xargs wc -l
</function_calls>
Let me try listing repository files more directly:
<function_calls>
#!/bin/bash
git ls-files --full-name | grep ".kt$" | head -20
</function_calls>
Now let me examine those files for Apple login logic:
<function_calls>
#!/bin/bash
Search for Apple in all Kotlin files
git ls-files --full-name | grep ".kt$" | xargs rg "apple|Apple" -i -l | head -10
</function_calls>
Let me get the full file content of UserController if it exists:
<function_calls>
#!/bin/bash
git ls-files --full-name | grep -i "usercontroller"
</function_calls>
Let me try a different pattern search:
<function_calls>
#!/bin/bash
git ls-files --full-name | grep ".kt$" | xargs grep -l "appleLoginCallback" 2>/dev/null
</function_calls>
Now let me examine the authentication/OAuth flow more carefully:
<function_calls>
#!/bin/bash
Search for all POST mapping with auth
git ls-files --full-name | grep ".kt$" | xargs grep -n "@PostMapping" 2>/dev/null | grep -i "auth" | head -20
</function_calls>
Let me search for session attribute handling:
<function_calls>
#!/bin/bash
Search for session state or state validation
git ls-files --full-name | grep ".kt$" | xargs grep -n "session.*state|setAttribute.*state|getAttribute.*state" 2>/dev/null | head -20
</function_calls>
Let me look for the Apple OAuth configuration:
<function_calls>
#!/bin/bash
Search for Apple OAuth configuration
git ls-files --full-name | xargs grep -n "apple.*oauth|AppleOauth" 2>/dev/null -i | head -20
</function_calls>
Let me search for where the Apple login is initiated:
<function_calls>
#!/bin/bash
Search for Apple login start or initiate
git ls-files --full-name | grep ".kt$" | xargs grep -n "appleLogin|apple.*login" 2>/dev/null -i | head -30
</function_calls>
Let me try searching for the URL that's hard-coded:
<function_calls>
#!/bin/bash
git ls-files --full-name | xargs grep -n "upbrella.co.kr" 2>/dev/null | head -20
</function_calls>
Let me check if there's configuration for base URL:
<function_calls>
#!/bin/bash
Look for property files
git ls-files --full-name | grep -E ".(yml|yaml|properties)$"
</function_calls>
Let me check the content of those config files:
<function_calls>
#!/bin/bash
Read application config files if they exist
git ls-files --full-name | grep -E "application.*.(yml|yaml|properties)$" | xargs cat 2>/dev/null | head -100
</function_calls>
Let me search for all OAuth-related endpoints:
<function_calls>
#!/bin/bash
git ls-files --full-name | grep ".kt$" | xargs grep -n "@.*Mapping.*oauth|@.*Mapping.*auth" 2>/dev/null -i | head -30
</function_calls>
Let me get a comprehensive list of all controller methods:
<function_calls>
#!/bin/bash
git ls-files --full-name | grep "Controller.kt$" | head -10
</function_calls>
Let me try to find the file using a broader search:
<function_calls>
#!/bin/bash
Search all files containing both "PostMapping" and "/auth/apple"
git ls-files --full-name | xargs grep -l "PostMapping.*apple|apple.*PostMapping" 2>/dev/null
</function_calls>
Let me search more broadly:
<function_calls>
#!/bin/bash
Find all files mentioning "auth/apple"
git ls-files --full-name | xargs grep -l "/auth/apple" 2>/dev/null
</function_calls>
Let me examine the git history or search the PR:
<function_calls>
#!/bin/bash
Check the git diff to see what files changed
git diff --name-only main...HEAD 2>/dev/null | head -20
</function_calls>
Let me try to get the current branch and see what files are in the PR:
<function_calls>
#!/bin/bash
git log --oneline -5
</function_calls>
Let me check the git status and recent changes:
<function_calls>
#!/bin/bash
git status
</function_calls>
Let me try to find files containing the exact method signature:
<function_calls>
#!/bin/bash
Search for the method signature
rg "fun appleLoginCallback" -t kt -B 5 -A 30 2>/dev/null
</function_calls>
Let me try using ast-grep to find this function:
<function_calls>
#!/bin/bash
Try to find the function with ast-grep
ast-grep --pattern 'fun appleLoginCallback($$$) { $$$ }' 2>/dev/null | head -50
</function_calls>
Let me search for all files in the repository:
<function_calls>
#!/bin/bash
List all kotlin files
find . -name "*.kt" -type f 2>/dev/null | head -50
</function_calls>
Great! Now let me examine the UserController file:
<function_calls>
#!/bin/bash
Read the UserController file
cat -n ./src/main/kotlin/upbrella/be/user/controller/UserController.kt | sed -n '1,150p'
</function_calls>
Perfect! Now let me check the OauthLoginService to understand how state is handled:
<function_calls>
#!/bin/bash
Find and examine OauthLoginService
find . -name "OauthLoginService" -o -name "oauthService*" | grep -v ".class"
</function_calls>
<function_calls>
#!/bin/bash
Find oauth service files
find . -type f -name "ervice.kt" | grep -i oauth | head -10
</function_calls>
Let me search for the service classes:
<function_calls>
#!/bin/bash
find . -type d -name "service" | head -5
</function_calls>
<function_calls>
#!/bin/bash
List service directory
ls -la ./src/main/kotlin/upbrella/be/*/service/ 2>/dev/null | head -100
</function_calls>
<function_calls>
#!/bin/bash
Find all Kotlin files in service directories
find ./src/main/kotlin -type f -name "*Service.kt" 2>/dev/null | head -20
</function_calls>
<function_calls>
#!/bin/bash
Search for OauthLoginService
rg "class OauthLoginService" -t kt -B 2 -A 20 2>/dev/null
</function_calls>
Let me search more broadly for oauth:
<function_calls>
#!/bin/bash
rg "processAppleLogin|getOauthToken" -t kt 2>/dev/null | head -20
</function_calls>
Let me check the entire service directory structure:
<function_calls>
#!/bin/bash
find ./src/main/kotlin -type d | head -30
</function_calls>
Let me look for the oauth package:
<function_calls>
#!/bin/bash
find ./src/main/kotlin -type d -name "oauth" -o -type d -name "auth"
</function_calls>
Let me examine the UserController imports to understand where oauthLoginService comes from:
<function_calls>
#!/bin/bash
Check the beginning of UserController to see imports
cat ./src/main/kotlin/upbrella/be/user/controller/UserController.kt | head -50
</function_calls>
<function_calls>
#!/bin/bash
Search for OauthLoginService anywhere in the codebase
rg "OauthLoginService" -t kt 2>/dev/null | head -20
</function_calls>
Let me check what's imported:
<function_calls>
#!/bin/bash
Get imports from UserController
grep -n "^import" ./src/main/kotlin/upbrella/be/user/controller/UserController.kt | grep -i oauth
</function_calls>
<function_calls>
#!/bin/bash
Get all imports
grep -n "^import" ./src/main/kotlin/upbrella/be/user/controller/UserController.kt
</function_calls>
<function_calls>
#!/bin/bash
Search for files containing OauthLoginService class definition
rg "class.*OauthLoginService" -t kt 2>/dev/null
</function_calls>
Let me search with a wildcard:
<function_calls>
#!/bin/bash
Search for interface or class definitions
rg "^class|^interface|^object" -t kt | grep -i oauth
</function_calls>
Let me look for service definitions:
<function_calls>
#!/bin/bash
Find all service classes
find ./src/main/kotlin -name "*Service.kt" -type f
</function_calls>
Let me check each service file:
<function_calls>
#!/bin/bash
Get file list
find ./src/main/kotlin -name "*Service.kt" -type f | xargs -I {} sh -c 'echo "=== {} ===" && head -20 "{}"'
</function_calls>
Let me look at all Kotlin files in the oauth package (if it exists):
<function_calls>
#!/bin/bash
List all directories
find ./src/main/kotlin/upbrella/be -type d 2>/dev/null
</function_calls>
<function_calls>
#!/bin/bash
Search in lent/login/oauth directories
ls -la ./src/main/kotlin/upbrella/be/lent/ 2>/dev/null || echo "lent dir not found"
ls -la ./src/main/kotlin/upbrella/be/login/ 2>/dev/null || echo "login dir not found"
ls -la ./src/main/kotlin/upbrella/be/oauth/ 2>/dev/null || echo "oauth dir not found"
</function_calls>
Let me search all Kotlin files:
<function_calls>
#!/bin/bash
Find all Kotlin files and search for the service class
find ./src/main/kotlin -name "*.kt" -type f -exec grep -l "OauthLoginService" {} ;
</function_calls>
Let me now check for other OAuth callbacks to understand the pattern:
<function_calls>
#!/bin/bash
Search for other login callbacks (google, kakao)
grep -n "LoginCallback|login.*callback" ./src/main/kotlin/upbrella/be/user/controller/UserController.kt -i
</function_calls>
Let me view more of the UserController file:
<function_calls>
#!/bin/bash
Get line count and view more sections
wc -l ./src/main/kotlin/upbrella/be/user/controller/UserController.kt
</function_calls>
<function_calls>
#!/bin/bash
View the full file to understand context
cat ./src/main/kotlin/upbrella/be/user/controller/UserController.kt
</function_calls>
Now let me search for the OauthLoginService:
<function_calls>
#!/bin/bash
Search all Kotlin files for OauthLoginService class or interface
rg "class|interface|object" -t kt -A 2 | grep -B 2 "OauthLoginService"
</function_calls>
Let me search differently:
<function_calls>
#!/bin/bash
Look for all files with OAuth in name
find . -name "oauth" -o -name "OAuth" 2>/dev/null | grep -v ".git" | head -20
</function_calls>
<function_calls>
#!/bin/bash
Search for login package
find ./src/main/kotlin -type d | grep -i login
</function_calls>
<function_calls>
#!/bin/bash
Check what's in the lent directory
ls -la ./src/main/kotlin/upbrella/be/lent/ 2>/dev/null
</function_calls>
<function_calls>
#!/bin/bash
Find all directories under be
find ./src/main/kotlin/upbrella/be -maxdepth 1 -type d
</function_calls>
<function_calls>
#!/bin/bash
List all subdirectories with service subdir
find ./src/main/kotlin/upbrella/be -name "service" -type d -exec ls {} ;
</function_calls>
<function_calls>
#!/bin/bash
Get lent/service directory contents
ls -la ./src/main/kotlin/upbrella/be/lent/service/
</function_calls>
<function_calls>
#!/bin/bash
List all files in that service directory
find ./src/main/kotlin/upbrella/be/lent/service -name "*.kt" -type f
</function_calls>
Great! Let me examine the OauthLoginService:
<function_calls>
#!/bin/bash
Read OauthLoginService
cat -n ./src/main/kotlin/upbrella/be/lent/service/OauthLoginService.kt
</function_calls>
Perfect! Now let me search for where the Apple login is initiated to understand state handling:
<function_calls>
#!/bin/bash
Search for Apple login initiation endpoints
grep -rn "@.*Mapping.*apple|/auth/apple" ./src/main/kotlin -i | head -20
</function_calls>
<function_calls>
#!/bin/bash
Search for any state-related session handling
grep -rn "state|State" ./src/main/kotlin/upbrella/be/user -i | head -30
</function_calls>
Let me search for state in the entire codebase:
<function_calls>
#!/bin/bash
Search for session state validation
grep -rn "session.*state|state.*session|getAttribute.*state|setAttribute.*state" ./src/main/kotlin -i | head -20
</function_calls>
<function_calls>
#!/bin/bash
Search for CSRF protection or state validation
grep -rn "csrf|CSRF|state.*validation|validate.*state" ./src/main/kotlin -i | head -20
</function_calls>
Now let me check if there's a login initiation endpoint:
<function_calls>
#!/bin/bash
Search for any mapping that initiates Apple login
grep -rn "GetMapping|PostMapping|@.*Mapping" ./src/main/kotlin/upbrella/be/user -B 2 -A 2 | grep -E "apple|Apple|login|Login" -i
</function_calls>
Let me check the configuration files:
<function_calls>
#!/bin/bash
Find and check configuration files
find . -name "application*.yml" -o -name "application*.yaml" -o -name "application*.properties" | head -10
</function_calls>
<function_calls>
#!/bin/bash
Check if properties file exists and examine it
cat ./src/main/resources/application.yml 2>/dev/null || cat ./src/main/resources/application.properties 2>/dev/null || cat ./src/main/resources/application.yaml 2>/dev/null
</function_calls>
Let me check the resources directory:
<function_calls>
#!/bin/bash
List resources directory
ls -la ./src/main/resources/ 2>/dev/null | head -20
</function_calls>
<function_calls>
#!/bin/bash
Find all config files
find ./src -name ".yml" -o -name ".yaml" -o -name "*.properties" 2>/dev/null
</function_calls>
Let me check what controllers exist and look for others:
<function_calls>
#!/bin/bash
Find all controller files
find ./src -name "*Controller.kt" 2>/dev/null
</function_calls>
Let me examine the google login callback to see if it has the same pattern:
<function_calls>
#!/bin/bash
View lines around googleLoginCallback
sed -n '130,180p' ./src/main/kotlin/upbrella/be/user/controller/UserController.kt
</function_calls>
<function_calls>
#!/bin/bash
View the rest of the file
sed -n '180,210p' ./src/main/kotlin/upbrella/be/user/controller/UserController.kt
</function_calls>
Let me check the OauthInfo classes:
<function_calls>
#!/bin/bash
Search for OauthInfo class
find ./src -name "OauthInfo" -o -name "oauthinfo*" -i
</function_calls>
<function_calls>
#!/bin/bash
Search for AppleOauthInfo
grep -rn "AppleOauthInfo|class.*OauthInfo" ./src/main/kotlin -i | head -20
</function_calls>
Let me search for configuration classes:
<function_calls>
#!/bin/bash
Find config or entity files
find ./src/main/kotlin -name "*Config.kt" -o -name "*Property.kt" -o -name "*Entity.kt" | head -20
</function_calls>
<function_calls>
#!/bin/bash
List all files in lent directory
find ./src/main/kotlin/upbrella/be/lent -name "*.kt" -type f
</function_calls>
<function_calls>
#!/bin/bash
Check config package
ls -la ./src/main/kotlin/upbrella/be/lent/config/ 2>/dev/null || echo "No config dir found"
</function_calls>
<function_calls>
#!/bin/bash
Find all files in lent
find ./src/main/kotlin/upbrella/be/lent -type f -name "*.kt" | xargs ls -la
</function_calls>
<function_calls>
#!/bin/bash
Show directory structure
find ./src/main/kotlin/upbrella/be/lent -type d
</function_calls>
<function_calls>
#!/bin/bash
View the OauthProperty file
cat ./src/main/kotlin/upbrella/be/lent/config/property/OauthProperty.kt
</function_calls>
Perfect! Now let me check if there's state validation:
<function_calls>
#!/bin/bash
Search for CSRF token, state generation, or validation
grep -rn "UUID|random|state|csrf" ./src/main/kotlin/upbrella/be/lent -i | head -30
</function_calls>
Let me check for @value or @ConfigurationProperties usage:
<function_calls>
#!/bin/bash
Check how OauthProperty is used
grep -rn "OauthProperty|@value" ./src/main/kotlin/upbrella/be -i | head -20
</function_calls>
Let me check the entire lent package structure:
<function_calls>
#!/bin/bash
List all Kotlin files in lent package
find ./src/main/kotlin/upbrella/be/lent -name "*.kt" -type f -exec echo "=== {} ===" ; -exec head -20 {} ;
</function_calls>
<function_calls>
#!/bin/bash
Get list of files
find ./src/main/kotlin/upbrella/be/lent -name "*.kt" -type f
</function_calls>
Let me examine each file:
<function_calls>
#!/bin/bash
OauthProperty (already showed)
Check OauthToken
echo "=== OauthToken ==="
cat ./src/main/kotlin/upbrella/be/lent/config/property/OauthToken.kt
echo ""
echo "=== AppleOauthUser ==="
cat ./src/main/kotlin/upbrella/be/lent/config/property/AppleOauthUser.kt
</function_calls>
<function_calls>
#!/bin/bash
Check remaining files
echo "=== AppleOauthInfo ==="
cat ./src/main/kotlin/upbrella/be/lent/config/property/AppleOauthInfo.kt
echo ""
echo "=== GoogleOauthInfo ==="
cat ./src/main/kotlin/upbrella/be/lent/config/property/GoogleOauthInfo.kt 2>/dev/null || echo "Not found"
</function_calls>
<function_calls>
#!/bin/bash
Check for any other oauth info classes
find ./src/main/kotlin -name "OauthInfo" -o -name "oauthInfo*" -i
</function_calls>
<function_calls>
#!/bin/bash
Check OauthProperty constructor details
cat ./src/main/kotlin/upbrella/be/lent/config/property/OauthProperty.kt
</function_calls>
Let me check if there's frontend URL configuration:
<function_calls>
#!/bin/bash
Search for frontend URL or redirect URL configuration
grep -rn "frontend|redirectUri|redirect.url|frontendUrl" ./src/main/kotlin -i | head -20
</function_calls>
<function_calls>
#!/bin/bash
Search in the entire codebase for the hardcoded URL
grep -rn "upbrella.co.kr" ./src --include=".kt" --include=".yml" --include=".yaml" --include=".properties"
</function_calls>
<function_calls>
#!/bin/bash
Search in test files as well to understand the patterns
find ./src -name "*.kt" -type f | xargs grep -l "upbrella.co.kr" | head -10
</function_calls>
Let me search for any other OAuth implementation in the repo for comparison:
<function_calls>
#!/bin/bash
Search for Google OAuth or Kakao OAuth
grep -rn "google.*callback|kakao.*callback|google.*login|kakao.*login" ./src/main/kotlin -i
</function_calls>
Let me check the full UserController again for Google callback:
<function_calls>
#!/bin/bash
Show lines around googleLoginCallback
sed -n '130,160p' ./src/main/kotlin/upbrella/be/user/controller/UserController.kt | cat -n
</function_calls>
Now let me search for any CSRF or state validation mechanism in the entire codebase:
<function_calls>
#!/bin/bash
Search for any CSRF or state-related code
grep -rn "state" ./src/main/kotlin/upbrella/be -i -A 2 -B 2 | head -100
</function_calls>
<function_calls>
#!/bin/bash
Check if there's any validation of the state parameter anywhere
rg "state.*validate|validate.*state|state.*check|check.*state" -i -t kt
</function_calls>
Let me check the README or documentation:
<function_calls>
#!/bin/bash
Check README or other documentation
find . -name "README*" -o -name "*.md" | head -10
</function_calls>
Let me look for any environment-specific configuration:
<function_calls>
#!/bin/bash
Find all yaml/properties files
find . -name ".yml" -o -name ".yaml" -o -name "*.properties" | head -20
</function_calls>
<function_calls>
#!/bin/bash
Try to find any application configuration
find ./src/main/resources -type f | head -20
</function_calls>
Let me check if there's a pom.xml or build.gradle:
<function_calls>
#!/bin/bash
Check for build files
ls -la | grep -E "pom.xml|build.gradle|settings.gradle"
</function_calls>
Let me check the OauthLoginService more carefully to see if it validates state:
<function_calls>
#!/bin/bash
Get the full OauthLoginService file with wc first
wc -l ./src/main/kotlin/upbrella/be/lent/service/OauthLoginService.kt
</function_calls>
<function_calls>
#!/bin/bash
View the full service file
cat -n ./src/main/kotlin/upbrella/be/lent/service/OauthLoginService.kt
</function_calls>
Perfect! Now let me verify the specific issues and create a summary. Let me also check if there are any tests that show how state should be handled:
<function_calls>
#!/bin/bash
Find test files related to OAuth
find ./src/test -name "OAuth" -o -name "oauth" -o -name "Apple" | head -10
</function_calls>
<function_calls>
#!/bin/bash
Find test files for UserController
find ./src/test -name "UserController"
</function_calls>
<function_calls>
#!/bin/bash
Find all test files
find ./src/test -name "*.kt" -type f | head -10
</function_calls>
Let me now verify the exact line numbers in the code to ensure accuracy:
<function_calls>
#!/bin/bash
Get the exact appleLoginCallback method with line numbers
sed -n '92,130p' ./src/main/kotlin/upbrella/be/user/controller/UserController.kt | cat -n
</function_calls>
Ensure CSRF state parameter validation is implemented, remove unused request parameters, and externalize hard-coded redirect URLs in OAuth callbacks
The review identifies legitimate issues that I've confirmed throughout the codebase:
-
Missing state validation: The
stateparameter is accepted at line 98 but never validated. Searching the entire codebase reveals no state persistence insessionor validation logic anywhere. TheOauthLoginService.getOauthToken()andprocessAppleLogin()methods do not use or verify state. This is a genuine CSRF vulnerability—implement state parameter storage during login initiation and verification in the callback before processing the authorization code. -
Unused request parameters: The
id_tokenanduserparameters (line 96) are not referenced in the method body; the actualid_tokencomes from the OAuth token response viaoauthLoginService.getOauthToken()at line 108. Remove these unused parameters from the signature to avoid confusion about which values the method actually uses. -
Hard-coded frontend URLs: Lines 106, 110, 116, 122, and 125 all hard-code
https://upbrella.co.krfor redirects. No configuration property exists in the codebase for frontend base URL. The same pattern appears ingoogleLoginCallback(lines 132+). Inject a configurablefrontendUrlproperty to support local, dev, and stage environments.
🤖 Prompt for AI Agents
In src/main/kotlin/upbrella/be/user/controller/UserController.kt around lines
92-130, the appleLoginCallback method accepts but never validates the state
parameter (CSRF risk), includes unused request params id_token and user, and
uses hard-coded frontend redirect URLs; fix by implementing state validation:
store a generated state in session when initiating Apple login and compare it to
the incoming state before exchanging the code, remove id_token and user from the
method signature (use the id token from oauthLoginService.getOauthToken), and
replace hard-coded https://upbrella.co.kr redirect URLs with an injected
configurable frontendBaseUrl (read from application properties) and use that for
RedirectView targets.
| return try { | ||
| Jwts.parserBuilder() | ||
| .setSigningKey(publicKey) | ||
| .build() | ||
| .parseClaimsJws(idToken) | ||
| .body | ||
| } catch (e: Exception) { | ||
| throw IllegalArgumentException("Invalid ID token: ${e.message}", e) | ||
| } |
There was a problem hiding this comment.
Missing issuer and audience validation — security risk.
The JWT verification only validates the signature but does not verify the iss (issuer) and aud (audience) claims. Per Apple's Sign in with Apple guidelines, you must verify:
issequalshttps://appleid.apple.comaudmatches your app's client_id
Without these checks, an attacker could use a valid Apple token issued for a different application.
Apply this diff to add proper claim validation:
- return try {
- Jwts.parserBuilder()
- .setSigningKey(publicKey)
- .build()
- .parseClaimsJws(idToken)
- .body
- } catch (e: Exception) {
- throw IllegalArgumentException("Invalid ID token: ${e.message}", e)
- }
+ return try {
+ Jwts.parserBuilder()
+ .setSigningKey(publicKey)
+ .requireIssuer("https://appleid.apple.com")
+ .build()
+ .parseClaimsJws(idToken)
+ .body
+ } catch (e: Exception) {
+ throw IllegalArgumentException("Invalid ID token: ${e.message}", e)
+ }Additionally, inject the expected clientId and validate the audience claim after parsing:
val aud = claims["aud"] as? String
if (aud != expectedClientId) {
throw IllegalArgumentException("Invalid audience: $aud")
}
🟢 구현내용
🧩 고민과 해결과정
Summary by CodeRabbit
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.