feat: 2fa backup codes #3
Conversation
Co-authored-by: Peer Richelsen <[email protected]>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
8 issues found across 16 files
Prompt for AI agents (all 8 issues)
Understand the root cause of the following 8 issues and fix them.
<file name="packages/features/auth/lib/next-auth-options.ts">
<violation number="1" location="packages/features/auth/lib/next-auth-options.ts:131">
Two-factor backup code validation and decryption logic duplicates apps/web/pages/api/auth/two-factor/totp/disable.ts (lines 14-27 of the provided snippet).</violation>
<violation number="2" location="packages/features/auth/lib/next-auth-options.ts:144">
TOCTOU race: backup code can be consumed twice by concurrent logins due to non-atomic read-check-write.</violation>
</file>
<file name="apps/web/components/settings/DisableTwoFactorModal.tsx">
<violation number="1" location="apps/web/components/settings/DisableTwoFactorModal.tsx:58">
Updating `TwoFactorAuthAPI.disable` to require `backupCode` breaks other callers (e.g. `apps/web/components/security/DisableTwoFactorModal.tsx` still invokes it with only password and totp), so the build will fail with “Expected 3 arguments, but got 2.” Please either supply the backup code at every call site or make the new argument optional.
*DEV MODE: This violation would have been filtered out by screening filters. Failing filters: typescriptUnderstanding.*
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: Invalid: Two distinct local TwoFactorAuthAPI modules exist (settings and security) with different disable() signatures, and each call site imports the correct local module matching its signature. No build error from argument count.</violation>
</file>
<file name="packages/ui/components/form/inputs/Input.tsx">
<violation number="1" location="packages/ui/components/form/inputs/Input.tsx:49">
Setting `tabIndex={-1}` on this password visibility `<button>` removes it from the tab order, so keyboard users can no longer focus and activate the toggle—this regresses accessibility and functionality.</violation>
</file>
<file name="apps/web/pages/api/auth/two-factor/totp/disable.ts">
<violation number="1" location="apps/web/pages/api/auth/two-factor/totp/disable.ts:61">
A security vulnerability exists where a backup code can be reused. In the "disable 2FA" API endpoint, a valid backup code is checked but not immediately invalidated. This allows the same backup code to be used for multiple attempts if other checks (like the password) fail. This contradicts the correct one-time-use implementation found in the login flow (`packages/features/auth/lib/next-auth-options.ts`).
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: Incorrect claim: password is checked first, so a failed password prevents any backupCode evaluation. If a valid backup code is provided, the endpoint proceeds to disable 2FA and clears backupCodes (sets null) in the final update, preventing reuse. No subsequent checks after a valid backup code would fail and leave it reusable.</violation>
</file>
<file name="apps/web/components/settings/EnableTwoFactorModal.tsx">
<violation number="1" location="apps/web/components/settings/EnableTwoFactorModal.tsx:64">
`useState([])` infers the state as `never[]`, so `setBackupCodes(body.backupCodes)` fails to compile. Please initialize the hook with an explicit `string[]` type.
*DEV MODE: This violation would have been filtered out by screening filters. Failing filters: typescriptUnderstanding.*</violation>
</file>
<file name="apps/web/components/auth/BackupCode.tsx">
<violation number="1" location="apps/web/components/auth/BackupCode.tsx:13">
Please associate this `Label` with the input by adding an `htmlFor` attribute so the backup code field has an accessible name.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: Accurate but purely accessibility-related and low impact per review guidelines; not critical enough to report.</violation>
</file>
<file name="apps/web/pages/api/auth/two-factor/totp/setup.ts">
<violation number="1" location="apps/web/pages/api/auth/two-factor/totp/setup.ts:68">
Uncaught exception risk: symmetricEncrypt may throw on invalid CALENDSO_ENCRYPTION_KEY length, causing 500.
*DEV MODE: This violation would have been filtered out by screening filters. Failing filters: uncertaintyLanguage.*</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| } | ||
|
|
||
| if (user.twoFactorEnabled) { | ||
| if (user.twoFactorEnabled && credentials.backupCode) { |
There was a problem hiding this comment.
Two-factor backup code validation and decryption logic duplicates apps/web/pages/api/auth/two-factor/totp/disable.ts (lines 14-27 of the provided snippet).
Prompt for AI agents
Address the following comment on packages/features/auth/lib/next-auth-options.ts at line 131:
<comment>Two-factor backup code validation and decryption logic duplicates apps/web/pages/api/auth/two-factor/totp/disable.ts (lines 14-27 of the provided snippet).</comment>
<file context>
@@ -126,7 +128,33 @@ const providers: Provider[] = [
}
- if (user.twoFactorEnabled) {
+ if (user.twoFactorEnabled && credentials.backupCode) {
+ if (!process.env.CALENDSO_ENCRYPTION_KEY) {
+ console.error("Missing encryption key; cannot proceed with backup code login.");
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: Duplicate Detection Agent
|
|
||
| try { | ||
| const response = await TwoFactorAuthAPI.disable(password, totpCode); | ||
| const response = await TwoFactorAuthAPI.disable(password, totpCode, backupCode); |
There was a problem hiding this comment.
Updating TwoFactorAuthAPI.disable to require backupCode breaks other callers (e.g. apps/web/components/security/DisableTwoFactorModal.tsx still invokes it with only password and totp), so the build will fail with “Expected 3 arguments, but got 2.” Please either supply the backup code at every call site or make the new argument optional.
DEV MODE: This violation would have been filtered out by screening filters. Failing filters: typescriptUnderstanding.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• GPT-5: Invalid: Two distinct local TwoFactorAuthAPI modules exist (settings and security) with different disable() signatures, and each call site imports the correct local module matching its signature. No build error from argument count.
Prompt for AI agents
Address the following comment on apps/web/components/settings/DisableTwoFactorModal.tsx at line 58:
<comment>Updating `TwoFactorAuthAPI.disable` to require `backupCode` breaks other callers (e.g. `apps/web/components/security/DisableTwoFactorModal.tsx` still invokes it with only password and totp), so the build will fail with “Expected 3 arguments, but got 2.” Please either supply the backup code at every call site or make the new argument optional.
*DEV MODE: This violation would have been filtered out by screening filters. Failing filters: typescriptUnderstanding.*
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: Invalid: Two distinct local TwoFactorAuthAPI modules exist (settings and security) with different disable() signatures, and each call site imports the correct local module matching its signature. No build error from argument count.</comment>
<file context>
@@ -33,33 +35,45 @@ const DisableTwoFactorAuthModal = ({
try {
- const response = await TwoFactorAuthAPI.disable(password, totpCode);
+ const response = await TwoFactorAuthAPI.disable(password, totpCode, backupCode);
if (response.status === 200) {
+ setTwoFactorLostAccess(false);
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
| <button className="text-emphasis h-9" type="button" onClick={() => toggleIsPasswordVisible()}> | ||
| <button | ||
| className="text-emphasis h-9" | ||
| tabIndex={-1} |
There was a problem hiding this comment.
Setting tabIndex={-1} on this password visibility <button> removes it from the tab order, so keyboard users can no longer focus and activate the toggle—this regresses accessibility and functionality.
Prompt for AI agents
Address the following comment on packages/ui/components/form/inputs/Input.tsx at line 49:
<comment>Setting `tabIndex={-1}` on this password visibility `<button>` removes it from the tab order, so keyboard users can no longer focus and activate the toggle—this regresses accessibility and functionality.</comment>
<file context>
@@ -44,7 +44,11 @@ export const PasswordField = forwardRef<HTMLInputElement, InputFieldProps>(funct
- <button className="text-emphasis h-9" type="button" onClick={() => toggleIsPasswordVisible()}>
+ <button
+ className="text-emphasis h-9"
+ tabIndex={-1}
+ type="button"
+ onClick={() => toggleIsPasswordVisible()}>
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
| tabIndex={-1} | |
| tabIndex={0} |
| const backupCodes = JSON.parse(symmetricDecrypt(user.backupCodes, process.env.CALENDSO_ENCRYPTION_KEY)); | ||
|
|
||
| // check if user-supplied code matches one | ||
| const index = backupCodes.indexOf(req.body.backupCode.replaceAll("-", "")); |
There was a problem hiding this comment.
A security vulnerability exists where a backup code can be reused. In the "disable 2FA" API endpoint, a valid backup code is checked but not immediately invalidated. This allows the same backup code to be used for multiple attempts if other checks (like the password) fail. This contradicts the correct one-time-use implementation found in the login flow (packages/features/auth/lib/next-auth-options.ts).
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• GPT-5: Incorrect claim: password is checked first, so a failed password prevents any backupCode evaluation. If a valid backup code is provided, the endpoint proceeds to disable 2FA and clears backupCodes (sets null) in the final update, preventing reuse. No subsequent checks after a valid backup code would fail and leave it reusable.
Prompt for AI agents
Address the following comment on apps/web/pages/api/auth/two-factor/totp/disable.ts at line 61:
<comment>A security vulnerability exists where a backup code can be reused. In the "disable 2FA" API endpoint, a valid backup code is checked but not immediately invalidated. This allows the same backup code to be used for multiple attempts if other checks (like the password) fail. This contradicts the correct one-time-use implementation found in the login flow (`packages/features/auth/lib/next-auth-options.ts`).
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: Incorrect claim: password is checked first, so a failed password prevents any backupCode evaluation. If a valid backup code is provided, the endpoint proceeds to disable 2FA and clears backupCodes (sets null) in the final update, preventing reuse. No subsequent checks after a valid backup code would fail and leave it reusable.</comment>
<file context>
@@ -43,8 +43,30 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
+ const backupCodes = JSON.parse(symmetricDecrypt(user.backupCodes, process.env.CALENDSO_ENCRYPTION_KEY));
+
+ // check if user-supplied code matches one
+ const index = backupCodes.indexOf(req.body.backupCode.replaceAll("-", ""));
+ if (index === -1) {
+ return res.status(400).json({ error: ErrorCode.IncorrectBackupCode });
</file context>
[internal] Confidence score: 10/10
[internal] Posted by: System Design Agent
| }; | ||
| const [step, setStep] = useState(SetupStep.ConfirmPassword); | ||
| const [password, setPassword] = useState(""); | ||
| const [backupCodes, setBackupCodes] = useState([]); |
There was a problem hiding this comment.
useState([]) infers the state as never[], so setBackupCodes(body.backupCodes) fails to compile. Please initialize the hook with an explicit string[] type.
DEV MODE: This violation would have been filtered out by screening filters. Failing filters: typescriptUnderstanding.
Prompt for AI agents
Address the following comment on apps/web/components/settings/EnableTwoFactorModal.tsx at line 64:
<comment>`useState([])` infers the state as `never[]`, so `setBackupCodes(body.backupCodes)` fails to compile. Please initialize the hook with an explicit `string[]` type.
*DEV MODE: This violation would have been filtered out by screening filters. Failing filters: typescriptUnderstanding.*</comment>
<file context>
@@ -54,16 +55,25 @@ const EnableTwoFactorModal = ({ onEnable, onCancel, open, onOpenChange }: Enable
};
const [step, setStep] = useState(SetupStep.ConfirmPassword);
const [password, setPassword] = useState("");
+ const [backupCodes, setBackupCodes] = useState([]);
+ const [backupCodesUrl, setBackupCodesUrl] = useState("");
const [dataUri, setDataUri] = useState("");
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
| const [backupCodes, setBackupCodes] = useState([]); | |
| const [backupCodes, setBackupCodes] = useState<string[]>([]); |
| ); | ||
|
|
||
| // check if user-supplied code matches one | ||
| const index = backupCodes.indexOf(credentials.backupCode.replaceAll("-", "")); |
There was a problem hiding this comment.
TOCTOU race: backup code can be consumed twice by concurrent logins due to non-atomic read-check-write.
Prompt for AI agents
Address the following comment on packages/features/auth/lib/next-auth-options.ts at line 144:
<comment>TOCTOU race: backup code can be consumed twice by concurrent logins due to non-atomic read-check-write.</comment>
<file context>
@@ -126,7 +128,33 @@ const providers: Provider[] = [
+ );
+
+ // check if user-supplied code matches one
+ const index = backupCodes.indexOf(credentials.backupCode.replaceAll("-", ""));
+ if (index === -1) throw new Error(ErrorCode.IncorrectBackupCode);
+
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: Functional Bugs Agent
|
|
||
| return ( | ||
| <div className={center ? "mx-auto !mt-0 max-w-sm" : "!mt-0 max-w-sm"}> | ||
| <Label className="mt-4">{t("backup_code")}</Label> |
There was a problem hiding this comment.
Please associate this Label with the input by adding an htmlFor attribute so the backup code field has an accessible name.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• GPT-5: Accurate but purely accessibility-related and low impact per review guidelines; not critical enough to report.
Prompt for AI agents
Address the following comment on apps/web/components/auth/BackupCode.tsx at line 13:
<comment>Please associate this `Label` with the input by adding an `htmlFor` attribute so the backup code field has an accessible name.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: Accurate but purely accessibility-related and low impact per review guidelines; not critical enough to report.</comment>
<file context>
@@ -0,0 +1,29 @@
+
+ return (
+ <div className={center ? "mx-auto !mt-0 max-w-sm" : "!mt-0 max-w-sm"}>
+ <Label className="mt-4">{t("backup_code")}</Label>
+
+ <p className="text-subtle mb-4 text-sm">{t("backup_code_instructions")}</p>
</file context>
[internal] Confidence score: 7/10
[internal] Posted by: General AI Review Agent
| id: session.user.id, | ||
| }, | ||
| data: { | ||
| backupCodes: symmetricEncrypt(JSON.stringify(backupCodes), process.env.CALENDSO_ENCRYPTION_KEY), |
There was a problem hiding this comment.
Uncaught exception risk: symmetricEncrypt may throw on invalid CALENDSO_ENCRYPTION_KEY length, causing 500.
DEV MODE: This violation would have been filtered out by screening filters. Failing filters: uncertaintyLanguage.
Prompt for AI agents
Address the following comment on apps/web/pages/api/auth/two-factor/totp/setup.ts at line 68:
<comment>Uncaught exception risk: symmetricEncrypt may throw on invalid CALENDSO_ENCRYPTION_KEY length, causing 500.
*DEV MODE: This violation would have been filtered out by screening filters. Failing filters: uncertaintyLanguage.*</comment>
<file context>
@@ -56,11 +57,15 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
id: session.user.id,
},
data: {
+ backupCodes: symmetricEncrypt(JSON.stringify(backupCodes), process.env.CALENDSO_ENCRYPTION_KEY),
twoFactorEnabled: false,
twoFactorSecret: symmetricEncrypt(secret, process.env.CALENDSO_ENCRYPTION_KEY),
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: Functional Bugs Agent
There was a problem hiding this comment.
9 issues found across 16 files
Prompt for AI agents (all 9 issues)
Understand the root cause of the following 9 issues and fix them.
<file name="apps/web/components/settings/TwoFactorAuthAPI.ts">
<violation number="1" location="apps/web/components/settings/TwoFactorAuthAPI.ts:22">
The `disable` function for 2FA in `apps/web/components/settings/TwoFactorAuthAPI.ts` duplicates and updates the `disable` function in `apps/web/components/security/TwoFactorAuthAPI.ts`. The two files are essentially identical, suggesting a deeper problem of file duplication which should be refactored into a single shared utility.</violation>
</file>
<file name="apps/web/pages/api/auth/two-factor/totp/disable.ts">
<violation number="1" location="apps/web/pages/api/auth/two-factor/totp/disable.ts:49">
2FA backup code validation logic duplicates packages/features/auth/lib/next-auth-options.ts. Severity: 8/10 - Critical security-related logic is duplicated, increasing risk of inconsistencies and bugs.</violation>
<violation number="2" location="apps/web/pages/api/auth/two-factor/totp/disable.ts:58">
Decrypt/parse of backupCodes can throw and is not caught, causing 500s with non-JSON error responses
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: Decryption/JSON.parse errors indicate server-side state issues; returning 500 is appropriate and consistent. The claim to catch and return 4xx/JSON is speculative with low impact.</violation>
<violation number="3" location="apps/web/pages/api/auth/two-factor/totp/disable.ts:61">
Unvalidated types for req.body.backupCode cause TypeError via replaceAll on non-string, returning 500 instead of 4xx</violation>
</file>
<file name="packages/ui/components/form/inputs/Input.tsx">
<violation number="1" location="packages/ui/components/form/inputs/Input.tsx:49">
Setting `tabIndex={-1}` on this button removes it from the tab order, so keyboard users can no longer reach the password visibility toggle. Please keep the button focusable so the control remains accessible.</violation>
</file>
<file name="apps/web/components/auth/BackupCode.tsx">
<violation number="1" location="apps/web/components/auth/BackupCode.tsx:7">
React component naming conflict: The new component in `apps/web/components/auth/BackupCode.tsx` is named `TwoFactor`, which duplicates the name of an existing component in `apps/web/components/auth/TwoFactor.tsx`. The component in `BackupCode.tsx` should be renamed to something like `BackupCodeInput` to avoid conflicts and improve clarity.
*DEV MODE: This violation would have been filtered out by screening filters. Failing filters: typescriptUnderstanding.*
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: Default exports are imported by file path, so having two files each exporting a default function named TwoFactor does not cause import conflicts. The internal function name only affects component displayName in DevTools, not module resolution. This is a low-severity clarity suggestion, not a technical violation.</violation>
</file>
<file name="apps/web/pages/api/auth/two-factor/totp/setup.ts">
<violation number="1" location="apps/web/pages/api/auth/two-factor/totp/setup.ts:78">
Missing req.body.password validation can cause runtime error in bcrypt compare</violation>
</file>
<file name="packages/features/auth/lib/next-auth-options.ts">
<violation number="1" location="packages/features/auth/lib/next-auth-options.ts:131">
Backup-code login unreachable for non-CAL (OAuth) users due to early check requiring totpCode; blocks completing 2FA with backup codes.</violation>
</file>
<file name="apps/web/components/settings/EnableTwoFactorModal.tsx">
<violation number="1" location="apps/web/components/settings/EnableTwoFactorModal.tsx:135">
onEnable not called on successful enable; only on Close. Closing modal via overlay/ESC/X skips notification and leaves stale state</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| }, | ||
|
|
||
| async disable(password: string, code: string) { | ||
| async disable(password: string, code: string, backupCode: string) { |
There was a problem hiding this comment.
The disable function for 2FA in apps/web/components/settings/TwoFactorAuthAPI.ts duplicates and updates the disable function in apps/web/components/security/TwoFactorAuthAPI.ts. The two files are essentially identical, suggesting a deeper problem of file duplication which should be refactored into a single shared utility.
Prompt for AI agents
Address the following comment on apps/web/components/settings/TwoFactorAuthAPI.ts at line 22:
<comment>The `disable` function for 2FA in `apps/web/components/settings/TwoFactorAuthAPI.ts` duplicates and updates the `disable` function in `apps/web/components/security/TwoFactorAuthAPI.ts`. The two files are essentially identical, suggesting a deeper problem of file duplication which should be refactored into a single shared utility.</comment>
<file context>
@@ -19,10 +19,10 @@ const TwoFactorAuthAPI = {
},
- async disable(password: string, code: string) {
+ async disable(password: string, code: string, backupCode: string) {
return fetch("/api/auth/two-factor/totp/disable", {
method: "POST",
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: Duplicate Detection Agent
|
|
||
| // if user has 2fa and using backup code | ||
| if (user.twoFactorEnabled && req.body.backupCode) { | ||
| if (!process.env.CALENDSO_ENCRYPTION_KEY) { |
There was a problem hiding this comment.
2FA backup code validation logic duplicates packages/features/auth/lib/next-auth-options.ts. Severity: 8/10 - Critical security-related logic is duplicated, increasing risk of inconsistencies and bugs.
Prompt for AI agents
Address the following comment on apps/web/pages/api/auth/two-factor/totp/disable.ts at line 49:
<comment>2FA backup code validation logic duplicates packages/features/auth/lib/next-auth-options.ts. Severity: 8/10 - Critical security-related logic is duplicated, increasing risk of inconsistencies and bugs.</comment>
<file context>
@@ -43,8 +43,30 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
+
+ // if user has 2fa and using backup code
+ if (user.twoFactorEnabled && req.body.backupCode) {
+ if (!process.env.CALENDSO_ENCRYPTION_KEY) {
+ console.error("Missing encryption key; cannot proceed with backup code login.");
+ throw new Error(ErrorCode.InternalServerError);
</file context>
[internal] Confidence score: 10/10
[internal] Posted by: Duplicate Detection Agent
| <button className="text-emphasis h-9" type="button" onClick={() => toggleIsPasswordVisible()}> | ||
| <button | ||
| className="text-emphasis h-9" | ||
| tabIndex={-1} |
There was a problem hiding this comment.
Setting tabIndex={-1} on this button removes it from the tab order, so keyboard users can no longer reach the password visibility toggle. Please keep the button focusable so the control remains accessible.
Prompt for AI agents
Address the following comment on packages/ui/components/form/inputs/Input.tsx at line 49:
<comment>Setting `tabIndex={-1}` on this button removes it from the tab order, so keyboard users can no longer reach the password visibility toggle. Please keep the button focusable so the control remains accessible.</comment>
<file context>
@@ -44,7 +44,11 @@ export const PasswordField = forwardRef<HTMLInputElement, InputFieldProps>(funct
- <button className="text-emphasis h-9" type="button" onClick={() => toggleIsPasswordVisible()}>
+ <button
+ className="text-emphasis h-9"
+ tabIndex={-1}
+ type="button"
+ onClick={() => toggleIsPasswordVisible()}>
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
| tabIndex={-1} | |
| tabIndex={0} |
| import { useLocale } from "@calcom/lib/hooks/useLocale"; | ||
| import { Label, TextField } from "@calcom/ui"; | ||
|
|
||
| export default function TwoFactor({ center = true }) { |
There was a problem hiding this comment.
React component naming conflict: The new component in apps/web/components/auth/BackupCode.tsx is named TwoFactor, which duplicates the name of an existing component in apps/web/components/auth/TwoFactor.tsx. The component in BackupCode.tsx should be renamed to something like BackupCodeInput to avoid conflicts and improve clarity.
DEV MODE: This violation would have been filtered out by screening filters. Failing filters: typescriptUnderstanding.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• GPT-5: Default exports are imported by file path, so having two files each exporting a default function named TwoFactor does not cause import conflicts. The internal function name only affects component displayName in DevTools, not module resolution. This is a low-severity clarity suggestion, not a technical violation.
Prompt for AI agents
Address the following comment on apps/web/components/auth/BackupCode.tsx at line 7:
<comment>React component naming conflict: The new component in `apps/web/components/auth/BackupCode.tsx` is named `TwoFactor`, which duplicates the name of an existing component in `apps/web/components/auth/TwoFactor.tsx`. The component in `BackupCode.tsx` should be renamed to something like `BackupCodeInput` to avoid conflicts and improve clarity.
*DEV MODE: This violation would have been filtered out by screening filters. Failing filters: typescriptUnderstanding.*
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: Default exports are imported by file path, so having two files each exporting a default function named TwoFactor does not cause import conflicts. The internal function name only affects component displayName in DevTools, not module resolution. This is a low-severity clarity suggestion, not a technical violation.</comment>
<file context>
@@ -0,0 +1,29 @@
+import { useLocale } from "@calcom/lib/hooks/useLocale";
+import { Label, TextField } from "@calcom/ui";
+
+export default function TwoFactor({ center = true }) {
+ const { t } = useLocale();
+ const methods = useFormContext();
</file context>
[internal] Confidence score: 10/10
[internal] Posted by: Duplicate Detection Agent
| return res.status(400).json({ error: ErrorCode.MissingBackupCodes }); | ||
| } | ||
|
|
||
| const backupCodes = JSON.parse(symmetricDecrypt(user.backupCodes, process.env.CALENDSO_ENCRYPTION_KEY)); |
There was a problem hiding this comment.
Decrypt/parse of backupCodes can throw and is not caught, causing 500s with non-JSON error responses
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• GPT-5: Decryption/JSON.parse errors indicate server-side state issues; returning 500 is appropriate and consistent. The claim to catch and return 4xx/JSON is speculative with low impact.
Prompt for AI agents
Address the following comment on apps/web/pages/api/auth/two-factor/totp/disable.ts at line 58:
<comment>Decrypt/parse of backupCodes can throw and is not caught, causing 500s with non-JSON error responses
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: Decryption/JSON.parse errors indicate server-side state issues; returning 500 is appropriate and consistent. The claim to catch and return 4xx/JSON is speculative with low impact.</comment>
<file context>
@@ -43,8 +43,30 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
+ return res.status(400).json({ error: ErrorCode.MissingBackupCodes });
+ }
+
+ const backupCodes = JSON.parse(symmetricDecrypt(user.backupCodes, process.env.CALENDSO_ENCRYPTION_KEY));
+
+ // check if user-supplied code matches one
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: Functional Bugs Agent
| const backupCodes = JSON.parse(symmetricDecrypt(user.backupCodes, process.env.CALENDSO_ENCRYPTION_KEY)); | ||
|
|
||
| // check if user-supplied code matches one | ||
| const index = backupCodes.indexOf(req.body.backupCode.replaceAll("-", "")); |
There was a problem hiding this comment.
Unvalidated types for req.body.backupCode cause TypeError via replaceAll on non-string, returning 500 instead of 4xx
Prompt for AI agents
Address the following comment on apps/web/pages/api/auth/two-factor/totp/disable.ts at line 61:
<comment>Unvalidated types for req.body.backupCode cause TypeError via replaceAll on non-string, returning 500 instead of 4xx</comment>
<file context>
@@ -43,8 +43,30 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
+ const backupCodes = JSON.parse(symmetricDecrypt(user.backupCodes, process.env.CALENDSO_ENCRYPTION_KEY));
+
+ // check if user-supplied code matches one
+ const index = backupCodes.indexOf(req.body.backupCode.replaceAll("-", ""));
+ if (index === -1) {
+ return res.status(400).json({ error: ErrorCode.IncorrectBackupCode });
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: Functional Bugs Agent
| const dataUri = await qrcode.toDataURL(keyUri); | ||
|
|
||
| return res.json({ secret, keyUri, dataUri }); | ||
| return res.json({ secret, keyUri, dataUri, backupCodes }); |
There was a problem hiding this comment.
Missing req.body.password validation can cause runtime error in bcrypt compare
Prompt for AI agents
Address the following comment on apps/web/pages/api/auth/two-factor/totp/setup.ts at line 78:
<comment>Missing req.body.password validation can cause runtime error in bcrypt compare</comment>
<file context>
@@ -70,5 +75,5 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
const dataUri = await qrcode.toDataURL(keyUri);
- return res.json({ secret, keyUri, dataUri });
+ return res.json({ secret, keyUri, dataUri, backupCodes });
}
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: Functional Bugs Agent
| } | ||
|
|
||
| if (user.twoFactorEnabled) { | ||
| if (user.twoFactorEnabled && credentials.backupCode) { |
There was a problem hiding this comment.
Backup-code login unreachable for non-CAL (OAuth) users due to early check requiring totpCode; blocks completing 2FA with backup codes.
Prompt for AI agents
Address the following comment on packages/features/auth/lib/next-auth-options.ts at line 131:
<comment>Backup-code login unreachable for non-CAL (OAuth) users due to early check requiring totpCode; blocks completing 2FA with backup codes.</comment>
<file context>
@@ -126,7 +128,33 @@ const providers: Provider[] = [
}
- if (user.twoFactorEnabled) {
+ if (user.twoFactorEnabled && credentials.backupCode) {
+ if (!process.env.CALENDSO_ENCRYPTION_KEY) {
+ console.error("Missing encryption key; cannot proceed with backup code login.");
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: Functional Bugs Agent
|
|
||
| if (response.status === 200) { | ||
| onEnable(); | ||
| setStep(SetupStep.DisplayBackupCodes); |
There was a problem hiding this comment.
onEnable not called on successful enable; only on Close. Closing modal via overlay/ESC/X skips notification and leaves stale state
Prompt for AI agents
Address the following comment on apps/web/components/settings/EnableTwoFactorModal.tsx at line 135:
<comment>onEnable not called on successful enable; only on Close. Closing modal via overlay/ESC/X skips notification and leaves stale state</comment>
<file context>
@@ -113,7 +132,7 @@ const EnableTwoFactorModal = ({ onEnable, onCancel, open, onOpenChange }: Enable
if (response.status === 200) {
- onEnable();
+ setStep(SetupStep.DisplayBackupCodes);
return;
}
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: Functional Bugs Agent
cubic is running your AI reviewPipeline
|
![cubic-staging[bot]](https://avatars.githubusercontent.com/u/189440881?s=60&v=4){kind=small}
There was a problem hiding this comment.
7 issues found across 16 files
Prompt for AI agents (all 7 issues)
Understand the root cause of the following 7 issues and fix them.
<file name="apps/web/pages/api/auth/two-factor/totp/disable.ts">
<violation number="1" location="apps/web/pages/api/auth/two-factor/totp/disable.ts:48">
Backup code validation logic for two-factor authentication is duplicated. This security-critical logic already exists in `packages/features/auth/lib/next-auth-options.ts`. It should be extracted into a shared utility function to ensure consistency and easier maintenance.</violation>
</file>
<file name="packages/ui/components/form/inputs/Input.tsx">
<violation number="1" location="packages/ui/components/form/inputs/Input.tsx:49">
Setting `tabIndex={-1}` on this toggle button removes it from the tab order, preventing keyboard users from toggling password visibility. Please keep the button keyboard-focusable for accessibility.</violation>
</file>
<file name="apps/web/pages/auth/login.tsx">
<violation number="1" location="apps/web/pages/auth/login.tsx:70">
Two-factor authentication recovery logic (state management for `twoFactorLostAccess`, conditional rendering of 2FA/BackupCode components, error handling for `IncorrectBackupCode` and `MissingBackupCodes`, and the 'Lost Access' button functionality) duplicates `apps/web/components/settings/DisableTwoFactorModal.tsx`.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: The login flow’s lost-access handling is tailored to toggling between primary password-based login, OTP submission, and backup codes, while DisableTwoFactorModal manages an authenticated settings modal with its own form state and API interactions. Although both components share a twoFactorLostAccess flag and display backup-code errors, the surrounding state transitions, button layouts, and side effects are context-specific. The similarity is superficial and does not constitute actionable duplication that should be refactored into a shared abstraction.</violation>
</file>
<file name="apps/web/components/settings/TwoFactorAuthAPI.ts">
<violation number="1" location="apps/web/components/settings/TwoFactorAuthAPI.ts:22">
Requiring `backupCode` here breaks existing callers like `security/DisableTwoFactorModal`, which still pass only password and TOTP. That yields a TypeScript "Expected 3 arguments, but got 2" error and blocks the build. Please make this parameter optional.
*DEV MODE: This violation would have been filtered out by screening filters. Failing filters: typescriptUnderstanding.*</violation>
</file>
<file name="apps/web/components/auth/BackupCode.tsx">
<violation number="1" location="apps/web/components/auth/BackupCode.tsx:7">
Rename this component to `BackupCode` so the React component name matches the file/functionality and avoids colliding with the existing `TwoFactor` component in devtools and traces.</violation>
<violation number="2" location="apps/web/components/auth/BackupCode.tsx:13">
Add `htmlFor="backup-code"` so the label is properly associated with the backup code input for accessibility.
*DEV MODE: This violation would have been filtered out by screening filters. Failing filters: functionalImpact.*
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: This suggestion is limited to adding an accessibility attribute (`htmlFor`) to match a nearby input. Although accurate, it targets a minor accessibility enhancement rather than a functional or high-impact issue, so it is filtered out under the review guidelines.</violation>
</file>
<file name="apps/web/components/settings/EnableTwoFactorModal.tsx">
<violation number="1" location="apps/web/components/settings/EnableTwoFactorModal.tsx:135">
The success branch no longer calls `onEnable()`, so if the user dismisses the backup-code dialog via the dialog close (ESC/overlay) the parent never updates and keeps showing two-factor as disabled even though it was enabled on the server.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| if (user.twoFactorEnabled) { | ||
|
|
||
| // if user has 2fa and using backup code | ||
| if (user.twoFactorEnabled && req.body.backupCode) { |
There was a problem hiding this comment.
Backup code validation logic for two-factor authentication is duplicated. This security-critical logic already exists in packages/features/auth/lib/next-auth-options.ts. It should be extracted into a shared utility function to ensure consistency and easier maintenance.
Prompt for AI agents
Address the following comment on apps/web/pages/api/auth/two-factor/totp/disable.ts at line 48:
<comment>Backup code validation logic for two-factor authentication is duplicated. This security-critical logic already exists in `packages/features/auth/lib/next-auth-options.ts`. It should be extracted into a shared utility function to ensure consistency and easier maintenance.</comment>
<file context>
@@ -43,8 +43,30 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
- if (user.twoFactorEnabled) {
+
+ // if user has 2fa and using backup code
+ if (user.twoFactorEnabled && req.body.backupCode) {
+ if (!process.env.CALENDSO_ENCRYPTION_KEY) {
+ console.error("Missing encryption key; cannot proceed with backup code login.");
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: Duplicate Detection Agent
| <button className="text-emphasis h-9" type="button" onClick={() => toggleIsPasswordVisible()}> | ||
| <button | ||
| className="text-emphasis h-9" | ||
| tabIndex={-1} |
There was a problem hiding this comment.
Setting tabIndex={-1} on this toggle button removes it from the tab order, preventing keyboard users from toggling password visibility. Please keep the button keyboard-focusable for accessibility.
Prompt for AI agents
Address the following comment on packages/ui/components/form/inputs/Input.tsx at line 49:
<comment>Setting `tabIndex={-1}` on this toggle button removes it from the tab order, preventing keyboard users from toggling password visibility. Please keep the button keyboard-focusable for accessibility.</comment>
<file context>
@@ -44,7 +44,11 @@ export const PasswordField = forwardRef<HTMLInputElement, InputFieldProps>(funct
- <button className="text-emphasis h-9" type="button" onClick={() => toggleIsPasswordVisible()}>
+ <button
+ className="text-emphasis h-9"
+ tabIndex={-1}
+ type="button"
+ onClick={() => toggleIsPasswordVisible()}>
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
| tabIndex={-1} | |
| tabIndex={0} |
| const methods = useForm<LoginValues>({ resolver: zodResolver(formSchema) }); | ||
| const { register, formState } = methods; | ||
| const [twoFactorRequired, setTwoFactorRequired] = useState(!!totpEmail || false); | ||
| const [twoFactorLostAccess, setTwoFactorLostAccess] = useState(false); |
There was a problem hiding this comment.
Two-factor authentication recovery logic (state management for twoFactorLostAccess, conditional rendering of 2FA/BackupCode components, error handling for IncorrectBackupCode and MissingBackupCodes, and the 'Lost Access' button functionality) duplicates apps/web/components/settings/DisableTwoFactorModal.tsx.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• GPT-5: The login flow’s lost-access handling is tailored to toggling between primary password-based login, OTP submission, and backup codes, while DisableTwoFactorModal manages an authenticated settings modal with its own form state and API interactions. Although both components share a twoFactorLostAccess flag and display backup-code errors, the surrounding state transitions, button layouts, and side effects are context-specific. The similarity is superficial and does not constitute actionable duplication that should be refactored into a shared abstraction.
Prompt for AI agents
Address the following comment on apps/web/pages/auth/login.tsx at line 70:
<comment>Two-factor authentication recovery logic (state management for `twoFactorLostAccess`, conditional rendering of 2FA/BackupCode components, error handling for `IncorrectBackupCode` and `MissingBackupCodes`, and the 'Lost Access' button functionality) duplicates `apps/web/components/settings/DisableTwoFactorModal.tsx`.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: The login flow’s lost-access handling is tailored to toggling between primary password-based login, OTP submission, and backup codes, while DisableTwoFactorModal manages an authenticated settings modal with its own form state and API interactions. Although both components share a twoFactorLostAccess flag and display backup-code errors, the surrounding state transitions, button layouts, and side effects are context-specific. The similarity is superficial and does not constitute actionable duplication that should be refactored into a shared abstraction.</comment>
<file context>
@@ -65,6 +67,7 @@ export default function Login({
const methods = useForm<LoginValues>({ resolver: zodResolver(formSchema) });
const { register, formState } = methods;
const [twoFactorRequired, setTwoFactorRequired] = useState(!!totpEmail || false);
+ const [twoFactorLostAccess, setTwoFactorLostAccess] = useState(false);
const [errorMessage, setErrorMessage] = useState<string | null>(null);
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: Duplicate Detection Agent
| }, | ||
|
|
||
| async disable(password: string, code: string) { | ||
| async disable(password: string, code: string, backupCode: string) { |
There was a problem hiding this comment.
Requiring backupCode here breaks existing callers like security/DisableTwoFactorModal, which still pass only password and TOTP. That yields a TypeScript "Expected 3 arguments, but got 2" error and blocks the build. Please make this parameter optional.
DEV MODE: This violation would have been filtered out by screening filters. Failing filters: typescriptUnderstanding.
Prompt for AI agents
Address the following comment on apps/web/components/settings/TwoFactorAuthAPI.ts at line 22:
<comment>Requiring `backupCode` here breaks existing callers like `security/DisableTwoFactorModal`, which still pass only password and TOTP. That yields a TypeScript "Expected 3 arguments, but got 2" error and blocks the build. Please make this parameter optional.
*DEV MODE: This violation would have been filtered out by screening filters. Failing filters: typescriptUnderstanding.*</comment>
<file context>
@@ -19,10 +19,10 @@ const TwoFactorAuthAPI = {
},
- async disable(password: string, code: string) {
+ async disable(password: string, code: string, backupCode: string) {
return fetch("/api/auth/two-factor/totp/disable", {
method: "POST",
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
| async disable(password: string, code: string, backupCode: string) { | |
| async disable(password: string, code: string, backupCode?: string) { |
| import { useLocale } from "@calcom/lib/hooks/useLocale"; | ||
| import { Label, TextField } from "@calcom/ui"; | ||
|
|
||
| export default function TwoFactor({ center = true }) { |
There was a problem hiding this comment.
Rename this component to BackupCode so the React component name matches the file/functionality and avoids colliding with the existing TwoFactor component in devtools and traces.
Prompt for AI agents
Address the following comment on apps/web/components/auth/BackupCode.tsx at line 7:
<comment>Rename this component to `BackupCode` so the React component name matches the file/functionality and avoids colliding with the existing `TwoFactor` component in devtools and traces.</comment>
<file context>
@@ -0,0 +1,29 @@
+import { useLocale } from "@calcom/lib/hooks/useLocale";
+import { Label, TextField } from "@calcom/ui";
+
+export default function TwoFactor({ center = true }) {
+ const { t } = useLocale();
+ const methods = useFormContext();
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
| export default function TwoFactor({ center = true }) { | |
| export default function BackupCode({ center = true }) { |
|
|
||
| return ( | ||
| <div className={center ? "mx-auto !mt-0 max-w-sm" : "!mt-0 max-w-sm"}> | ||
| <Label className="mt-4">{t("backup_code")}</Label> |
There was a problem hiding this comment.
Add htmlFor="backup-code" so the label is properly associated with the backup code input for accessibility.
DEV MODE: This violation would have been filtered out by screening filters. Failing filters: functionalImpact.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• GPT-5: This suggestion is limited to adding an accessibility attribute (htmlFor) to match a nearby input. Although accurate, it targets a minor accessibility enhancement rather than a functional or high-impact issue, so it is filtered out under the review guidelines.
Prompt for AI agents
Address the following comment on apps/web/components/auth/BackupCode.tsx at line 13:
<comment>Add `htmlFor="backup-code"` so the label is properly associated with the backup code input for accessibility.
*DEV MODE: This violation would have been filtered out by screening filters. Failing filters: functionalImpact.*
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: This suggestion is limited to adding an accessibility attribute (`htmlFor`) to match a nearby input. Although accurate, it targets a minor accessibility enhancement rather than a functional or high-impact issue, so it is filtered out under the review guidelines.</comment>
<file context>
@@ -0,0 +1,29 @@
+
+ return (
+ <div className={center ? "mx-auto !mt-0 max-w-sm" : "!mt-0 max-w-sm"}>
+ <Label className="mt-4">{t("backup_code")}</Label>
+
+ <p className="text-subtle mb-4 text-sm">{t("backup_code_instructions")}</p>
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
| <Label className="mt-4">{t("backup_code")}</Label> | |
| <Label className="mt-4" htmlFor="backup-code">{t("backup_code")}</Label> |
|
|
||
| if (response.status === 200) { | ||
| onEnable(); | ||
| setStep(SetupStep.DisplayBackupCodes); |
There was a problem hiding this comment.
The success branch no longer calls onEnable(), so if the user dismisses the backup-code dialog via the dialog close (ESC/overlay) the parent never updates and keeps showing two-factor as disabled even though it was enabled on the server.
Prompt for AI agents
Address the following comment on apps/web/components/settings/EnableTwoFactorModal.tsx at line 135:
<comment>The success branch no longer calls `onEnable()`, so if the user dismisses the backup-code dialog via the dialog close (ESC/overlay) the parent never updates and keeps showing two-factor as disabled even though it was enabled on the server.</comment>
<file context>
@@ -113,7 +132,7 @@ const EnableTwoFactorModal = ({ onEnable, onCancel, open, onOpenChange }: Enable
if (response.status === 200) {
- onEnable();
+ setStep(SetupStep.DisplayBackupCodes);
return;
}
</file context>
[internal] Confidence score: 7/10
[internal] Posted by: General AI Review Agent
cubic is running your AI reviewPipeline
|
There was a problem hiding this comment.
6 issues found across 16 files
Prompt for AI agents (all 6 issues)
Understand the root cause of the following 6 issues and fix them.
<file name="apps/web/pages/api/auth/two-factor/totp/disable.ts">
<violation number="1" location="apps/web/pages/api/auth/two-factor/totp/disable.ts:48">
Backup code validation logic duplicates functionality in `packages/features/auth/lib/next-auth-options.ts`. This critical authentication logic should be centralized in a shared utility.</violation>
</file>
<file name="packages/ui/components/form/inputs/Input.tsx">
<violation number="1" location="packages/ui/components/form/inputs/Input.tsx:49">
Setting `tabIndex={-1}` removes this password-visibility button from the tab order, so keyboard users can no longer reveal or hide the password. Please keep it focusable.</violation>
</file>
<file name="apps/web/components/settings/TwoFactorAuthAPI.ts">
<violation number="1" location="apps/web/components/settings/TwoFactorAuthAPI.ts:22">
New disable() signature requires backupCode but existing callers still pass only two args, breaking the 2FA disable flow.
*DEV MODE: This violation would have been filtered out by screening filters. Failing filters: typescriptUnderstanding.*
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: apps/web/components/security/DisableTwoFactorModal.tsx imports `./TwoFactorAuthAPI`, which is a different module (apps/web/components/security/TwoFactorAuthAPI.ts) whose disable signature still takes only two arguments. The updated three-argument disable function is defined under apps/web/components/settings/TwoFactorAuthAPI.ts and its callers (e.g., settings/DisableTwoFactorModal.tsx) already pass all three arguments. Therefore there is no TypeScript error or broken flow caused by the change.</violation>
</file>
<file name="packages/features/auth/lib/next-auth-options.ts">
<violation number="1" location="packages/features/auth/lib/next-auth-options.ts:131">
Broken authentication: backup code login bypasses password verification, allowing single-factor login.</violation>
<violation number="2" location="packages/features/auth/lib/next-auth-options.ts:131">
SSO users can never reach this new backup-code branch because earlier guards still throw when `totpCode` is empty. Please update those conditions to accept a backup code as an alternative second factor; otherwise SSO users remain locked out when relying on backup codes.</violation>
</file>
<file name="apps/web/components/settings/EnableTwoFactorModal.tsx">
<violation number="1" location="apps/web/components/settings/EnableTwoFactorModal.tsx:135">
Successful enable no longer triggers onEnable, so closing the modal outside the footer button leaves the parent UI in the old "2FA disabled" state.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| if (user.twoFactorEnabled) { | ||
|
|
||
| // if user has 2fa and using backup code | ||
| if (user.twoFactorEnabled && req.body.backupCode) { |
There was a problem hiding this comment.
Backup code validation logic duplicates functionality in packages/features/auth/lib/next-auth-options.ts. This critical authentication logic should be centralized in a shared utility.
Prompt for AI agents
Address the following comment on apps/web/pages/api/auth/two-factor/totp/disable.ts at line 48:
<comment>Backup code validation logic duplicates functionality in `packages/features/auth/lib/next-auth-options.ts`. This critical authentication logic should be centralized in a shared utility.</comment>
<file context>
@@ -43,8 +43,30 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
- if (user.twoFactorEnabled) {
+
+ // if user has 2fa and using backup code
+ if (user.twoFactorEnabled && req.body.backupCode) {
+ if (!process.env.CALENDSO_ENCRYPTION_KEY) {
+ console.error("Missing encryption key; cannot proceed with backup code login.");
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: Duplicate Detection Agent
| <button className="text-emphasis h-9" type="button" onClick={() => toggleIsPasswordVisible()}> | ||
| <button | ||
| className="text-emphasis h-9" | ||
| tabIndex={-1} |
There was a problem hiding this comment.
Setting tabIndex={-1} removes this password-visibility button from the tab order, so keyboard users can no longer reveal or hide the password. Please keep it focusable.
Prompt for AI agents
Address the following comment on packages/ui/components/form/inputs/Input.tsx at line 49:
<comment>Setting `tabIndex={-1}` removes this password-visibility button from the tab order, so keyboard users can no longer reveal or hide the password. Please keep it focusable.</comment>
<file context>
@@ -44,7 +44,11 @@ export const PasswordField = forwardRef<HTMLInputElement, InputFieldProps>(funct
- <button className="text-emphasis h-9" type="button" onClick={() => toggleIsPasswordVisible()}>
+ <button
+ className="text-emphasis h-9"
+ tabIndex={-1}
+ type="button"
+ onClick={() => toggleIsPasswordVisible()}>
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
| tabIndex={-1} | |
| tabIndex={0} |
| }, | ||
|
|
||
| async disable(password: string, code: string) { | ||
| async disable(password: string, code: string, backupCode: string) { |
There was a problem hiding this comment.
New disable() signature requires backupCode but existing callers still pass only two args, breaking the 2FA disable flow.
DEV MODE: This violation would have been filtered out by screening filters. Failing filters: typescriptUnderstanding.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• GPT-5: apps/web/components/security/DisableTwoFactorModal.tsx imports ./TwoFactorAuthAPI, which is a different module (apps/web/components/security/TwoFactorAuthAPI.ts) whose disable signature still takes only two arguments. The updated three-argument disable function is defined under apps/web/components/settings/TwoFactorAuthAPI.ts and its callers (e.g., settings/DisableTwoFactorModal.tsx) already pass all three arguments. Therefore there is no TypeScript error or broken flow caused by the change.
Prompt for AI agents
Address the following comment on apps/web/components/settings/TwoFactorAuthAPI.ts at line 22:
<comment>New disable() signature requires backupCode but existing callers still pass only two args, breaking the 2FA disable flow.
*DEV MODE: This violation would have been filtered out by screening filters. Failing filters: typescriptUnderstanding.*
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: apps/web/components/security/DisableTwoFactorModal.tsx imports `./TwoFactorAuthAPI`, which is a different module (apps/web/components/security/TwoFactorAuthAPI.ts) whose disable signature still takes only two arguments. The updated three-argument disable function is defined under apps/web/components/settings/TwoFactorAuthAPI.ts and its callers (e.g., settings/DisableTwoFactorModal.tsx) already pass all three arguments. Therefore there is no TypeScript error or broken flow caused by the change.</comment>
<file context>
@@ -19,10 +19,10 @@ const TwoFactorAuthAPI = {
},
- async disable(password: string, code: string) {
+ async disable(password: string, code: string, backupCode: string) {
return fetch("/api/auth/two-factor/totp/disable", {
method: "POST",
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: Functional Bugs Agent
| async disable(password: string, code: string, backupCode: string) { | |
| async disable(password: string, code: string, backupCode?: string) { |
| } | ||
|
|
||
| if (user.twoFactorEnabled) { | ||
| if (user.twoFactorEnabled && credentials.backupCode) { |
There was a problem hiding this comment.
Broken authentication: backup code login bypasses password verification, allowing single-factor login.
Prompt for AI agents
Address the following comment on packages/features/auth/lib/next-auth-options.ts at line 131:
<comment>Broken authentication: backup code login bypasses password verification, allowing single-factor login.</comment>
<file context>
@@ -126,7 +128,33 @@ const providers: Provider[] = [
}
- if (user.twoFactorEnabled) {
+ if (user.twoFactorEnabled && credentials.backupCode) {
+ if (!process.env.CALENDSO_ENCRYPTION_KEY) {
+ console.error("Missing encryption key; cannot proceed with backup code login.");
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: Security Vulnerabilities Agent
| } | ||
|
|
||
| if (user.twoFactorEnabled) { | ||
| if (user.twoFactorEnabled && credentials.backupCode) { |
There was a problem hiding this comment.
SSO users can never reach this new backup-code branch because earlier guards still throw when totpCode is empty. Please update those conditions to accept a backup code as an alternative second factor; otherwise SSO users remain locked out when relying on backup codes.
Prompt for AI agents
Address the following comment on packages/features/auth/lib/next-auth-options.ts at line 131:
<comment>SSO users can never reach this new backup-code branch because earlier guards still throw when `totpCode` is empty. Please update those conditions to accept a backup code as an alternative second factor; otherwise SSO users remain locked out when relying on backup codes.</comment>
<file context>
@@ -126,7 +128,33 @@ const providers: Provider[] = [
}
- if (user.twoFactorEnabled) {
+ if (user.twoFactorEnabled && credentials.backupCode) {
+ if (!process.env.CALENDSO_ENCRYPTION_KEY) {
+ console.error("Missing encryption key; cannot proceed with backup code login.");
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
|
|
||
| if (response.status === 200) { | ||
| onEnable(); | ||
| setStep(SetupStep.DisplayBackupCodes); |
There was a problem hiding this comment.
Successful enable no longer triggers onEnable, so closing the modal outside the footer button leaves the parent UI in the old "2FA disabled" state.
Prompt for AI agents
Address the following comment on apps/web/components/settings/EnableTwoFactorModal.tsx at line 135:
<comment>Successful enable no longer triggers onEnable, so closing the modal outside the footer button leaves the parent UI in the old "2FA disabled" state.</comment>
<file context>
@@ -113,7 +132,7 @@ const EnableTwoFactorModal = ({ onEnable, onCancel, open, onOpenChange }: Enable
if (response.status === 200) {
- onEnable();
+ setStep(SetupStep.DisplayBackupCodes);
return;
}
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: Functional Bugs Agent
1 participant
Summary by cubic
Adds 2FA backup codes across setup, login, and disable flows so users can regain access without their authenticator. Codes are generated, encrypted, shown once with copy/download options, and consumed on use.
New Features
Migration