Add Google OAuth sign-in via Auth.js (#136)#287
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
CodeAnt AI is reviewing your PR. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Auth.js (NextAuth v5) Google OAuth integration with account-linking, makes password hashes nullable for OAuth-only users, adds DB table and migrations for provider accounts, introduces an oauth-complete bridge to mint the app Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Web Browser
participant App as Next.js App
participant AuthJS as Auth.js
participant Provider as OAuth Provider
participant DB as Database
participant iOS as iOS App
rect rgba(100,150,255,0.5)
Note over Client,AuthJS: Web OAuth flow (Auth.js + oauth-complete bridge)
Client->>App: Click "Continue with Google" -> /api/auth/signin/google
App->>AuthJS: Redirect to Auth.js signin endpoint
AuthJS->>Provider: OAuth handshake (consent)
Provider->>Client: Redirect callback with code
Client->>AuthJS: Callback -> Auth.js exchanges code, runs signIn callback
AuthJS->>DB: Lookup/link (provider, providerAccountId) or email match
DB-->>AuthJS: userId
AuthJS->>AuthJS: Mint Auth.js session (includes userId)
AuthJS->>App: Redirect to /api/auth/oauth-complete?return=...
App->>AuthJS: auth() -> retrieve session
AuthJS-->>App: Session with userId
App->>DB: Verify user exists
DB-->>App: user row
App->>App: createToken(userId,email) -> setAuthCookie(sp_token)
App->>App: clearAuthJsCookies()
App->>Client: Redirect to original return target (sp_token set)
end
rect rgba(150,200,100,0.5)
Note over iOS,App: iOS ASWebAuthenticationSession flow
iOS->>App: Open ASWebAuthenticationSession to /api/auth/signin/google
App->>AuthJS: Same web OAuth handshake
AuthJS->>App: Redirect to /api/auth/oauth-complete
App->>App: setAuthCookie (sp_token) accessible to HTTPCookieStorage.shared
App->>iOS: Redirect to custom URL scheme
iOS->>iOS: Read sp_token from HTTPCookieStorage and use for API requests
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
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 unit tests (beta)
Comment |
codeant-ai
Bot
added
the
size:XL
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
|
CodeAnt AI finished reviewing your PR. |
Owner
Author
|
@cursor review |
Owner
Author
|
@graphite-app re-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: 6
🧹 Nitpick comments (2)
src/app/api/auth/logout/route.ts (1)
5-13: Extract Auth.js cookie cleanup into a shared helper.This duplicates logic already present in
src/app/api/auth/oauth-complete/route.ts. Centralizing the prefix list + delete loop avoids drift and keeps auth-cookie hygiene consistent.Also applies to: 21-26
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/auth/logout/route.ts` around lines 5 - 13, Extract the Auth.js cookie cleanup into a shared helper so both logout and oauth-complete reuse the same logic: move the AUTHJS_COOKIE_PREFIXES array and the cookie deletion loop into a single exported function (e.g., clearAuthJsCookies(req, res) or clearAuthJsCookiesFromHeaders) and replace the duplicate code in route.ts and oauth-complete/route.ts to call that helper; ensure the helper iterates prefixes ("authjs.", "__Secure-authjs.", "__Host-authjs.") and removes cookies (including chunked variants like session-token.0) consistently for the current request/response context.src/middleware.ts (1)
17-28: Tighten these public matches to route boundaries.
startsWith()is right for/api/auth/signin/<provider>and/api/auth/callback/<provider>, but it is broader than intended for exact-only endpoints like/api/auth/sessionand/api/auth/error. As written, a future route such as/api/auth/session-revokewould bypasssp_tokenverification just because it shares the prefix. Split exact-only entries back intopublicExactPaths, and use a segment-aware prefix helper for the two true prefix cases.Suggested tightening
const publicExactPaths = [ "/api/auth/signup", "/api/auth/login", "/api/auth/logout", "/api/auth/google/callback", "/api/auth/oauth-complete", + "/api/auth/csrf", + "/api/auth/session", + "/api/auth/providers", + "/api/auth/error", ]; const publicPrefixPaths = [ "/api/board", "/api/auth/password-reset", - "/api/auth/signin", - "/api/auth/callback", - "/api/auth/csrf", - "/api/auth/session", - "/api/auth/providers", - "/api/auth/error", + "/api/auth/signin", + "/api/auth/callback", ]; + +const hasPathPrefix = (pathname: string, prefix: string) => + pathname === prefix || pathname.startsWith(`${prefix}/`); if ( publicExactPaths.includes(pathname) || - publicPrefixPaths.some((p) => pathname.startsWith(p)) + publicPrefixPaths.some((p) => hasPathPrefix(pathname, p)) ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/middleware.ts` around lines 17 - 28, The current startsWith-based bypass list is too broad; change the public route handling so exact-only endpoints (e.g., "/api/auth/session", "/api/auth/csrf", "/api/auth/providers", "/api/auth/error") are checked via a publicExactPaths set, and only the true prefix routes ("/api/auth/signin" and "/api/auth/callback") are matched with a segment-aware helper (e.g., isSegmentPrefix(path, prefix) that returns true only when path === prefix or path startsWith(prefix + "/")). Update the sp_token bypass logic to first check publicExactPaths for exact equality and then check publicPrefixPaths via isSegmentPrefix so routes like "/api/auth/session-revoke" no longer bypass verification.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/mobile-oauth-integration.md`:
- Around line 50-58: The code blocks showing the HTTP request and JSON body for
the POST /api/auth/apple-native example need fenced-language tags to satisfy
markdownlint MD040: add an HTTP/text (or http) fence to the request line block
containing "POST /api/auth/apple-native" and add a json fence to the request
body block (the block containing "identityToken", "authorizationCode", and
"fullName"); apply the same change to the other occurrence mentioned (lines
84-87) so both the URL/request and the JSON body blocks are explicitly tagged.
In `@README.md`:
- Line 3: The README currently mixes two production host variants
("www.still-point.me" vs "still-point.me"), which can break strict OAuth
redirect URI matching; search for every occurrence of the non-canonical host
string "still-point.me" (examples near Line 85 and Line 208) and replace them
with the canonical "www.still-point.me", ensuring all examples, redirect URIs,
marketing links, and any mention of the production origin consistently use
"www.still-point.me" (also update any plaintext, markdown links, or code
snippets that reference the bare domain).
In `@src/app/api/auth/login/route.ts`:
- Around line 35-43: The SSO-only branch leaks account state by returning a
different error message; change the handler so both the user-not-found and
SSO-only (user.passwordHash falsy) paths return the identical generic 401 JSON
error (e.g., "Invalid email or password") while still performing the dummy
verifyPassword call using verifyPassword(password, DUMMY_PASSWORD_HASH) to keep
timing consistent; keep any provider-specific guidance out of the public
response (you can surface it via a separate, non-enumerating flow or internal
log) and update the NextResponse.json call in the branch that checks
user.passwordHash to match the generic response used for missing users.
In `@src/app/privacy/page.tsx`:
- Around line 192-199: Update the privacy text in the page component where the
paragraph contains the phrase "we receive your verified email address": replace
the absolute wording with a safer phrase such as "we receive your email address
and, when available, its verification status" so the sentence now reads that we
receive the email address and, when available, verification status; modify the
JSX paragraph in the privacy page component (the paragraph containing "we
receive your verified email address") accordingly.
In `@src/lib/auth-config.ts`:
- Around line 91-139: The read-then-insert flow around oauthAccounts/users (the
block that reads existingLink, emailMatch, inserts into users and oauthAccounts
and calls generateUniqueUsername) must be made atomic: wrap that whole
decision-and-create sequence in a database transaction using the same db
instance, and on insert conflicts (unique constraint failures when inserting
into oauthAccounts or users) catch the error and re-query oauthAccounts and
users to obtain the already-created user/link instead of failing; specifically,
perform the logic inside a transaction, attempt inserts for users and
oauthAccounts, and on constraint errors re-read by provider+providerAccountId
(oauthAccounts) and by email (users) to set userId appropriately so concurrent
sign-ins "lose" gracefully and return the existing user rather than throwing.
- Around line 159-164: The redirect handler in async redirect({ url, baseUrl })
unconditionally rewrites to /api/auth/oauth-complete and drops any callbackUrl
(e.g., stillpoint://oauth-complete) from the incoming URL; update this function
to parse the incoming URL's query for callbackUrl and, when present, append it
to the returned /api/auth/oauth-complete URL so the oauth-complete endpoint
receives and can later redirect to the original custom-scheme callback; ensure
the logic still returns the unmodified url if it already starts with
`${baseUrl}/api/auth/oauth-complete` and preserve existing behavior otherwise.
---
Nitpick comments:
In `@src/app/api/auth/logout/route.ts`:
- Around line 5-13: Extract the Auth.js cookie cleanup into a shared helper so
both logout and oauth-complete reuse the same logic: move the
AUTHJS_COOKIE_PREFIXES array and the cookie deletion loop into a single exported
function (e.g., clearAuthJsCookies(req, res) or clearAuthJsCookiesFromHeaders)
and replace the duplicate code in route.ts and oauth-complete/route.ts to call
that helper; ensure the helper iterates prefixes ("authjs.", "__Secure-authjs.",
"__Host-authjs.") and removes cookies (including chunked variants like
session-token.0) consistently for the current request/response context.
In `@src/middleware.ts`:
- Around line 17-28: The current startsWith-based bypass list is too broad;
change the public route handling so exact-only endpoints (e.g.,
"/api/auth/session", "/api/auth/csrf", "/api/auth/providers", "/api/auth/error")
are checked via a publicExactPaths set, and only the true prefix routes
("/api/auth/signin" and "/api/auth/callback") are matched with a segment-aware
helper (e.g., isSegmentPrefix(path, prefix) that returns true only when path ===
prefix or path startsWith(prefix + "/")). Update the sp_token bypass logic to
first check publicExactPaths for exact equality and then check publicPrefixPaths
via isSegmentPrefix so routes like "/api/auth/session-revoke" no longer bypass
verification.
🪄 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: 41cab944-618b-47d0-a65e-3af5e2053da2
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
.env.exampleREADME.mddocs/mobile-oauth-integration.mddrizzle/oauth_accounts_incremental.sqldrizzle/users_nullable_password_incremental.sqlpackage.jsonsrc/app/api/auth/[...nextauth]/route.tssrc/app/api/auth/login/route.tssrc/app/api/auth/logout/route.tssrc/app/api/auth/oauth-complete/route.tssrc/app/privacy/page.tsxsrc/components/AuthScreen.tsxsrc/db/schema.tssrc/lib/auth-config.tssrc/middleware.ts
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 103e475. Configure here.
CodeRabbit + CodeAnt + Cursor BugBot rounds 1: - auth-config.ts: link/create flow is now atomic via INSERT ... ON CONFLICT DO NOTHING on both users (email) and oauth_accounts (provider, providerAccountId), with fallback lookup if a concurrent callback raced ahead. - auth-config.ts: redirect callback no longer swallows Auth.js error redirects (e.g. /app?error=AccessDenied). Errors land on /app and pass through unchanged so AuthScreen renders inline; only success paths route through the sp_token bridge. - auth-config.ts: username collision suffixes use crypto.randomUUID() in the iteration loop too, not just the last-resort fallback. - login/route.ts: OAuth-only accounts return the same generic "Invalid credentials" 401 as wrong-password / unknown-email, with the dummy bcrypt timing path preserved. The friendlier "use SSO" message was an account-enumeration leak. - oauth-complete/route.ts + logout/route.ts: extracted shared clearAuthJsCookies helper to src/lib/authJsCookies.ts; both routes import from the same source. - README.md / privacy/page.tsx / lib/email.ts: canonical production URL unified to https://www.still-point.me (matches the user-stated canonical and the OAuth redirect URIs). - privacy/page.tsx: softened wording on email-verified guarantees. - docs/mobile-oauth-integration.md: code fences carry languages (markdownlint MD040 compliance). iOS smoke lane failure is a pre-existing UI test flake (keyboard focus on auth.passwordField) — no iOS files touched in this PR.
Owner
Author
|
iOS smoke lane ( Failure was in
This is a known iOS Simulator UI test flake pattern — keyboard not fully presented before the test types. This PR touches zero iOS files (no The new commit |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/lib/auth-config.ts (1)
209-223:⚠️ Potential issue | 🟠 MajorDon't drop the original
callbackUrlwhen routing through the bridge.Successful OAuth redirects are always rewritten to
/api/auth/oauth-complete, so a mobile callback likestillpoint://oauth-completenever reaches the bridge. Withsrc/app/api/auth/oauth-complete/route.ts:15-44currently redirecting to/app, the documented mobile flow cannot complete.Possible direction
- return `${baseUrl}/api/auth/oauth-complete`; + const bridgeUrl = new URL("/api/auth/oauth-complete", baseUrl); + const incoming = new URL(url, baseUrl); + const callbackUrl = incoming.searchParams.get("callbackUrl"); + if (callbackUrl) { + bridgeUrl.searchParams.set("callbackUrl", callbackUrl); + } + return bridgeUrl.toString();The bridge endpoint should then read that query param and redirect to it after minting
sp_token, instead of always landing on/app.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/privacy/page.tsx`:
- Around line 119-121: The privacy text in the Privacy page component that
starts with the <strong>Account and profile.</strong> sentence currently uses
exclusive wording ("either a password hash ... or a link to a single sign-on
provider") which incorrectly implies both cannot coexist; update that sentence
to non-exclusive wording such as "a password hash, a link to a single sign-on
provider, or both" (or "may include") so it accurately reflects that linked
accounts can have both a password hash and SSO linkage; modify the JSX fragment
containing that sentence in the page.tsx privacy component accordingly.
In `@src/lib/auth-config.ts`:
- Around line 165-171: The insert can fail when two sign-ins generate the same
username; wrap the insert in a retry loop that calls
generateUniqueUsername(seed) again and retries when the DB returns a
unique-constraint error for the username (e.g., constraint
users_username_lower_unique or the DB-specific unique violation code). Keep the
existing .onConflictDoNothing({ target: users.email }) behavior: if the insert
returns no row because email conflicted, query users by email to return the
existing id; otherwise, on username-unique errors regenerate username and retry
(limit attempts and surface a clear error if exceeded). Update the code around
the generateUniqueUsername call and the db.insert(users)...returning({ id:
users.id }) block to implement this retry logic.
🪄 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: 1db6ffca-bcf2-42a9-bca2-520fc77c63ea
📒 Files selected for processing (9)
README.mddocs/mobile-oauth-integration.mdsrc/app/api/auth/login/route.tssrc/app/api/auth/logout/route.tssrc/app/api/auth/oauth-complete/route.tssrc/app/privacy/page.tsxsrc/lib/auth-config.tssrc/lib/authJsCookies.tssrc/lib/email.ts
✅ Files skipped from review due to trivial changes (3)
- src/lib/email.ts
- docs/mobile-oauth-integration.md
- README.md
🚧 Files skipped from review as they are similar to previous changes (3)
- src/app/api/auth/login/route.ts
- src/app/api/auth/logout/route.ts
- src/app/api/auth/oauth-complete/route.ts
- auth-config.ts: createUserWithUsernameRetry() wraps the new-user insert in a retry loop that catches Postgres unique_violation 23505 on users_username_lower_unique and regenerates the username. The pre-check in generateUniqueUsername() reduces but does not eliminate the race; two concurrent first-time OAuth sign-ins with the same name seed could both pre-check, both pick the same candidate, both attempt the insert. First wins; loser regenerates and retries (max 5 retries). - oauth-complete/route.ts: clearAuthJsCookies() in the finally block is now wrapped in its own try/catch so a thrown error during cleanup does not prevent the NextResponse.redirect from running. Cookie cleanup is best-effort; the redirect must always execute. - privacy/page.tsx: account/profile bullet now reads "any combination of a password hash and links to single sign-on providers" (non-exclusive). A user can validly have both a password and one or more linked providers; the prior "either/or" wording was a policy accuracy gap. Local CR was rate-limited on the final pass; self-reviewed the diff.
|
CodeAnt AI is running Incremental review |
codeant-ai
Bot
added
size:XL
|
CodeAnt AI Incremental review completed. |
|
CodeAnt AI is running the review. |
codeant-ai
Bot
added
size:XL
|
CodeAnt AI finished running the review. |
|
CodeAnt AI is running the review. |
codeant-ai
Bot
added
size:XL
Sequence DiagramThis diagram shows how the web app routes Google sign-in through Auth.js and the oauth-complete bridge to mint the existing app session token while cleaning up Auth.js cookies and redirecting back to the app. sequenceDiagram
participant User
participant WebApp
participant AuthJS
participant Google
participant OAuthBridge
participant AppBackend
User->>WebApp: Click Continue with Google
WebApp->>AuthJS: Start Google OAuth sign-in
AuthJS->>Google: Redirect for OAuth consent
Google-->>AuthJS: Return OAuth callback
AuthJS->>OAuthBridge: Redirect to oauth-complete with session
OAuthBridge->>AuthJS: Read OAuth session for user id
OAuthBridge->>AppBackend: Exchange OAuth session for app token
AppBackend-->>OAuthBridge: Return user info and sp_token
OAuthBridge->>AuthJS: Clear Auth.js cookies
OAuthBridge-->>WebApp: Redirect to app with sp_token cookie
Generated by CodeAnt AI |
|
CodeAnt AI finished running the review. |
|
CodeAnt AI is running the review. |
codeant-ai
Bot
added
size:XL
Sequence DiagramThis diagram shows how Google OAuth via Auth.js v5 is wired into the existing sp_token JWT cookie flow, from the user starting Google sign-in through the OAuth handshake to the final authenticated redirect into the app. sequenceDiagram
participant User
participant WebApp
participant AuthBackend
participant Google
User->>WebApp: Click Continue with Google
WebApp->>AuthBackend: Navigate to Google sign-in route
AuthBackend->>Google: Redirect user to Google consent
Google-->>AuthBackend: Return auth code and profile
AuthBackend->>AuthBackend: Resolve or create user and link OAuth account
AuthBackend->>AuthBackend: Bridge OAuth session to sp_token cookie
AuthBackend-->>User: Redirect to app with sp_token session
Generated by CodeAnt AI |
|
CodeAnt AI finished running the review. |
|
CodeAnt AI is running the review. |
codeant-ai
Bot
added
size:XL
Sequence DiagramThis PR adds Google OAuth sign-in via Auth.js and introduces an oauth-complete bridge that converts the short-lived Auth.js session into the existing sp_token cookie, so the rest of the app continues to rely on the existing auth model. sequenceDiagram
participant User
participant AuthScreen
participant AuthRoutes
participant Backend
User->>AuthScreen: Click Continue with Google
AuthScreen->>AuthRoutes: Navigate to sign in with Google
AuthRoutes->>AuthRoutes: Complete Google OAuth and create session
AuthRoutes->>Backend: Redirect to oauth complete with session and return path
Backend->>Backend: Read session and mint sp_token cookie
Backend->>Backend: Clear temporary auth cookies
Backend-->>User: Redirect to app with sp_token session
Generated by CodeAnt AI |
Comment on lines
+28
to
+29
| CREATE UNIQUE INDEX IF NOT EXISTS "oauth_accounts_provider_account_unique" | ||
| ON "oauth_accounts" ("provider", "provider_account_id"); |
There was a problem hiding this comment.
Suggestion: The uniqueness rule only enforces (provider, provider_account_id) and does not enforce at most one account per provider per user. That allows multiple identities from the same provider to be linked to one local user via email matching, which can lead to incorrect or unsafe account linkage over time (for example when provider emails are reassigned). Add a uniqueness constraint on (user_id, provider) so each user can have only one identity per provider. [security]
Severity Level: Major ⚠️
- ❌ OAuth sign-in (`/api/auth/[...nextauth]`) may mis-link identities.
- ⚠️ Potential account takeover if provider reassigns verified email.Steps of Reproduction ✅
1. Apply the incremental schema in `drizzle/oauth_accounts_incremental.sql` so
`oauth_accounts` is created with only a uniqueness constraint on (`provider`,
`provider_account_id`) and no constraint on (`user_id`, `provider`) (lines 18–32 in
`drizzle/oauth_accounts_incremental.sql`).
2. A user signs in with Google for the first time via `/api/auth/[...nextauth]` (route
defined in `src/app/api/auth/[...nextauth]/route.ts:1–3`, which exports `GET`/`POST` from
`handlers` in `src/lib/auth-config.ts:154`). In the `signIn` callback in
`src/lib/auth-config.ts` (lines 13–67 of the slice starting at `Showing lines 168 to
247`), there is no existing `oauth_accounts` row for this (`provider`,
`providerAccountId`), so `linkProviderToUser()` is called.
3. `linkProviderToUser()` in `src/lib/auth-config.ts` (lines 12–47 of the slice starting
at `Showing lines 40 to 119`) inserts a row into `oauth_accounts` with (`userId`,
`provider`, `providerAccountId`) and uses `ON CONFLICT DO NOTHING` targeting only
(`provider`, `providerAccountId`) (lines 21–27 of that slice). This results in a single
row for this Google identity in `oauth_accounts`, consistent with the mapping described in
`src/db/schema.ts:17–27` and `docs/mobile-oauth-integration.md:13–19`.
4. Later, the identity provider reassigns the same verified email address to a different
Google account (for example, in a Google Workspace or future Microsoft/Facebook/Apple
scenario). When the new account signs in via the same `/api/auth/[...nextauth]` flow, the
`signIn` callback again finds no existing `oauth_accounts` row for its (`provider`,
`providerAccountId`), but the verified email matches the existing `users.email`, so
`linkProviderToUser()` is invoked with the existing `userId`. Because the table schema at
`drizzle/oauth_accounts_incremental.sql:18–32` does not enforce uniqueness on (`user_id`,
`provider`), the second insert for the same `user_id` and provider but a different
`provider_account_id` succeeds, creating multiple same-provider identities for one user.
This contradicts the "one per provider" intent described in `src/db/schema.ts:13–16` and
allows email-based reassignment over time to attach multiple distinct provider identities
for the same provider to a single local account; adding a unique constraint on (`user_id`,
`provider`) would cause that second insertion to fail instead of silently mis-linking.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:** drizzle/oauth_accounts_incremental.sql
**Line:** 28:29
**Comment:**
*Security: The uniqueness rule only enforces `(provider, provider_account_id)` and does not enforce at most one account per provider per user. That allows multiple identities from the same provider to be linked to one local user via email matching, which can lead to incorrect or unsafe account linkage over time (for example when provider emails are reassigned). Add a uniqueness constraint on `(user_id, provider)` so each user can have only one identity per provider.
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
+211
to
+225
| const [emailMatch] = await db | ||
| .select({ id: users.id }) | ||
| .from(users) | ||
| .where(eq(users.email, email)) | ||
| .limit(1); | ||
|
|
||
| let targetUserId: string; | ||
| if (emailMatch) { | ||
| targetUserId = emailMatch.id; | ||
| } else { | ||
| const seed = | ||
| (typeof profile.name === "string" && profile.name) || | ||
| email.split("@")[0] || | ||
| "user"; | ||
| targetUserId = await createUserWithUsernameRetry(email, seed); |
There was a problem hiding this comment.
Suggestion: Auto-linking a new OAuth identity to any existing row found by email creates an account-takeover path because local signup accounts are not email-verified. A malicious user can pre-register someone else's email with password auth, and when the real owner later signs in with Google, this code links that Google identity to the attacker-controlled account. Require a stronger prerequisite before email linking (for example, only link accounts already marked as email-verified, or require explicit authenticated linking from an already logged-in session) instead of unconditional email-match linking. [security]
Severity Level: Critical 🚨
- ❌ Google OAuth mis-links to potentially attacker-created account.
- ❌ Victim Google login reuses attacker's existing user data.
- ⚠️ Attackers can pre-register arbitrary targets' emails.
- ⚠️ No email verification column to distinguish trusted accounts.Steps of Reproduction ✅
1. Create a local password account using someone else's email via the signup API at
`src/app/api/auth/signup/route.ts:9-55`. Send `POST /api/auth/signup` with JSON `{"email":
"victim@example.com", "username": "attackeruser", "password": "AttackerPass123"}`; the
handler lowercases the email (line 31), checks only format/uniqueness, and directly
inserts into `users` with `passwordHash` (lines 47–55) without any email verification
step.
2. Observe the resulting `users` row in `src/db/schema.ts:18-26`, where `email` is unique
and there is no `emailVerified` column; the comment at line 25 notes `passwordHash` is
nullable for OAuth-only accounts, but nothing distinguishes verified vs unverified email
ownership. The attacker now controls a canonical `users` row for `victim@example.com` and
can log in via `POST /api/auth/login` (`src/app/api/auth/login/route.ts:11-52`).
3. Later, the real owner signs in with Google using the Auth.js route `GET
/api/auth/[...nextauth]` wired in `src/app/api/auth/[...nextauth]/route.ts:1-3` to
`handlers` from `src/lib/auth-config.ts`. During the OAuth callback, the
`callbacks.signIn` function at `src/lib/auth-config.ts:168-235` receives the Google
profile, reads `profile.email`, and confirms `email_verified` from the provider (lines
170–178), then normalizes the email with `const email = rawEmail.trim().toLowerCase();`
(line 180).
4. In that same sign-in callback, because the attacker's `users` row already exists for
`victim@example.com`, the code at `src/lib/auth-config.ts:211-219` executes:
```ts
const [emailMatch] = await db
.select({ id: users.id })
.from(users)
.where(eq(users.email, email))
.limit(1);
let targetUserId: string;
if (emailMatch) {
targetUserId = emailMatch.id;
} else {
// ...
}
Since emailMatch is truthy, targetUserId is set to the attacker-created user id.
linkProviderToUser at src/lib/auth-config.ts:55-87 then inserts (provider, providerAccountId, userId) into oauth_accounts, binding the victim's verified Google
identity to the attacker's existing users row.
- After this link, the OAuth bridge endpoint
GET /api/auth/oauth-completeat
src/app/api/auth/oauth-complete/route.ts:27-64callsauth()(line 33) which returns a
session containinguserIdset in the sign-in callback (lines 231–243 of
auth-config.ts), loads that user fromusers(line 39), and mints ansp_tokenfor
thatuser.id(line 43). The victim's subsequent Google sign-ins now consistently
authenticate them into the attacker-createdusersaccount, while the attacker still logs
in via email/password, so both effectively share and can control the same account. This
cross-owner binding arises specifically because any existingusersrow matched by email
(without a local "email verified" or owner-binding check) is auto-linked to the new OAuth
identity.
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20src%2Flib%2Fauth-config.ts%0A%2A%2ALine%3A%2A%2A%20211%3A225%0A%2A%2AComment%3A%2A%2A%0A%09%2ASecurity%3A%20Auto-linking%20a%20new%20OAuth%20identity%20to%20any%20existing%20row%20found%20by%20email%20creates%20an%20account-takeover%20path%20because%20local%20signup%20accounts%20are%20not%20email-verified.%20A%20malicious%20user%20can%20pre-register%20someone%20else%27s%20email%20with%20password%20auth%2C%20and%20when%20the%20real%20owner%20later%20signs%20in%20with%20Google%2C%20this%20code%20links%20that%20Google%20identity%20to%20the%20attacker-controlled%20account.%20Require%20a%20stronger%20prerequisite%20before%20email%20linking%20%28for%20example%2C%20only%20link%20accounts%20already%20marked%20as%20email-verified%2C%20or%20require%20explicit%20authenticated%20linking%20from%20an%20already%20logged-in%20session%29%20instead%20of%20unconditional%20email-match%20linking.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20src%2Flib%2Fauth-config.ts%0A%2A%2ALine%3A%2A%2A%20211%3A225%0A%2A%2AComment%3A%2A%2A%0A%09%2ASecurity%3A%20Auto-linking%20a%20new%20OAuth%20identity%20to%20any%20existing%20row%20found%20by%20email%20creates%20an%20account-takeover%20path%20because%20local%20signup%20accounts%20are%20not%20email-verified.%20A%20malicious%20user%20can%20pre-register%20someone%20else%27s%20email%20with%20password%20auth%2C%20and%20when%20the%20real%20owner%20later%20signs%20in%20with%20Google%2C%20this%20code%20links%20that%20Google%20identity%20to%20the%20attacker-controlled%20account.%20Require%20a%20stronger%20prerequisite%20before%20email%20linking%20%28for%20example%2C%20only%20link%20accounts%20already%20marked%20as%20email-verified%2C%20or%20require%20explicit%20authenticated%20linking%20from%20an%20already%20logged-in%20session%29%20instead%20of%20unconditional%20email-match%20linking.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** src/lib/auth-config.ts
**Line:** 211:225
**Comment:**
*Security: Auto-linking a new OAuth identity to any existing row found by email creates an account-takeover path because local signup accounts are not email-verified. A malicious user can pre-register someone else's email with password auth, and when the real owner later signs in with Google, this code links that Google identity to the attacker-controlled account. Require a stronger prerequisite before email linking (for example, only link accounts already marked as email-verified, or require explicit authenticated linking from an already logged-in session) instead of unconditional email-match linking.
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 how the new Google OAuth flow uses Auth.js to authenticate the user, link or create an account, mint the existing sp_token cookie, and surface any OAuth errors back on the auth screen. sequenceDiagram
participant User
participant WebApp
participant AuthServer
participant Backend
participant Database
User->>WebApp: Click Continue with Google
WebApp->>AuthServer: Start Google sign in with callbackUrl
AuthServer->>Database: Complete Google OAuth, find or create user, link provider account
AuthServer->>Backend: Redirect to oauth-complete with session
Backend->>AuthServer: Read Auth.js session and user id
Backend->>Database: Load user and mint sp_token token
Backend-->>User: Clear Auth.js cookies, set sp_token cookie, redirect to app or error page
WebApp->>WebApp: Read error code from URL and show user friendly message
Generated by CodeAnt AI |
| const store = await cookies(); | ||
| for (const cookie of store.getAll()) { | ||
| if (AUTHJS_COOKIE_PREFIXES.some((p) => cookie.name.startsWith(p))) { | ||
| store.delete(cookie.name); |
There was a problem hiding this comment.
Suggestion: Deleting prefixed Auth.js cookies by name alone can fail for __Secure-/__Host- cookies, because those cookies are only accepted when the Secure/host-prefix constraints are preserved on the clearing Set-Cookie. If the browser rejects the delete header, OAuth session cookies remain and users can get stuck with stale Auth.js state. Clear each matched cookie with explicit attributes (at least path: "/", and secure handling compatible with the request/protocol) so the expiration write is accepted. [logic error]
Severity Level: Critical 🚨
- ❌ OAuth completion may leave Auth.js session cookies uncleared.
- ❌ Logout endpoint may not clear secure Auth.js cookies.
- ⚠️ Users can be stuck in repeated OAuth bridge loops.Steps of Reproduction ✅
1. Configure and run the app so Google OAuth is available; Auth.js is wired to hand off to
the bridge endpoint `/api/auth/oauth-complete` (implementation in
`src/app/api/auth/oauth-complete/route.ts`, lines 1–25) after a successful OAuth callback.
2. Complete a Google OAuth sign-in so Auth.js sets its own cookies, including `authjs.*`,
`__Secure-authjs.*`, or `__Host-authjs.*` (the prefixes explicitly listed in
`AUTHJS_COOKIE_PREFIXES` at `src/lib/authJsCookies.ts:7-10`), and then redirects into the
`/api/auth/oauth-complete` route.
3. In the OAuth bridge route's `finally` block at
`src/app/api/auth/oauth-complete/route.ts:11-21`, `clearAuthJsCookies()` from
`src/lib/authJsCookies.ts:15-22` is called; this function iterates over all cookies and,
for any whose name starts with the Auth.js prefixes, calls `store.delete(cookie.name)` at
line 19 without specifying attributes like `path` or `secure`.
4. For cookies whose names start with `__Secure-authjs.` or `__Host-authjs.`, the browser
enforces prefix rules requiring `Secure`/host constraints on any `Set-Cookie` header;
because `store.delete(cookie.name)` only supplies the name and relies on defaults, the
resulting deletion `Set-Cookie` can be rejected, leaving those Auth.js cookies present. On
a subsequent call to `/api/auth/oauth-complete` or to the logout handler `POST
/api/auth/logout` in `src/app/api/auth/logout/route.ts:5-18` (which also calls
`clearAuthJsCookies()` at line 14), the stale Auth.js cookies remain, causing users to be
treated as still in an Auth.js session and potentially looping through the OAuth bridge
instead of starting from a clean state.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/authJsCookies.ts
**Line:** 19:19
**Comment:**
*Logic Error: Deleting prefixed Auth.js cookies by name alone can fail for `__Secure-`/`__Host-` cookies, because those cookies are only accepted when the `Secure`/host-prefix constraints are preserved on the clearing `Set-Cookie`. If the browser rejects the delete header, OAuth session cookies remain and users can get stuck with stale Auth.js state. Clear each matched cookie with explicit attributes (at least `path: "/"`, and secure handling compatible with the request/protocol) so the expiration write is accepted.
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
| } | ||
|
|
||
| export const { handlers, auth, signIn, signOut } = NextAuth({ | ||
| trustHost: true, |
There was a problem hiding this comment.
Suggestion: Enabling trustHost: true makes Auth.js trust the incoming host headers when deriving baseUrl; if your edge/proxy does not strictly rewrite/validate Host and forwarded host headers, this creates a host-header poisoning risk in OAuth redirects. Use a fixed canonical auth URL (for example via env-configured base URL) or enforce strict trusted-host validation at the edge before trusting request host values. [security]
Severity Level: Major ⚠️
- ❌ OAuth redirect URLs can be generated for attacker-controlled hosts.
- ⚠️ sp_token bridge at `/api/auth/oauth-complete` may run on spoofed host.Steps of Reproduction ✅
1. Incoming OAuth requests are handled by NextAuth handlers at
`src/app/api/auth/[...nextauth]/route.ts:1-3`, which import the `handlers` from
`src/lib/auth-config.ts:154-149`.
2. The NextAuth configuration in `src/lib/auth-config.ts:155` sets `trustHost: true`,
instructing Auth.js to derive its `baseUrl` from the incoming request host headers.
3. Trigger a Google sign-in by calling `GET /api/auth/signin/google` with a spoofed `Host`
header (e.g. `Host: attacker.example`) through infrastructure that does not
normalize/validate the host; this request is routed to the NextAuth handler described in
step 1.
4. Auth.js uses the trusted request host to construct its OAuth authorization and callback
URLs and to drive the redirect callback at `src/lib/auth-config.ts:108-147`, so the
generated redirect/bridge URLs (e.g. to `/api/auth/oauth-complete`) now point at the
attacker-controlled host, demonstrating host-header poisoning when the edge does not
enforce a canonical host.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:** 155:155
**Comment:**
*Security: Enabling `trustHost: true` makes Auth.js trust the incoming host headers when deriving `baseUrl`; if your edge/proxy does not strictly rewrite/validate `Host` and forwarded host headers, this creates a host-header poisoning risk in OAuth redirects. Use a fixed canonical auth URL (for example via env-configured base URL) or enforce strict trusted-host validation at the edge before trusting request host values.
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 Google-based sign in using Auth.js and links provider accounts to users, then bridges the Auth.js session into the existing sp_token cookie so the rest of the app continues to rely on the same JWT-based auth model. sequenceDiagram
participant User
participant AuthScreen
participant AuthBackend
participant OAuthBridge
participant Database
participant Google
User->>AuthScreen: Click Continue with Google
AuthScreen->>AuthBackend: Request Google sign in with callback
AuthBackend->>Google: Redirect user to Google OAuth
Google-->>AuthBackend: Return profile with verified email
AuthBackend->>Database: Find or create user and link provider account
AuthBackend-->>OAuthBridge: Redirect to oauth complete with return path
OAuthBridge->>AuthBackend: Read Auth.js session for user id
OAuthBridge->>Database: Load user and mint sp_token cookie
OAuthBridge-->>AuthScreen: Redirect to app with signed in session
Generated by CodeAnt AI |
|
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
1 participant
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
Wires up Google as a first OAuth sign-in provider, using Auth.js v5 for the OAuth handshake and bridging into the existing
sp_tokenJWT cookie so the rest of the auth model is unchanged. Account linking by verified email;oauth_accountstable records(provider, provider_account_id) -> user_id.users.password_hashis now nullable so OAuth-only accounts are valid.Microsoft, Facebook, and Apple are deferred to #284 / #285 / #286 — this PR exercises the phased-rollout exception in #136's AC. Architecture for all four providers (web + iOS) is documented in
docs/mobile-oauth-integration.mdso the follow-ups have a single spec to land against.Architecture
oauth_accountstable; nullableusers.password_hash. Two*_incremental.sqlfiles for manual apply on existing branches.src/lib/auth-config.ts): Google provider only,signIncallback does the find-or-create + link, with an explicit account-takeover guard (provider identity is resolved BEFORE email match — an existing(provider, providerAccountId)link wins, never gets re-linked to a different user)./api/auth/[...nextauth]: Auth.js catch-all (signin / callback / csrf / providers / session / error)./api/auth/oauth-complete: Auth.js redirects here after a successful OAuth callback. Reads the Auth.js session, mints ansp_tokenJWT (same shape as password login), drops the Auth.js cookies, redirects to/app. Wrapped in try/catch/finally so transient errors redirect to/app?error=...and Auth.js cookies are always cleared (no half-completed sessions).authjs./__Secure-authjs./__Host-authjs.) covers session-token, csrf, callback-url, pkce, state, nonce, and chunked variants in one shot. Used by both oauth-complete and logout./api/auth/oauth-complete(sp_token doesn't exist yet at hand-off time). The existing/api/auth/google/*Calendar-OAuth routes (Partner scheduling: Shared scheduling + auto-add to Google Calendar #204) are NOT touched — different URL segments, no collision.password_hashis null, instead of 500-ing on bcrypt-of-null.?error=...params (e.g.oauth_session_missing) are mapped to friendly inline messages and stripped from the URL after display.https://www.still-point.me/so future OAuth/integration work has one canonical reference.Security
email_verifieddefaults to false when absent on the OIDC profile (Google always sets it; any future provider that omits it will be rejected rather than implicitly trusted).(provider, providerAccountId)is resolved before email match.crypto.randomUUID()(notMath.random()) for collision resistance.next-authpinned to exact5.0.0-beta.31(no caret on beta).sp_tokenis HttpOnly + Secure (in prod) — same posture as before.Phased rollout
Per #136 AC: only Google ships in this PR. The other three providers are filed as #284 (Microsoft), #285 (Facebook), #286 (Apple).
.env.exampledocuments env var names + redirect URIs for all four so the follow-ups are mostly credential-wiring + UI buttons.Closes #136
Test plan
npm run buildpassescoderabbit review --prompt-onlypasses with no findingsoauth_accountsmigration applies cleanly on a fresh DB (idempotent IF NOT EXISTS)users_nullable_passwordmigration applies cleanly (idempotent DROP NOT NULL)npx drizzle-kit pushproduces no schema drift after the migrations are applied/appwhile logged out shows the new "Continue with Google" button above the email/password form/api/auth/signin/google, then to Google's consent screen/api/auth/callback/google→/api/auth/oauth-complete→/appwithsp_tokencookie setGET /api/auth/mereturns the user record after the redirectoauth_accountsrow linking that provider identity to the existing user (no duplicate user created)password_hashIS NULL and one matchingoauth_accountsrow(provider, providerAccountId)is already linked to user A but whose email matches user B resolves to user A (existing link wins; no new row is inserted)password_hash) returns generic 401Invalid credentials(no crash; message hardened during review to avoid enumeration leak)POST /api/auth/logoutclears bothsp_tokenand any Auth.js cookies (verify in browser devtools)/app?error=...with a friendly message rendered inline byAuthScreen/api/auth/google/callback(Google Calendar OAuth, Partner scheduling: Shared scheduling + auto-add to Google Calendar #204) still works for already-connected users — no regression from the catch-alldocs/mobile-oauth-integration.mddescribes both Pattern A (Apple native) and Pattern B (web flow overASWebAuthenticationSession) and links to the four issuesidentityTokenvalues appear in production logs after a sign-in attempt🤖 Generated with Claude Code
CodeAnt-AI Description
Add Google sign-in and handle OAuth-only accounts safely
What Changed
www.still-point.mecanonical domain, and documented OAuth usage for the web and iOSImpact
✅ Shorter sign-in for Google users✅ Fewer account-enumeration leaks✅ Clearer OAuth failure messages🔄 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.