From b706693f82a8cf50aad8103655964975ed3b2abe Mon Sep 17 00:00:00 2001 From: Daniel Salazar Date: Mon, 15 Jun 2026 16:07:44 -0700 Subject: [PATCH] wip: hardening (#3266) --- src/backend/clients/event/types.ts | 27 ++ .../controllers/auth/AuthController.test.ts | 240 +++++++++++++----- .../controllers/auth/AuthController.ts | 130 +++++++--- .../webdav/WebDAVController.test.ts | 107 ++++++++ .../controllers/webdav/WebDAVController.ts | 20 ++ .../core/http/middleware/gates.test.ts | 57 ++++- src/backend/core/http/middleware/gates.ts | 83 +++++- src/backend/server.ts | 17 +- .../UI/UIWindowPhoneVerificationRequired.js | 15 +- 9 files changed, 572 insertions(+), 124 deletions(-) diff --git a/src/backend/clients/event/types.ts b/src/backend/clients/event/types.ts index 17035a659..8927314c6 100644 --- a/src/backend/clients/event/types.ts +++ b/src/backend/clients/event/types.ts @@ -131,6 +131,12 @@ export type EventMap = { data?: unknown; abuse?: unknown; trail?: Array; + /** + * Set by the abuse harness for flagged signups — the id under which the + * decision trail is persisted to KV (`abuse:trail:`), shared back on + * the request for log / support correlation. + */ + trail_id?: string; /** Device signals forwarded verbatim from the signup request body. */ fingerprint?: string | null; dfp_telemetry_id?: string | null; @@ -168,6 +174,27 @@ export type EventMap = { user_uid: string; email: string; }; + // Phone-reuse cap is pure mechanism here — the abuse extension answers + // (emitted via `emitAndWait`) by counting how many OTHER accounts have + // already verified this number and flipping `allowed` to false when the + // cross-account limit is hit. No extension listening → `allowed` stays + // true and the send proceeds. + 'puter.phone-verification.check': { + user_id: number; + user_uid: string; + phone: string; + allowed: boolean; + reason: string | null; + [key: string]: unknown; + }; + // Fire-and-forget signal that a code was actually sent — the abuse + // extension bumps its per-number / per-account send-velocity counters off + // this. No-op with no extension listening. + 'puter.phone-verification.sent': { + user_id: number; + user_uid: string; + phone: string; + }; 'user.phone-verified': { user_id: number; user_uid: string; diff --git a/src/backend/controllers/auth/AuthController.test.ts b/src/backend/controllers/auth/AuthController.test.ts index bfd9cae71..1e0f36b98 100644 --- a/src/backend/controllers/auth/AuthController.test.ts +++ b/src/backend/controllers/auth/AuthController.test.ts @@ -1809,70 +1809,13 @@ describe('AuthController.handleSendConfirmEmail', () => { }); }); -describe('AuthController.handleSendConfirmPhone attempt cap', () => { - it('allows two code sends, then hard-blocks the third (429)', async () => { - const { actor } = await makeUserAndActor(); - // Stub the Prelude client: report configured + supported, and a - // successful send — so we exercise the KV-backed attempt cap, not the - // network or the country gate. - const ctrl = controller as { clients: { prelude: unknown } }; - const realPrelude = ctrl.clients.prelude; - const createVerification = vi.fn(async () => ({ status: 'success' })); - ctrl.clients.prelude = { - isConfigured: () => true, - isCountrySupported: () => true, - defaultCountry: 'US', - createVerification, - }; - try { - const send = () => - controller.handleSendConfirmPhone( - makeReq({ phone: '+14155550123' }, { actor }), - makeRes(), - ); - await send(); // 1st — allowed - await send(); // 2nd — allowed - expect(createVerification).toHaveBeenCalledTimes(2); - // 3rd — over the lifetime cap → 429, and no SMS dispatched. - await expect(send()).rejects.toMatchObject({ statusCode: 429 }); - expect(createVerification).toHaveBeenCalledTimes(2); - } finally { - ctrl.clients.prelude = realPrelude; - } - }); - - it('does not burn an attempt when the send fails upstream', async () => { - const { actor } = await makeUserAndActor(); - const ctrl = controller as { clients: { prelude: unknown } }; - const realPrelude = ctrl.clients.prelude; - // First call throws (upstream error), second succeeds. - const createVerification = vi - .fn() - .mockRejectedValueOnce(new Error('prelude down')) - .mockResolvedValue({ status: 'success' }); - ctrl.clients.prelude = { - isConfigured: () => true, - isCountrySupported: () => true, - defaultCountry: 'US', - createVerification, - }; - try { - const send = () => - controller.handleSendConfirmPhone( - makeReq({ phone: '+14155550123' }, { actor }), - makeRes(), - ); - // Upstream failure → 502, attempt NOT counted. - await expect(send()).rejects.toMatchObject({ statusCode: 502 }); - // Two real sends still available afterwards. - await send(); - await send(); - await expect(send()).rejects.toMatchObject({ statusCode: 429 }); - } finally { - ctrl.clients.prelude = realPrelude; - } - }); -}); +// The per-account / per-number SMS send caps used to live here as a hardcoded +// `MAX_PHONE_VERIFY_SENDS` in the backend. They now live entirely in the abuse +// extension (no abuse thresholds in the OSS repo): the backend only asks via +// `puter.phone-verification.check` and reports sends via +// `puter.phone-verification.sent`. The cap behavior is covered by the +// extension's phoneVerification / phoneSendLog tests; the backend side (it +// forwards a veto, and emits `sent` only on success) is covered below. describe('AuthController.handleSendConfirmPhone validation', () => { // Stub the Prelude client (a real external boundary) so we exercise the @@ -1960,6 +1903,175 @@ describe('AuthController.handleSendConfirmPhone validation', () => { }); }); +describe('AuthController phone verification — staging & reuse', () => { + const stubPrelude = (over: Record = {}) => ({ + isConfigured: () => true, + isCountrySupported: () => true, + defaultCountry: 'US', + createVerification: vi.fn(async () => ({ status: 'success' })), + checkVerification: vi.fn(async () => ({ status: 'success' })), + ...over, + }); + const withClients = async ( + over: { prelude?: unknown; event?: unknown }, + fn: () => Promise, + ): Promise => { + const ctrl = controller as { + clients: { prelude: unknown; event: unknown }; + }; + const realPrelude = ctrl.clients.prelude; + const realEvent = ctrl.clients.event; + if ('prelude' in over) ctrl.clients.prelude = over.prelude; + if ('event' in over) ctrl.clients.event = over.event; + try { + await fn(); + } finally { + ctrl.clients.prelude = realPrelude; + ctrl.clients.event = realEvent; + } + }; + + it('stages the number in KV and does NOT write it to the user row before verification', async () => { + const { user, actor } = await makeUserAndActor(); + await withClients({ prelude: stubPrelude() }, async () => { + await controller.handleSendConfirmPhone( + makeReq({ phone: '+14155550123' }, { actor }), + makeRes(), + ); + }); + const after = await server.stores.user.getById(user.id, { + force: true, + }); + // The unverified number is not persisted to the indexed column … + expect(after!.phone ?? null).toBeNull(); + // … it's staged in KV instead, for /confirm-phone to read back. + const { res: staged } = await server.stores.kv.get({ + key: `phone-verify-pending:${user.id}`, + }); + expect(staged).toBe('+14155550123'); + }); + + it('forwards an abuse-extension veto as 429 with the opaque reason, and sends nothing', async () => { + const { user, actor } = await makeUserAndActor(); + const emitAndWait = vi.fn( + async ( + name: string, + ev: { allowed: boolean; reason: string | null }, + ) => { + if (name === 'puter.phone-verification.check') { + ev.allowed = false; + ev.reason = 'phone_already_used'; + } + }, + ); + const createVerification = vi.fn(async () => ({ status: 'success' })); + await withClients( + { + prelude: stubPrelude({ createVerification }), + event: { emitAndWait, emit: vi.fn() }, + }, + async () => { + // 429 (not 403), and the extension's reason is forwarded + // verbatim for the client to message on — the backend never + // interprets it. + await expect( + controller.handleSendConfirmPhone( + makeReq({ phone: '+14155550123' }, { actor }), + makeRes(), + ), + ).rejects.toMatchObject({ + statusCode: 429, + fields: { reason: 'phone_already_used' }, + }); + }, + ); + // Vetoed before the send — no SMS dispatched, nothing staged. + expect(createVerification).not.toHaveBeenCalled(); + const { res: staged } = await server.stores.kv.get({ + key: `phone-verify-pending:${user.id}`, + }); + expect(staged ?? null).toBeNull(); + }); + + it('emits puter.phone-verification.sent after a successful send', async () => { + const { actor } = await makeUserAndActor(); + const emit = vi.fn(); + const emitAndWait = vi.fn(async () => {}); // no veto + await withClients( + { prelude: stubPrelude(), event: { emit, emitAndWait } }, + async () => { + await controller.handleSendConfirmPhone( + makeReq({ phone: '+14155550123' }, { actor }), + makeRes(), + ); + }, + ); + expect(emit).toHaveBeenCalledWith( + 'puter.phone-verification.sent', + expect.objectContaining({ phone: '+14155550123' }), + expect.anything(), + ); + }); + + it('does NOT emit the sent signal when the send fails upstream', async () => { + const { actor } = await makeUserAndActor(); + const emit = vi.fn(); + const emitAndWait = vi.fn(async () => {}); + await withClients( + { + prelude: stubPrelude({ + createVerification: vi.fn(async () => { + throw new Error('prelude down'); + }), + }), + event: { emit, emitAndWait }, + }, + async () => { + await expect( + controller.handleSendConfirmPhone( + makeReq({ phone: '+14155550123' }, { actor }), + makeRes(), + ), + ).rejects.toMatchObject({ statusCode: 502 }); + }, + ); + const sentCalls = emit.mock.calls.filter( + (c) => c[0] === 'puter.phone-verification.sent', + ); + expect(sentCalls).toHaveLength(0); + }); + + it('confirms against the staged number and persists it to the row only on success', async () => { + const { user, actor } = await makeUserAndActor({ + requires_phone_verification: 1, + }); + // No phone on the row; the number lives only in the KV staging slot. + await server.stores.kv.set({ + key: `phone-verify-pending:${user.id}`, + value: '+14155550123', + }); + const checkVerification = vi.fn(async () => ({ status: 'success' })); + await withClients( + { prelude: stubPrelude({ checkVerification }) }, + async () => { + const res = makeRes(); + await controller.handleConfirmPhone( + makeReq({ code: '123456' }, { actor }), + res, + ); + expect(res.body).toMatchObject({ phone_verified: true }); + }, + ); + expect(checkVerification).toHaveBeenCalledWith('+14155550123', '123456'); + const after = await server.stores.user.getById(user.id, { + force: true, + }); + expect(after!.requires_phone_verification).toBe(false); + // Persisted to the row only now, on success. + expect(after!.phone).toBe('+14155550123'); + }); +}); + describe('AuthController.handleConfirmPhone', () => { const stubPrelude = (over: Record = {}) => ({ isConfigured: () => true, diff --git a/src/backend/controllers/auth/AuthController.ts b/src/backend/controllers/auth/AuthController.ts index d89b4b95e..9c9fb1997 100644 --- a/src/backend/controllers/auth/AuthController.ts +++ b/src/backend/controllers/auth/AuthController.ts @@ -609,6 +609,11 @@ export class AuthController extends PuterController { // Populated by the abuse extension's v2 harness; persisted to the // user row below so the signup-time reputation is referable later. reputation: null as number | null, + // Stamped by the abuse harness for flagged signups — the id keying + // the `abuse:trail:` decision trail (carrying both the live and + // shadow trails). Surfaced to a blocked user as the Request Code so + // the code they quote support leads straight to their trail. + trail_id: undefined as string | undefined, }; try { await this.clients.event?.emitAndWait( @@ -620,9 +625,14 @@ export class AuthController extends PuterController { console.warn('[signup] validate hook failed:', e); } if (!validateEvent.allow) { + // Pass the trail id back to a blocked user as the Request Code (when + // the harness stamped one), embedded in the message so the existing + // signup-block UI surfaces it without a GUI change. + const requestCode = validateEvent.trail_id; throw new HttpError( 403, - validateEvent.message ?? 'Signup blocked', + (validateEvent.message ?? 'Signup blocked') + + (requestCode ? ` Request Code: ${requestCode}` : ''), { ...(validateEvent.code ? { legacyCode: validateEvent.code as never } @@ -1087,30 +1097,66 @@ export class AuthController extends PuterController { { legacyCode: 'phone_country_not_supported' as never }, ); - // Hard lifetime cap: at most MAX_PHONE_VERIFY_SENDS SMS codes per user, - // tracked in KV so it survives logout / re-login (unlike the per-window - // rate limit above). Once exhausted the user can't request another code - // — and so can't clear the gate. KV read failures fail open: this is - // abuse/cost control, not a security boundary, so a KV blip must not - // lock signups out. Only successful sends are counted (incremented - // below), so a bad number / upstream error doesn't burn an attempt. - const MAX_PHONE_VERIFY_SENDS = 2; - const attemptsKey = `phone-verify-attempts:${user.id}`; - let priorAttempts = 0; + // Abuse caps live ENTIRELY in a listening abuse extension, consulted + // via `puter.phone-verification.check`. The backend ships no thresholds + // or detection of its own (so none of it is readable in the open-source + // repo): it forwards the user / number / ip, and the extension decides + // `allowed` plus an opaque `reason` (per-account + per-number send + // velocity, cross-account reuse, …). With no extension listening + // `allowed` stays true. Fail-open on a hook error — this is abuse/cost + // control, not a security boundary (the route rate limit and the country + // cost cap remain), so a flaky hook must not lock signups out. + const abuseCheck = { + user_id: user.id, + user_uid: user.uuid, + phone: parsed.e164, + allowed: true, + reason: null as string | null, + }; try { - const { res } = await this.stores.kv.get({ key: attemptsKey }); - if (typeof res === 'number') priorAttempts = res; + await this.clients.event?.emitAndWait( + 'puter.phone-verification.check', + abuseCheck, + {}, + ); } catch (e) { - console.warn('[send-confirm-phone] attempt-count read failed:', e); + console.warn('[send-confirm-phone] abuse-check hook failed:', e); } - if (priorAttempts >= MAX_PHONE_VERIFY_SENDS) + // Forward the verdict verbatim: a generic 429 plus the opaque reason for + // the client to message on. The backend never interprets the reason — + // its meaning lives in the extension (which sets it) and the GUI (which + // displays it), so no abuse semantics leak into the OSS repo. + if (abuseCheck.allowed === false) throw new HttpError( 429, - 'You have used all of your phone verification attempts.', - { legacyCode: 'phone_verify_attempts_exhausted' as never }, + 'Phone verification is unavailable for this number right now.', + { + legacyCode: 'phone_verification_unavailable' as never, + fields: abuseCheck.reason + ? { reason: abuseCheck.reason } + : {}, + }, ); - await this.stores.user.update(user.id, { phone: parsed.e164 }); + // Stage the parsed number as pending in KV (NOT on the user row) so a + // never-confirmed number is never written to the indexed `phone` + // column. /confirm-phone reads it back and persists it to the row only + // once Prelude confirms the code. ~1h TTL covers the code's lifetime. + // Stored before the send so we never dispatch an SMS we couldn't later + // confirm against. + const pendingPhoneKey = `phone-verify-pending:${user.id}`; + try { + await this.stores.kv.set({ + key: pendingPhoneKey, + value: parsed.e164, + expireAt: Math.floor(Date.now() / 1000) + 60 * 60, + }); + } catch (e) { + console.warn('[send-confirm-phone] pending-store failed:', e); + throw new HttpError(503, 'Could not start phone verification.', { + legacyCode: 'service_unavailable' as never, + }); + } const ip = req.ip || req.socket?.remoteAddress || undefined; try { @@ -1137,17 +1183,22 @@ export class AuthController extends PuterController { }); } - // Count the successful send against the lifetime cap (best-effort; a KV - // write failure shouldn't fail a code that was already sent). ~30d TTL - // so abandoned counters self-clean rather than living forever. + // Tell the abuse extension a code was actually sent, so it can bump its + // send-velocity counters (per number + per account). Fire-and-forget; + // the backend keeps no send counts of its own. Only reached after a + // successful send, so an upstream error never burns quota. try { - await this.stores.kv.set({ - key: attemptsKey, - value: priorAttempts + 1, - expireAt: Math.floor(Date.now() / 1000) + 30 * 24 * 60 * 60, - }); - } catch (e) { - console.warn('[send-confirm-phone] attempt-count write failed:', e); + this.clients.event?.emit( + 'puter.phone-verification.sent' as never, + { + user_id: user.id, + user_uid: user.uuid, + phone: parsed.e164, + } as never, + {}, + ); + } catch { + // ignore — best-effort velocity signal } res.json({}); } @@ -1181,7 +1232,22 @@ export class AuthController extends PuterController { res.json({ phone_verified: true, original_client_socket_id }); return; } - if (!user.phone) + // The number being verified is the one staged at send time (KV + // pending), not the user row — we don't persist an unverified number. + // Fall back to a row value for accounts that already have one on file + // (legacy / a number persisted by a prior verified flow). + const pendingPhoneKey = `phone-verify-pending:${user.id}`; + let pendingPhone: string | null = null; + try { + const { res: staged } = await this.stores.kv.get({ + key: pendingPhoneKey, + }); + if (typeof staged === 'string' && staged) pendingPhone = staged; + } catch (e) { + console.warn('[confirm-phone] pending read failed:', e); + } + if (!pendingPhone) pendingPhone = user.phone ?? null; + if (!pendingPhone) throw new HttpError( 400, 'No phone number on file. Request a code first.', @@ -1195,7 +1261,7 @@ export class AuthController extends PuterController { let status; try { ({ status } = await this.clients.prelude.checkVerification( - user.phone, + pendingPhone, String(code), )); } catch (e) { @@ -1210,8 +1276,10 @@ export class AuthController extends PuterController { return; } + // Verified — persist the number now (and only now) and clear the gate. await this.stores.user.update(user.id, { requires_phone_verification: 0, + phone: pendingPhone, }); try { @@ -1220,7 +1288,7 @@ export class AuthController extends PuterController { { user_id: user.id, user_uid: user.uuid, - phone: user.phone, + phone: pendingPhone, } as never, {}, ); diff --git a/src/backend/controllers/webdav/WebDAVController.test.ts b/src/backend/controllers/webdav/WebDAVController.test.ts index 24cbcdb0e..3eeb0a706 100644 --- a/src/backend/controllers/webdav/WebDAVController.test.ts +++ b/src/backend/controllers/webdav/WebDAVController.test.ts @@ -4,6 +4,7 @@ import type { Request, Response } from 'express'; import { Readable } from 'node:stream'; import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; import { v4 as uuidv4 } from 'uuid'; +import { hash as bcryptHash } from 'bcrypt'; import { PuterRouter } from '../../core/http/PuterRouter.js'; import { PuterServer } from '../../server.js'; import { setupTestServer } from '../../testUtil.js'; @@ -228,6 +229,112 @@ describe('WebDAVController', () => { }); }); + describe('pending-verification gate', () => { + // WebDAV must enforce the same gate every other authenticated route + // gets from requireVerifiedAccount — it dispatches off a single use() + // with no route options, so the middleware is never wired in and it + // has to call assertVerifiedAccount itself. Without it, an account + // still pending email/phone/card verification could read/write its + // whole filesystem over the `dav` subdomain. + const gatedActor = (flags: Record) => ({ + user: { + id: 1, + uuid: 'gated-uuid', + username: 'gated', + ...flags, + }, + }); + + it('rejects a session actor pending phone verification with 403', async () => { + const { res, captured } = makeRes(); + await dispatchMiddleware( + makeReq({ + method: 'PROPFIND', + actor: gatedActor({ requires_phone_verification: true }), + }), + res, + noop, + ); + expect(captured.statusCode).toBe(403); + }); + + it('rejects a session actor pending card verification with 403', async () => { + const { res, captured } = makeRes(); + await dispatchMiddleware( + makeReq({ + method: 'GET', + actor: gatedActor({ requires_card_verification: true }), + }), + res, + noop, + ); + expect(captured.statusCode).toBe(403); + }); + + it('rejects a session actor with an unconfirmed email with 403', async () => { + const { res, captured } = makeRes(); + await dispatchMiddleware( + makeReq({ + method: 'PROPFIND', + actor: gatedActor({ + requires_email_confirmation: true, + email_confirmed: false, + }), + }), + res, + noop, + ); + expect(captured.statusCode).toBe(403); + }); + + it('enforces the gate on the Basic-auth path (flags carried onto the built actor)', async () => { + const username = `webdav-gated-${Math.random() + .toString(36) + .slice(2, 10)}`; + const created = await server.stores.user.create({ + username, + uuid: uuidv4(), + password: await bcryptHash('correct-horse', 4), + email: `${username}@test.local`, + free_storage: 100 * 1024 * 1024, + requires_email_confirmation: false, + }); + await server.stores.user.update(created.id, { + requires_phone_verification: 1, + }); + + const { res, captured } = makeRes(); + await dispatchMiddleware( + makeReq({ + method: 'PROPFIND', + headers: { + authorization: basicAuth(username, 'correct-horse'), + }, + }), + res, + noop, + ); + expect(captured.statusCode).toBe(403); + }); + + it('lets a fully-verified session actor through the gate', async () => { + const { res, captured } = makeRes(); + await dispatchMiddleware( + makeReq({ + method: 'OPTIONS', + actor: gatedActor({ + requires_phone_verification: false, + requires_card_verification: false, + requires_email_confirmation: false, + }), + }), + res, + noop, + ); + expect(captured.statusCode).toBe(200); + }); + }); + describe('unsupported methods', () => { it('returns 405 for unknown HTTP methods', async () => { const { res, captured } = makeRes(); diff --git a/src/backend/controllers/webdav/WebDAVController.ts b/src/backend/controllers/webdav/WebDAVController.ts index cf53b102a..0036d6ffc 100644 --- a/src/backend/controllers/webdav/WebDAVController.ts +++ b/src/backend/controllers/webdav/WebDAVController.ts @@ -23,6 +23,7 @@ import { posix as pathPosix } from 'node:path'; import { EventMap } from '../../clients/event/types.js'; import type { Actor } from '../../core/actor.js'; import { HttpError } from '../../core/http/HttpError.js'; +import { assertVerifiedAccount } from '../../core/http/middleware/gates.js'; import type { PuterRouter } from '../../core/http/PuterRouter.js'; import { verify as verifyOtp } from '../../services/auth/OTPUtil.js'; import { expandTildePath } from '../../services/fs/resolveNode.js'; @@ -89,6 +90,16 @@ export class WebDAVController extends PuterController { const actor = await this.#resolveActor(req, res); if (!actor) return; // 401 already sent + // Apply the same pending-verification gate every other authenticated + // route gets from `requireVerifiedAccount`. WebDAV dispatches every + // method off a single `router.use` with no route options, so that + // middleware is never inserted into its chain — without this call an + // account still pending email / phone / card verification could read, + // write, and delete its entire filesystem over the `dav` subdomain, + // bypassing the gate. Throws a 403 HttpError, surfaced by the catch in + // registerRoutes. + assertVerifiedAccount(actor.user); + // Expand `~`/`~/...` against the authenticated actor's username. // WebDAV doesn't standardize `~`, but some clients do — and the // pre-existing behaviour silently expanded it via the FS store. @@ -233,6 +244,15 @@ export class WebDAVController extends PuterController { email_confirmed: user.email_confirmed ?? false, requires_email_confirmation: user.requires_email_confirmation ?? false, + // Carry the signup-time verification flags so the + // assertVerifiedAccount() gate in #dispatch can see them on + // the Basic-auth path too (the cookie / `-token` paths get + // them from AuthService#actorUserFromRow). Omitting these is + // what let phone/card-gated accounts through WebDAV. + requires_phone_verification: + user.requires_phone_verification ?? false, + requires_card_verification: + user.requires_card_verification ?? false, }, }; } diff --git a/src/backend/core/http/middleware/gates.test.ts b/src/backend/core/http/middleware/gates.test.ts index a32839c37..8d7c7fbd8 100644 --- a/src/backend/core/http/middleware/gates.test.ts +++ b/src/backend/core/http/middleware/gates.test.ts @@ -25,7 +25,7 @@ import { adminOnlyGate, allowedAppIdsGate, requireAuthGate, - requireEmailConfirmedGate, + requireVerifiedAccount, requireNonAccessTokenGate, requireUserActorGate, requireVerifiedGate, @@ -440,11 +440,11 @@ describe('requireVerifiedGate', () => { }); }); -// ── requireEmailConfirmedGate ─────────────────────────────────────── +// ── requireVerifiedAccount ────────────────────────────────────────── -describe('requireEmailConfirmedGate', () => { +describe('requireVerifiedAccount', () => { it('passes through users that do not require confirmation (e.g. legacy/temp)', () => { - const got = runGate(requireEmailConfirmedGate(), { + const got = runGate(requireVerifiedAccount(), { actor: { user: { uuid: 'u-1', @@ -457,7 +457,7 @@ describe('requireEmailConfirmedGate', () => { }); it('passes through confirmed users even when confirmation is required', () => { - const got = runGate(requireEmailConfirmedGate(), { + const got = runGate(requireVerifiedAccount(), { actor: { user: { uuid: 'u-1', @@ -470,7 +470,7 @@ describe('requireEmailConfirmedGate', () => { }); it('returns 403 email_confirmation_required for pending-confirmation users', () => { - const got = runGate(requireEmailConfirmedGate(), { + const got = runGate(requireVerifiedAccount(), { actor: { user: { uuid: 'u-1', @@ -482,8 +482,51 @@ describe('requireEmailConfirmedGate', () => { expectHttpError(got, 403, 'email_confirmation_required'); }); + it('returns 403 phone_verification_required while the phone gate is set', () => { + const got = runGate(requireVerifiedAccount(), { + actor: { + user: { + uuid: 'u-1', + requires_email_confirmation: false, + email_confirmed: true, + requires_phone_verification: true, + }, + }, + }); + expectHttpError(got, 403, 'phone_verification_required'); + }); + + it('returns 403 card_verification_required while the card gate is set', () => { + const got = runGate(requireVerifiedAccount(), { + actor: { + user: { + uuid: 'u-1', + requires_email_confirmation: false, + email_confirmed: true, + requires_card_verification: true, + }, + }, + }); + expectHttpError(got, 403, 'card_verification_required'); + }); + + it('passes through once every gate is cleared', () => { + const got = runGate(requireVerifiedAccount(), { + actor: { + user: { + uuid: 'u-1', + requires_email_confirmation: true, + email_confirmed: true, + requires_phone_verification: false, + requires_card_verification: false, + }, + }, + }); + expect(got).toBeUndefined(); + }); + it("passes through when there's no actor (auth gate handled it)", () => { - const got = runGate(requireEmailConfirmedGate(), {}); + const got = runGate(requireVerifiedAccount(), {}); expect(got).toBeUndefined(); }); }); diff --git a/src/backend/core/http/middleware/gates.ts b/src/backend/core/http/middleware/gates.ts index adb1312d2..66a03a4c0 100644 --- a/src/backend/core/http/middleware/gates.ts +++ b/src/backend/core/http/middleware/gates.ts @@ -224,29 +224,84 @@ export const requireVerifiedGate = (strictFlag: boolean): RequestHandler => { }; /** - * Reject authenticated users whose account is pending email confirmation - * (`requires_email_confirmation && !email_confirmed`). Runs on every - * authenticated route by default; routes that set `allowUnconfirmed: true` - * skip this gate. + * Reject authenticated users whose account is still pending any signup-time + * verification — email confirmation, SMS phone verification, or credit-card + * verification. The abuse harness sets the phone/card flags on low-reputation + * signups (in place of a hard block), and this gate is what actually keeps + * those accounts out of the product until the flag clears: the flags live on + * `req.actor.user`, so every authenticated route enforces them, not just the + * GUI modal. * - * Returns 403 with `email_confirmation_required` so clients can show the - * confirmation prompt instead of a generic error. + * Runs on every authenticated route by default; routes that set + * `allowUnconfirmed: true` opt out (the verification endpoints themselves, + * plus essential flows like whoami / logout / save-account so a pending + * account can still reach the screens that clear the gate). + * + * Returns 403 with a per-gate legacy code (`email_confirmation_required` / + * `phone_verification_required` / `card_verification_required`) so clients can + * show the right prompt instead of a generic error. There is no state where a + * user should be allowed in with one verification pending, so any pending gate + * rejects. */ -export const requireEmailConfirmedGate = (): RequestHandler => { +export const requireVerifiedAccount = (): RequestHandler => { return (req, _res, next) => { - const user = req.actor?.user; - if (user?.requires_email_confirmation && !user?.email_confirmed) { - next( - new HttpError(403, 'Please confirm your email to continue', { - legacyCode: 'email_confirmation_required', - }), - ); + try { + assertVerifiedAccount(req.actor?.user); + } catch (err) { + next(err); return; } next(); }; }; +/** + * The pending-verification check, factored out of {@link requireVerifiedAccount} + * so auth paths that build their own actor outside the route-option machinery + * can enforce the exact same gate. The WebDAV controller is the motivating + * case: it dispatches every method off a single `router.use`, so + * `requireVerifiedAccount` is never wired into its chain — it has to call this + * directly. Keeping one implementation is the point: a verification gate added + * here is picked up by every caller, so the paths can't drift (which is how + * WebDAV came to bypass the phone/card gate to begin with). + * + * Throws 403 with a per-gate legacy code (`email_confirmation_required` / + * `phone_verification_required` / `card_verification_required`) so clients can + * show the right prompt instead of a generic error. There is no state where a + * user should be let in with any verification pending, so the first pending + * gate rejects. + */ +export const assertVerifiedAccount = ( + user: + | { + requires_email_confirmation?: unknown; + email_confirmed?: unknown; + requires_phone_verification?: unknown; + requires_card_verification?: unknown; + } + | undefined, +): void => { + if (user?.requires_email_confirmation && !user?.email_confirmed) { + throw new HttpError(403, 'Please confirm your email to continue', { + legacyCode: 'email_confirmation_required', + }); + } + if (user?.requires_phone_verification) { + throw new HttpError( + 403, + 'Please verify your phone number to continue', + { + legacyCode: 'phone_verification_required' as never, + }, + ); + } + if (user?.requires_card_verification) { + throw new HttpError(403, 'Please verify your card to continue', { + legacyCode: 'card_verification_required' as never, + }); + } +}; + /** * Reject unless the actor is acting through one of the named apps. * App-under-user actors are permitted iff `actor.app.uid` is in the allowList; diff --git a/src/backend/server.ts b/src/backend/server.ts index c5bffaff2..551887a2f 100644 --- a/src/backend/server.ts +++ b/src/backend/server.ts @@ -37,7 +37,7 @@ import { adminOnlyGate, allowedAppIdsGate, requireAuthGate, - requireEmailConfirmedGate, + requireVerifiedAccount, requireNonAccessTokenGate, requireUserActorGate, requireVerifiedGate, @@ -830,13 +830,16 @@ export class PuterServer { mwChain.push(requireAuthGate()); } - // Default-on email confirmation gate. Every authenticated route - // rejects users pending confirmation unless `allowUnconfirmed` - // opts out. This prevents unconfirmed accounts from accessing - // AI, FS, driver, etc. endpoints while still allowing essential - // flows (logout, confirm-email, whoami, save-account, …). + // Default-on account-verification gate. Every authenticated route + // rejects accounts still pending any signup-time verification — + // email confirmation, SMS phone verification, or card verification — + // unless `allowUnconfirmed` opts out. This is what keeps low-reputation + // signups (which the abuse harness flags instead of hard-blocking) out + // of AI, FS, driver, etc. endpoints server-side, not just behind the + // GUI modal, while still allowing essential flows (logout, + // confirm-email / -phone, card verification, whoami, save-account, …). if (needsAuth && !opts.allowUnconfirmed) { - mwChain.push(requireEmailConfirmedGate()); + mwChain.push(requireVerifiedAccount()); } // block access tokens by default diff --git a/src/gui/src/UI/UIWindowPhoneVerificationRequired.js b/src/gui/src/UI/UIWindowPhoneVerificationRequired.js index e8cdf53a7..119f97fc2 100644 --- a/src/gui/src/UI/UIWindowPhoneVerificationRequired.js +++ b/src/gui/src/UI/UIWindowPhoneVerificationRequired.js @@ -24,6 +24,17 @@ import UIWindow from './UIWindow.js'; // 2. Enter the 6-digit code → POST /confirm-phone (Prelude validates it). // The 6-digit code UX mirrors UIWindowEmailConfirmationRequired.js. Used as a // hard gate for low-reputation signups, so by default it has no close button. + +// Friendly, non-accusatory messages for the abuse `reason` codes the backend +// forwards on a refused send. The backend never interprets these (the abuse +// policy lives in a private extension); the wording is presented here. Unknown +// reasons fall back to the server's own message. +const SEND_REASON_MESSAGES = { + phone_already_used: 'This phone number has already been used to verify the maximum number of accounts. Please use a different number, or email hi@puter.com for help.', + phone_send_limit: 'This phone number has received too many verification codes recently. Please try again later, or use a different number.', + phone_verify_attempts_exhausted: 'You\'ve used all of your phone verification attempts. Email hi@puter.com and we\'ll help you finish.', +}; + function UIWindowPhoneVerificationRequired (options) { return new Promise(async (resolve) => { options = options ?? {}; @@ -205,8 +216,10 @@ function UIWindowPhoneVerificationRequired (options) { $(el_window).find('.digit-input').first().focus(); }, error: function (xhr) { + const reason = xhr.responseJSON?.reason; showError( - xhr.responseJSON?.error ?? + SEND_REASON_MESSAGES[reason] ?? + xhr.responseJSON?.error ?? 'Could not send a code to that number.', ); },