diff --git a/src/backend/core/http/middleware/userProtected.test.ts b/src/backend/core/http/middleware/userProtected.test.ts index 7fb810534..5e7344210 100644 --- a/src/backend/core/http/middleware/userProtected.test.ts +++ b/src/backend/core/http/middleware/userProtected.test.ts @@ -303,18 +303,41 @@ describe('userProtected — verifyIdentity (step 3)', () => { expect(refused.legacyCode).toBe('too_many_requests'); }); - it('spends the budget on wrong answers only', async () => { + it('bounds attempts rather than rounds, so a burst cannot outrun it', async () => { const user = await makeUserWithPassword('correct-horse'); const [, , verifyIdentity] = buildGate(); - const right = () => + const guess = () => run( verifyIdentity, - withUser(user, { body: { password: 'correct-horse' } }), + withUser(user, { body: { password: 'wrong-guess' } }), ); - // Well past the limit, and none of it is charged. - for (let i = 0; i < 20; i++) expect(await right()).toBeUndefined(); - expect(await right()).toBeUndefined(); + // All at once: the budget is spent before any comparison runs, so + // the same ten get through whether they arrive in step or together. + const answers = (await Promise.all( + Array.from({ length: 30 }, guess), + )) as HttpError[]; + const compared = answers.filter( + (a) => a.legacyCode === 'password_mismatch', + ); + expect(compared).toHaveLength(10); + }); + + it('keeps an exhausted budget on one route away from the others', async () => { + const user = await makeUserWithPassword('correct-horse'); + const [, , verifyIdentity] = buildGate(); + const on = (path: string, password: string) => + run(verifyIdentity, { + ...withUser(user, { body: { password } }), + route: { path }, + } as Partial); + + for (let i = 0; i < 10; i++) await on('/a', 'wrong-guess'); + expect(((await on('/a', 'wrong-guess')) as HttpError).statusCode).toBe( + 429, + ); + // A different route behind the same gate still answers. + expect(await on('/b', 'correct-horse')).toBeUndefined(); }); it('returns 403 password_required when password account submits no credentials', async () => { diff --git a/src/backend/core/http/middleware/userProtected.ts b/src/backend/core/http/middleware/userProtected.ts index 8f5dab260..5945d23fb 100644 --- a/src/backend/core/http/middleware/userProtected.ts +++ b/src/backend/core/http/middleware/userProtected.ts @@ -21,7 +21,7 @@ import type { Request, RequestHandler, Response, NextFunction } from 'express'; import bcrypt from 'bcrypt'; import { isPlainUserActor } from '../../actor'; import { HttpError } from '../HttpError'; -import { checkRateLimit, peekRateLimit } from './rateLimit.js'; +import { checkRateLimit } from './rateLimit.js'; import type { IConfig } from '../../../types'; import type { UserStore, UserRow } from '../../../stores/user/UserStore'; import type { OIDCService } from '../../../services/auth/OIDCService'; @@ -49,7 +49,9 @@ import type { TokenService } from '../../../services/auth/TokenService'; * cookie (signed via `services.token.sign('oidc-state')`) is required. * OIDC-only accounts (no password) MUST use the revalidation cookie — * password path returns `oidc_revalidation_required` with a `revalidate_url` - * so the GUI can open the OIDC popup. + * so the GUI can open the OIDC popup. Each password check is charged to a + * per-account, per-route hourly budget, so a route behind this gate is + * bounded whether or not it declares a limit of its own. */ const REVALIDATION_COOKIE_NAME = 'puter_revalidation'; @@ -139,7 +141,7 @@ async function buildRevalidateFields( }; } -/** Wrong passwords per account per hour, matching the sibling routes. */ +/** Password checks per account per route per hour, as the siblings bound. */ const PASSWORD_ATTEMPT_LIMIT = 10; const PASSWORD_ATTEMPT_WINDOW_MS = 60 * 60_000; const PASSWORD_ATTEMPT_SCOPE = 'user-protected-password'; @@ -225,10 +227,14 @@ export const createUserProtectedGate = ( fields, }); } - // Charged by the outcome, so a right answer spends nothing. - const budgetKey = `${PASSWORD_ATTEMPT_SCOPE}:${user.id}`; + // Charged before the comparison, or everything already in + // flight is answered before any of it is spent. Per route, so + // one exhausted budget cannot close the other five. + const budgetKey = + `${PASSWORD_ATTEMPT_SCOPE}:${req.route?.path ?? req.path}` + + `:${user.id}`; if ( - !(await peekRateLimit( + !(await checkRateLimit( budgetKey, PASSWORD_ATTEMPT_LIMIT, PASSWORD_ATTEMPT_WINDOW_MS, @@ -247,16 +253,10 @@ export const createUserProtectedGate = ( } catch { match = false; } - if (!match) { - await checkRateLimit( - budgetKey, - PASSWORD_ATTEMPT_LIMIT, - PASSWORD_ATTEMPT_WINDOW_MS, - ); + if (!match) throw new HttpError(400, 'Password mismatch', { legacyCode: 'password_mismatch', }); - } return next(); }