Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: POC for multi account support #360

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

elie222
Copy link
Owner

@elie222 elie222 commented Feb 25, 2025

Summary by CodeRabbit

  • New Features
    • Introduces a comprehensive multi-account management upgrade that enables seamless account switching and linking via OAuth.
    • Adds new UI elements, including an account switcher in the sidebar and an add-account button, to simplify managing multiple email accounts.
    • Enhances session handling for smoother transitions between active accounts.
    • Updates user account details to provide an enriched overview of linked accounts.

Copy link

vercel bot commented Feb 25, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
inbox-zero ❌ Failed (Inspect) Feb 25, 2025 10:04am

Copy link
Contributor

coderabbitai bot commented Feb 25, 2025

Walkthrough

This pull request introduces several account management enhancements. A new authentication wrapper enables account switching based on headers or cookies, while dedicated API routes handle account linking, adding, and switching. New React components for account switching and adding accounts streamline the user experience, and middleware is added for OAuth account linking and active account header injection. Additionally, the Prisma schema is updated to adjust the Account and User models and to create new relationships with other entities.

Changes

File(s) Change Summary
apps/web/app/api/auth/[...nextauth]/auth.ts Introduced new auth function for account switching; original logic renamed to originalAuth to preserve existing authentication.
apps/web/app/api/auth/account-linking-handler/route.ts
apps/web/app/api/user/add-account/route.ts
apps/web/app/api/user/switch-account/route.ts
apps/web/app/api/user/me/route.ts
Added new API endpoints for account linking, adding, switching, and enhanced user data retrieval with account details.
apps/web/components/AccountSwitcher.tsx
apps/web/components/AddEmailAccount.tsx
apps/web/components/SideNav.tsx
Added React components for switching and adding accounts; integrated AccountSwitcher into the sidebar.
apps/web/middleware.ts
apps/web/middleware/account-linking.ts
apps/web/middleware/account-switcher.ts
Introduced middleware for handling OAuth account linking and injecting active account headers.
apps/web/prisma/schema.prisma Updated Account and User models: removed unique constraint, added several fields to Account, adjusted relationships with related models, and replaced User's promptHistory with apiKeys.

Sequence Diagram(s)

sequenceDiagram
    participant U as User
    participant AS as AccountSwitcher Component
    participant SA as Switch-Account API Endpoint
    participant DB as Database
    U->>AS: Select new account from dropdown
    AS->>SA: POST request to switch-account API
    SA->>DB: Validate account access
    DB-->>SA: Return validation result
    SA->>U: Success (set active account cookie)
    U->>AS: Refresh session view
Loading
sequenceDiagram
    participant U as User
    participant AE as AddEmailAccount Component
    participant AA as Add-Account API Endpoint
    participant O as OAuth Provider
    participant AL as Account Linking Handler
    U->>AE: Click "Add Account" button
    AE->>AA: POST request to add-account API
    AA->>U: Respond with Google OAuth URL & set cookies
    U->>O: Redirect to OAuth Provider
    O->>AL: Callback after authentication
    AL->>U: Finalize account linking
Loading

Poem

I'm a rabbit hopping through lines of code so bright,
New routes and middleware set my ears alight,
Switching accounts with a joyful leap,
Linking and adding as I happily peep,
In a world of changes, my whiskers dance all night!

Warning

There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

apps/web/app/api/user/me/route.ts

Oops! Something went wrong! :(

ESLint: 8.57.1

Error: Failed to load parser '@typescript-eslint/parser' declared in 'apps/web/.eslintrc.json': Cannot find module '@typescript-eslint/parser'
Require stack:

  • /apps/web/.eslintrc.json
    at Module._resolveFilename (node:internal/modules/cjs/loader:1248:15)
    at Function.resolve (node:internal/modules/helpers:145:19)
    at Object.resolve (/node_modules/.pnpm/@eslint[email protected]/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2346:46)
    at ConfigArrayFactory._loadParser (/node_modules/.pnpm/@eslint[email protected]/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3325:39)
    at ConfigArrayFactory._normalizeObjectConfigDataBody (/node_modules/.pnpm/@eslint[email protected]/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3099:43)
    at _normalizeObjectConfigDataBody.next ()
    at ConfigArrayFactory._normalizeObjectConfigData (/node_modules/.pnpm/@eslint[email protected]/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3040:20)
    at _normalizeObjectConfigData.next ()
    at ConfigArrayFactory.loadInDirectory (/node_modules/.pnpm/@eslint[email protected]/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2886:28)
    at CascadingConfigArrayFactory._loadConfigInAncestors (/node_modules/.pnpm/@eslint[email protected]/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3871:46)
apps/web/components/SideNav.tsx

Oops! Something went wrong! :(

ESLint: 8.57.1

Error: Failed to load parser '@typescript-eslint/parser' declared in 'apps/web/.eslintrc.json': Cannot find module '@typescript-eslint/parser'
Require stack:

  • /apps/web/.eslintrc.json
    at Module._resolveFilename (node:internal/modules/cjs/loader:1248:15)
    at Function.resolve (node:internal/modules/helpers:145:19)
    at Object.resolve (/node_modules/.pnpm/@eslint[email protected]/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2346:46)
    at ConfigArrayFactory._loadParser (/node_modules/.pnpm/@eslint[email protected]/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3325:39)
    at ConfigArrayFactory._normalizeObjectConfigDataBody (/node_modules/.pnpm/@eslint[email protected]/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3099:43)
    at _normalizeObjectConfigDataBody.next ()
    at ConfigArrayFactory._normalizeObjectConfigData (/node_modules/.pnpm/@eslint[email protected]/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3040:20)
    at _normalizeObjectConfigData.next ()
    at ConfigArrayFactory.loadInDirectory (/node_modules/.pnpm/@eslint[email protected]/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2886:28)
    at CascadingConfigArrayFactory._loadConfigInAncestors (/node_modules/.pnpm/@eslint[email protected]/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3871:46)
✨ Finishing Touches
  • 📝 Generate Docstrings (Beta)

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (7)
apps/web/middleware.ts (1)

18-30: Middleware implementation looks good but lacks explicit error handling.

The middleware implementation follows a good pattern by sequencing the middleware functions. However, there's no explicit error handling if either middleware function throws an exception.

Consider adding a try/catch block to prevent unhandled exceptions:

export async function middleware(request: NextRequest) {
+  try {
    // Apply the account linking middleware first (only runs on specific routes)
    const linkingResponse = await accountLinkingMiddleware(request);
    if (linkingResponse) return linkingResponse;

    // Apply the account switcher middleware next
    const response = await accountSwitcherMiddleware(
      request,
      NextResponse.next(),
    );

    return response;
+  } catch (error) {
+    console.error("Middleware error:", error);
+    return NextResponse.next();
+  }
}
apps/web/app/api/user/add-account/route.ts (1)

22-35: Cookie security considerations.

The cookie implementation is generally good with proper security settings based on environment. However, there are some considerations for production environments.

Consider adding the httpOnly flag to prevent client-side JavaScript access to these cookies, especially since they contain sensitive linking information:

response.cookies.set("link_account", "true", {
  path: "/",
  sameSite: "lax",
  secure: process.env.NODE_ENV === "production",
+  httpOnly: true,
  maxAge: 60 * 5, // 5 minutes
});

response.cookies.set("original_user_id", session.user.id, {
  path: "/",
  sameSite: "lax",
  secure: process.env.NODE_ENV === "production",
+  httpOnly: true,
  maxAge: 60 * 5, // 5 minutes
});
apps/web/middleware/account-switcher.ts (1)

9-10: Remove unused parameter res.

The res parameter is not utilized within this middleware. Consider removing it from the function signature to keep the code clean and concise.

-export async function accountSwitcherMiddleware(
-  req: NextRequest,
-  res: NextResponse,
-) {
+export async function accountSwitcherMiddleware(req: NextRequest) {
apps/web/middleware/account-linking.ts (1)

25-27: Return explicit response for missing original user ID.

Returning undefined here might lead to ambiguous behavior in subsequent middleware. Consider returning a response object (e.g., a 400 status) to clearly signal this error condition.

    if (!originalUserId) {
      logger.error("Missing original user ID for account linking");
-     return;
+     return NextResponse.json({ error: "Missing original user ID" }, { status: 400 });
    }
apps/web/components/AddEmailAccount.tsx (1)

54-67: Offer an explicit label or tooltip for the button.

When children is not provided, the default text is "Add Email Account". To improve consistency and user clarity, you might consider always providing a short label or tooltip explaining the button’s function.

apps/web/components/AccountSwitcher.tsx (1)

24-106: Good implementation of account switching functionality

This is a well-structured implementation of the account switcher component. I particularly like:

  • The conditional rendering at line 70 to hide the switcher when there's only one account
  • Proper session management with useSession and state updates via useEffect
  • Good error handling in the account switching function

Consider enhancing the error handling in the UI to provide feedback to users when account switching fails. Currently, errors are only logged to the console (lines 59-60 and 62). Adding toast notifications or inline error messages would improve the user experience.

 import { cn } from "@/utils";
 import { AddEmailAccount } from "@/components/AddEmailAccount";
+import { useToast } from "@/components/ui/use-toast";
 
 interface AccountSwitcherProps {
   className?: string;
 }
 
 export function AccountSwitcher({ className }: AccountSwitcherProps) {
   const { data: session } = useSession();
   const router = useRouter();
+  const { toast } = useToast();
   const [activeEmail, setActiveEmail] = useState<string | null>(null);

And then update the error handling:

       } else {
         console.error("Failed to switch account");
+        toast({
+          title: "Account switch failed",
+          description: "Unable to switch account. Please try again.",
+          variant: "destructive",
+        });
       }
     } catch (error) {
       console.error("Error switching account:", error);
+      toast({
+        title: "Account switch error",
+        description: "An unexpected error occurred. Please try again.",
+        variant: "destructive",
+      });
     }
apps/web/app/api/user/switch-account/route.ts (1)

56-62: Consider making cookie expiration configurable

The cookie expiration time is hardcoded to 30 days. For a feature that manages account access, this value should ideally be configurable.

Consider extracting this to an environment variable or a configuration constant:

+// Cookie expiration time in days
+const ACCOUNT_COOKIE_EXPIRATION_DAYS = 30;

// Set a cookie to indicate the active account
const response = NextResponse.json({ success: true });
response.cookies.set("active_account", email, {
  path: "/",
  sameSite: "lax",
  secure: process.env.NODE_ENV === "production",
-  maxAge: 60 * 60 * 24 * 30, // 30 days
+  maxAge: 60 * 60 * 24 * ACCOUNT_COOKIE_EXPIRATION_DAYS,
});
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ce2dcd8 and 786b8ef.

📒 Files selected for processing (10)
  • apps/web/app/api/auth/[...nextauth]/auth.ts (2 hunks)
  • apps/web/app/api/auth/account-linking-handler/route.ts (1 hunks)
  • apps/web/app/api/user/add-account/route.ts (1 hunks)
  • apps/web/app/api/user/switch-account/route.ts (1 hunks)
  • apps/web/components/AccountSwitcher.tsx (1 hunks)
  • apps/web/components/AddEmailAccount.tsx (1 hunks)
  • apps/web/components/TopNav.tsx (2 hunks)
  • apps/web/middleware.ts (1 hunks)
  • apps/web/middleware/account-linking.ts (1 hunks)
  • apps/web/middleware/account-switcher.ts (1 hunks)
🔇 Additional comments (15)
apps/web/components/TopNav.tsx (2)

26-26: Import addition looks good.

The import statement for the AccountSwitcher component is correctly added and follows the project's import pattern.


121-125: Account switcher integration looks clean.

The AccountSwitcher component is well-integrated into the menu structure with proper padding. This implementation properly maintains the existing dropdown pattern while adding the new functionality.

apps/web/middleware.ts (2)

1-4: Import statements are appropriate.

All necessary imports are included for the middleware functionality, with clear purpose for each imported item.


5-16: Configuration matcher is properly defined.

The matcher configuration correctly excludes static assets and public files from middleware processing, which helps with performance.

apps/web/app/api/user/add-account/route.ts (4)

1-7: Necessary imports and logger setup look good.

The imports are appropriate and the scoped logger follows project patterns.


8-12: Authentication check is correctly implemented.

The route properly checks for an authenticated user session before proceeding.


14-20: Auth URL construction looks good.

The implementation correctly builds the Google OAuth URL with proper parameters for account linking.


37-45: Error handling and logging look good.

The implementation includes appropriate logging and error handling with proper status codes.

apps/web/app/api/auth/[...nextauth]/auth.ts (2)

4-4: Import addition is appropriate.

The new import for cookies and headers is needed for the account switching functionality.


10-10: Correct renaming of auth function.

Renaming the original auth function allows for extending its functionality while maintaining backward compatibility.

apps/web/middleware/account-switcher.ts (1)

12-19: Consider handling empty or invalid cookie values.

If active_account is an empty string, it will still be set in the headers. You may want to verify that it's not empty or provide a fallback if an invalid value is encountered, to avoid potential downstream errors.

apps/web/middleware/account-linking.ts (1)

19-21: Verify cookies if partially present.

Currently, you strictly check link_account and original_user_id. If other cookies relevant to account linking are later introduced, ensure you validate their presence, or degrade gracefully if they're not found.

apps/web/components/AddEmailAccount.tsx (1)

34-43: Validate the redirection URL before using it.

The code immediately redirects to result.authUrl. If authUrl is missing or malformed, it could cause an unexpected redirect failure. Consider verifying that authUrl is present and valid before assigning it to window.location.href.

apps/web/app/api/auth/account-linking-handler/route.ts (1)

52-58: Clarify the logic for already-linked accounts.

When the new account’s userId matches the originalUserId, the handler immediately returns success. Ensure the calling code and UI are prepared for scenarios where the account is already linked, in order to properly notify or update the user interface.

apps/web/app/api/user/switch-account/route.ts (1)

21-30: Temporary implementation needs a more detailed roadmap

The implementation is correctly validating user access to accounts, but as noted in the comments, this is a temporary solution.

Since this is marked as "temporary" with a TODO in the comments, it would be helpful to have more clarity on:

  1. What the proper account relationships will look like
  2. When this implementation is expected to be replaced
  3. How to track this technical debt

Consider creating a GitHub issue to track this and reference it in a comment.

Comment on lines +30 to +76
// Create a wrapper for the auth function to handle account switching
export const auth = async () => {
// Get the original session
const session = await originalAuth();

// If no session, just return it
if (!session?.user) {
return session;
}

try {
// Check if there's an active account in headers or cookies
const headersList = headers();
const activeAccount =
headersList.get("x-active-account") ||
cookies().get("active_account")?.value;

// If there's no active account or it's the same as the current user, return the original session
if (!activeAccount || activeAccount === session.user.email) {
return session;
}

// For now, since we don't have a proper model to track connected accounts,
// we can only use the simplest implementation that allows a user
// to switch to their own email account
if (activeAccount !== session.user.email) {
logger.info("Account switch requested", {
from: session.user.email,
to: activeAccount,
});

// Update the session with the active account's email
return {
...session,
user: {
...session.user,
email: activeAccount,
},
};
}

return session;
} catch (error) {
logger.error("Error in auth account switching", { error });
return session;
}
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Account switching implementation needs security enhancement.

The account switching implementation works but has security implications that could be addressed.

The current implementation allows switching to any account specified in the header/cookie without verification that the account belongs to the user. Consider implementing verification that the requested account is actually linked to the user:

export const auth = async () => {
  // Get the original session
  const session = await originalAuth();

  // If no session, just return it
  if (!session?.user) {
    return session;
  }

  try {
    // Check if there's an active account in headers or cookies
    const headersList = headers();
    const activeAccount =
      headersList.get("x-active-account") ||
      cookies().get("active_account")?.value;

    // If there's no active account or it's the same as the current user, return the original session
    if (!activeAccount || activeAccount === session.user.email) {
      return session;
    }

+    // Verify that the requested account is linked to the user
+    const linkedAccounts = await getLinkedAccountsForUser(session.user.id);
+    if (!linkedAccounts.includes(activeAccount)) {
+      logger.warn("Attempted to switch to unlinked account", {
+        userId: session.user.id,
+        attemptedAccount: activeAccount
+      });
+      return session;
+    }

    // For now, since we don't have a proper model to track connected accounts,
    // we can only use the simplest implementation that allows a user
    // to switch to their own email account
    if (activeAccount !== session.user.email) {
      logger.info("Account switch requested", {
        from: session.user.email,
        to: activeAccount,
      });

      // Update the session with the active account's email
      return {
        ...session,
        user: {
          ...session.user,
          email: activeAccount,
        },
      };
    }

    return session;
  } catch (error) {
    logger.error("Error in auth account switching", { error });
    return session;
  }
};

Note: This implementation requires creating a getLinkedAccountsForUser function that retrieves the linked accounts from your database.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Create a wrapper for the auth function to handle account switching
export const auth = async () => {
// Get the original session
const session = await originalAuth();
// If no session, just return it
if (!session?.user) {
return session;
}
try {
// Check if there's an active account in headers or cookies
const headersList = headers();
const activeAccount =
headersList.get("x-active-account") ||
cookies().get("active_account")?.value;
// If there's no active account or it's the same as the current user, return the original session
if (!activeAccount || activeAccount === session.user.email) {
return session;
}
// For now, since we don't have a proper model to track connected accounts,
// we can only use the simplest implementation that allows a user
// to switch to their own email account
if (activeAccount !== session.user.email) {
logger.info("Account switch requested", {
from: session.user.email,
to: activeAccount,
});
// Update the session with the active account's email
return {
...session,
user: {
...session.user,
email: activeAccount,
},
};
}
return session;
} catch (error) {
logger.error("Error in auth account switching", { error });
return session;
}
};
// Create a wrapper for the auth function to handle account switching
export const auth = async () => {
// Get the original session
const session = await originalAuth();
// If no session, just return it
if (!session?.user) {
return session;
}
try {
// Check if there's an active account in headers or cookies
const headersList = headers();
const activeAccount =
headersList.get("x-active-account") ||
cookies().get("active_account")?.value;
// If there's no active account or it's the same as the current user, return the original session
if (!activeAccount || activeAccount === session.user.email) {
return session;
}
// Verify that the requested account is linked to the user
const linkedAccounts = await getLinkedAccountsForUser(session.user.id);
if (!linkedAccounts.includes(activeAccount)) {
logger.warn("Attempted to switch to unlinked account", {
userId: session.user.id,
attemptedAccount: activeAccount,
});
return session;
}
// For now, since we don't have a proper model to track connected accounts,
// we can only use the simplest implementation that allows a user
// to switch to their own email account
if (activeAccount !== session.user.email) {
logger.info("Account switch requested", {
from: session.user.email,
to: activeAccount,
});
// Update the session with the active account's email
return {
...session,
user: {
...session.user,
email: activeAccount,
},
};
}
return session;
} catch (error) {
logger.error("Error in auth account switching", { error });
return session;
}
};

Comment on lines +62 to +90

// Update the account to be associated with the original user
await prisma.account.update({
where: {
id: accountId,
},
data: {
userId: originalUserId,
},
});

// Delete the temporary user (if it's not the original user)
// But first check if there are any other accounts linked to it
const otherAccounts = await prisma.account.findMany({
where: {
userId: tempUser.id,
},
});

if (otherAccounts.length === 0) {
await prisma.user.delete({
where: {
id: tempUser.id,
},
});

logger.info("Deleted temporary user", { userId: tempUser.id });
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Wrap account update and user deletion in a transaction.

The update of the newAccount user and potential deletion of tempUser could create inconsistencies if one operation fails. Consider wrapping these operations within a database transaction to maintain atomicity.

// Example with Prisma transaction
    await prisma.$transaction(async (prismaTx) => {
      await prismaTx.account.update({
        where: { id: accountId },
        data: { userId: originalUserId },
      });

      const otherAccounts = await prismaTx.account.findMany({
        where: { userId: tempUser.id },
      });

      if (otherAccounts.length === 0) {
        await prismaTx.user.delete({
          where: { id: tempUser.id },
        });
      }
    });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Update the account to be associated with the original user
await prisma.account.update({
where: {
id: accountId,
},
data: {
userId: originalUserId,
},
});
// Delete the temporary user (if it's not the original user)
// But first check if there are any other accounts linked to it
const otherAccounts = await prisma.account.findMany({
where: {
userId: tempUser.id,
},
});
if (otherAccounts.length === 0) {
await prisma.user.delete({
where: {
id: tempUser.id,
},
});
logger.info("Deleted temporary user", { userId: tempUser.id });
}
await prisma.$transaction(async (prismaTx) => {
await prismaTx.account.update({
where: { id: accountId },
data: { userId: originalUserId },
});
const otherAccounts = await prismaTx.account.findMany({
where: { userId: tempUser.id },
});
if (otherAccounts.length === 0) {
await prismaTx.user.delete({
where: { id: tempUser.id },
});
}
});

Comment on lines +38 to +53
try {
const body = await request.json();
const { email } = switchAccountSchema.parse(body);

// Check if the user can access this account
const hasAccess = await canAccessAccount(session.user.id, email);
if (!hasAccess) {
logger.error("Unauthorized access attempt", {
userId: session.user.id,
targetEmail: email,
});
return NextResponse.json(
{ error: "You don't have access to this account" },
{ status: 403 },
);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Enhance validation to prevent account spoofing

The current implementation only checks if the target email matches the current user's email, which is a good start, but could be enhanced.

For a multi-account feature, especially in the context of a "Proof of Concept", it's important to verify that the user actually owns or has permission to access each account they're trying to switch to. Consider implementing additional checks such as:

  1. Verifying the email exists in your database
  2. Checking for explicit account linking records
  3. Implementing a more robust access control system

These enhancements would help prevent potential security issues where a user might attempt to spoof accounts.


🏁 Script executed:

#!/bin/bash
# Verify if there are any other checks for account ownership in the codebase
rg -A 5 "account.*ownership|account.*access|email.*verify" --type ts

Length of output: 9218


Action Required: Strengthen Account Ownership Checks

After verifying the codebase, it appears that the only measure in place in apps/web/app/api/user/switch-account/route.ts is the call to canAccessAccount, which likely performs a basic check (e.g., matching the current session user against the target email). Other endpoints in our repository rely heavily on token and permission validations but do not include an explicit check that confirms whether the target email is indeed linked to the user’s account. To prevent account spoofing, please consider the following enhancements:

  • Verify Email Existence: Before switching accounts, query the database to ensure the target email corresponds to a valid, existing account.
  • Check Explicit Account Linkage: Implement checks to confirm that the target account is explicitly linked to the current user, rather than relying solely on email matching.
  • Adopt More Robust Access Control: Consider aligning your checks with our other endpoints (e.g., those handling Google account tokens) by incorporating additional permissions or ownership verifications.

These changes will further safeguard against malicious attempts to access unauthorized accounts.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
apps/web/prisma/schema.prisma (1)

1-529: Ensure account-switching middleware is in place

This schema supports multi-account functionality, but you'll need middleware to handle account switching logic, authentication, and maintaining the active account context.

Make sure you have implemented or planned:

  1. Middleware to track the active account (via headers/cookies)
  2. Session management that includes active account information
  3. API endpoints for switching between accounts
  4. Proper error handling for account access permissions
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 786b8ef and 263bea9.

📒 Files selected for processing (3)
  • apps/web/app/api/user/me/route.ts (1 hunks)
  • apps/web/components/SideNav.tsx (2 hunks)
  • apps/web/prisma/schema.prisma (13 hunks)
🔇 Additional comments (10)
apps/web/app/api/user/me/route.ts (1)

36-41: Good addition of accounts data to the user response

This change correctly adds the account information to the user data returned by the API, which will enable the account switcher functionality in the UI.

apps/web/components/SideNav.tsx (2)

52-52: Good import of the new AccountSwitcher component

This import enables the multi-account functionality in the sidebar.


259-259: Well-placed integration of the AccountSwitcher component

The AccountSwitcher component is appropriately placed in the SidebarFooter, replacing the previously commented-out NavUser component.

apps/web/prisma/schema.prisma (7)

16-17: Crucial schema change to enable multi-account support

Removing the @unique constraint from userId is a key change that allows a user to have multiple accounts, which is essential for the multi-account functionality.


28-45: Comprehensive account fields migration

You've appropriately moved user-specific fields like email, image, and profile information to the Account model, which aligns with the multi-account architecture.


50-59: Well-structured account relationships

The relationships between Account and other entities are properly defined, which will ensure data integrity when working with multiple accounts.


106-106: Good model restructuring

Changing promptHistory to apiKeys in the User model is a good decision that better represents the relationship between users and API keys.


264-266: Important relations for ExecutedRule

The addition of account relation to ExecutedRule ensures that rules are properly associated with specific accounts, which is crucial for multi-account support.


333-334:

❓ Verification inconclusive

Check for missing database migrations

Ensure that database migrations are created and tested for these schema changes. When adding required fields to existing models, you'll need a migration strategy to populate these fields for existing records.

Also applies to: 353-354, 374-375, 397-398, 426-427


🏁 Script executed:

#!/bin/bash
# Check for migration files in the repository
fd "migration" --type d

Length of output: 51


Action Required: Verify Comprehensive Database Migrations for Schema Changes

We've confirmed that the apps/web/prisma/migrations/ directory exists, which indicates that migration files are being generated. However, please double-check that a migration specifically covering the schema changes at:

  • Lines 333-334 (accountId and its relation in schema.prisma)
  • Lines 353-354, 374-375, 397-398, and 426-427

...has been created and thoroughly tested. Also, ensure that the migration strategy includes populating the newly required fields for existing records safely to avoid data inconsistencies in production.


48-48:

❓ Verification inconclusive

Verify client requirements for auto-categorization

The default value for autoCategorizeSenders is set to false. Consider whether this should be a user preference that carries over from existing accounts.


🏁 Script executed:

#!/bin/bash
# Check if this setting exists elsewhere in the codebase
rg "autoCategorizeSenders" --type ts

Length of output: 1795


Action: Confirm Auto-Categorization Setting Behavior with Client

The autoCategorizeSenders field in apps/web/prisma/schema.prisma is currently set to false by default. Our search confirms this property is consistently used across the codebase (e.g., in apps/web/utils/actions/categorize.ts, apps/web/app/(app)/smart-categories/Uncategorized.tsx, apps/web/app/(app)/smart-categories/page.tsx, and related API modules). Please verify that this default value aligns with the client's requirements—specifically, whether existing account preferences should carry over or if this should be a user-configurable setting with a different default behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant