-
Notifications
You must be signed in to change notification settings - Fork 0
feat: 2fa backup codes #9
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?
feat: 2fa backup codes #9
Conversation
Co-authored-by: Peer Richelsen <[email protected]>
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.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Greptile OverviewConfidence Score: 2/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant User
participant Frontend
participant API
participant DB
Note over User,DB: 2FA Setup with Backup Codes
User->>Frontend: Enable 2FA
Frontend->>API: POST /api/auth/two-factor/totp/setup
API->>API: Generate TOTP secret (32 chars)
API->>API: Generate 10 backup codes (crypto.randomBytes)
API->>DB: Save encrypted backupCodes & twoFactorSecret
API-->>Frontend: Return QR code, secret, backupCodes
Frontend->>User: Display QR code
User->>Frontend: Enter TOTP code
Frontend->>API: Verify TOTP & enable 2FA
API-->>Frontend: Success
Frontend->>User: Display backup codes (download/copy)
Note over User,DB: Login with Backup Code
User->>Frontend: Enter email/password
Frontend->>API: POST /api/auth/callback/credentials
API-->>Frontend: SecondFactorRequired
Frontend->>User: Show 2FA input with "Lost access" option
User->>Frontend: Click "Lost access" → Enter backup code
Frontend->>API: Submit credentials with backupCode
API->>DB: Fetch encrypted backupCodes
API->>API: Decrypt & check if code matches
API->>API: Set matching code to null
API->>DB: Re-encrypt & save backupCodes array
API-->>Frontend: Authentication success
Frontend->>User: Redirect to app
Note over User,DB: Disable 2FA with Backup Code
User->>Frontend: Toggle 2FA off
Frontend->>User: Show password + 2FA/backup code input
User->>Frontend: Click "Lost access" → Enter backup code
Frontend->>API: POST /api/auth/two-factor/totp/disable
API->>DB: Verify backup code exists & matches
API->>DB: Set backupCodes=null, twoFactorEnabled=false
API-->>Frontend: Success
Frontend->>User: 2FA disabled
|
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.
16 files reviewed, 3 comments
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
syntax: component is named TwoFactor but file is BackupCode.tsx
| export default function TwoFactor({ center = true }) { | |
| export default function BackupCode({ center = true }) { |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/components/auth/BackupCode.tsx
Line: 7:7
Comment:
**syntax:** component is named `TwoFactor` but file is `BackupCode.tsx`
```suggestion
export default function BackupCode({ center = true }) {
```
How can I resolve this? If you propose a fix, please make it concise.| const index = backupCodes.indexOf(credentials.backupCode.replaceAll("-", "")); | ||
| if (index === -1) throw new Error(ErrorCode.IncorrectBackupCode); | ||
|
|
||
| // delete verified backup code and re-encrypt remaining | ||
| backupCodes[index] = 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: setting used codes to null creates a security issue - indexOf will match null values if user submits "null" as string after JSON parsing, allowing reuse of invalidated codes
| const index = backupCodes.indexOf(credentials.backupCode.replaceAll("-", "")); | |
| if (index === -1) throw new Error(ErrorCode.IncorrectBackupCode); | |
| // delete verified backup code and re-encrypt remaining | |
| backupCodes[index] = null; | |
| const index = backupCodes.indexOf(credentials.backupCode.replaceAll("-", "")); | |
| if (index === -1) throw new Error(ErrorCode.IncorrectBackupCode); | |
| // delete verified backup code and re-encrypt remaining | |
| backupCodes.splice(index, 1); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/features/auth/lib/next-auth-options.ts
Line: 144:148
Comment:
**logic:** setting used codes to `null` creates a security issue - `indexOf` will match `null` values if user submits "null" as string after JSON parsing, allowing reuse of invalidated codes
```suggestion
const index = backupCodes.indexOf(credentials.backupCode.replaceAll("-", ""));
if (index === -1) throw new Error(ErrorCode.IncorrectBackupCode);
// delete verified backup code and re-encrypt remaining
backupCodes.splice(index, 1);
```
How can I resolve this? If you propose a fix, please make it concise.| {t("forgot")} | ||
| </Link> | ||
| </div> | ||
| </div> |
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.
style: triple ternary nesting hurts readability
| </div> | |
| {twoFactorRequired && ( | |
| twoFactorLostAccess ? <BackupCode center /> : <TwoFactor center /> | |
| )} |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/pages/auth/login.tsx
Line: 218:218
Comment:
**style:** triple ternary nesting hurts readability
```suggestion
{twoFactorRequired && (
twoFactorLostAccess ? <BackupCode center /> : <TwoFactor center />
)}
```
How can I resolve this? If you propose a fix, please make it concise.
Test 3
Replicated from ai-code-review-evaluation/cal.com-greptile#3