feat: 2fa backup codes #10
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| import React from "react"; | ||
| import { useFormContext } from "react-hook-form"; | ||
|
|
||
| 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(); | ||
|
|
||
| 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> | ||
|
|
||
| <TextField | ||
| id="backup-code" | ||
| label="" | ||
| defaultValue="" | ||
| placeholder="XXXXX-XXXXX" | ||
| minLength={10} // without dash | ||
| maxLength={11} // with dash | ||
| required | ||
| {...methods.register("backupCode")} | ||
| /> | ||
| </div> | ||
| ); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,7 @@ import { useForm } from "react-hook-form"; | |
| import { ErrorCode } from "@calcom/features/auth/lib/ErrorCode"; | ||
| import { useCallbackRef } from "@calcom/lib/hooks/useCallbackRef"; | ||
| import { useLocale } from "@calcom/lib/hooks/useLocale"; | ||
| import { Button, Dialog, DialogContent, DialogFooter, Form, PasswordField, showToast } from "@calcom/ui"; | ||
|
|
||
| import TwoFactor from "@components/auth/TwoFactor"; | ||
|
|
||
|
|
@@ -28,6 +28,7 @@ interface EnableTwoFactorModalProps { | |
|
|
||
| enum SetupStep { | ||
| ConfirmPassword, | ||
| DisplayBackupCodes, | ||
| DisplayQrCode, | ||
| EnterTotpCode, | ||
| } | ||
|
|
@@ -54,16 +55,25 @@ const EnableTwoFactorModal = ({ onEnable, onCancel, open, onOpenChange }: Enable | |
|
|
||
| const setupDescriptions = { | ||
| [SetupStep.ConfirmPassword]: t("2fa_confirm_current_password"), | ||
| [SetupStep.DisplayBackupCodes]: t("backup_code_instructions"), | ||
| [SetupStep.DisplayQrCode]: t("2fa_scan_image_or_use_code"), | ||
| [SetupStep.EnterTotpCode]: t("2fa_enter_six_digit_code"), | ||
| }; | ||
| const [step, setStep] = useState(SetupStep.ConfirmPassword); | ||
| const [password, setPassword] = useState(""); | ||
| const [backupCodes, setBackupCodes] = useState([]); | ||
| const [backupCodesUrl, setBackupCodesUrl] = useState(""); | ||
| const [dataUri, setDataUri] = useState(""); | ||
| const [secret, setSecret] = useState(""); | ||
| const [isSubmitting, setIsSubmitting] = useState(false); | ||
| const [errorMessage, setErrorMessage] = useState<string | null>(null); | ||
|
|
||
| const resetState = () => { | ||
| setPassword(""); | ||
| setErrorMessage(null); | ||
| setStep(SetupStep.ConfirmPassword); | ||
| }; | ||
|
|
||
| async function handleSetup(e: React.FormEvent) { | ||
| e.preventDefault(); | ||
|
|
||
|
|
@@ -79,6 +89,15 @@ const EnableTwoFactorModal = ({ onEnable, onCancel, open, onOpenChange }: Enable | |
| const body = await response.json(); | ||
|
|
||
| if (response.status === 200) { | ||
| setBackupCodes(body.backupCodes); | ||
|
|
||
| // create backup codes download url | ||
| const textBlob = new Blob([body.backupCodes.map(formatBackupCode).join("\n")], { | ||
| type: "text/plain", | ||
| }); | ||
| if (backupCodesUrl) URL.revokeObjectURL(backupCodesUrl); | ||
| setBackupCodesUrl(URL.createObjectURL(textBlob)); | ||
|
|
||
| setDataUri(body.dataUri); | ||
| setSecret(body.secret); | ||
| setStep(SetupStep.DisplayQrCode); | ||
|
|
@@ -113,7 +132,7 @@ const EnableTwoFactorModal = ({ onEnable, onCancel, open, onOpenChange }: Enable | |
| const body = await response.json(); | ||
|
|
||
| if (response.status === 200) { | ||
| setStep(SetupStep.DisplayBackupCodes); | ||
| return; | ||
| } | ||
|
|
||
|
|
@@ -141,13 +160,18 @@ const EnableTwoFactorModal = ({ onEnable, onCancel, open, onOpenChange }: Enable | |
| } | ||
| }, [form, handleEnableRef, totpCode]); | ||
|
|
||
| const formatBackupCode = (code: string) => `${code.slice(0, 5)}-${code.slice(5, 10)}`; | ||
|
|
||
| return ( | ||
| <Dialog open={open} onOpenChange={onOpenChange}> | ||
| <DialogContent | ||
| title={step === SetupStep.DisplayBackupCodes ? t("backup_codes") : t("enable_2fa")} | ||
| description={setupDescriptions[step]} | ||
| type="creation"> | ||
| <WithStep step={SetupStep.ConfirmPassword} current={step}> | ||
| <form onSubmit={handleSetup}> | ||
| <div className="mb-4"> | ||
| <PasswordField | ||
| label={t("password")} | ||
| type="password" | ||
| name="password" | ||
|
|
@@ -173,6 +197,15 @@ const EnableTwoFactorModal = ({ onEnable, onCancel, open, onOpenChange }: Enable | |
| </p> | ||
| </> | ||
| </WithStep> | ||
| <WithStep step={SetupStep.DisplayBackupCodes} current={step}> | ||
| <> | ||
| <div className="mt-5 grid grid-cols-2 gap-1 text-center font-mono md:pl-10 md:pr-10"> | ||
| {backupCodes.map((code) => ( | ||
| <div key={code}>{formatBackupCode(code)}</div> | ||
| ))} | ||
| </div> | ||
| </> | ||
| </WithStep> | ||
| <Form handleSubmit={handleEnable} form={form}> | ||
| <WithStep step={SetupStep.EnterTotpCode} current={step}> | ||
| <div className="-mt-4 pb-2"> | ||
|
|
@@ -186,9 +219,16 @@ const EnableTwoFactorModal = ({ onEnable, onCancel, open, onOpenChange }: Enable | |
| </div> | ||
| </WithStep> | ||
| <DialogFooter className="mt-8" showDivider> | ||
| {step !== SetupStep.DisplayBackupCodes ? ( | ||
| <Button | ||
| color="secondary" | ||
| onClick={() => { | ||
| onCancel(); | ||
| resetState(); | ||
| }}> | ||
| {t("cancel")} | ||
| </Button> | ||
| ) : null} | ||
| <WithStep step={SetupStep.ConfirmPassword} current={step}> | ||
| <Button | ||
| type="submit" | ||
|
|
@@ -218,6 +258,35 @@ const EnableTwoFactorModal = ({ onEnable, onCancel, open, onOpenChange }: Enable | |
| {t("enable")} | ||
| </Button> | ||
| </WithStep> | ||
| <WithStep step={SetupStep.DisplayBackupCodes} current={step}> | ||
| <> | ||
| <Button | ||
| color="secondary" | ||
| data-testid="backup-codes-close" | ||
| onClick={(e) => { | ||
| e.preventDefault(); | ||
| resetState(); | ||
| onEnable(); | ||
| }}> | ||
| {t("close")} | ||
| </Button> | ||
| <Button | ||
| color="secondary" | ||
| data-testid="backup-codes-copy" | ||
| onClick={(e) => { | ||
| e.preventDefault(); | ||
| navigator.clipboard.writeText(backupCodes.map(formatBackupCode).join("\n")); | ||
| showToast(t("backup_codes_copied"), "success"); | ||
| }}> | ||
| {t("copy")} | ||
| </Button> | ||
| <a download="cal-backup-codes.txt" href={backupCodesUrl}> | ||
| <Button color="primary" data-testid="backup-codes-download"> | ||
| {t("download")} | ||
| </Button> | ||
| </a> | ||
| </> | ||
| </WithStep> | ||
| </DialogFooter> | ||
| </Form> | ||
| </DialogContent> | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,10 +19,10 @@ const TwoFactorAuthAPI = { | |
| }); | ||
| }, | ||
|
|
||
| async disable(password: string, code: string, backupCode: string) { | ||
| return fetch("/api/auth/two-factor/totp/disable", { | ||
| method: "POST", | ||
| body: JSON.stringify({ password, code, backupCode }), | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| }, | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,8 +43,30 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse) | |
| return res.status(400).json({ error: ErrorCode.IncorrectPassword }); | ||
| } | ||
| } | ||
|
|
||
| // 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); | ||
| } | ||
|
|
||
| if (!user.backupCodes) { | ||
| 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 | ||
| const index = backupCodes.indexOf(req.body.backupCode.replaceAll("-", "")); | ||
| if (index === -1) { | ||
| return res.status(400).json({ error: ErrorCode.IncorrectBackupCode }); | ||
| } | ||
|
|
||
| // we delete all stored backup codes at the end, no need to do this here | ||
|
|
||
|
Comment on lines
+48
to
+67
Contributor
There was a problem hiding this comment. Critical: Multiple security and reliability issues in backup code validation. Three critical issues must be addressed:
🔒 Proposed fixes // if user has 2fa and using backup code
if (user.twoFactorEnabled && req.body.backupCode) {
+ // Validate backupCode is a string
+ if (typeof req.body.backupCode !== 'string') {
+ return res.status(400).json({ error: ErrorCode.IncorrectBackupCode });
+ }
+
if (!process.env.CALENDSO_ENCRYPTION_KEY) {
console.error("Missing encryption key; cannot proceed with backup code login.");
throw new Error(ErrorCode.InternalServerError);
}
if (!user.backupCodes) {
return res.status(400).json({ error: ErrorCode.MissingBackupCodes });
}
- const backupCodes = JSON.parse(symmetricDecrypt(user.backupCodes, process.env.CALENDSO_ENCRYPTION_KEY));
+ let backupCodes: string[];
+ try {
+ backupCodes = JSON.parse(symmetricDecrypt(user.backupCodes, process.env.CALENDSO_ENCRYPTION_KEY));
+ } catch (error) {
+ console.error("Failed to decrypt backup codes:", error);
+ return res.status(500).json({ error: ErrorCode.InternalServerError });
+ }
// 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 });
}
- // we delete all stored backup codes at the end, no need to do this here
+ // Remove the used backup code to prevent reuse
+ backupCodes.splice(index, 1);
+
+ // Update user with remaining backup codes
+ await prisma.user.update({
+ where: { id: session.user.id },
+ data: {
+ backupCodes: backupCodes.length > 0
+ ? symmetricEncrypt(JSON.stringify(backupCodes), process.env.CALENDSO_ENCRYPTION_KEY)
+ : null,
+ },
+ });
// if user has 2fa and NOT using backup code, try totp
} else if (user.twoFactorEnabled) {Note: You'll need to import
🤖 Prompt for AI Agents |
||
| // if user has 2fa and NOT using backup code, try totp | ||
| } else if (user.twoFactorEnabled) { | ||
| if (!req.body.code) { | ||
| return res.status(400).json({ error: ErrorCode.SecondFactorRequired }); | ||
| // throw new Error(ErrorCode.SecondFactorRequired); | ||
|
|
@@ -82,6 +104,7 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse) | |
| id: session.user.id, | ||
| }, | ||
| data: { | ||
| backupCodes: null, | ||
| twoFactorEnabled: false, | ||
| twoFactorSecret: null, | ||
| }, | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| import crypto from "crypto"; | ||
| import type { NextApiRequest, NextApiResponse } from "next"; | ||
| import { authenticator } from "otplib"; | ||
| import qrcode from "qrcode"; | ||
|
|
@@ -56,11 +57,15 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse) | |
| // bytes without updating the sanity checks in the enable and login endpoints. | ||
| const secret = authenticator.generateSecret(20); | ||
|
|
||
| // generate backup codes with 10 character length | ||
| const backupCodes = Array.from(Array(10), () => crypto.randomBytes(5).toString("hex")); | ||
|
|
||
| await prisma.user.update({ | ||
| where: { | ||
| 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), | ||
| }, | ||
|
|
@@ -70,5 +75,5 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse) | |
| const keyUri = authenticator.keyuri(name, "Cal", secret); | ||
| const dataUri = await qrcode.toDataURL(keyUri); | ||
|
|
||
| return res.json({ secret, keyUri, dataUri, backupCodes }); | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function name should be
BackupCode, notTwoFactor.The exported function is named
TwoFactorbut the file isBackupCode.tsxand it renders backup code input, not TOTP input. This creates confusion since there's already aTwoFactorcomponent inTwoFactor.tsx. The naming mismatch could cause maintainability issues and confusion when reading the code.🔧 Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents