-
Notifications
You must be signed in to change notification settings - Fork 12
feat: 2fa backup codes #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: enhance-two-factor-security-foundation
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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 | ||
|
Comment on lines
+22
to
+23
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: Validation logic is flawed. If backup codes are 'XXXXX-XXXXX' format (11 chars), minLength of 10 allows invalid shorter inputs. Consider using exact length validation or regex pattern. |
||
| required | ||
| {...methods.register("backupCode")} | ||
| /> | ||
| </div> | ||
| ); | ||
| } | ||
| 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, TextField } from "@calcom/ui"; | ||||||||
| 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")], { | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: calls formatBackupCode before it's defined (line 163), causing a reference error
Suggested change
|
||||||||
| 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) { | ||||||||
| onEnable(); | ||||||||
| 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={t("enable_2fa")} description={setupDescriptions[step]} type="creation"> | ||||||||
| <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"> | ||||||||
| <TextField | ||||||||
| <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> | ||||||||
| <Button color="secondary" onClick={onCancel}> | ||||||||
| {t("cancel")} | ||||||||
| </Button> | ||||||||
| {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> | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,10 +19,10 @@ const TwoFactorAuthAPI = { | |
| }); | ||
| }, | ||
|
|
||
| async disable(password: string, code: string) { | ||
| async disable(password: string, code: string, backupCode: string) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: The |
||
| return fetch("/api/auth/two-factor/totp/disable", { | ||
| method: "POST", | ||
| body: JSON.stringify({ password, code }), | ||
| body: JSON.stringify({ password, code, backupCode }), | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: Sending empty/undefined backup codes in the request body may cause issues. Consider only including non-empty values in the JSON payload. |
||
| headers: { | ||
| "Content-Type": "application/json", | ||
| }, | ||
|
|
||
| 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 | ||||||
| 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."); | ||||||
| throw new Error(ErrorCode.InternalServerError); | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Error message mentions 'backup code login' but this is a disable endpoint, not login
Suggested change
|
||||||
| } | ||||||
|
|
||||||
| 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 | ||||||
|
|
||||||
| // 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, | ||||||
| }, | ||||||
|
|
||||||
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.
logic: Function name 'TwoFactor' doesn't match the component's purpose or file name. Should be 'BackupCode' or similar.