Implement MFA #2244
Implement MFA #2244
Conversation
|
WalkthroughThis set of changes introduces comprehensive multi-factor authentication (MFA) support across the application. The database schema and Prisma models are updated to add MFA-related fields to the User model and a new MfaBackupCode model. New backend services implement TOTP-based MFA setup, validation, disabling, and login verification, including rate limiting and secure secret management. The authentication flows for email and GitHub are updated to detect and require MFA when enabled. New Remix routes and React components provide user interfaces for MFA setup, disabling, and login, including OTP input, QR code display, and recovery code management. Email notifications are added for MFA status changes. Supporting utilities, path builders, and UI primitives are also extended or introduced as needed. 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used📓 Path-based instructions (4)`apps/webapp/**/*.ts`: In the webapp, all environment variables must be accessed through the `env` export of `env.server.ts`, instead of directly accessing `process.env`.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/webapp.mdc) List of files the instruction was applied to:
`apps/webapp/**/*.{ts,tsx}`: When importing from `@trigger.dev/core` in the weba...
📄 Source: CodeRabbit Inference Engine (.cursor/rules/webapp.mdc) List of files the instruction was applied to:
`**/*.{ts,tsx}`: Always prefer using isomorphic code like fetch, ReadableStream,...
📄 Source: CodeRabbit Inference Engine (.github/copilot-instructions.md) List of files the instruction was applied to:
`{packages/core,apps/webapp}/**/*.{ts,tsx}`: We use zod a lot in packages/core and in the webapp
📄 Source: CodeRabbit Inference Engine (.github/copilot-instructions.md) List of files the instruction was applied to:
🧠 Learnings (1)apps/webapp/app/services/mfa/multiFactorAuthentication.server.ts (1)🧬 Code Graph Analysis (1)apps/webapp/app/services/mfa/multiFactorAuthentication.server.ts (4)
⏰ 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). (25)
🔇 Additional comments (9)
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
apps/webapp/app/routes/auth.github.callback.tsx (1)
11-13: Refactor duplicated cookie parsing logic.The redirect cookie parsing logic is duplicated. Consider extracting it to avoid repetition.
export let loader: LoaderFunction = async ({ request }) => { + const cookie = request.headers.get("Cookie"); + const redirectValue = await redirectCookie.parse(cookie); + const redirectTo = redirectValue ?? "/"; + try { - const cookie = request.headers.get("Cookie"); - const redirectValue = await redirectCookie.parse(cookie); - const redirectTo = redirectValue ?? "/"; logger.debug("auth.github.callback loader", { redirectTo, }); const authuser = await authenticator.authenticate("github", request, { successRedirect: undefined, // Don't auto-redirect, we'll handle it failureRedirect: undefined, // Don't auto-redirect on failure either }); logger.debug("auth.github.callback authuser", { authuser, }); // If we get here, user doesn't have MFA - complete login normally return redirect(redirectTo); } catch (error) { // Check if this is an MFA_REQUIRED error if (error instanceof MfaRequiredError) { // User has MFA enabled - store pending user ID and redirect to MFA page const session = await getUserSession(request); session.set("pending-mfa-user-id", error.userId); - - const cookie = request.headers.get("Cookie"); - const redirectValue = await redirectCookie.parse(cookie); - const redirectTo = redirectValue ?? "/"; session.set("pending-mfa-redirect-to", redirectTo); return redirect("/login/mfa", { headers: { "Set-Cookie": await commitSession(session), }, }); } // Regular authentication failure, redirect to login page logger.debug("auth.github.callback error", { error }); return redirect("/login"); } };Also applies to: 37-39
apps/webapp/app/services/mfa/multiFactorAuthentication.server.ts (2)
216-234: Optimize recovery code insertionThe recovery codes are inserted one by one in a loop, which is inefficient.
Use batch insertion with
createMany:- // Hash and store the recovery codes - for (const code of recoveryCodes) { - const hashedCode = await createHash("SHA-512", "hex").digest(code); - await this.#prismaClient.mfaBackupCode.create({ - data: { - userId, - code: hashedCode, - }, - }); - } + // Hash and store the recovery codes + const hashedCodes = await Promise.all( + recoveryCodes.map(async (code) => ({ + userId, + code: await createHash("SHA-512", "hex").digest(code), + })) + ); + + await this.#prismaClient.mfaBackupCode.createMany({ + data: hashedCodes, + });
297-343: Good implementation of TOTP verification with replay protectionThe replay attack prevention using
mfaLastUsedCodeis a good security measure. The generic error messages prevent information leakage.Consider implementing rate limiting at the service level as well, in addition to the route-level rate limiting, for defense in depth.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (30)
apps/webapp/app/components/navigation/AccountSideMenu.tsx(2 hunks)apps/webapp/app/components/primitives/CopyButton.tsx(3 hunks)apps/webapp/app/components/primitives/Dialog.tsx(2 hunks)apps/webapp/app/components/primitives/InputOTP.tsx(1 hunks)apps/webapp/app/components/primitives/Switch.tsx(2 hunks)apps/webapp/app/models/message.server.ts(2 hunks)apps/webapp/app/routes/account.security/route.tsx(1 hunks)apps/webapp/app/routes/auth.github.callback.tsx(1 hunks)apps/webapp/app/routes/login.magic/route.tsx(1 hunks)apps/webapp/app/routes/login.mfa/route.tsx(1 hunks)apps/webapp/app/routes/magic.tsx(1 hunks)apps/webapp/app/routes/resources.account.mfa.setup/MfaDisableDialog.tsx(1 hunks)apps/webapp/app/routes/resources.account.mfa.setup/MfaSetupDialog.tsx(1 hunks)apps/webapp/app/routes/resources.account.mfa.setup/MfaToggle.tsx(1 hunks)apps/webapp/app/routes/resources.account.mfa.setup/route.tsx(1 hunks)apps/webapp/app/routes/resources.account.mfa.setup/useMfaSetup.ts(1 hunks)apps/webapp/app/routes/storybook.switch/route.tsx(1 hunks)apps/webapp/app/services/emailAuth.server.tsx(2 hunks)apps/webapp/app/services/gitHubAuth.server.ts(2 hunks)apps/webapp/app/services/mfa/mfaRateLimiter.server.ts(1 hunks)apps/webapp/app/services/mfa/multiFactorAuthentication.server.ts(1 hunks)apps/webapp/app/services/session.server.ts(1 hunks)apps/webapp/app/utils/pathBuilder.ts(1 hunks)apps/webapp/package.json(3 hunks)internal-packages/database/prisma/migrations/20250705082744_add_mfa_schema/migration.sql(1 hunks)internal-packages/database/prisma/schema.prisma(2 hunks)internal-packages/emails/emails/mfa-disabled.tsx(1 hunks)internal-packages/emails/emails/mfa-enabled.tsx(1 hunks)internal-packages/emails/src/index.tsx(3 hunks)packages/build/src/package.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
`apps/webapp/**/*.{ts,tsx}`: When importing from `@trigger.dev/core` in the weba...
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, never import from the root@trigger.dev/corepath; always use one of the subpath exports as defined in the package's package.json.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/webapp.mdc)
List of files the instruction was applied to:
apps/webapp/app/routes/login.magic/route.tsxapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/services/session.server.tsapps/webapp/app/routes/storybook.switch/route.tsxapps/webapp/app/services/gitHubAuth.server.tsapps/webapp/app/services/emailAuth.server.tsxapps/webapp/app/components/primitives/Dialog.tsxapps/webapp/app/routes/magic.tsxapps/webapp/app/components/primitives/Switch.tsxapps/webapp/app/services/mfa/mfaRateLimiter.server.tsapps/webapp/app/components/primitives/CopyButton.tsxapps/webapp/app/routes/resources.account.mfa.setup/MfaDisableDialog.tsxapps/webapp/app/components/navigation/AccountSideMenu.tsxapps/webapp/app/models/message.server.tsapps/webapp/app/routes/auth.github.callback.tsxapps/webapp/app/routes/resources.account.mfa.setup/MfaToggle.tsxapps/webapp/app/routes/account.security/route.tsxapps/webapp/app/components/primitives/InputOTP.tsxapps/webapp/app/routes/resources.account.mfa.setup/useMfaSetup.tsapps/webapp/app/routes/resources.account.mfa.setup/MfaSetupDialog.tsxapps/webapp/app/routes/resources.account.mfa.setup/route.tsxapps/webapp/app/routes/login.mfa/route.tsxapps/webapp/app/services/mfa/multiFactorAuthentication.server.ts
`**/*.tsx`: When using React hooks for realtime updates, use `@trigger.dev/react-hooks` and provide a Public Access Token via context or props.
**/*.tsx: When using React hooks for realtime updates, use@trigger.dev/react-hooksand provide a Public Access Token via context or props.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/writing-tasks.mdc)
List of files the instruction was applied to:
apps/webapp/app/routes/login.magic/route.tsxapps/webapp/app/routes/storybook.switch/route.tsxapps/webapp/app/services/emailAuth.server.tsxapps/webapp/app/components/primitives/Dialog.tsxinternal-packages/emails/src/index.tsxapps/webapp/app/routes/magic.tsxapps/webapp/app/components/primitives/Switch.tsxapps/webapp/app/components/primitives/CopyButton.tsxapps/webapp/app/routes/resources.account.mfa.setup/MfaDisableDialog.tsxinternal-packages/emails/emails/mfa-disabled.tsxapps/webapp/app/components/navigation/AccountSideMenu.tsxinternal-packages/emails/emails/mfa-enabled.tsxapps/webapp/app/routes/auth.github.callback.tsxapps/webapp/app/routes/resources.account.mfa.setup/MfaToggle.tsxapps/webapp/app/routes/account.security/route.tsxapps/webapp/app/components/primitives/InputOTP.tsxapps/webapp/app/routes/resources.account.mfa.setup/MfaSetupDialog.tsxapps/webapp/app/routes/resources.account.mfa.setup/route.tsxapps/webapp/app/routes/login.mfa/route.tsx
`**/*.{ts,tsx}`: Always prefer using isomorphic code like fetch, ReadableStream,...
**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
Use strict mode
No default exports, use function declarations
📄 Source: CodeRabbit Inference Engine (.github/copilot-instructions.md)
List of files the instruction was applied to:
apps/webapp/app/routes/login.magic/route.tsxapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/services/session.server.tsapps/webapp/app/routes/storybook.switch/route.tsxapps/webapp/app/services/gitHubAuth.server.tsapps/webapp/app/services/emailAuth.server.tsxapps/webapp/app/components/primitives/Dialog.tsxinternal-packages/emails/src/index.tsxapps/webapp/app/routes/magic.tsxapps/webapp/app/components/primitives/Switch.tsxapps/webapp/app/services/mfa/mfaRateLimiter.server.tsapps/webapp/app/components/primitives/CopyButton.tsxapps/webapp/app/routes/resources.account.mfa.setup/MfaDisableDialog.tsxinternal-packages/emails/emails/mfa-disabled.tsxapps/webapp/app/components/navigation/AccountSideMenu.tsxinternal-packages/emails/emails/mfa-enabled.tsxapps/webapp/app/models/message.server.tsapps/webapp/app/routes/auth.github.callback.tsxapps/webapp/app/routes/resources.account.mfa.setup/MfaToggle.tsxapps/webapp/app/routes/account.security/route.tsxapps/webapp/app/components/primitives/InputOTP.tsxapps/webapp/app/routes/resources.account.mfa.setup/useMfaSetup.tsapps/webapp/app/routes/resources.account.mfa.setup/MfaSetupDialog.tsxapps/webapp/app/routes/resources.account.mfa.setup/route.tsxapps/webapp/app/routes/login.mfa/route.tsxapps/webapp/app/services/mfa/multiFactorAuthentication.server.ts
`{packages/core,apps/webapp}/**/*.{ts,tsx}`: We use zod a lot in packages/core and in the webapp
{packages/core,apps/webapp}/**/*.{ts,tsx}: We use zod a lot in packages/core and in the webapp
📄 Source: CodeRabbit Inference Engine (.github/copilot-instructions.md)
List of files the instruction was applied to:
apps/webapp/app/routes/login.magic/route.tsxapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/services/session.server.tsapps/webapp/app/routes/storybook.switch/route.tsxapps/webapp/app/services/gitHubAuth.server.tsapps/webapp/app/services/emailAuth.server.tsxapps/webapp/app/components/primitives/Dialog.tsxapps/webapp/app/routes/magic.tsxapps/webapp/app/components/primitives/Switch.tsxapps/webapp/app/services/mfa/mfaRateLimiter.server.tsapps/webapp/app/components/primitives/CopyButton.tsxapps/webapp/app/routes/resources.account.mfa.setup/MfaDisableDialog.tsxapps/webapp/app/components/navigation/AccountSideMenu.tsxapps/webapp/app/models/message.server.tsapps/webapp/app/routes/auth.github.callback.tsxapps/webapp/app/routes/resources.account.mfa.setup/MfaToggle.tsxapps/webapp/app/routes/account.security/route.tsxapps/webapp/app/components/primitives/InputOTP.tsxapps/webapp/app/routes/resources.account.mfa.setup/useMfaSetup.tsapps/webapp/app/routes/resources.account.mfa.setup/MfaSetupDialog.tsxapps/webapp/app/routes/resources.account.mfa.setup/route.tsxapps/webapp/app/routes/login.mfa/route.tsxapps/webapp/app/services/mfa/multiFactorAuthentication.server.ts
`apps/webapp/**/*.ts`: In the webapp, all environment variables must be accessed through the `env` export of `env.server.ts`, instead of directly accessing `process.env`.
apps/webapp/**/*.ts: In the webapp, all environment variables must be accessed through theenvexport ofenv.server.ts, instead of directly accessingprocess.env.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/webapp.mdc)
List of files the instruction was applied to:
apps/webapp/app/utils/pathBuilder.tsapps/webapp/app/services/session.server.tsapps/webapp/app/services/gitHubAuth.server.tsapps/webapp/app/services/mfa/mfaRateLimiter.server.tsapps/webapp/app/models/message.server.tsapps/webapp/app/routes/resources.account.mfa.setup/useMfaSetup.tsapps/webapp/app/services/mfa/multiFactorAuthentication.server.ts
🧠 Learnings (15)
packages/build/src/package.json (4)
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-30T13:21:33.994Z
Learning: Applies to {packages/core,apps/webapp}/**/*.{ts,tsx} : We use zod a lot in packages/core and in the webapp
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-30T13:21:33.994Z
Learning: Applies to **/*.{ts,tsx} : No default exports, use function declarations
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-30T13:21:33.994Z
Learning: Applies to **/*.{ts,tsx} : Use strict mode
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-30T13:21:33.994Z
Learning: Applies to **/*.{ts,tsx} : Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
apps/webapp/package.json (2)
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-30T13:21:33.994Z
Learning: Applies to {packages/core,apps/webapp}/**/*.{ts,tsx} : We use zod a lot in packages/core and in the webapp
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-06-30T13:21:59.438Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : When importing from `@trigger.dev/core` in the webapp, never import from the root `@trigger.dev/core` path; always use one of the subpath exports as defined in the package's package.json.
internal-packages/emails/src/index.tsx (1)
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.528Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Schema tasks must use `schemaTask` from `@trigger.dev/sdk/v3` and validate payloads using a schema (e.g., Zod).
apps/webapp/app/routes/resources.account.mfa.setup/MfaDisableDialog.tsx (1)
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.528Z
Learning: Applies to **/*.tsx : When using React hooks for realtime updates, use `@trigger.dev/react-hooks` and provide a Public Access Token via context or props.
internal-packages/emails/emails/mfa-disabled.tsx (1)
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.528Z
Learning: Applies to **/*.tsx : When using React hooks for realtime updates, use `@trigger.dev/react-hooks` and provide a Public Access Token via context or props.
apps/webapp/app/components/navigation/AccountSideMenu.tsx (1)
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.528Z
Learning: Applies to **/*.tsx : When using React hooks for realtime updates, use `@trigger.dev/react-hooks` and provide a Public Access Token via context or props.
internal-packages/emails/emails/mfa-enabled.tsx (2)
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.528Z
Learning: Applies to **/*.tsx : When using React hooks for realtime updates, use `@trigger.dev/react-hooks` and provide a Public Access Token via context or props.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.528Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Schema tasks must use `schemaTask` from `@trigger.dev/sdk/v3` and validate payloads using a schema (e.g., Zod).
apps/webapp/app/routes/resources.account.mfa.setup/MfaToggle.tsx (1)
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.528Z
Learning: Applies to **/*.tsx : When using React hooks for realtime updates, use `@trigger.dev/react-hooks` and provide a Public Access Token via context or props.
apps/webapp/app/routes/account.security/route.tsx (2)
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.528Z
Learning: Applies to **/*.tsx : When using React hooks for realtime updates, use `@trigger.dev/react-hooks` and provide a Public Access Token via context or props.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-06-30T13:21:59.438Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : When importing from `@trigger.dev/core` in the webapp, never import from the root `@trigger.dev/core` path; always use one of the subpath exports as defined in the package's package.json.
apps/webapp/app/components/primitives/InputOTP.tsx (1)
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.528Z
Learning: Applies to **/*.tsx : When using React hooks for realtime updates, use `@trigger.dev/react-hooks` and provide a Public Access Token via context or props.
apps/webapp/app/routes/resources.account.mfa.setup/useMfaSetup.ts (1)
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.528Z
Learning: Applies to **/*.tsx : When using React hooks for realtime updates, use `@trigger.dev/react-hooks` and provide a Public Access Token via context or props.
internal-packages/database/prisma/migrations/20250705082744_add_mfa_schema/migration.sql (2)
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-30T13:21:33.994Z
Learning: Applies to internal-packages/database/**/*.{ts,tsx} : We use prisma in internal-packages/database for our database interactions using PostgreSQL
Learnt from: ericallam
PR: triggerdotdev/trigger.dev#2086
File: internal-packages/database/prisma/migrations/20250511145836_runtime_environment_add_is_branchable_environment/migration.sql:1-3
Timestamp: 2025-05-27T19:30:34.004Z
Learning: In modern PostgreSQL versions (11+), adding a column with a constant default value (like DEFAULT false, DEFAULT 0, DEFAULT 'text') does NOT require a table rewrite. PostgreSQL stores the default value in the catalog and applies it virtually when reading rows. Only non-constant defaults or more complex scenarios require table rewrites. Avoid suggesting multi-step migrations for simple constant defaults.
apps/webapp/app/routes/resources.account.mfa.setup/MfaSetupDialog.tsx (1)
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.528Z
Learning: Applies to **/*.tsx : When using React hooks for realtime updates, use `@trigger.dev/react-hooks` and provide a Public Access Token via context or props.
apps/webapp/app/services/mfa/multiFactorAuthentication.server.ts (1)
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-30T13:21:33.994Z
Learning: Applies to internal-packages/database/**/*.{ts,tsx} : We use prisma in internal-packages/database for our database interactions using PostgreSQL
internal-packages/database/prisma/schema.prisma (1)
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-30T13:21:33.994Z
Learning: Applies to internal-packages/database/**/*.{ts,tsx} : We use prisma in internal-packages/database for our database interactions using PostgreSQL
🧬 Code Graph Analysis (12)
apps/webapp/app/routes/storybook.switch/route.tsx (1)
apps/webapp/app/components/primitives/Switch.tsx (1)
Switch(52-107)
apps/webapp/app/services/gitHubAuth.server.ts (1)
apps/webapp/app/services/mfa/multiFactorAuthentication.server.ts (4)
user(248-267)user(269-285)user(287-295)MfaRequiredError(18-25)
apps/webapp/app/components/primitives/Dialog.tsx (1)
apps/webapp/app/components/primitives/ShortcutKey.tsx (1)
ShortcutKey(38-55)
internal-packages/emails/src/index.tsx (2)
internal-packages/emails/emails/mfa-enabled.tsx (1)
MfaEnabledEmailSchema(7-10)internal-packages/emails/emails/mfa-disabled.tsx (1)
MfaDisabledEmailSchema(7-10)
apps/webapp/app/components/primitives/Switch.tsx (1)
apps/webapp/app/utils/cn.ts (1)
cn(31-33)
apps/webapp/app/components/primitives/CopyButton.tsx (1)
apps/webapp/app/utils/cn.ts (1)
cn(31-33)
internal-packages/emails/emails/mfa-disabled.tsx (1)
internal-packages/emails/emails/mfa-enabled.tsx (1)
apps/webapp/app/components/navigation/AccountSideMenu.tsx (2)
apps/webapp/app/components/navigation/SideMenuItem.tsx (1)
SideMenuItem(7-53)apps/webapp/app/utils/pathBuilder.ts (1)
accountSecurityPath(63-65)
internal-packages/emails/emails/mfa-enabled.tsx (1)
internal-packages/emails/emails/mfa-disabled.tsx (1)
apps/webapp/app/routes/auth.github.callback.tsx (3)
apps/webapp/app/routes/magic.tsx (1)
loader(8-39)apps/webapp/app/routes/login.mfa/route.tsx (1)
loader(44-76)apps/webapp/app/services/mfa/multiFactorAuthentication.server.ts (1)
MfaRequiredError(18-25)
apps/webapp/app/components/primitives/InputOTP.tsx (2)
apps/webapp/app/components/primitives/ShortcutKey.tsx (1)
variants(14-19)apps/webapp/app/utils/cn.ts (1)
cn(31-33)
apps/webapp/app/routes/resources.account.mfa.setup/useMfaSetup.ts (1)
apps/webapp/app/routes/resources.account.mfa.setup/route.tsx (1)
action(52-139)
⏰ 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). (25)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (10, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (9, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (60)
apps/webapp/app/utils/pathBuilder.ts (1)
63-65: LGTM! Clean path builder addition.The new
accountSecurityPathfunction follows the established pattern of other path builders in this file and correctly returns the expected route for the account security page.apps/webapp/app/routes/login.magic/route.tsx (1)
116-116: Cosmetic change looks good.The color change from
text-primarytotext-indigo-500for the inbox icon is a straightforward UI update.Note: The AI summary mentions MFA handling changes in the loader function, but those changes are not visible in the provided code segment.
Likely an incorrect or invalid review comment.
apps/webapp/app/routes/storybook.switch/route.tsx (1)
9-9: Good addition for testing the new labelPosition prop.This storybook entry properly demonstrates the new
labelPosition="right"functionality of the Switch component, providing visual verification of the feature.apps/webapp/app/services/session.server.ts (1)
56-56: Good addition for MFA functionality.Adding
mfaEnabledAtto the session user data is essential for MFA implementation, allowing components and services throughout the application to check the user's MFA status.apps/webapp/app/services/emailAuth.server.tsx (3)
9-9: Import looks good.The
MfaRequiredErrorimport is correctly added for the MFA integration.
40-44: MFA check implementation is correct.The MFA check is properly placed after successful authentication and before returning the user. The logic correctly throws
MfaRequiredErrorwhen MFA is enabled, which will be caught by the magic link route for proper MFA flow handling.
48-51: Error handling is appropriate.The special handling for
MfaRequiredErroris correct - these are expected flow control errors that should be re-thrown without logging, while other errors continue to be logged as actual problems.apps/webapp/app/components/primitives/Dialog.tsx (3)
39-41: Props definition looks good.The
showCloseButtonprop is properly typed as optional boolean with appropriate intersection with the base component props.
42-42: Default value is well-chosen.Setting
showCloseButton = trueas the default maintains backward compatibility while allowing opt-out when needed.
55-66: Conditional rendering is correctly implemented.The close button rendering is properly gated by the
showCloseButtonprop, maintaining the existing functionality when enabled.apps/webapp/app/services/gitHubAuth.server.ts (3)
8-8: Import is consistent.The
MfaRequiredErrorimport matches the pattern used in other authentication strategies.
44-48: MFA check is properly implemented.The MFA check follows the same pattern as the email authentication strategy, correctly throwing
MfaRequiredErrorwhen MFA is enabled. This will be caught by the GitHub callback route for proper MFA flow handling.
54-57: Error handling is consistent.The error handling pattern matches the email auth strategy, properly re-throwing
MfaRequiredErrorwithout logging while maintaining logging for other errors.apps/webapp/app/components/navigation/AccountSideMenu.tsx (3)
1-1: Icon import is appropriate.The
LockClosedIconis a fitting choice for the security menu item.
5-10: Path import is correctly added.The
accountSecurityPathimport is properly added to the existing import statement.
50-56: Security menu item is well-implemented.The new menu item follows the established patterns with:
- Appropriate icon (
LockClosedIcon)- Consistent styling (
text-rose-500for active state)- Correct path usage (
accountSecurityPath())- Proper data attribute for testing/analytics
apps/webapp/app/components/primitives/Switch.tsx (4)
49-49: Prop type is well-defined.The
labelPositionprop is properly typed with clear options and good naming.
53-53: Default value maintains compatibility.Setting
labelPosition = "left"as the default ensures backward compatibility with existing usage.
71-93: Element extraction improves readability.The refactoring to extract
labelElementandswitchElementinto separate variables makes the conditional rendering logic clearer and easier to understand.
101-103: Conditional rendering is correctly implemented.The label positioning logic properly renders the label on the left or right based on the
labelPositionprop, maintaining clean conditional logic.internal-packages/emails/src/index.tsx (3)
17-18: LGTM!The import statements for MFA email components and schemas are correctly added and follow the existing import pattern.
33-34: LGTM!The MFA email schemas are properly added to the discriminated union, maintaining type safety for email delivery.
125-136: LGTM!The template logic for MFA emails follows the existing pattern consistently. The subject lines are clear and descriptive, and the component usage matches the established conventions.
apps/webapp/app/components/primitives/CopyButton.tsx (3)
30-31: LGTM!The addition of an optional
childrenprop enhances the component's flexibility while maintaining type safety.
41-42: LGTM!The children prop is properly added to the component's parameter destructuring.
71-90: LGTM!The refactoring to use
LeadingIconprop instead of rendering icons as children is clean and maintains proper icon positioning. The conditional styling based on button variant ensures consistent visual feedback across different button states.apps/webapp/app/routes/magic.tsx (2)
2-6: LGTM!The new imports are appropriate for the MFA flow implementation, including the
MfaRequiredErrorand session management utilities.
9-38: LGTM!The refactored loader function properly handles MFA requirements with explicit error handling. The try-catch pattern allows for controlled flow management, and session storage correctly persists the pending MFA state for continuation after verification.
apps/webapp/app/routes/account.security/route.tsx (3)
14-20: LGTM!The meta function properly sets the page title following the existing application pattern.
22-28: LGTM!The loader function correctly requires user authentication and returns the user data using type-safe JSON handling.
30-49: LGTM!The component structure follows the established layout patterns and properly integrates with the MFA setup component by passing the user's MFA status as a boolean prop.
internal-packages/emails/emails/mfa-enabled.tsx (2)
7-17: LGTM!The Zod schema definition and type inference follow the established pattern for email templates, ensuring type safety and consistency.
19-62: LGTM!The email template follows the established pattern with appropriate content for MFA enablement notification. The security guidance is helpful and the styling is consistent with other email templates. The personalization with the user's email address is properly implemented.
apps/webapp/app/models/message.server.ts (3)
3-3: LGTM: Import statement correctly updatedThe import of
typedjsonfromremix-typedjsonis appropriate for supporting the new typed JSON helper functions.
124-141: LGTM: Well-implemented typed JSON helper for success messagesThe
typedJsonWithSuccessMessagefunction provides a type-safe alternative to the existingjsonWithSuccessMessagefunction. The implementation follows the established pattern and maintains consistency with the existing codebase.
143-160: LGTM: Well-implemented typed JSON helper for error messagesThe
typedJsonWithErrorMessagefunction provides a type-safe alternative to the existingjsonWithErrorMessagefunction. The implementation follows the established pattern and maintains consistency with the existing codebase.internal-packages/emails/emails/mfa-disabled.tsx (2)
7-10: LGTM: Proper zod schema definitionThe
MfaDisabledEmailSchemacorrectly defines the email type as a literal and includes the requireduserEmailfield, following the established pattern for email schemas.
19-48: LGTM: Well-structured email componentThe email component follows the established pattern from the
mfa-enabled.tsxfile and includes:
- Proper preview text for email clients
- Clear messaging about MFA being disabled
- Security warnings for unauthorized changes
- Instructions for re-enabling MFA
- Consistent styling and branding
The content is appropriate and security-focused.
apps/webapp/app/services/mfa/mfaRateLimiter.server.ts (3)
6-25: LGTM: Well-configured MFA rate limiterThe rate limiter initialization is properly implemented with:
- Appropriate rate limit of 10 attempts per minute using sliding window
- Singleton pattern for efficiency
- Privacy-conscious logging (failures only)
- Proper Redis configuration using environment variables
The configuration strikes a good balance between security and usability.
27-34: LGTM: Useful custom error classThe
MfaRateLimitErrorclass extendsErrorand includes aretryAfterproperty that provides valuable information about when the user can retry, improving user experience.
41-48: LGTM: Proper rate limit check implementationThe
checkMfaRateLimitfunction correctly:
- Uses the rate limiter singleton
- Throws appropriate error when limit is exceeded
- Calculates proper retry delay
- Provides clear JSDoc documentation
apps/webapp/app/routes/resources.account.mfa.setup/MfaToggle.tsx (2)
7-10: LGTM: Clear and well-typed props interfaceThe
MfaTogglePropsinterface is properly defined with clear boolean state and callback function for toggle handling.
12-35: LGTM: Well-structured MFA toggle componentThe component implementation is clean and includes:
- Proper form structure with POST method
- Clear descriptive text about MFA functionality
- Switch component with appropriate state binding
- Dynamic label text based on current state
- Proper callback handling for state changes
apps/webapp/app/routes/resources.account.mfa.setup/MfaDisableDialog.tsx (3)
19-25: LGTM: Well-defined props interfaceThe
MfaDisableDialogPropsinterface properly defines all necessary props for dialog state management, form submission, and error handling.
34-58: LGTM: Proper state management and event handlingThe component correctly manages internal state for:
- TOTP and recovery code inputs
- Input mode switching
- Form submission and cancellation
The event handlers properly reset state and coordinate with parent callbacks.
60-152: LGTM: Comprehensive dialog UI implementationThe dialog implementation includes:
- Proper form structure with submission handling
- Clear UI for both TOTP and recovery code inputs
- Intuitive mode switching between authentication methods
- Appropriate input validation and error display
- Loading states with spinner and disabled buttons
- Proper accessibility with autofocus and form labels
The component provides a complete and user-friendly MFA disable experience.
apps/webapp/app/routes/auth.github.callback.tsx (1)
10-52: MFA integration looks good!The implementation correctly handles MFA requirements by catching
MfaRequiredErrorand redirecting to the MFA login page with proper session state management. This follows the same pattern as the magic link authentication.internal-packages/database/prisma/migrations/20250705082744_add_mfa_schema/migration.sql (1)
1-27: Database migration is well-structured!The migration correctly:
- Adds nullable MFA columns to the User table (safe operation in PostgreSQL 11+)
- Creates the MfaBackupCode table with appropriate constraints
- Sets up foreign keys with proper cascade behaviors (SET NULL for secret reference, RESTRICT for backup codes)
- Ensures backup code uniqueness per user with the composite unique index
internal-packages/database/prisma/schema.prisma (2)
49-60: MFA schema additions are well-designed!The User model extensions properly support MFA with:
- Optional timestamp for tracking when MFA was enabled
- Secure secret storage via SecretReference relation
- Replay attack prevention with
mfaLastUsedCodehash storage- One-to-many relation for backup codes
62-76: MfaBackupCode model is properly structured!The model includes all necessary fields for secure backup code management:
- Hash storage for the actual codes (secure)
- Usage tracking with
usedAttimestamp- Proper foreign key relation to User
- Composite unique constraint ensuring code uniqueness per user
apps/webapp/app/routes/resources.account.mfa.setup/route.tsx (3)
13-32: Well-structured form validation schema!The discriminated union approach with zod provides type-safe validation for all MFA actions, ensuring each action has the required fields.
69-139: Comprehensive MFA action handling!The switch statement properly handles all MFA operations with appropriate:
- Service method calls for each action
- Success/error response formatting
- Proper error messages for validation failures
- Redirect with success message after saving recovery codes
52-54: No change needed for MFA status checks
TherequireUserIdcall correctly retrieves the user’s ID, and theMultiFactorAuthenticationServiceimmediately fetches the full user record—explicitly includingmfaEnabledAt—before any enable/disable or validation logic. You don’t needrequireUserhere.Likely an incorrect or invalid review comment.
apps/webapp/app/routes/resources.account.mfa.setup/MfaSetupDialog.tsx (3)
61-74: Download functionality is correctly implemented!The recovery codes download uses standard browser APIs properly:
- Creates a text blob with proper MIME type
- Generates a temporary URL
- Triggers download with descriptive filename
- Cleans up resources (removes element and revokes URL)
159-181: OTP input implementation is user-friendly!The 6-digit OTP input:
- Uses individual slots for better UX
- Supports Enter key submission when complete
- Has proper autoFocus on first slot
- Validates length before enabling submit button
76-133: Recovery codes display is well-designed!The recovery codes view provides:
- Clear instructions for users
- Grid layout for easy readability
- Both download and copy options for flexibility
- Proper form handling to continue the flow
apps/webapp/app/components/primitives/InputOTP.tsx (1)
1-111: Well-implemented OTP input componentsThe implementation follows React best practices with proper component composition, variant-based styling, and accessibility attributes. The use of context for slot state management is appropriate, and the error handling for undefined context values is correct.
apps/webapp/app/routes/login.mfa/route.tsx (1)
167-317: Well-structured MFA login componentThe component properly manages state for both TOTP and recovery code flows, with good UX considerations like clearing input on submission and managing error visibility. The use of effects to handle navigation state changes is appropriate.
apps/webapp/app/routes/resources.account.mfa.setup/useMfaSetup.ts (1)
232-302: Well-designed hook APIThe hook provides a clean API with logical action methods and computed properties for UI state. The separation of concerns between state management and UI interactions is well-implemented.
apps/webapp/app/services/mfa/multiFactorAuthentication.server.ts (1)
345-386: Proper recovery code usage trackingThe implementation correctly marks recovery codes as used and checks for unused codes only. This prevents reuse of recovery codes.
apps/webapp/app/services/mfa/multiFactorAuthentication.server.ts
Outdated
Show resolved
Hide resolved
apps/webapp/app/services/mfa/multiFactorAuthentication.server.ts
Outdated
Show resolved
Hide resolved
Implements TOTP multi-factor authentication for both magic link and Github authentication. Uses @better-auth/utils for the TOTP generation/validation. Rate limits TOTP validation to 10 validations per minute.