fix(oauth): Update tokens when duplicate account detected for same user#1219
fix(oauth): Update tokens when duplicate account detected for same user#1219
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThree files updated: documentation for PR workflows now includes branch name display instruction; Google and Outlook OAuth callback routes refactored to centralize token update logic via new internal helper functions that handle account token persistence more consistently. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Update Google and Microsoft account token handling on duplicate account detection in
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.claude/commands/create-pr.md (1)
61-62: Capitalize "Markdown" as a proper noun.The static analysis tool flagged that "markdown" should be capitalized as "Markdown" when referring to the markup language. Minor documentation clarity improvement.
🔎 Proposed fix
-Display the returned PR URL as a markdown link on its own line, formatted as: `[PR #<number>](<url>)` so it's clickable. +Display the returned PR URL as a Markdown link on its own line, formatted as: `[PR #<number>](<url>)` so it's clickable.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.claude/commands/create-pr.mdapps/web/app/api/google/linking/callback/route.tsapps/web/app/api/outlook/linking/callback/route.ts
🧰 Additional context used
📓 Path-based instructions (20)
!(pages/_document).{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
Don't use the next/head module in pages/_document.js on Next.js projects
Files:
.claude/commands/create-pr.mdapps/web/app/api/google/linking/callback/route.tsapps/web/app/api/outlook/linking/callback/route.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/data-fetching.mdc)
**/*.{ts,tsx}: For API GET requests to server, use theswrpackage
Useresult?.serverErrorwithtoastErrorfrom@/components/Toastfor error handling in async operations
**/*.{ts,tsx}: Use wrapper functions for Gmail message operations (get, list, batch, etc.) from @/utils/gmail/message.ts instead of direct API calls
Use wrapper functions for Gmail thread operations from @/utils/gmail/thread.ts instead of direct API calls
Use wrapper functions for Gmail label operations from @/utils/gmail/label.ts instead of direct API calls
**/*.{ts,tsx}: For early access feature flags, create hooks using the naming conventionuse[FeatureName]Enabledthat return a boolean fromuseFeatureFlagEnabled("flag-key")
For A/B test variant flags, create hooks using the naming conventionuse[FeatureName]Variantthat define variant types, useuseFeatureFlagVariantKey()with type casting, and provide a default "control" fallback
Use kebab-case for PostHog feature flag keys (e.g.,inbox-cleaner,pricing-options-2)
Always define types for A/B test variant flags (e.g.,type PricingVariant = "control" | "variant-a" | "variant-b") and provide type safety through type casting
**/*.{ts,tsx}: Don't use primitive type aliases or misleading types
Don't use empty type parameters in type aliases and interfaces
Don't use this and super in static contexts
Don't use any or unknown as type constraints
Don't use the TypeScript directive @ts-ignore
Don't use TypeScript enums
Don't export imported variables
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions
Don't use TypeScript namespaces
Don't use non-null assertions with the!postfix operator
Don't use parameter properties in class constructors
Don't use user-defined types
Useas constinstead of literal types and type annotations
Use eitherT[]orArray<T>consistently
Initialize each enum member value explicitly
Useexport typefor types
Use `impo...
Files:
apps/web/app/api/google/linking/callback/route.tsapps/web/app/api/outlook/linking/callback/route.ts
apps/web/app/api/**/route.ts
📄 CodeRabbit inference engine (.cursor/rules/fullstack-workflow.mdc)
apps/web/app/api/**/route.ts: Create GET API routes usingwithAuthorwithEmailAccountmiddleware inapps/web/app/api/*/route.ts, export response types asGetExampleResponsetype alias for client-side type safety
Always export response types from GET routes asGet[Feature]Responseusing type inference from the data fetching function for type-safe client consumption
Do NOT use POST API routes for mutations - always use server actions withnext-safe-actioninstead
Files:
apps/web/app/api/google/linking/callback/route.tsapps/web/app/api/outlook/linking/callback/route.ts
**/app/**/route.ts
📄 CodeRabbit inference engine (.cursor/rules/get-api-route.mdc)
**/app/**/route.ts: Always wrap GET API route handlers withwithAuthorwithEmailAccountmiddleware for consistent error handling and authentication in Next.js App Router
Infer and export response type for GET API routes usingAwaited<ReturnType<typeof functionName>>pattern in Next.js
Use Prisma for database queries in GET API routes
Return responses usingNextResponse.json()in GET API routes
Do not use try/catch blocks in GET API route handlers when usingwithAuthorwithEmailAccountmiddleware, as the middleware handles error handling
Files:
apps/web/app/api/google/linking/callback/route.tsapps/web/app/api/outlook/linking/callback/route.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/prisma-enum-imports.mdc)
Always import Prisma enums from
@/generated/prisma/enumsinstead of@/generated/prisma/clientto avoid Next.js bundling errors in client componentsImport Prisma using the project's centralized utility:
import prisma from '@/utils/prisma'
Files:
apps/web/app/api/google/linking/callback/route.tsapps/web/app/api/outlook/linking/callback/route.ts
apps/web/app/**/[!.]*/route.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Use kebab-case for route directories in Next.js App Router (e.g.,
api/hello-world/route)
Files:
apps/web/app/api/google/linking/callback/route.tsapps/web/app/api/outlook/linking/callback/route.ts
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Import specific lodash functions rather than entire lodash library to minimize bundle size (e.g.,
import groupBy from 'lodash/groupBy')
apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Do not export types/interfaces that are only used within the same file. Export later if needed
Files:
apps/web/app/api/google/linking/callback/route.tsapps/web/app/api/outlook/linking/callback/route.ts
apps/web/app/api/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/security-audit.mdc)
apps/web/app/api/**/*.{ts,tsx}: API routes must usewithAuth,withEmailAccount, orwithErrormiddleware for authentication
All database queries must include user scoping withemailAccountIdoruserIdfiltering in WHERE clauses
Request parameters must be validated before use; avoid direct parameter usage without type checking
Use generic error messages instead of revealing internal details; throwSafeErrorinstead of exposing user IDs, resource IDs, or system information
API routes should only return necessary fields usingselectin database queries to prevent unintended information disclosure
Cron endpoints must usehasCronSecretorhasPostCronSecretto validate cron requests and prevent unauthorized access
Request bodies should use Zod schemas for validation to ensure type safety and prevent injection attacks
Files:
apps/web/app/api/google/linking/callback/route.tsapps/web/app/api/outlook/linking/callback/route.ts
**/app/api/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/security.mdc)
**/app/api/**/*.ts: ALL API routes that handle user data MUST use appropriate middleware: usewithEmailAccountfor email-scoped operations, usewithAuthfor user-scoped operations, or usewithErrorwith proper validation for public/custom auth endpoints
UsewithEmailAccountmiddleware for operations scoped to a specific email account, including reading/writing emails, rules, schedules, or any operation usingemailAccountId
UsewithAuthmiddleware for user-level operations such as user settings, API keys, and referrals that use onlyuserId
UsewithErrormiddleware only for public endpoints, custom authentication logic, or cron endpoints. For cron endpoints, MUST usehasCronSecret()orhasPostCronSecret()validation
Cron endpoints without proper authentication can be triggered by anyone. CRITICAL: All cron endpoints MUST validate cron secret usinghasCronSecret(request)orhasPostCronSecret(request)and capture unauthorized attempts withcaptureException()
Always validate request bodies using Zod schemas to ensure type safety and prevent invalid data from reaching database operations
Maintain consistent error response format across all API routes to avoid information disclosure while providing meaningful error feedback
Files:
apps/web/app/api/google/linking/callback/route.tsapps/web/app/api/outlook/linking/callback/route.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/security.mdc)
**/*.ts: ALL database queries MUST be scoped to the authenticated user/account by including user/account filtering in WHERE clauses to prevent unauthorized data access
Always validate that resources belong to the authenticated user before performing operations, using ownership checks in WHERE clauses or relationships
Always validate all input parameters for type, format, and length before using them in database queries
Use SafeError for error responses to prevent information disclosure. Generic error messages should not reveal internal IDs, logic, or resource ownership details
Only return necessary fields in API responses using Prisma'sselectoption. Never expose sensitive data such as password hashes, private keys, or system flags
Prevent Insecure Direct Object References (IDOR) by validating resource ownership before operations. AllfindUnique/findFirstcalls MUST include ownership filters
Prevent mass assignment vulnerabilities by explicitly whitelisting allowed fields in update operations instead of accepting all user-provided data
Prevent privilege escalation by never allowing users to modify system fields, ownership fields, or admin-only attributes through user input
AllfindManyqueries MUST be scoped to the user's data by including appropriate WHERE filters to prevent returning data from other users
Use Prisma relationships for access control by leveraging nested where clauses (e.g.,emailAccount: { id: emailAccountId }) to validate ownership
Files:
apps/web/app/api/google/linking/callback/route.tsapps/web/app/api/outlook/linking/callback/route.ts
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)
**/*.{tsx,ts}: Use Shadcn UI and Tailwind for components and styling
Usenext/imagepackage for images
For API GET requests to server, use theswrpackage with hooks likeuseSWRto fetch data
For text inputs, use theInputcomponent withregisterPropsfor form integration and error handling
Files:
apps/web/app/api/google/linking/callback/route.tsapps/web/app/api/outlook/linking/callback/route.ts
**/*.{tsx,ts,css}
📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)
Implement responsive design with Tailwind CSS using a mobile-first approach
Files:
apps/web/app/api/google/linking/callback/route.tsapps/web/app/api/outlook/linking/callback/route.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{js,jsx,ts,tsx}: Don't useaccessKeyattribute on any HTML element
Don't setaria-hidden="true"on focusable elements
Don't add ARIA roles, states, and properties to elements that don't support them
Don't use distracting elements like<marquee>or<blink>
Only use thescopeprop on<th>elements
Don't assign non-interactive ARIA roles to interactive HTML elements
Make sure label elements have text content and are associated with an input
Don't assign interactive ARIA roles to non-interactive HTML elements
Don't assigntabIndexto non-interactive HTML elements
Don't use positive integers fortabIndexproperty
Don't include "image", "picture", or "photo" in img alt prop
Don't use explicit role property that's the same as the implicit/default role
Make static elements with click handlers use a valid role attribute
Always include atitleelement for SVG elements
Give all elements requiring alt text meaningful information for screen readers
Make sure anchors have content that's accessible to screen readers
AssigntabIndexto non-interactive HTML elements witharia-activedescendant
Include all required ARIA attributes for elements with ARIA roles
Make sure ARIA properties are valid for the element's supported roles
Always include atypeattribute for button elements
Make elements with interactive roles and handlers focusable
Give heading elements content that's accessible to screen readers (not hidden witharia-hidden)
Always include alangattribute on the html element
Always include atitleattribute for iframe elements
AccompanyonClickwith at least one of:onKeyUp,onKeyDown, oronKeyPress
AccompanyonMouseOver/onMouseOutwithonFocus/onBlur
Include caption tracks for audio and video elements
Use semantic elements instead of role attributes in JSX
Make sure all anchors are valid and navigable
Ensure all ARIA properties (aria-*) are valid
Use valid, non-abstract ARIA roles for elements with ARIA roles
Use valid AR...
Files:
apps/web/app/api/google/linking/callback/route.tsapps/web/app/api/outlook/linking/callback/route.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
**/*.{js,ts,jsx,tsx}: Use lodash utilities for common operations (arrays, objects, strings)
Import specific lodash functions to minimize bundle size (e.g.,import groupBy from 'lodash/groupBy')
Files:
apps/web/app/api/google/linking/callback/route.tsapps/web/app/api/outlook/linking/callback/route.ts
**/{app,pages}/**/{route,+page}.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/logging.mdc)
**/{app,pages}/**/{route,+page}.{ts,tsx}: Use middleware wrappers (withError, withAuth, withEmailAccount, withEmailProvider) that automatically create loggers with request context in API routes
Enrich logger context within route handlers using logger.with() to add request-specific fields like messageId
Files:
apps/web/app/api/google/linking/callback/route.tsapps/web/app/api/outlook/linking/callback/route.ts
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
apps/web/**/*.{ts,tsx,js,jsx}: Use@/path aliases for imports from project root
Prefer self-documenting code over comments; use descriptive variable and function names instead of explaining intent with comments
Add helper functions to the bottom of files, not the top
All imports go at the top of files, no mid-file dynamic imports
Files:
apps/web/app/api/google/linking/callback/route.tsapps/web/app/api/outlook/linking/callback/route.ts
apps/web/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
Follow NextJS app router structure with (app) directory
Files:
apps/web/app/api/google/linking/callback/route.tsapps/web/app/api/outlook/linking/callback/route.ts
apps/web/**/*.{ts,tsx,js,jsx,json,css}
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
Format code with Prettier
Files:
apps/web/app/api/google/linking/callback/route.tsapps/web/app/api/outlook/linking/callback/route.ts
apps/web/**/*.{example,ts,json}
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
Add environment variables to
.env.example,env.ts, andturbo.json
Files:
apps/web/app/api/google/linking/callback/route.tsapps/web/app/api/outlook/linking/callback/route.ts
apps/web/app/api/**/*.ts
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
apps/web/app/api/**/*.ts: Create GET API routes wrapped withwithAuthorwithEmailAccountmiddleware for fetching data
Export response types from GET API routes usingexport type GetXResponse = Awaited<ReturnType<typeof getData>>
Files:
apps/web/app/api/google/linking/callback/route.tsapps/web/app/api/outlook/linking/callback/route.ts
🧠 Learnings (3)
📚 Learning: 2025-11-25T14:39:23.326Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/security.mdc:0-0
Timestamp: 2025-11-25T14:39:23.326Z
Learning: Applies to app/api/**/*.ts : Use `withEmailAccount` middleware for operations scoped to a specific email account (reading/writing emails, rules, schedules, etc.) - provides `emailAccountId`, `userId`, and `email` in `request.auth`
Applied to files:
apps/web/app/api/google/linking/callback/route.tsapps/web/app/api/outlook/linking/callback/route.ts
📚 Learning: 2025-11-25T14:39:04.892Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/security-audit.mdc:0-0
Timestamp: 2025-11-25T14:39:04.892Z
Learning: Applies to apps/web/app/api/**/route.ts : All database queries must include user/account filtering with `emailAccountId` or `userId` in WHERE clauses to prevent IDOR vulnerabilities
Applied to files:
apps/web/app/api/google/linking/callback/route.ts
📚 Learning: 2025-11-25T14:39:27.909Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/security.mdc:0-0
Timestamp: 2025-11-25T14:39:27.909Z
Learning: Applies to **/app/api/**/*.ts : Use `withEmailAccount` middleware for operations scoped to a specific email account, including reading/writing emails, rules, schedules, or any operation using `emailAccountId`
Applied to files:
apps/web/app/api/google/linking/callback/route.tsapps/web/app/api/outlook/linking/callback/route.ts
🪛 LanguageTool
.claude/commands/create-pr.md
[uncategorized] ~61-~61: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ..." ``` Display the returned PR URL as a markdown link on its own line, formatted as: `[P...
(MARKDOWN_NNP)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (9)
.claude/commands/create-pr.md (1)
61-62: Documentation additions are clear and improve user guidance.The new instructions to display the PR URL as a clickable link and show the branch name created align well with the command workflow and provide helpful user feedback.
apps/web/app/api/google/linking/callback/route.ts (4)
182-183: LGTM! Necessary field addition for token updates.The
idfield is correctly added to the select statement to enable token updates via the new helper function when handling duplicate accounts.
186-195: Excellent fix for the duplicate account token update issue.This properly addresses the core problem described in the PR: when a duplicate account is detected during re-authentication, tokens are now updated instead of being ignored. The improved logging with
accountIdenhances observability for debugging re-authentication flows.
221-221: Good refactor to centralize token update logic.Using the
updateGoogleAccountTokenshelper ensures consistent token handling across both the duplicate-account path and the update-tokens path.
288-315: Well-implemented token update helper that preserves refresh_token.The conditional spread on lines 306-308 correctly preserves the existing
refresh_tokenwhen the provider doesn't return one (which is typical for Google OAuth on subsequent re-authentications). This addresses the concerns raised in previous reviews about refresh tokens being overwritten withnull.The helper is appropriately:
- Internal (not exported)
- Colocated at the bottom of the file
- Handles
expiry_datetoexpires_atconversion for Google's OAuth response formatapps/web/app/api/outlook/linking/callback/route.ts (4)
234-234: LGTM! Necessary field addition for token updates.Consistent with the Google file, the
idfield is added to support the token update helper when handling duplicate accounts.
238-247: Excellent fix for Microsoft re-authentication issue.This addresses the specific failure mode mentioned in the PR description where Microsoft users couldn't re-authenticate after password changes. The token update now ensures
refresh_tokenisn't left as NULL when duplicate accounts are detected.
273-276: Good refactor to centralize token update logic.Consistent with the Google file, this uses the dedicated helper to ensure uniform token handling across different code paths.
341-381: Well-structured token helpers for Microsoft OAuth.The implementation properly handles Microsoft OAuth token response variations:
parseMicrosoftExpiresAthandles bothexpires_at(Unix timestamp) andexpires_in(relative seconds)- Conditional spread on lines 373-375 preserves existing
refresh_tokenwhen provider doesn't return one- Good refactor to extract the expires_at calculation logic that was previously inline (lines 162-171)
The helpers are appropriately internal and colocated, following coding guidelines.
1 participant
User description
oauth: Fix refresh token not being saved on re-authentication
When a user re-authenticates (e.g., after password change), the OAuth callback could hit a unique constraint error if the account already exists. Previously, the duplicate handler just logged and continued without updating the tokens, leaving refresh_token as NULL.
Generated description
Below is a concise technical summary of the changes proposed in this PR:
Rectify OAuth callback handlers for Google and Outlook to ensure access and refresh tokens are consistently updated during user re-authentication, preventing
refresh_tokenfrom becomingNULLand resolving authentication failures. Refactor token update logic into dedicated helper functions (updateGoogleAccountTokens,updateMicrosoftAccountTokens) to centralize persistence mechanisms and improve code maintainability.selectclause to retrieveaccountIdand invoking new helper functions (updateGoogleAccountTokens,updateMicrosoftAccountTokens) to ensure tokens are saved, preventingrefresh_tokenfrom beingNULL. The refactoring centralizes token update operations, including conditional refresh token updates to preserve existing tokens if not provided by the provider, and extracts date parsing for Microsoft tokens.Modified files (2)
Latest Contributors(2)
.claude/commands/create-pr.mddocumentation to include a new instruction to display the name of the created branch after executing thegh pr createcommand. This improves the user experience by providing more immediate feedback on the branch used for the pull request.Modified files (1)
Latest Contributors(1)
Summary by CodeRabbit
Documentation
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.