Revert "Refactor exports. Fix account linking. Remove unused props."#659
Revert "Refactor exports. Fix account linking. Remove unused props."#659johnlowe399-blip wants to merge 1 commit intoelie222:stagingfrom
Conversation
|
@johnlowe399-blip is attempting to deploy a commit to the Inbox Zero OSS Program Team on Vercel. A member of the Team first needs to authorize it. |
|
|
WalkthroughThis update refactors authentication and session retrieval across the codebase. It standardizes session fetching by passing request headers explicitly, renames and restructures authentication exports, updates cookie and provider configurations, and modifies account linking logic to use provider account IDs. Several components and utilities are updated to utilize the new authentication client interface. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WebApp
participant AuthAPI
participant Headers
User->>WebApp: Request (any protected resource)
WebApp->>Headers: Retrieve request headers
WebApp->>AuthAPI: auth.api.getSession({ headers })
AuthAPI-->>WebApp: Session (user info or null)
WebApp->>User: Continue with session context
sequenceDiagram
participant User
participant WebApp
participant AuthClient
participant Provider
User->>WebApp: Initiate social sign-in
WebApp->>AuthClient: signIn.social(provider, { callbackURL, newUserCallbackURL })
AuthClient->>Provider: Redirect to provider with params
Provider-->>WebApp: Redirect with auth code
WebApp->>AuthClient: Handle callback, establish session
Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Possibly related issues
Possibly related PRs
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
apps/web/utils/auth.ts (1)
140-151: Re-enable critical auth event handlers in apps/web/utils/auth.tsI searched the entire codebase for
handleSignIn,handleLinkAccount, andhandlePendingPremiumInviteand found no alternative implementations—these hooks only live inapps/web/utils/auth.tsand are now entirely disabled.• File: apps/web/utils/auth.ts
• Lines: ~140–151Commenting out these handlers breaks:
- New user sign-in workflows (Loops/Resend contact creation)
- Sign-up analytics and referral tracking
- Pending premium-invite processing
- Account linking and EmailAccount record creation
Please re-enable the
signInevent and database hooks or provide a replacement implementation to restore these core flows.
🧹 Nitpick comments (2)
apps/web/app/(landing)/login/page.tsx (1)
8-8: Remove unused importThe
AutoLogOutimport is added but the component remains commented out. Consider removing the import if the component won't be used, or implement it if it's needed.-import AutoLogOut from "@/app/(landing)/login/error/AutoLogOut";Or uncomment and implement the AutoLogOut functionality:
- {/* <AutoLogOut loggedIn={loggedIn} /> */} + <AutoLogOut loggedIn={loggedIn} />Also applies to: 112-112
apps/web/utils/auth.ts (1)
1-475: Summary: Review the implications of reverting authentication changesThis revert introduces several significant changes to the authentication configuration:
Cookie Security: The new configuration with
sameSite: "none"requires careful CSRF protection and HTTPS in all environments.Disabled Event Handlers: Critical functionality for user onboarding, account linking, and premium features is commented out. Ensure alternative implementations exist or re-enable these handlers.
Session Retrieval Pattern: The removal of the auth function aligns with a pattern where session retrieval is done explicitly at call sites rather than centrally.
Given that this is a revert PR (#659 reverting #658), please ensure:
- The revert is intentional and necessary
- Any security improvements from the reverted PR are preserved
- The commented-out code is either re-enabled or has alternative implementations
| async function getEmailClients(client: EmailProvider) { | ||
| try { | ||
| const messages = await client.getSentMessages(50); | ||
| const messages = await client.getMessages("from:me", 50); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify cross-provider compatibility of the "from:me" query syntax.
The change from getSentMessages(50) to getMessages("from:me", 50) introduces Gmail-specific query syntax. While functionally equivalent for Gmail, this may not work consistently across different email providers (Outlook/Exchange).
Let's verify how different providers handle this query syntax:
🏁 Script executed:
#!/bin/bash
# Description: Check how the EmailProvider interface handles getMessages with query parameters across different providers
# Search for EmailProvider interface and implementations
ast-grep --pattern 'interface EmailProvider {
$$$
getMessages($$$)
$$$
}'
# Find all provider implementations
ast-grep --pattern 'class $_Provider implements EmailProvider {
$$$
getMessages($$$) {
$$$
}
$$$
}'
# Look for any provider-specific query handling
rg -A 10 "getMessages.*query" --type tsLength of output: 12163
Use provider-abstracted sent-message method instead of Gmail-only query
The call
const messages = await client.getMessages("from:me", 50);injects Gmail’s “from:me” syntax into generic assessment logic. Outlook’s implementation simply forwards that string to Microsoft Graph and won’t return sent messages as intended.
To preserve cross-provider compatibility and keep Gmail-specific details isolated in apps/web/utils/gmail, use the interface’s getSentMessages here (or branch on client.name === "google" if you need to retain the exact Gmail query):
• File: apps/web/utils/assess.ts, getEmailClients (around line 131)
- const messages = await client.getMessages("from:me", 50);
+ const messages = await client.getSentMessages(50);
// -- or, if you must keep the Gmail query:
// + const messages =
// + client.name === "google"
// + ? await client.getMessages("from:me", 50)
// + : await client.getSentMessages(50);This ensures all providers correctly retrieve sent messages without leaking provider-specific syntax into shared utilities.
🤖 Prompt for AI Agents
In apps/web/utils/assess.ts at line 131, replace the Gmail-specific call
client.getMessages("from:me", 50) with the provider-abstracted method
client.getSentMessages() to retrieve sent messages. This avoids using Gmail-only
query syntax in generic assessment logic and ensures compatibility with other
providers like Outlook. If necessary, use a conditional check on client.name to
handle provider-specific behavior, but prefer the interface method to keep
provider details encapsulated.
| export const auth = betterAuth({ | ||
| advanced: { | ||
| database: { | ||
| generateId: false, | ||
| }, | ||
| crossSubDomainCookies: { | ||
| enabled: true, | ||
| domain: env.COOKIE_DOMAIN, | ||
| }, | ||
| cookie: { | ||
| sameSite: "none", | ||
| secure: true, | ||
| domain: env.COOKIE_DOMAIN, | ||
| path: "/", | ||
| }, | ||
| defaultCookieAttributes: { | ||
| secure: true, | ||
| sameSite: "none", | ||
| }, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify cookie configuration for security and development compatibility
The cookie configuration with sameSite: "none" and secure: true has important implications:
-
Security:
sameSite: "none"allows cross-site requests, which could expose the application to CSRF attacks. Ensure CSRF protection is properly implemented. -
Development: These settings require HTTPS even in development. Consider environment-specific configuration to avoid issues with local development.
-
Domain validation: The
COOKIE_DOMAINshould be validated to prevent configuration errors.
🏁 Script executed:
#!/bin/bash
# Check if CSRF protection is implemented in the codebase
echo "Checking for CSRF protection implementation..."
rg -A 5 "csrf|CSRF" --type ts --type tsx
# Check for environment-specific cookie configuration
echo -e "\nChecking for environment-specific configurations..."
fd -e ts -e tsx -e js | xargs rg "NODE_ENV.*development.*cookie|cookie.*NODE_ENV.*development" -A 3
# Check how COOKIE_DOMAIN is used and validated
echo -e "\nChecking COOKIE_DOMAIN usage and validation..."
rg "COOKIE_DOMAIN" --type ts --type tsx -A 3Length of output: 576
🏁 Script executed:
#!/bin/bash
# Search for any CSRF protection usage in the web app
echo "Checking for CSRF protection patterns..."
rg -A 5 'csrf|CSRF' apps/web
# Look for NODE_ENV checks around cookie configuration
echo -e "\nChecking for environment-specific cookie configuration..."
rg -A 3 'NODE_ENV' apps/web
# Inspect where COOKIE_DOMAIN is referenced and whether it's validated
echo -e "\nChecking for COOKIE_DOMAIN usage and validation..."
rg -A 3 'COOKIE_DOMAIN' apps/webLength of output: 5419
Adjust cookie settings for development and enforce domain validation
The current configuration in apps/web/utils/auth.ts unconditionally sets
sameSite: "none",
secure: truewhich will:
- Block cookies in non-HTTPS dev environments (e.g., http://localhost)
- Allow cross-site requests without explicit CSRF safeguards
Recommendations:
- Scope
sameSiteandsecureto production only:const isProd = env.NODE_ENV === "production"; cookie: { sameSite: isProd ? "none" : "lax", secure: isProd, domain: env.COOKIE_DOMAIN, path: "/", }, defaultCookieAttributes: { sameSite: isProd ? "none" : "lax", secure: isProd, },
- In apps/web/env.ts, tighten
COOKIE_DOMAINvalidation:// from: COOKIE_DOMAIN: z.string().default("getinboxzero.com"), // to: COOKIE_DOMAIN: z.string().url().default("https://getinboxzero.com"),
- Verify that
betterAuthprovides CSRF protection (tokens or same-site enforcement). If not, integrate a CSRF middleware or token strategy.
Files to update:
- apps/web/utils/auth.ts (lines defining
cookieanddefaultCookieAttributes) - apps/web/env.ts (COOKIE_DOMAIN schema)
🤖 Prompt for AI Agents
In apps/web/utils/auth.ts around lines 25 to 43, update the cookie settings to
conditionally set sameSite and secure attributes based on the environment by
defining a boolean isProd from env.NODE_ENV and using it to set sameSite to
"none" and secure to true only in production, otherwise use "lax" and false
respectively for development. Also, in apps/web/env.ts, modify the COOKIE_DOMAIN
schema to validate as a URL instead of a plain string by changing
z.string().default("getinboxzero.com") to
z.string().url().default("https://getinboxzero.com"). Finally, verify if
betterAuth includes CSRF protection and if not, add appropriate CSRF middleware
or token handling.
|
Hey, what is this? Closing for now. |
3 participants
Reverts #658
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Documentation