Sign in with Apple: web (Auth.js) + native iOS bridge (#286)#340
Merged
Conversation
Implement Auth.js Apple provider with JWT client_secret minting from APPLE_* env vars, shared oauth_accounts resolution with Hide My Email–friendly sub-first linking, POST /api/auth/apple-native with JWKS verification for native tokens, Apple button on AuthScreen, privacy policy link, mobile docs, and iOS SignInWithAppleButton wiring. Closes #286 Co-authored-by: Bretton Auerbach <auerbachb@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds native iOS "Sign in with Apple" end-to-end: iOS UI and payload builder, shared DTOs and API client, backend POST /api/auth/apple-native with Apple JWKS/JWT verification, Apple client-secret minting, OAuth user-resolution/linking, tests, docs, privacy/UI updates, and middleware allowlist entry. ChangesApple Sign-in Implementation (Web + Native iOS)
Sequence DiagramsequenceDiagram
actor User as iOS User
participant App as iOS App
participant ASAuth as ASAuthorizationController
participant Backend as Backend (/api/auth/apple-native)
participant Apple as Apple JWKS
participant DB as Database
User->>App: Tap "Sign in with Apple"
App->>ASAuth: Request authorization (email/fullName)
ASAuth->>App: ASAuthorizationAppleIDCredential (identityToken, authorizationCode, user)
App->>App: AppleSignInController.nativeSignInRequest(...) → payload
App->>Backend: POST /api/auth/apple-native { identityToken, authorizationCode, user? }
Backend->>Apple: Fetch JWKS / validate token (issuer/aud)
Apple-->>Backend: JWKS
Backend->>Backend: jose.jwtVerify → extract sub, email, email_verified
Backend->>DB: resolveOAuthUserId(provider: "apple", providerAccountId: sub, email, profile)
DB-->>Backend: userId / user record
Backend->>Backend: create token, set Set-Cookie: sp_token
Backend-->>App: { user, token } + sp_token cookie
App->>App: APIClient.signInWithApple saves token to AuthTokenStore
App-->>User: Navigate to app (logged in)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📋 Issue PlannerBuilt with CodeRabbit's Coding Plans for faster development and fewer bugs. View plan used: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 4/5 reviews remaining, refill in 12 minutes. Comment |
Owner
Author
|
@CodeAnt-AI review |
|
CodeAnt AI is running the review. |
Owner
Author
|
@graphite-app re-review |
codeant-ai
Bot
added
the
size:XL
Sequence DiagramThis diagram shows the new native iOS Sign in with Apple flow, where the app sends Apple's identity token to a dedicated backend endpoint that verifies it, resolves or creates the user, and returns a session token. sequenceDiagram
participant User
participant IOSApp as iOS App
participant Backend
participant AppleID as Apple identity service
User->>IOSApp: Tap Sign in with Apple
IOSApp->>IOSApp: Request Apple credential and build native request body
IOSApp->>Backend: POST apple-native with identity token and user info
Backend->>AppleID: Verify identity token via JWKS and claims
AppleID-->>Backend: Validated sub and email
Backend->>Backend: Resolve or create user and link Apple identity
Backend-->>IOSApp: Return user and auth token and set sp_token cookie
IOSApp->>IOSApp: Save token and mark user as logged in
Generated by CodeAnt AI |
The ASAuthorization button’s UIKit hosting can break XCUITest hit-testing and layout for email/password on CI simulators. Gate the control on SP_UI_TEST_MODE so production still shows Apple sign-in; document the behavior in mobile-oauth-integration.md. Co-authored-by: Bretton Auerbach <auerbachb@users.noreply.github.com>
|
User cursoragent@cursor.com does not have a PR Review subscription. Go to Team management and add this email to the PR Review subscription. |
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
|
CodeAnt AI finished running the review. |
![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
🧹 Nitpick comments (1)
src/lib/auth.ts (1)
6-63: ⚡ Quick winUse the exported cookie name as the single source of truth.
src/middleware.tsand the Apple-native test still hardcode"sp_token", so this new export only helps if those call sites switch over too.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/auth.ts` around lines 6 - 63, The exported SP_TOKEN_COOKIE constant is not being used elsewhere, so replace hardcoded "sp_token" usages with the single source of truth: import SP_TOKEN_COOKIE from the auth module and use it wherever cookies or headers are read/written (e.g., in your middleware and the Apple-native test), including in code that sets/deletes cookies (functions like setAuthCookie/clearAuthCookie) and any direct cookie.get or cookie.delete calls and assertions in tests; search for the literal "sp_token" and swap it to SP_TOKEN_COOKIE so all cookie operations reference the exported constant.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.example:
- Around line 71-76: The Apple-related env vars are out of alphabetical order
and trigger dotenv-linter UnorderedKey warnings; reorder the keys AUTH_APPLE_ID,
APPLE_KEY_ID, APPLE_PRIVATE_KEY, APPLE_TEAM_ID (or alphabetically: APPLE_KEY_ID,
APPLE_PRIVATE_KEY, APPLE_TEAM_ID, AUTH_APPLE_ID) so they are grouped in key
order—update the block containing AUTH_APPLE_ID, APPLE_TEAM_ID, APPLE_KEY_ID,
APPLE_PRIVATE_KEY to the correct alphabetical sequence to satisfy dotenv-linter.
In `@ios/StillPointApp/ViewModels/AuthViewModel.swift`:
- Around line 56-61: Replace the Apple-specific in-flight gate with a shared
auth-in-flight flag so all auth flows are mutually exclusive: add a single Bool
(e.g., isAuthInFlight) to the AuthViewModel, change signInWithApple(using:) to
guard and set/defer that shared flag instead of isAppleSignInInFlight, and
update the email/password sign-in method (the existing signIn.../submit method)
to use the same isAuthInFlight guard/set/defer and clear error/resetMessage the
same way; remove or stop using isAppleSignInInFlight so both flows cannot run
concurrently and both respect the same in-flight lifecycle.
In `@src/app/api/auth/apple-native/route.ts`:
- Around line 58-66: The current token validation block rejects tokens missing
payload.email, preventing returning Apple users whose subsequent tokens omit
email; change the logic in the route (the emailClaim/email/email_verified checks
around payload) to make email optional: do not return 401 when email is absent,
only normalize and set email when present and verified (keep the existing
trim/toLowerCase and email_verified check when email exists). Update calls to
resolveOAuthUserId to accept an optional email parameter and ensure
resolveOAuthUserId (and its signature) first attempts to find an existing
account by provider='apple' and provider_account_id=payload.sub before requiring
an email for account creation or linking, so only creation paths validate
email/email_verified.
In `@src/lib/apple-client-secret.ts`:
- Around line 5-7: The CACHE_MAX_MS value currently caches the Apple client JWT
longer than its actual lifetime (SECRET_TTL_SECONDS), causing expired client
secrets; update the caching logic in src/lib/apple-client-secret.ts to compute
cache TTL from the token expiry (use SECRET_TTL_SECONDS or parse the JWT exp)
instead of the hardcoded 140-day window—replace CACHE_MAX_MS with a derived
value like SECRET_TTL_SECONDS * 1000 (or calculate ms until exp) and ensure any
functions that read/write the cached secret (e.g., the get/createClientSecret or
cache handling around lines referenced 49-57) use that expiry-derived TTL so the
secret is invalidated before Apple rejects it.
In `@src/lib/oauth-user-resolution.ts`:
- Around line 122-127: The function resolveOAuthUserId currently requires email
in its params which prevents handling repeat Apple sign-ins that only provide
providerAccountId (`sub`); change the params signature to make email optional
(email?: string) and update any internal checks to only require email when it’s
needed (e.g., when creating a new user or linking an account), while still first
resolving by provider and providerAccountId (`provider`, `providerAccountId`)
before falling back to email-based flows; update usages/call sites of
resolveOAuthUserId and any validation logic that assumed email was always
present so that Apple repeat sign-ins succeed without an email.
---
Nitpick comments:
In `@src/lib/auth.ts`:
- Around line 6-63: The exported SP_TOKEN_COOKIE constant is not being used
elsewhere, so replace hardcoded "sp_token" usages with the single source of
truth: import SP_TOKEN_COOKIE from the auth module and use it wherever cookies
or headers are read/written (e.g., in your middleware and the Apple-native
test), including in code that sets/deletes cookies (functions like
setAuthCookie/clearAuthCookie) and any direct cookie.get or cookie.delete calls
and assertions in tests; search for the literal "sp_token" and swap it to
SP_TOKEN_COOKIE so all cookie operations reference the exported constant.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 89d89ad7-3a43-4b11-82a9-88e5b5f9b8b2
📒 Files selected for processing (19)
.env.exampledocs/mobile-oauth-integration.mdios/StillPoint.xcodeproj/project.pbxprojios/StillPointApp/Services/AppleSignInController.swiftios/StillPointApp/StillPoint.entitlementsios/StillPointApp/ViewModels/AuthViewModel.swiftios/StillPointApp/Views/AuthView.swiftios/StillPointShared/Sources/StillPointShared/APIClient.swiftios/StillPointShared/Sources/StillPointShared/DTOs/DTOs.swiftsrc/app/api/auth/apple-native/route.test.tssrc/app/api/auth/apple-native/route.tssrc/app/privacy/page.tsxsrc/components/AuthScreen.tsxsrc/db/schema.tssrc/lib/apple-client-secret.tssrc/lib/auth-config.tssrc/lib/auth.tssrc/lib/oauth-user-resolution.tssrc/middleware.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ios/StillPointApp/Views/AuthView.swift`:
- Around line 76-77: In the Sign in with Apple failure branch update the handler
to inspect the provided error (e.g., case .failure(let error)) and ignore
cancellation errors instead of setting vm.error; specifically cast to
ASAuthorizationError and if (error as? ASAuthorizationError)?.code == .canceled
return/do nothing, otherwise assign vm.error = "Sign in with Apple failed." (or
the existing message) so only real failures surface; look for the failure case
around the SignInWithAppleButton/authorization result and modify that branch
accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a41c4410-b27b-4015-ac7f-601064d86b18
📒 Files selected for processing (2)
docs/mobile-oauth-integration.mdios/StillPointApp/Views/AuthView.swift
✅ Files skipped from review due to trivial changes (1)
- docs/mobile-oauth-integration.md
- iOS: single isAuthInFlight for email/password + Apple; ignore ASAuthorization user cancel; wire submit + Apple button to shared gate - apple-native: allow JWT without email for returning users (sub link); OAuthEmailRequiredError for first sign-in; no untrusted body email - oauth-user-resolution: optional email after existing link check - auth-config: Google/Apple require email_verified; try/catch Apple secret mint at init so bad .p8 does not break Google - apple-client-secret: cache by JWT expiry with refresh skew - .env.example: dotenv-linter key order; docs: token email behavior Co-authored-by: Bretton Auerbach <auerbachb@users.noreply.github.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/app/api/auth/apple-native/route.test.ts (1)
46-68: ⚡ Quick winConsider adding tests for JWT verification failures and email_verified=false.
The current suite covers happy paths and the
OAuthEmailRequiredErrorcase. Additional tests forjwtVerifythrowing (invalid/expired token) andemail_verified: falserejection would strengthen coverage of error paths.📋 Suggested test cases
test("returns 401 when jwtVerify throws", async () => { jwtVerify.mockRejectedValueOnce(new Error("invalid signature")); const { POST } = await import("./route"); const req = { json: async () => ({ identityToken: "bad.token.sig" }), } as unknown as NextRequest; const res = await POST(req); expect(res.status).toBe(401); }); test("returns 403 when email_verified is false", async () => { jwtVerify.mockResolvedValue({ payload: { sub: "apple-sub-unverified", email: "test@example.com", email_verified: false, }, }); const { POST } = await import("./route"); const req = { json: async () => ({ identityToken: "tok" }), } as unknown as NextRequest; const res = await POST(req); expect(res.status).toBe(403); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/auth/apple-native/route.test.ts` around lines 46 - 68, Add two tests to src/app/api/auth/apple-native/route.test.ts covering JWT verification failures and unverified emails: (1) mock jwtVerify to reject (e.g., mockRejectedValueOnce(new Error(...))) and call the POST handler from "./route" with a request containing an invalid identityToken to assert the response status is 401; (2) mock jwtVerify to resolve with payload.email_verified = false (and a unique sub/email), call POST with a valid-looking identityToken and assert the response status is 403. Use the existing jwtVerify and POST references in the test file and follow the same request shape used elsewhere (json method on the NextRequest-like object).src/lib/oauth-user-resolution.ts (1)
80-113: 💤 Low valueConsider logging username retry exhaustion for observability.
When all retries are exhausted (line 112), this throws without any logging. For rare but real contention scenarios, a warning log before throwing would help diagnose production issues.
🔧 Suggested improvement
+ console.warn(`OAuth user creation: exhausted ${MAX_USERNAME_RETRIES} username retries for email ${email}`); throw new Error("Failed to allocate a unique username after retries");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/oauth-user-resolution.ts` around lines 80 - 113, The createUserWithUsernameRetry function currently throws after MAX_USERNAME_RETRIES without logging; add a warning log just before the final throw that records the email, seed, number of attempts (MAX_USERNAME_RETRIES + 1 or attempts variable), and any relevant context (e.g., that username allocation exhausted retries) using the same logger used elsewhere in this module (e.g., processLogger or the existing logger instance) at warning level to aid observability; keep the log concise and then rethrow the same Error("Failed to allocate a unique username after retries").
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/app/api/auth/apple-native/route.test.ts`:
- Around line 46-68: Add two tests to
src/app/api/auth/apple-native/route.test.ts covering JWT verification failures
and unverified emails: (1) mock jwtVerify to reject (e.g.,
mockRejectedValueOnce(new Error(...))) and call the POST handler from "./route"
with a request containing an invalid identityToken to assert the response status
is 401; (2) mock jwtVerify to resolve with payload.email_verified = false (and a
unique sub/email), call POST with a valid-looking identityToken and assert the
response status is 403. Use the existing jwtVerify and POST references in the
test file and follow the same request shape used elsewhere (json method on the
NextRequest-like object).
In `@src/lib/oauth-user-resolution.ts`:
- Around line 80-113: The createUserWithUsernameRetry function currently throws
after MAX_USERNAME_RETRIES without logging; add a warning log just before the
final throw that records the email, seed, number of attempts
(MAX_USERNAME_RETRIES + 1 or attempts variable), and any relevant context (e.g.,
that username allocation exhausted retries) using the same logger used elsewhere
in this module (e.g., processLogger or the existing logger instance) at warning
level to aid observability; keep the log concise and then rethrow the same
Error("Failed to allocate a unique username after retries").
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: afa23f22-bc57-420e-aded-a23c2ed48027
📒 Files selected for processing (9)
.env.exampledocs/mobile-oauth-integration.mdios/StillPointApp/ViewModels/AuthViewModel.swiftios/StillPointApp/Views/AuthView.swiftsrc/app/api/auth/apple-native/route.test.tssrc/app/api/auth/apple-native/route.tssrc/lib/apple-client-secret.tssrc/lib/auth-config.tssrc/lib/oauth-user-resolution.ts
✅ Files skipped from review due to trivial changes (2)
- .env.example
- src/lib/apple-client-secret.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/api/auth/apple-native/route.ts
|
CodeAnt AI is running the review. |
codeant-ai
Bot
added
size:XL
Sequence DiagramThis PR adds a native iOS Sign in with Apple flow where the app obtains an Apple identity token, the backend verifies it, and shared oauth user resolution links or creates accounts before issuing the standard auth token. sequenceDiagram
participant User
participant iOSApp
participant AppleIdentity
participant AuthBackend
participant Database
User->>iOSApp: Tap Sign in with Apple
iOSApp->>AppleIdentity: Request Apple authorization
AppleIdentity-->>iOSApp: Return identity token with Apple sub and optional email
iOSApp->>AuthBackend: Send apple-native request with identity token and user payload
AuthBackend->>AppleIdentity: Verify identity token via JWKS and audience
AuthBackend->>Database: Resolve or create user by Apple sub and email
Database-->>AuthBackend: User record and id
AuthBackend-->>iOSApp: Return user and auth token, set sp_token cookie
iOSApp->>iOSApp: Save token and mark user as logged in
Generated by CodeAnt AI |
| 17A248000000000000000002 /* AppBlockingManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppBlockingManager.swift; sourceTree = "<group>"; }; | ||
| 17A248000000000000000004 /* AppBlockingSettingsView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppBlockingSettingsView.swift; sourceTree = "<group>"; }; | ||
| 17A248000000000000000005 /* StillPoint.entitlements */ = {isa = PBXFileReference; lastKnownFileType = text.plist.entitlements; path = StillPoint.entitlements; sourceTree = "<group>"; }; | ||
| 17A248000000000000000007 /* AppleSignInController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppleSignInController.swift; sourceTree = "<group>"; }; |
There was a problem hiding this comment.
Suggestion: The project now references AppleSignInController.swift in the file references and sources build phase, but that file is not present in the repository. Xcode will fail the build with a missing input/source file error. Add the actual file at the referenced path or remove these project entries until the file exists. [possible bug]
Severity Level: Critical 🚨
- ❌ StillPoint iOS target fails to compile.
- ❌ CI builds for iOS app will be broken.
- ⚠️ Developers blocked from running iOS app locally.Steps of Reproduction ✅
1. Open the iOS project at `ios/StillPoint.xcodeproj` in Xcode; note the PBXFileReference
for `AppleSignInController.swift` at `ios/StillPoint.xcodeproj/project.pbxproj:54`, where
the file is declared with `path = AppleSignInController.swift`.
2. Observe that `AppleSignInController.swift` is also added to the Sources build phase at
`ios/StillPoint.xcodeproj/project.pbxproj:328` as `17A248000000000000000008 /*
AppleSignInController.swift in Sources */`, meaning it is treated as a required
compilation unit.
3. Check the repository filesystem for the referenced file; a glob search for
`**/AppleSignInController.swift` under `/workspace` returns no matches, confirming that
`AppleSignInController.swift` does not exist anywhere in the repo.
4. Build the StillPoint iOS target in Xcode; during compilation, Xcode attempts to load
`AppleSignInController.swift` from the project but fails with a "file not found" / missing
source file error, causing the iOS target build to fail.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** ios/StillPoint.xcodeproj/project.pbxproj
**Line:** 54:54
**Comment:**
*Possible Bug: The project now references `AppleSignInController.swift` in the file references and sources build phase, but that file is not present in the repository. Xcode will fail the build with a missing input/source file error. Add the actual file at the referenced path or remove these project entries until the file exists.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+50
to
+53
| const { payload } = await jwtVerify(identityToken, APPLE_JWKS, { | ||
| issuer: ISSUER, | ||
| audience: defaultIOSAudience(), | ||
| }); |
There was a problem hiding this comment.
Suggestion: The identity token is accepted without validating a nonce, so any valid Apple ID token for this audience can be replayed within its validity window to create a session. Bind the request to a client-generated nonce (and verify the token's nonce claim) before issuing app auth tokens. [security]
Severity Level: Critical 🚨
- ❌ Native iOS Apple sign-in tokens replayable during validity.
- ⚠️ Weakens Apple identity assurance and OIDC best practices.Steps of Reproduction ✅
1. On native iOS, user taps Sign in with Apple; `AuthViewModel.signInWithApple(...)` at
`ios/StillPointApp/ViewModels/AuthViewModel.swift:56-71` calls
`APIClient.shared.signInWithApple(request)`.
2. `APIClient.signInWithApple` at
`ios/StillPointShared/Sources/StillPointShared/APIClient.swift:138-143` POSTs the
`AppleNativeSignInRequest` (containing `identityToken`) to `POST /api/auth/apple-native`
without any nonce field.
3. The request reaches the public route `/api/auth/apple-native`, which is explicitly
exempted from JWT middleware in `src/middleware.ts:6-11,35-49`, and is handled by `POST()`
in `src/app/api/auth/apple-native/route.ts:34-118`.
4. Inside `POST()`, the server verifies the `identityToken` with `jwtVerify(identityToken,
APPLE_JWKS, { issuer: ISSUER, audience: defaultIOSAudience() })` at `route.ts:50-53` but
never checks a `nonce` claim or any client-supplied nonce; any party who has captured this
still-valid Apple identity token can replay it in a new `POST /api/auth/apple-native` body
and the handler will again pass verification, resolve the same user via
`resolveOAuthUserId` at `src/lib/oauth-user-resolution.ts:132-178`, and issue a fresh app
auth token via `createToken` at `route.ts:99-109`.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/app/api/auth/apple-native/route.ts
**Line:** 50:53
**Comment:**
*Security: The identity token is accepted without validating a nonce, so any valid Apple ID token for this audience can be replayed within its validity window to create a session. Bind the request to a client-generated nonce (and verify the token's nonce claim) before issuing app auth tokens.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| params.delete("error"); | ||
| const search = params.toString(); | ||
| const callbackUrl = `${window.location.pathname}${search ? `?${search}` : ""}`; | ||
| void signIn("apple", { callbackUrl }); |
There was a problem hiding this comment.
Suggestion: The Apple sign-in button is always shown, but the backend only enables the Apple provider when Apple env vars are configured; in environments where it is disabled, this click path always fails with a configuration error. Gate rendering of this button behind the same provider-availability condition used by auth configuration. [api mismatch]
Severity Level: Major ⚠️
- ⚠️ Web Apple button broken when backend Apple disabled.
- ⚠️ Confusing UX; users see configuration error on click.Steps of Reproduction ✅
1. Configure and run the Next.js app without Apple env vars (`AUTH_APPLE_ID`,
`APPLE_TEAM_ID`, `APPLE_KEY_ID`, `APPLE_PRIVATE_KEY`), so `appleEnvReady` is false in
`src/lib/auth-config.ts:27-31`.
2. In this configuration, `appleClientSecretForWeb` remains `undefined`, so the Apple
provider is omitted from the NextAuth providers array at `auth-config.ts:43-59`, meaning
server-side Auth.js has no `apple` provider configured.
3. Visit the web login page that renders `AuthScreen` (component defined in
`src/components/AuthScreen.tsx:22-371`); the Apple button is always rendered via the JSX
block at `AuthScreen.tsx:168-206` regardless of provider availability.
4. Click "Continue with Apple"; the `onClick` handler at `AuthScreen.tsx:170-176` calls
`void signIn("apple", { callbackUrl });`, which invokes NextAuth's `/api/auth/signin` flow
with `provider=apple`, but since the `apple` provider is not registered server-side,
Auth.js returns a configuration error (mapped by `OAUTH_ERROR_MESSAGES.Configuration` at
`AuthScreen.tsx:10-20`), so this UI path always fails in such environments.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/components/AuthScreen.tsx
**Line:** 175:175
**Comment:**
*Api Mismatch: The Apple sign-in button is always shown, but the backend only enables the Apple provider when Apple env vars are configured; in environments where it is disabled, this click path always fails with a configuration error. Gate rendering of this button behind the same provider-availability condition used by auth configuration.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+73
to
+75
| if (!isEmailVerifiedForProvider(provider, profile as { email_verified?: string | boolean })) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Suggestion: The new verification gate rejects Apple logins whenever email_verified is absent, but Apple can omit email-related claims on subsequent sign-ins. That breaks repeat sign-in for already-linked Apple accounts even though user resolution is designed to work from provider sub alone; only enforce email verification when an email is actually being used for new-account creation/link-by-email. [api mismatch]
Severity Level: Critical 🚨
- ❌ Apple web OAuth sign-in fails for repeat logins.
- ❌ /api/auth/oauth-complete bridge never issues sp_token.
- ⚠️ Users must fall back to password-based authentication.Steps of Reproduction ✅
1. A user completes an initial Apple OAuth sign-in via the web Auth screen
(`src/components/AuthScreen.tsx` lines 69-107), which calls `signIn("apple", { callbackUrl
})` and creates an `oauthAccounts` row for (`provider="apple"`, `providerAccountId=sub`)
via `resolveOAuthUserId` in `src/lib/oauth-user-resolution.ts` lines 132-178.
2. On a subsequent Apple web sign-in (same Apple ID, e.g., from another browser), the Auth
screen again invokes `signIn("apple")` (`src/components/AuthScreen.tsx` line 76), which
routes to the Auth.js handler at `src/app/api/auth/[...nextauth]/route.ts:1` re-exporting
`handlers` from `src/lib/auth-config.ts`.
3. The `signIn` callback in `src/lib/auth-config.ts` lines 64-88 executes for provider
`"apple"`. If Apple omits email-related claims on this repeat sign-in (documented behavior
mirrored in the native flow comment at `src/lib/oauth-user-resolution.ts` lines 119-131),
`profile.email_verified` is `undefined` even though an `oauthAccounts` link already exists
for this (`provider`, `providerAccountId`) pair.
4. At `src/lib/auth-config.ts` line 73, `isEmailVerifiedForProvider("apple", profile as {
email_verified?: string | boolean })` returns `false` because it requires `ev === true ||
ev === "true"` for Apple (lines 13-24), so the `signIn` callback immediately returns
`false` (lines 73-75) without calling `resolveOAuthUserId`. The `/api/auth/[...nextauth]`
flow terminates as a failed sign-in, and the bridge at
`src/app/api/auth/oauth-complete/route.ts` lines 27-44 never sees a valid session,
blocking repeat Apple web logins even though `resolveOAuthUserId` is explicitly designed
to resolve users by (`provider`, `providerAccountId`) alone when an existing
`oauthAccounts` row is present.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/lib/auth-config.ts
**Line:** 73:75
**Comment:**
*Api Mismatch: The new verification gate rejects Apple logins whenever `email_verified` is absent, but Apple can omit email-related claims on subsequent sign-ins. That breaks repeat sign-in for already-linked Apple accounts even though user resolution is designed to work from provider `sub` alone; only enforce email verification when an email is actually being used for new-account creation/link-by-email.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
|
CodeAnt AI finished running the review. |
|
CodeAnt AI is running the review. |
codeant-ai
Bot
added
size:XL
Sequence DiagramThis PR adds Sign in with Apple for the web via Auth.js and for native iOS via a dedicated Apple native endpoint, both sharing a common user resolution flow that links Apple identities to Still Point accounts and issues the standard session token. sequenceDiagram
participant User
participant WebApp
participant IOSApp
participant Apple
participant AuthBackend
participant Database
User->>WebApp: Click Continue with Apple
WebApp->>AuthBackend: Request web Apple sign in
AuthBackend->>Apple: Redirect user to Apple and receive profile
AuthBackend->>Database: Resolve user by Apple sub and email, issue session
User->>IOSApp: Tap Sign in with Apple
IOSApp->>Apple: Perform native Apple sign in
IOSApp->>AuthBackend: Send identity token and optional user info
AuthBackend->>Database: Verify token, resolve user, mint token and set cookie
Generated by CodeAnt AI |
Comment on lines
+66
to
+68
| } catch { | ||
| return NextResponse.json({ error: "Invalid identity token" }, { status: 401 }); | ||
| } |
There was a problem hiding this comment.
Suggestion: This catch block converts every verification-time failure into 401 Invalid identity token, including non-auth failures like JWKS fetch/network errors. That misclassifies server-side outages as client auth failures and will incorrectly reject valid sign-ins. Handle JWT validation errors separately and return a 5xx for key-fetch or infrastructure failures. [incorrect condition logic]
Severity Level: Major ⚠️
- ⚠️ Apple JWKS outages misreported as invalid credentials.
- ⚠️ Native iOS sign-in fails during server network issues.
- ⚠️ Difficult to distinguish user error from infrastructure failure.Steps of Reproduction ✅
1. Start the backend with the PR code and ensure it cannot reach Apple's JWKS endpoint
(for example, by running in an environment without outbound internet or blocking
`https://appleid.apple.com/auth/keys`), so `createRemoteJWKSet(new
URL(\"https://appleid.apple.com/auth/keys\"))` at
`src/app/api/auth/apple-native/route.ts:9` will encounter network errors when used.
2. On an iOS device, complete a valid Sign in with Apple flow so that
`AppleSignInController.nativeSignInRequest` at
`ios/StillPointApp/Services/AppleSignInController.swift:9-35` produces an
`AppleNativeSignInRequest` containing a real Apple `identityToken`, and
`APIClient.signInWithApple` at
`ios/StillPointShared/Sources/StillPointShared/APIClient.swift:138-143` sends `POST
/api/auth/apple-native` with this body (as documented in
`docs/mobile-oauth-integration.md:7-21`).
3. The backend handler `POST` in `src/app/api/auth/apple-native/route.ts:34-68` attempts
to verify the token with `jwtVerify(identityToken, APPLE_JWKS, { issuer: ISSUER, audience:
defaultIOSAudience() })` at lines 50-53; due to the JWKS/network failure this call throws
before any payload validation.
4. The `catch` block at `src/app/api/auth/apple-native/route.ts:66-68` catches this
infrastructure error and unconditionally returns `401` with `{ error: "Invalid identity
token" }`, causing the client to see an authentication failure for a valid identity token
instead of a `5xx` indicating a server-side outage.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/app/api/auth/apple-native/route.ts
**Line:** 66:68
**Comment:**
*Incorrect Condition Logic: This catch block converts every verification-time failure into `401 Invalid identity token`, including non-auth failures like JWKS fetch/network errors. That misclassifies server-side outages as client auth failures and will incorrectly reject valid sign-ins. Handle JWT validation errors separately and return a 5xx for key-fetch or infrastructure failures.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+21
to
23
| if (provider === "apple") { | ||
| return ev === true || ev === "true"; | ||
| } |
There was a problem hiding this comment.
Suggestion: The Apple branch rejects sign-in unless email_verified is explicitly present and true, but Apple repeat authorizations may omit email-related claims. That makes existing linked Apple users fail sign-in even though identity (sub) is valid. For Apple, only enforce email verification when an email claim is present/needed for linking, not on every callback. [incorrect condition logic]
Severity Level: Critical 🚨
- ❌ Returning Apple web sign-ins rejected when email_verified claim missing.
- ⚠️ OAuth bridge /api/auth/oauth-complete never issues sp_token.Steps of Reproduction ✅
1. A user clicks the Apple sign-in button in `src/components/AuthScreen.tsx:69-77`, which
calls `signIn("apple", { callbackUrl })` and sends the browser through the Auth.js Apple
OAuth flow handled by `src/app/api/auth/[...nextauth]/route.ts:1-3` using the NextAuth
config from `src/lib/auth-config.ts`.
2. After Apple redirects back, Auth.js invokes the `signIn` callback at
`src/lib/auth-config.ts:64-87` with `account.provider === "apple"`,
`account.providerAccountId` set from Apple `sub`, and a `profile` object where `email` is
present but `email_verified` is omitted or not strictly `true` (a scenario the native path
already anticipates by treating email/email_verified as optional in
`src/app/api/auth/apple-native/route.ts:58-65`).
3. Inside the callback, `rawEmail` is derived from `profile.email` at
`src/lib/auth-config.ts:66-67`, then `isEmailVerifiedForProvider(provider, profile)` is
called at `73`, which for `provider === "apple"` returns `false` whenever
`profile.email_verified` is anything other than `true` or `"true"` due to the branch at
`21-22`.
4. Because the `signIn` callback immediately returns `false` when
`isEmailVerifiedForProvider` returns false (`src/lib/auth-config.ts:73-75`), Auth.js
denies the sign-in and never reaches the `/api/auth/oauth-complete` bridge in
`src/app/api/auth/oauth-complete/route.ts:18-27`, so even users with an existing
`oauth_accounts` link for this Apple `sub` are rejected whenever Apple omits or does not
assert `email_verified` on a repeat web authorization.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/lib/auth-config.ts
**Line:** 21:23
**Comment:**
*Incorrect Condition Logic: The Apple branch rejects sign-in unless `email_verified` is explicitly present and true, but Apple repeat authorizations may omit email-related claims. That makes existing linked Apple users fail sign-in even though identity (`sub`) is valid. For Apple, only enforce email verification when an email claim is present/needed for linking, not on every callback.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
| var isSubmitting = false | ||
| var isRequestingPasswordReset = false | ||
| /// True while any sign-in path (email/password or Apple) is in flight. | ||
| var isAuthInFlight = false |
There was a problem hiding this comment.
Suggestion: The new in-flight auth guard uses mutable shared state in a non-actor-isolated view model, so concurrent tasks can race on isAuthInFlight and both enter auth flows before the flag is observed. This can trigger overlapping login/Apple requests and inconsistent UI state updates; isolate this view model on @MainActor (or convert to an actor) so the check/set sequence is serialized. [race condition]
Severity Level: Major ⚠️
- ❌ Overlapping login requests from AuthView hit auth endpoints.
- ⚠️ UI error state races between concurrent auth completions.Steps of Reproduction ✅
1. Launch the iOS app so that `AuthView` (ios/StillPointApp/Views/AuthView.swift:5-8) is
shown; this view owns `@State private var vm = AuthViewModel()` (line 7), using
`AuthViewModel` from ios/StillPointApp/ViewModels/AuthViewModel.swift:4-15.
2. In `AuthView`, tap the primary auth button twice in quick succession; each tap executes
the `Button` at ios/StillPointApp/Views/AuthView.swift:119-137, which starts a new `Task {
if let user = await vm.submit() { appVM.didLogin(user:) } }`, calling
`AuthViewModel.submit()` concurrently.
3. In `AuthViewModel.submit()` (ios/StillPointApp/ViewModels/AuthViewModel.swift:27-53),
both async tasks execute the guard and flag sequence `guard isValid, !isAuthInFlight else
{ return nil }; isAuthInFlight = true; defer { isAuthInFlight = false }` on the
non-actor-isolated property `var isAuthInFlight = false` (line 14); because
`AuthViewModel` is only `@Observable` and not `@MainActor` or an actor, the two tasks can
race and both observe `isAuthInFlight == false` before either write is applied.
4. As a result of the data race, both tasks proceed into the network calls
(`APIClient.shared.signup` / `login` at AuthViewModel.swift:35-44) or, similarly, `Task {
if let user = await vm.signInWithApple(using: body) }` in `AuthView`
(AuthView.swift:71-75) calling `signInWithApple(using:)` (AuthViewModel.swift:56-71),
allowing overlapping login or Apple sign-in requests and racing updates to `vm.error` and
`isAuthInFlight`, which this suggestion proposes to prevent by isolating the view model on
`@MainActor` or converting it to an actor.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** ios/StillPointApp/ViewModels/AuthViewModel.swift
**Line:** 14:14
**Comment:**
*Race Condition: The new in-flight auth guard uses mutable shared state in a non-actor-isolated view model, so concurrent tasks can race on `isAuthInFlight` and both enter auth flows before the flag is observed. This can trigger overlapping login/Apple requests and inconsistent UI state updates; isolate this view model on `@MainActor` (or convert to an actor) so the check/set sequence is serialized.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+168
to
+170
| private var isUiTestMode: Bool { | ||
| ProcessInfo.processInfo.environment["SP_UI_TEST_MODE"] == "1" | ||
| } |
There was a problem hiding this comment.
Suggestion: The UI-test gate checks SP_UI_TEST_MODE with an exact "1" comparison, but the rest of the app treats this flag as truthy ("1", "true", "yes", "on"). If CI/test launch sets "true" (or another truthy value), Apple sign-in will still render and can reintroduce the simulator hit-testing break this change is trying to avoid. Reuse the same truthy parsing logic used by UI-test config. [incorrect condition logic]
Severity Level: Major ⚠️
- ❌ Apple sign-in button visible in some UI test runs.
- ⚠️ XCTest login interactions become flaky on CI simulators.Steps of Reproduction ✅
1. Configure the app to run in UI-test fixture mode by setting `SP_UI_TEST_MODE` to a
non-"1" truthy value such as `"true"` in the launch environment (for example via Xcode
scheme or test runner); the shared UITestConfig parser in
ios/StillPointShared/Sources/StillPointShared/APIClient.swift:748-765 calls
`truthy(env["SP_UI_TEST_MODE"])` with `truthy` defined at lines 762-765 to treat `"1"`,
`"true"`, `"yes"`, and `"on"` as enabling UI test mode.
2. Start the app so that `UITestConfig.fromProcessInfo()` (APIClient.swift:748-760)
returns a non-nil config, putting the network layer into UI test mode, and note that other
views such as `RootView.isUITestMode` (ios/StillPointApp/Views/RootView.swift:159-163) and
`CompletionView.isUITestMode` (ios/StillPointApp/Views/CompletionView.swift:335-340) also
use a local `truthy` helper on `SP_UI_TEST_MODE`, thus treating `"true"` as UI test mode.
3. Navigate to `AuthView` (ios/StillPointApp/Views/AuthView.swift:5-10), which defines
`private var isUiTestMode: Bool { ProcessInfo.processInfo.environment["SP_UI_TEST_MODE"]
== "1" }` at lines 168-170, so with `SP_UI_TEST_MODE="true"` this property incorrectly
evaluates to `false` even though the rest of the UI-test plumbing considers the app to be
in test mode.
4. Observe that the Apple sign-in UI remains visible because the `SignInWithAppleButton`
block is guarded by `if !isUiTestMode` (AuthView.swift:55-90); in this misconfigured but
supported truthy case, the app is running with UI test fixtures (APIClient's
`uiTestConfig` is active) yet the Apple button still renders, reintroducing the
hit-testing instability in CI that this UI-test gate is intended to avoid, which
consistent truthy parsing for `isUiTestMode` would prevent.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** ios/StillPointApp/Views/AuthView.swift
**Line:** 168:170
**Comment:**
*Incorrect Condition Logic: The UI-test gate checks `SP_UI_TEST_MODE` with an exact `"1"` comparison, but the rest of the app treats this flag as truthy (`"1"`, `"true"`, `"yes"`, `"on"`). If CI/test launch sets `"true"` (or another truthy value), Apple sign-in will still render and can reintroduce the simulator hit-testing break this change is trying to avoid. Reuse the same truthy parsing logic used by UI-test config.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
|
CodeAnt AI finished running the review. |
|
CodeAnt AI is running the review. |
codeant-ai
Bot
added
size:XL
Sequence DiagramThis diagram shows the new native iOS Sign in with Apple path where the app exchanges an Apple identity token with the backend, which verifies it, resolves or creates the user via shared OAuth linking, and returns an auth token and cookie. sequenceDiagram
participant User
participant iOSApp
participant Apple
participant Backend
participant Database
User->>iOSApp: Tap Sign in with Apple
iOSApp->>Apple: Start native Apple sign in
Apple-->>iOSApp: Return identity token and profile
iOSApp->>Backend: POST apple-native with identity token
Backend->>Apple: Verify token with Apple keys
Backend->>Database: Resolve or create user for Apple account
Database-->>Backend: User record
Backend-->>iOSApp: User and auth token, set sp_token cookie
Generated by CodeAnt AI |
Comment on lines
+28
to
+32
| guard isValid, !isAuthInFlight else { return nil } | ||
| isAuthInFlight = true | ||
| error = nil | ||
| resetMessage = nil | ||
| defer { isSubmitting = false } | ||
| defer { isAuthInFlight = false } |
There was a problem hiding this comment.
Suggestion: isAuthInFlight is used as a concurrency guard, but this view model is not actor-isolated, so two async auth tasks can read false and both set it to true concurrently. That allows overlapping sign-in requests and inconsistent UI state. Isolate this state on @MainActor (or another actor/lock) so the check-and-set is serialized. [race condition]
Severity Level: Major ⚠️
- ❌ AuthView can issue duplicate login or signup requests.
- ⚠️ UI single-flight guard can be bypassed under contention.
- ⚠️ Backend may see concurrent Apple-native sign-in attempts.Steps of Reproduction ✅
1. Open the iOS app and navigate to the auth screen, which uses `AuthView` at
`ios/StillPointApp/Views/AuthView.swift:5-10` and creates `@State private var vm =
AuthViewModel()` at line 7.
2. Observe that the email/password submit button and the Sign in with Apple button both
trigger asynchronous tasks that call into `AuthViewModel` (`vm.submit()` at
`AuthView.swift:119-123`, `vm.signInWithApple(using:)` at `AuthView.swift:71-73`).
3. Note that `AuthViewModel` is a plain `@Observable` class without `@MainActor` isolation
(`AuthViewModel.swift:4-5`), and that the concurrency guard uses `isAuthInFlight` around
network calls (`submit()` at `AuthViewModel.swift:27-54`, `signInWithApple(using:)` at
`AuthViewModel.swift:56-71`), with the non-atomic check-and-set sequence `guard ...
!isAuthInFlight` / `isAuthInFlight = true` / `defer { isAuthInFlight = false }` at lines
28-32 and 57-61.
4. On a real device or simulator, tap the Sign in with Apple button (or the email/password
"Enter" button) twice in quick succession so that two independent `Task { ... }` blocks
are created (`AuthView.swift:71-75` or `AuthView.swift:119-124`); because `AuthViewModel`
is not actor-isolated, these tasks can execute `submit()`/`signInWithApple(using:)`
concurrently on different threads, both reading `isAuthInFlight == false` before either
write occurs, allowing both to set `isAuthInFlight = true` and proceed to call
`APIClient.shared.login`/`signup`/`signInWithApple` in parallel, resulting in overlapping
auth requests and inconsistent UI state despite the intended single-flight guard.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** ios/StillPointApp/ViewModels/AuthViewModel.swift
**Line:** 28:32
**Comment:**
*Race Condition: `isAuthInFlight` is used as a concurrency guard, but this view model is not actor-isolated, so two async auth tasks can read `false` and both set it to `true` concurrently. That allows overlapping sign-in requests and inconsistent UI state. Isolate this state on `@MainActor` (or another actor/lock) so the check-and-set is serialized.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+79
to
+84
| const userId = await resolveOAuthUserId({ | ||
| provider, | ||
| providerAccountId, | ||
| email, | ||
| profile: { name: typeof profile.name === "string" ? profile.name : null }, | ||
| }); |
There was a problem hiding this comment.
Suggestion: The new OAuth resolution call is wired as if an email is always present, but Apple web sign-ins can omit email on subsequent logins. This breaks returning Apple users because the sign-in callback path still depends on a non-empty email before resolution. Update the sign-in flow to allow email to be undefined for Apple and let resolveOAuthUserId resolve by (provider, providerAccountId) first. [api mismatch]
Severity Level: Critical 🚨
- ❌ Returning Apple web users cannot sign in.
- ❌ Existing Apple-linked users blocked from web access.
- ⚠️ Inconsistent behavior vs native `apple-native` route.Steps of Reproduction ✅
1. Enable the Apple web provider by setting `AUTH_APPLE_ID`, `APPLE_TEAM_ID`,
`APPLE_KEY_ID`, and `APPLE_PRIVATE_KEY`, which makes `appleEnvReady` truthy and mints a
client secret in `src/lib/auth-config.ts:27-38`, causing the Apple provider to be included
in `providers` at `src/lib/auth-config.ts:51-58`.
2. From the web app, open `/app` which renders `AuthScreen` via `AuthScreen` usage in
`src/app/app/page.tsx:5` and `:16`, then click the "Continue with Apple" button wired to
`void signIn("apple", { callbackUrl })` in `src/components/AuthScreen.tsx:168-176`,
initiating the Auth.js Apple OAuth flow.
3. On the user's first Apple web sign-in, Apple returns `profile.email` and `profile.sub`;
the `signIn` callback in `src/lib/auth-config.ts:64-88` runs, passes the email and
`(provider, providerAccountId)` into `resolveOAuthUserId` at
`src/lib/auth-config.ts:79-84`, which in turn creates or links a user account based on
`(provider, providerAccountId)` and email as implemented in
`src/lib/oauth-user-resolution.ts:132-178`, and sign-in succeeds.
4. On a subsequent Apple sign-in for the same Apple ID (Apple's documented behavior is to
omit `email` after initial consent), Auth.js's Apple provider passes a `profile` where
`profile.email` is missing; the `signIn` callback at `src/lib/auth-config.ts:64-68`
computes `rawEmail` as `null` and immediately returns `false` at line 67, so
`resolveOAuthUserId` at lines 79-84 is never called, even though it is explicitly designed
to resolve by existing `(provider, providerAccountId)` links first (see
`resolveOAuthUserId` JSDoc at `src/lib/oauth-user-resolution.ts:128-131`), resulting in
returning Apple users being rejected from the web login flow instead of being resolved by
their existing `oauth_accounts` link.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/lib/auth-config.ts
**Line:** 79:84
**Comment:**
*Api Mismatch: The new OAuth resolution call is wired as if an email is always present, but Apple web sign-ins can omit email on subsequent logins. This breaks returning Apple users because the sign-in callback path still depends on a non-empty email before resolution. Update the sign-in flow to allow `email` to be undefined for Apple and let `resolveOAuthUserId` resolve by `(provider, providerAccountId)` first.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+100
to
+109
| const res = NextResponse.json({ | ||
| user: { | ||
| id: user.id, | ||
| email: user.email, | ||
| username: user.username, | ||
| isPublic: user.isPublic, | ||
| currentDay: user.currentDay, | ||
| }, | ||
| token, | ||
| }); |
There was a problem hiding this comment.
Suggestion: This endpoint always includes the JWT in the JSON response body, unlike the other auth routes that only return token for the iOS client header. Returning it unconditionally exposes a bearer token to any browser JavaScript caller and weakens the HttpOnly-cookie boundary. Match the existing auth contract by only including token for trusted native clients (or omit it entirely and rely on the cookie). [security]
Severity Level: Critical 🚨
- ❌ Apple native auth returns bearer token to all callers.
- ⚠️ Browser JavaScript can read JWT despite HttpOnly cookie.Steps of Reproduction ✅
1. Inspect the email/password login handler at `src/app/api/auth/login/route.ts:11-22`,
where `includeToken` is derived from `request.headers.get("x-still-point-client") ===
"ios"` and only then used to include `token` in the JSON (`...(includeToken ? { token } :
{}),` at lines 53-61).
2. Inspect the signup handler at `src/app/api/auth/signup/route.ts:9-12`, which uses the
same `includeToken` pattern and conditionally spreads `{ token }` into the JSON response
(`...(includeToken ? { token } : {}),` at lines 70-80), confirming the existing auth
contract: web callers get only `user`, iOS-native callers (with `x-still-point-client:
ios`) get `user + token`.
3. Inspect the new Apple native route at `src/app/api/auth/apple-native/route.ts:99-109`,
where after creating the JWT via `createToken`, the response is constructed as:
- `const res = NextResponse.json({ user: { ... }, token, });`
with no header-based gating and `token` always present in the JSON payload.
4. Inspect the middleware configuration at `src/middleware.ts:6-24`, where
`/api/auth/apple-native` is listed in `publicExactPaths`, meaning requests to this
endpoint bypass JWT auth and always reach the handler.
5. Run the app and, from any JavaScript-capable client (e.g., browser `fetch` or Postman
script), send `POST /api/auth/apple-native` with a valid Apple `identityToken` in the JSON
body (as in test setup `route.test.ts:31-37`, where the body only needs `identityToken` /
`authorizationCode`). Once `jwtVerify` passes and `resolveOAuthUserId` returns, the
handler at `route.ts:99-109` responds with JSON containing both `user` and `token`.
6. In that JavaScript client, call `await res.json()` on the response and observe that
`json.token` holds the same bearer token that is also being set as an HttpOnly cookie via
`res.cookies.set(SP_TOKEN_COOKIE, token, ...)` at
`src/app/api/auth/apple-native/route.ts:110-116`, meaning any caller (not just trusted
native clients) can read the JWT directly from the JSON body, unlike the other auth routes
which only expose `token` when explicitly flagged as `x-still-point-client: ios`.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/app/api/auth/apple-native/route.ts
**Line:** 100:109
**Comment:**
*Security: This endpoint always includes the JWT in the JSON response body, unlike the other auth routes that only return `token` for the iOS client header. Returning it unconditionally exposes a bearer token to any browser JavaScript caller and weakens the HttpOnly-cookie boundary. Match the existing auth contract by only including `token` for trusted native clients (or omit it entirely and rely on the cookie).
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
|
CodeAnt AI finished running the review. |
|
CodeAnt AI is running the review. |
codeant-ai
Bot
added
size:XL
Sequence DiagramThis PR adds a native iOS Sign in with Apple path that verifies Apple identity tokens on the backend, reuses shared OAuth account resolution, and returns a JWT plus sp_token cookie for the logged-in user. sequenceDiagram
participant User
participant iOS App
participant Apple ID
participant Auth API
participant Database
User->>iOS App: Tap Sign in with Apple
iOS App->>Apple ID: Request Apple credentials
Apple ID-->>iOS App: Return identity token and user details
iOS App->>Auth API: POST apple-native with identity token
Auth API->>Apple ID: Verify identity token with JWKS keys
Apple ID-->>Auth API: Validated claims with sub and optional email
Auth API->>Database: Resolve or create user for Apple sub and email
Database-->>Auth API: User record
Auth API-->>iOS App: User and auth token (sets sp_token cookie)
Generated by CodeAnt AI |
Comment on lines
+57
to
+61
| guard !isAuthInFlight else { return nil } | ||
| isAuthInFlight = true | ||
| error = nil | ||
| resetMessage = nil | ||
| defer { isAuthInFlight = false } |
There was a problem hiding this comment.
Suggestion: isAuthInFlight is being used as a concurrency gate from async methods, but this model is not actor-isolated, so concurrent Task executions can read/write it at the same time and both proceed. This can allow duplicate auth requests and nondeterministic UI state. Isolate the view model to @MainActor (or make it an actor) so these state mutations are serialized. [race condition]
Severity Level: Major ⚠️
- ❌ iOS login can send overlapping auth requests.
- ⚠️ Sign-in buttons' disabled state may flicker or desync.
- ⚠️ Potential data races on error/reset UI state.Steps of Reproduction ✅
1. Open `AuthView` in the iOS app (`ios/StillPointApp/Views/AuthView.swift:5-10`), which
creates a single shared view model instance via `@State private var vm = AuthViewModel()`.
2. Trigger the email/password login path by tapping the primary button; this runs the
`Button` action at `AuthView.swift:119-124`, which launches `Task { if let user = await
vm.submit() { ... } }` without any actor isolation on `AuthViewModel`.
3. Inside that task, `AuthViewModel.submit()` (`AuthViewModel.swift:27-54`) executes
`guard isValid, !isAuthInFlight else { return nil }` and then mutates `isAuthInFlight` and
other properties (`error`, `resetMessage`) on a non-actor-isolated class instance shared
with the SwiftUI view.
4. At the same time, SwiftUI reads `vm.isAuthInFlight` on the main actor to drive
`.disabled` / `.opacity` for both the Apple button (`AuthView.swift:88-89`) and the submit
button (`AuthView.swift:135-137`), and can also launch another task via the Apple path
(`Task { if let user = await vm.signInWithApple(using: body) ... }` at
`AuthView.swift:71-75`) which calls `signInWithApple(using:)`
(`AuthViewModel.swift:56-71`) and reuses the same `isAuthInFlight` guard and mutations.
Because `AuthViewModel` is neither `@MainActor` nor an `actor`, these concurrent `Task`
executions and main-actor view updates can read/write `isAuthInFlight` and related state
at the same time, allowing overlapping auth flows and data races on this flag.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** ios/StillPointApp/ViewModels/AuthViewModel.swift
**Line:** 57:61
**Comment:**
*Race Condition: `isAuthInFlight` is being used as a concurrency gate from async methods, but this model is not actor-isolated, so concurrent `Task` executions can read/write it at the same time and both proceed. This can allow duplicate auth requests and nondeterministic UI state. Isolate the view model to `@MainActor` (or make it an actor) so these state mutations are serialized.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+168
to
+176
| <button | ||
| type="button" | ||
| onClick={() => { | ||
| const params = new URLSearchParams(window.location.search); | ||
| params.delete("error"); | ||
| const search = params.toString(); | ||
| const callbackUrl = `${window.location.pathname}${search ? `?${search}` : ""}`; | ||
| void signIn("apple", { callbackUrl }); | ||
| }} |
There was a problem hiding this comment.
Suggestion: The Apple sign-in button is always rendered, but Apple auth is conditionally disabled in server config when required env vars are missing. In those environments the button is dead and consistently sends users into a provider configuration error path. Gate rendering of this button behind the same availability check (or fetch provider availability from the server). [incomplete implementation]
Severity Level: Major ⚠️
- ❌ Web Apple login button leads to configuration error.
- ⚠️ Users see broken Apple sign-in in some deployments.Steps of Reproduction ✅
1. Ensure Apple env vars are absent so `appleEnvReady` is false in
`src/lib/auth-config.ts:27-31`, leaving `appleClientSecretForWeb` undefined and the Apple
provider array entry omitted at `src/lib/auth-config.ts:51-58`.
2. Start the app and navigate to `/app`, which renders the `StillPoint` component in
`src/app/app/page.tsx:58-399`; when `user` is null at `src/app/app/page.tsx:386-395`, it
renders `<AuthScreen onLogin={handleLogin} />`.
3. On the Auth screen, observe the Apple button defined at
`src/components/AuthScreen.tsx:168-205`, which always renders and whose `onClick` handler
at `src/components/AuthScreen.tsx:170-175` calls `void signIn("apple", { callbackUrl });`.
4. Click "Continue with Apple"; NextAuth's `signIn("apple")` call cannot find a configured
`apple` provider (because it was not added to `providers` when `appleClientSecretForWeb`
is undefined), so Auth.js redirects back to the error page `/app` (configured at
`src/lib/auth-config.ts:60-62`) with an error query (e.g. `error=Configuration`),
resulting in a visible "Sign-in is temporarily unavailable." message from
`OAUTH_ERROR_MESSAGES` at `src/components/AuthScreen.tsx:10-20` and a consistently
non-functional Apple sign-in button in environments where Apple is not configured.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/components/AuthScreen.tsx
**Line:** 168:176
**Comment:**
*Incomplete Implementation: The Apple sign-in button is always rendered, but Apple auth is conditionally disabled in server config when required env vars are missing. In those environments the button is dead and consistently sends users into a provider configuration error path. Gate rendering of this button behind the same availability check (or fetch provider availability from the server).
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+51
to
+58
| ...(appleClientSecretForWeb | ||
| ? [ | ||
| Apple({ | ||
| clientId: process.env.AUTH_APPLE_ID!, | ||
| clientSecret: appleClientSecretForWeb, | ||
| }), | ||
| ] | ||
| : []), |
There was a problem hiding this comment.
Suggestion: The Apple provider was added, but the existing sign-in flow still requires profile.email before user resolution. Apple commonly omits email on subsequent authorizations, so returning Apple users will be rejected before resolveOAuthUserId can match by providerAccountId. Make email mandatory only for providers that always supply it, or allow Apple through when providerAccountId is present. [api mismatch]
Severity Level: Critical 🚨
- ❌ Returning Apple web logins can be consistently rejected.
- ⚠️ Apple users may lose access to existing accounts.Steps of Reproduction ✅
1. Configure Apple Sign In so `appleEnvReady` is true at `src/lib/auth-config.ts:27-31`,
allowing `getAppleClientSecret()` at `src/lib/auth-config.ts:35-37` to set
`appleClientSecretForWeb` and register the Apple provider in the `providers` array at
`src/lib/auth-config.ts:51-58`.
2. A user signs in with Apple via the web button in
`src/components/AuthScreen.tsx:168-205`, which calls `signIn("apple", { callbackUrl })` at
`src/components/AuthScreen.tsx:170-175`; Auth.js invokes the `callbacks.signIn` handler in
`src/lib/auth-config.ts:64-88`.
3. On this first sign-in, Apple supplies `profile.email`, so `rawEmail` at
`src/lib/auth-config.ts:66` is a string; `isEmailVerifiedForProvider()` at
`src/lib/auth-config.ts:13-25` passes for `provider === "apple"`, and
`resolveOAuthUserId()` at `src/lib/auth-config.ts:79-84` creates a user and stores a
`(provider, providerAccountId)` link as implemented in
`src/lib/oauth-user-resolution.ts:132-179`.
4. On a subsequent Apple authorization for the same user, Apple omits `email` from the
profile (the repo's own comment in `OAuthEmailRequiredError` at
`src/lib/oauth-user-resolution.ts:119-131` notes this behavior for repeat Apple sign-ins),
so `rawEmail` becomes null at `src/lib/auth-config.ts:66`; the guard `if (!rawEmail)
return false;` at `src/lib/auth-config.ts:67` aborts before `resolveOAuthUserId()` can
reuse the existing `(provider, providerAccountId)` link, causing Auth.js to reject the
sign-in and redirect back to `/app` with an error even though `providerAccountId` uniquely
identifies the returning Apple user.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/lib/auth-config.ts
**Line:** 51:58
**Comment:**
*Api Mismatch: The Apple provider was added, but the existing sign-in flow still requires `profile.email` before user resolution. Apple commonly omits email on subsequent authorizations, so returning Apple users will be rejected before `resolveOAuthUserId` can match by `providerAccountId`. Make email mandatory only for providers that always supply it, or allow Apple through when `providerAccountId` is present.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
| return NextResponse.json({ error: "Invalid identity token" }, { status: 401 }); | ||
| } | ||
|
|
||
| const emailForResolution = emailFromToken; |
There was a problem hiding this comment.
Suggestion: The resolver input ignores body.user.email entirely and uses only the JWT email claim, so first-time native Apple sign-ins will fail with "Email required" whenever Apple omits the email claim in the token even though the app sent the first-sign-in email payload. Use a safe fallback path for the first-sign-in email payload when the token email is absent. [incomplete implementation]
Severity Level: Major ⚠️
- ❌ Some new native Apple users receive 400 Email required.
- ⚠️ Native Sign in with Apple onboarding can silently fail.
- ⚠️ Ignores trusted first-sign-in email payload from iOS.Steps of Reproduction ✅
1. On the first native Apple sign-in, `AppleSignInController.nativeSignInRequest` builds
an `AppleNativeSignInRequest` where `user` includes `email` and `name` taken from
`ASAuthorizationAppleIDCredential.email` and `.fullName`
(`ios/StillPointApp/Services/AppleSignInController.swift:20-27`), and this request is sent
via `APIClient.signInWithApple` to `POST /api/auth/apple-native`
(`ios/StillPointShared/Sources/StillPointShared/APIClient.swift:19-25`).
2. The server route `POST` handler at `src/app/api/auth/apple-native/route.ts:34-45`
parses the JSON body and then verifies the `identityToken` using `jwtVerify`
(`route.ts:47-53`), extracting `payload.email` into `emailFromToken` only if the token
includes a non-empty `email` claim and `email_verified` is true (`route.ts:58-63`).
3. When the `identityToken` lacks an `email` claim (while the client has still sent
`user.email` in the body), `emailFromToken` remains `undefined`, and `emailForResolution`
is set to `emailFromToken` (`route.ts:70`), meaning the subsequent call to
`resolveOAuthUserId` passes `email: undefined` despite `body.user.email` being present
(`route.ts:75-82`).
4. Inside `resolveOAuthUserId` (`src/lib/oauth-user-resolution.ts:132-159`), if there is
no existing `(provider, providerAccountId)` link (`existingLink` check at lines 140-153),
the `email` parameter is normalized to an empty string and, because it is falsy,
`OAuthEmailRequiredError` is thrown (`oauth-user-resolution.ts:155-159`), causing the
route to return HTTP 400 `"Email required for first sign-in with this Apple ID"`
(`route.ts:83-88`) even though a valid first-sign-in email was supplied in
`body.user.email` from iOS but ignored by the server.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/app/api/auth/apple-native/route.ts
**Line:** 70:70
**Comment:**
*Incomplete Implementation: The resolver input ignores `body.user.email` entirely and uses only the JWT email claim, so first-time native Apple sign-ins will fail with "Email required" whenever Apple omits the email claim in the token even though the app sent the first-sign-in email payload. Use a safe fallback path for the first-sign-in email payload when the token email is absent.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
|
CodeAnt AI finished running the review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Summary
Adds Sign in with Apple for the web using Auth.js’s built-in Apple provider and a native iOS path via
POST /api/auth/apple-native(JWKS verification — Auth.js does not handle native identity tokens).Account linking uses
oauth_accountswithprovider='apple'andsubasprovider_account_id, with email only for new-user / link-by-email; subsequent sign-ins resolve by Applesub, so Hide My Email relay addresses stay consistent.iOS + XCUITest
The Sign in with Apple control is hidden when
SP_UI_TEST_MODE=1(UI tests) so the system button’s UIKit hosting does not break email/password hit-testing on CI simulators. Production / dev app runs still show the button.Key changes
src/lib/apple-client-secret.ts— ES256 client secret JWT for the Services ID (cached ~5 months)src/lib/oauth-user-resolution.ts— shared resolution logic for Auth.js + native routesrc/lib/auth-config.ts— Apple provider when all fourAUTH_APPLE_ID/APPLE_*vars are setsrc/app/api/auth/apple-native/route.ts— verifyidentityTokenagainst Apple JWKS; optionalAUTH_APPLE_IOS_AUDIENCE(defaults to bundle idcom.brettonauerbach.stillpoint)src/components/AuthScreen.tsx— black Apple button usingsignIn("apple")(Auth.js POST+CSRF contract)docs/mobile-oauth-integration.md— native handshake, Hide My Email caveats, UI test gating note, pointers to Implement Sign in with Apple server-to-server notification endpoint #338 / Register Communication Email Sources with Apple Private Email Relay (Hide My Email deliverability) #339SignInWithAppleButton,AppleSignInController.swift,APIClient.signInWithApple, Sign in with Apple entitlement; UI test gating inAuthViewEnv vars (verify in Vercel Production + local
.env.local; never commit.p8)AUTH_APPLE_ID— Services ID (com.brettonauerbach.stillpoint.web.signin)APPLE_TEAM_ID,APPLE_KEY_ID,APPLE_PRIVATE_KEYTesting performed (automated)
npm run test:unitnpm run buildManual QA (required — agent cannot complete)
gh secret listwas 403 in this environment)coderabbit review --prompt-onlyis not available in the agent environment (binary missing).Closes #286
CodeAnt-AI Description
Add Sign in with Apple on web and native iOS
What Changed
Impact
✅ Faster Apple sign-in on web and iPhone✅ Fewer failed repeat logins for Hide My Email users✅ More reliable CI sign-in tests on iOS🔄 Retrigger CodeAnt AI Review
Details
💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.