From c8f7b1cedaec36d01832bf1ba3b6ede35022266c Mon Sep 17 00:00:00 2001 From: Daniel Salazar Date: Mon, 26 Oct 2026 22:07:52 -0700 Subject: [PATCH] fix(auth): support recovery codes when disabling two-factor auth (#4374) --- doc/contributing-apis.md | 2 +- .../AuthController.elevation.http.test.ts | 85 ++++++++++++++++++ .../controllers/auth/AuthController.ts | 39 ++++++-- .../auth/recoveryDisable2fa.http.test.ts | 89 +++++++++++++++++++ src/gui/src/UI/Settings/UIWindowDisable2FA.js | 7 +- .../UI/Settings/UIWindowDisable2FA.test.js | 8 ++ src/gui/src/i18n/translations/en.js | 3 +- 7 files changed, 221 insertions(+), 12 deletions(-) create mode 100644 src/backend/controllers/auth/AuthController.elevation.http.test.ts create mode 100644 src/backend/controllers/auth/recoveryDisable2fa.http.test.ts diff --git a/doc/contributing-apis.md b/doc/contributing-apis.md index 1bf280e8a..86759dff8 100644 --- a/doc/contributing-apis.md +++ b/doc/contributing-apis.md @@ -47,7 +47,7 @@ Both are supported ways to define an API. **Prefer a controller when you need fi Provider sign-in requires the account's authenticator or a recovery code when 2FA is enabled, including signup callbacks that resolve an existing account. The five-minute challenge uses the same single-use `/login/otp` and `/login/recovery-code` verification as password sign-in. The GUI carries it in a URL fragment and removes it before initialization. A session and successful popup-return proof are issued only after verification; cancelling retains the popup's signed retry context without marking it authenticated. -Disabling 2FA through `/user-protected/disable-2fa` requires both password or OIDC revalidation and the current six-digit authenticator `code`. Missing, malformed, or incorrect codes leave the secret and recovery codes intact. A team's requirement for 2FA still prevents disabling it. +Disabling 2FA through `/user-protected/disable-2fa` requires both password or OIDC revalidation and either the current six-digit authenticator `code` or an unused recovery code in `code`. Recovery codes are consumed atomically. Missing or incorrect codes leave the secret and remaining recovery codes intact. A team's requirement for 2FA still prevents disabling it. Email changes through `/user-protected/change-email` require the password or OIDC revalidation cookie, plus a six-digit `code` from the authenticator when 2FA is enabled. Missing or malformed codes return `400 bad_request`; incorrect codes return `401 code_mismatch`. Both include `factor: 'otp'`. Confirmation links are bound to that verified request. A 2FA-enabled account must request a new link if its existing link was issued without second-factor verification. diff --git a/src/backend/controllers/auth/AuthController.elevation.http.test.ts b/src/backend/controllers/auth/AuthController.elevation.http.test.ts new file mode 100644 index 000000000..5722d7d57 --- /dev/null +++ b/src/backend/controllers/auth/AuthController.elevation.http.test.ts @@ -0,0 +1,85 @@ +/* + * Copyright (C) 2024-present Puter Technologies Inc. + * + * This file is part of Puter. + * + * Puter is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { setupPuterTestEnv, type PuterTestEnv } from '../../testUtil.js'; + +describe('auth elevation cookies over HTTP', () => { + let env: PuterTestEnv; + + beforeAll(async () => { + env = await setupPuterTestEnv(); + }, 120_000); + + afterAll(async () => { + await env?.shutdown(); + }); + + it.each(['bearer', 'cookie'])( + 'API host returns a usable token without setting cookies (%s auth)', + async (credential) => { + const { token, password, username } = env.users.user; + const res = await fetch(new URL('/auth/elevate', env.apiOrigin), { + method: 'POST', + headers: { + 'content-type': 'application/json', + ...(credential === 'cookie' + ? { cookie: `puter_auth_token=${token}` } + : { authorization: `Bearer ${token}` }), + }, + body: JSON.stringify({ password }), + }); + expect(res.status).toBe(200); + expect(res.headers.get('set-cookie')).toBeNull(); + const body = (await res.json()) as { + elevated: boolean; + token: string; + }; + expect(body.elevated).toBe(true); + expect(typeof body.token).toBe('string'); + const user = await env.server.stores.user.getByUsername(username); + const { verifyStepUpSession } = + await import('../../core/http/middleware/stepUpSession.js'); + expect( + verifyStepUpSession( + { + headers: { 'x-puter-elevation': body.token }, + actor: { user: { uuid: user!.uuid } }, + } as never, + { tokenService: env.server.services.token }, + ), + ).toBe(true); + }, + ); + + it('GUI host sets the elevation cookie and omits the raw token for cookie auth', async () => { + const { token, password } = env.users.user; + const res = await fetch(new URL('/auth/elevate', env.origin), { + method: 'POST', + headers: { + 'content-type': 'application/json', + cookie: `puter_auth_token=${token}`, + }, + body: JSON.stringify({ password }), + }); + expect(res.status).toBe(200); + expect(res.headers.get('set-cookie')).toContain('puter_elevated='); + expect(await res.json()).toEqual({ elevated: true }); + }); +}); diff --git a/src/backend/controllers/auth/AuthController.ts b/src/backend/controllers/auth/AuthController.ts index 758c785b0..1ab6f6486 100644 --- a/src/backend/controllers/auth/AuthController.ts +++ b/src/backend/controllers/auth/AuthController.ts @@ -4275,7 +4275,27 @@ export class AuthController extends PuterController { ); } - this.#requireAccountTwoFactor(user, req.body?.code); + const code = req.body?.code; + if ( + user.otp_enabled && + typeof code === 'string' && + /^[A-Z0-9]{8}$/.test(code) + ) { + await this.#assertSecondFactorAttemptsRemain(user.uuid); + const consumed = await this.stores.user.consumeOtpRecoveryCode( + user.id, + hashRecoveryCode(code), + ); + if (!consumed) { + await this.#recordSecondFactorFailure(user.uuid); + throw new HttpError(401, 'Incorrect code.', { + legacyCode: 'code_mismatch', + fields: { factor: 'otp' }, + }); + } + } else { + this.#requireAccountTwoFactor(user, code); + } await this.stores.user.update(user.id, { otp_enabled: false, @@ -4485,11 +4505,14 @@ export class AuthController extends PuterController { } const token = signStepUpToken(this.services.token, user as never); - res.cookie( - STEP_UP_COOKIE_NAME, - token, - stepUpCookieOptions(this.config), - ); + const isApiHost = req.subdomains?.at(-1) === 'api'; + if (!isApiHost) { + res.cookie( + STEP_UP_COOKIE_NAME, + token, + stepUpCookieOptions(this.config), + ); + } // A browser reads its elevation back from the httpOnly cookie and never // needs the raw value; handing it to page JS would put the second factor @@ -4498,7 +4521,9 @@ export class AuthController extends PuterController { // password/TOTP — this only avoids needless exposure, it isn't a gate. const cookieName = this.config.cookie_name ?? 'puter_token'; const usedSessionCookie = - !!req.token && req.token === req.cookies?.[cookieName]; + !isApiHost && + !!req.token && + req.token === req.cookies?.[cookieName]; res.json( usedSessionCookie ? { elevated: true } : { elevated: true, token }, ); diff --git a/src/backend/controllers/auth/recoveryDisable2fa.http.test.ts b/src/backend/controllers/auth/recoveryDisable2fa.http.test.ts new file mode 100644 index 000000000..9c862caa1 --- /dev/null +++ b/src/backend/controllers/auth/recoveryDisable2fa.http.test.ts @@ -0,0 +1,89 @@ +import { v4 as uuidv4 } from 'uuid'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { hashRecoveryCode } from '../../services/auth/OTPUtil.js'; +import { + createTestUser, + setupPuterTestEnv, + type PuterTestEnv, +} from '../../testUtil.js'; + +describe('2FA after signing in with a recovery code', () => { + let env: PuterTestEnv; + const password = 'lost-authenticator-password'; + + beforeAll(async () => { + env = await setupPuterTestEnv(); + }, 120_000); + afterAll(async () => { + await env?.shutdown(); + }); + + const post = ( + path: string, + body: Record, + cookie?: string, + ) => + fetch(new URL(path, env.origin), { + method: 'POST', + headers: { + 'content-type': 'application/json', + ...(cookie ? { cookie } : {}), + }, + body: JSON.stringify(body), + }); + + it('allows an unused recovery code to disable 2FA after password revalidation', async () => { + const username = `lost2fa_${uuidv4().slice(0, 8)}`; + await createTestUser(env.server, { username, password }); + const user = (await env.server.stores.user.getByUsername(username))!; + await env.server.stores.user.update(user.id, { + email: `${username}@example.com`, + email_confirmed: true, + otp_enabled: true, + otp_secret: 'TESTSECRETBASE32', + otp_recovery_codes: ['RECOVER1', 'RECOVER2', 'RECOVER3'] + .map(hashRecoveryCode) + .join(','), + }); + + // Sign in with a recovery code: works, and yields a web session. + const login = await post('/login', { username, password }); + expect(login.status).toBe(202); + const { otp_jwt_token } = await login.json(); + const recovered = await post('/login/recovery-code', { + token: otp_jwt_token, + code: 'RECOVER1', + }); + expect((await recovered.json()).next_step).toBe('complete'); + const cookie = recovered.headers + .getSetCookie() + .find((value) => value.startsWith('puter_auth_token='))! + .split(';')[0]!; + + const reused = await post( + '/user-protected/disable-2fa', + { password, code: 'RECOVER1' }, + cookie, + ); + expect(reused.status).toBe(401); + expect( + (await env.server.stores.user.getById(user.id, { force: true }))! + .otp_enabled, + ).toBeTruthy(); + + const disable = await post( + '/user-protected/disable-2fa', + { password, code: 'RECOVER2' }, + cookie, + ); + expect(disable.status).toBe(200); + expect(await disable.json()).toMatchObject({ success: true }); + + const after = await env.server.stores.user.getById(user.id, { + force: true, + }); + expect(after!.otp_enabled).toBeFalsy(); + expect(after!.otp_secret).toBeNull(); + expect(after!.otp_recovery_codes).toBeNull(); + }); +}); diff --git a/src/gui/src/UI/Settings/UIWindowDisable2FA.js b/src/gui/src/UI/Settings/UIWindowDisable2FA.js index 6d33a4a7a..d945c79e5 100644 --- a/src/gui/src/UI/Settings/UIWindowDisable2FA.js +++ b/src/gui/src/UI/Settings/UIWindowDisable2FA.js @@ -53,8 +53,8 @@ async function UIWindowDisable2FA (options) { h += ''; h += ''; h += '
'; - h += ``; - h += ``; + h += ``; + h += ``; h += '
'; h += ``; h += ''; @@ -204,7 +204,8 @@ async function UIWindowDisable2FA (options) { credentials: 'include', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ - code: $(el_window).find('.disable-2fa-code').val() || undefined, + // Normalized like the login window's recovery input. + code: $(el_window).find('.disable-2fa-code').val()?.trim().toUpperCase() || undefined, password: password !== undefined && password !== '' ? password : undefined, }), }); diff --git a/src/gui/src/UI/Settings/UIWindowDisable2FA.test.js b/src/gui/src/UI/Settings/UIWindowDisable2FA.test.js index 3576a2587..48db5193b 100644 --- a/src/gui/src/UI/Settings/UIWindowDisable2FA.test.js +++ b/src/gui/src/UI/Settings/UIWindowDisable2FA.test.js @@ -59,6 +59,14 @@ describe('disabling 2FA', () => { await expect(Promise.resolve(state.result.promise)).resolves.toBe(true); }); + it('trims and uppercases a pasted recovery code', async () => { + await open({ otp: true }); + jQuery(state.el).find('.disable-2fa-code').val(' abcd2345\n'); + fetch.mockResolvedValue(reply(200)); + await submit(); + expect(bodyOf(0)).toEqual({ password: 'password', code: 'ABCD2345' }); + }); + it.each([400, 401])('leaves 2FA enabled and permits retry after a code rejection (%s)', async status => { await open({ otp: true }); jQuery(state.el).find('.disable-2fa-code').val('000123'); diff --git a/src/gui/src/i18n/translations/en.js b/src/gui/src/i18n/translations/en.js index 1125488f6..be91aedf8 100644 --- a/src/gui/src/i18n/translations/en.js +++ b/src/gui/src/i18n/translations/en.js @@ -177,7 +177,8 @@ const en = { directory_depth_limit_exceeded: 'This folder is nested too deeply. Move it somewhere closer to the top before adding more folders inside it.', disable_2fa: 'Disable 2FA', disable_2fa_confirm: 'Are you sure you want to disable 2FA?', - disable_2fa_instructions: 'Confirm your identity and enter the code from your authenticator app to disable 2FA.', + disable_2fa_instructions: 'Confirm your identity and enter an authenticator code or an unused recovery code to disable 2FA.', + disable_2fa_code_instruction: 'Enter a 6-digit authenticator code or an unused recovery code.', disassociate_dir: 'Disassociate Directory', display_name: 'Display Name', directory: 'Directory',