mirror of
https://github.com/nestriness/nestri.git
synced 2026-11-11 18:02:29 +02:00
fix(auth): bound sign-in code sends per mailbox and per caller
The send budget was keyed by every submitted form field, so adding a field with a new value each time made a fresh budget, and one mailbox could be sent codes without limit. A claim is now built only from the claim fields (email, phone by default); anything else is dropped before it is counted and before it reaches sendCode. A budget per mailbox does nothing against a caller holding a list of addresses: each send goes to a different stranger, and the deployment's whole sending allowance goes with them. Add a second budget per requesting address, 10 codes per 24 hours by default. The window is a day rather than an hour because a caller that paces itself stays under any hourly figure a person would also fit under. The address lookup the device flow used is moved into util so both read the caller the same way.
This commit is contained in:
@@ -234,7 +234,7 @@ import { HtmlRenderer, type Renderer } from './ui/render.js';
|
|||||||
import { type DeviceConfirmWording, IssuerScreens } from './ui/issuer.js';
|
import { type DeviceConfirmWording, IssuerScreens } from './ui/issuer.js';
|
||||||
import type { ChooseOption, Screen } from './ui/screen.js';
|
import type { ChooseOption, Screen } from './ui/screen.js';
|
||||||
import type { Theme } from './ui/theme.js';
|
import type { Theme } from './ui/theme.js';
|
||||||
import { getRelativeUrl, isDomainMatch, lazy } from './util.js';
|
import { getRelativeUrl, isDomainMatch, lazy, requestAddress } from './util.js';
|
||||||
|
|
||||||
/** @internal */
|
/** @internal */
|
||||||
export const aws = awsHandle;
|
export const aws = awsHandle;
|
||||||
@@ -624,13 +624,7 @@ export function issuer<
|
|||||||
const deviceGuessWindow = input.deviceVerification?.guessWindow ?? 600;
|
const deviceGuessWindow = input.deviceVerification?.guessWindow ?? 600;
|
||||||
const deviceRequestLimit = input.deviceVerification?.requestLimit ?? 10;
|
const deviceRequestLimit = input.deviceVerification?.requestLimit ?? 10;
|
||||||
const deviceRequestWindow = input.deviceVerification?.requestWindow ?? 600;
|
const deviceRequestWindow = input.deviceVerification?.requestWindow ?? 600;
|
||||||
const deviceAddress =
|
const deviceAddress = input.deviceVerification?.address ?? requestAddress;
|
||||||
input.deviceVerification?.address ??
|
|
||||||
((req: Request) =>
|
|
||||||
req.headers.get('cf-connecting-ip') ??
|
|
||||||
req.headers.get('x-forwarded-for')?.split(',')[0]?.trim() ??
|
|
||||||
req.headers.get('x-real-ip') ??
|
|
||||||
undefined);
|
|
||||||
const renderer = input.renderer ?? HtmlRenderer({ theme: input.theme });
|
const renderer = input.renderer ?? HtmlRenderer({ theme: input.theme });
|
||||||
|
|
||||||
/** What a token for `clientID` lives for. See {@link IssuerInput.clientTTL}. */
|
/** What a token for `clientID` lives for. See {@link IssuerInput.clientTTL}. */
|
||||||
|
|||||||
@@ -56,6 +56,7 @@ import { Context } from 'hono';
|
|||||||
|
|
||||||
import { generateUnbiasedDigits, generateUnbiasedString, timingSafeCompare } from '../random.js';
|
import { generateUnbiasedDigits, generateUnbiasedString, timingSafeCompare } from '../random.js';
|
||||||
import { Storage } from '../storage/storage.js';
|
import { Storage } from '../storage/storage.js';
|
||||||
|
import { requestAddress } from '../util.js';
|
||||||
import { MARK_CODE } from '../ui/mark.js';
|
import { MARK_CODE } from '../ui/mark.js';
|
||||||
import type { Screen } from '../ui/screen.js';
|
import type { Screen } from '../ui/screen.js';
|
||||||
import { Provider } from './provider.js';
|
import { Provider } from './provider.js';
|
||||||
@@ -121,6 +122,47 @@ export interface CodeProviderConfig<
|
|||||||
* @default 30
|
* @default 30
|
||||||
*/
|
*/
|
||||||
resendInterval?: number;
|
resendInterval?: number;
|
||||||
|
/**
|
||||||
|
* The form fields that make up a claim. Anything else submitted is dropped
|
||||||
|
* before the claim is counted or sent to.
|
||||||
|
*
|
||||||
|
* The send budget is keyed by the claim, so a claim built from every field
|
||||||
|
* in the form is a budget the caller can mint at will: one extra field
|
||||||
|
* with a new value each time is a new key, and the same mailbox gets an
|
||||||
|
* unbounded number of codes.
|
||||||
|
*
|
||||||
|
* @default ['email', 'phone']
|
||||||
|
*/
|
||||||
|
claimFields?: string[];
|
||||||
|
/**
|
||||||
|
* How many codes one caller may have sent, to any claims, inside
|
||||||
|
* {@link addressWindow}.
|
||||||
|
*
|
||||||
|
* The per-claim budget protects a mailbox. This one protects everything
|
||||||
|
* else: without it a caller with a list of addresses can spend the whole
|
||||||
|
* sending allowance of the deployment, at five codes each, on strangers.
|
||||||
|
*
|
||||||
|
* @default 10
|
||||||
|
*/
|
||||||
|
maxSendsPerAddress?: number;
|
||||||
|
/**
|
||||||
|
* The window {@link maxSendsPerAddress} is counted over, in seconds.
|
||||||
|
*
|
||||||
|
* A day rather than the hour a mailbox gets, because the caller this is
|
||||||
|
* for paces itself: a handful of addresses an hour stays under any hourly
|
||||||
|
* budget a person would also fit under, and still adds up to hundreds of
|
||||||
|
* strangers mailed by the end of the day.
|
||||||
|
*
|
||||||
|
* @default 86400
|
||||||
|
*/
|
||||||
|
addressWindow?: number;
|
||||||
|
/**
|
||||||
|
* Who is asking, for {@link maxSendsPerAddress}. A request it returns
|
||||||
|
* nothing for is not counted.
|
||||||
|
*
|
||||||
|
* @default the address the proxy in front reports
|
||||||
|
*/
|
||||||
|
address?: (req: Request) => string | undefined;
|
||||||
/**
|
/**
|
||||||
* What to ask for at each step of the flow.
|
* What to ask for at each step of the flow.
|
||||||
*
|
*
|
||||||
@@ -210,6 +252,10 @@ export function CodeProvider<Claims extends Record<string, string> = Record<stri
|
|||||||
const maxSends = config.maxSends ?? 5;
|
const maxSends = config.maxSends ?? 5;
|
||||||
const sendWindow = config.sendWindow ?? 60 * 60;
|
const sendWindow = config.sendWindow ?? 60 * 60;
|
||||||
const resendInterval = config.resendInterval ?? 30;
|
const resendInterval = config.resendInterval ?? 30;
|
||||||
|
const claimFields = config.claimFields ?? ['email', 'phone'];
|
||||||
|
const maxSendsPerAddress = config.maxSendsPerAddress ?? 10;
|
||||||
|
const addressWindow = config.addressWindow ?? 60 * 60 * 24;
|
||||||
|
const address = config.address ?? requestAddress;
|
||||||
|
|
||||||
function generate() {
|
function generate() {
|
||||||
return generateUnbiasedDigits(length);
|
return generateUnbiasedDigits(length);
|
||||||
@@ -230,13 +276,33 @@ export function CodeProvider<Claims extends Record<string, string> = Record<stri
|
|||||||
*/
|
*/
|
||||||
function claimKey(claims: Record<string, string>) {
|
function claimKey(claims: Record<string, string>) {
|
||||||
const flattened = Object.entries(claims)
|
const flattened = Object.entries(claims)
|
||||||
.filter(([key]) => key !== 'action')
|
|
||||||
.map(([key, value]) => `${key}=${String(value).trim().toLowerCase()}`)
|
.map(([key, value]) => `${key}=${String(value).trim().toLowerCase()}`)
|
||||||
.sort()
|
.sort()
|
||||||
.join('&');
|
.join('&');
|
||||||
return ['oauth:code:claim', flattened];
|
return ['oauth:code:claim', flattened];
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** Where the sends one caller has asked for are counted. */
|
||||||
|
function addressKey(value: string) {
|
||||||
|
return ['oauth:code:address', value];
|
||||||
|
}
|
||||||
|
|
||||||
|
type SendRecord = { at: number; count: number; since: number };
|
||||||
|
|
||||||
|
/** The record after one more send, opening a new window if the last one closed. */
|
||||||
|
function counted(record: SendRecord | null, now: number, window: number): SendRecord {
|
||||||
|
const open = record && now - record.since < window * 1000;
|
||||||
|
return {
|
||||||
|
at: now,
|
||||||
|
count: open ? record.count + 1 : 1,
|
||||||
|
since: open ? record.since : now
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function exhausted(record: SendRecord | null, now: number, limit: number, window: number) {
|
||||||
|
return !!record && now - record.since < window * 1000 && record.count >= limit;
|
||||||
|
}
|
||||||
|
|
||||||
return {
|
return {
|
||||||
type: 'code',
|
type: 'code',
|
||||||
display: { name: 'Email', icon: MARK_CODE },
|
display: { name: 'Email', icon: MARK_CODE },
|
||||||
@@ -267,29 +333,35 @@ export function CodeProvider<Claims extends Record<string, string> = Record<stri
|
|||||||
const action = fd.get('action')?.toString();
|
const action = fd.get('action')?.toString();
|
||||||
|
|
||||||
if (action === 'request' || action === 'resend') {
|
if (action === 'request' || action === 'resend') {
|
||||||
const claims = Object.fromEntries(fd) as Claims;
|
const claims = {} as Record<string, string>;
|
||||||
delete claims.action;
|
for (const field of claimFields) {
|
||||||
|
const value = fd.get(field);
|
||||||
|
if (typeof value === 'string') claims[field] = value;
|
||||||
|
}
|
||||||
|
|
||||||
// Asked for too soon, or too many times for this mailbox.
|
// Asked for too soon, or too many times for this mailbox,
|
||||||
// Both answers are the same on purpose: saying which would
|
// or too many times from this caller. All three answers are
|
||||||
// tell a caller whether the address they typed has had a
|
// the same on purpose: saying which would tell a caller
|
||||||
// code sent to it lately, which is a fact about somebody
|
// whether the address they typed has had a code sent to it
|
||||||
// else's mailbox.
|
// lately, which is a fact about somebody else's mailbox.
|
||||||
const now = Date.now();
|
const now = Date.now();
|
||||||
const sent = await Storage.get<{ at: number; count: number; since: number }>(
|
const sent = await Storage.get<SendRecord>(ctx.storage, claimKey(claims));
|
||||||
ctx.storage,
|
|
||||||
claimKey(claims)
|
|
||||||
);
|
|
||||||
const open = sent && now - sent.since < sendWindow * 1000;
|
|
||||||
if (sent && now - sent.at < resendInterval * 1000) {
|
if (sent && now - sent.at < resendInterval * 1000) {
|
||||||
return transition(c, state ?? { type: 'start' }, fd, { type: 'rate_limit' });
|
return transition(c, state ?? { type: 'start' }, fd, { type: 'rate_limit' });
|
||||||
}
|
}
|
||||||
if (open && sent.count >= maxSends) {
|
if (exhausted(sent, now, maxSends, sendWindow)) {
|
||||||
|
return transition(c, state ?? { type: 'start' }, fd, { type: 'rate_limit' });
|
||||||
|
}
|
||||||
|
const caller = address(c.req.raw);
|
||||||
|
const fromCaller = caller
|
||||||
|
? await Storage.get<SendRecord>(ctx.storage, addressKey(caller))
|
||||||
|
: null;
|
||||||
|
if (exhausted(fromCaller, now, maxSendsPerAddress, addressWindow)) {
|
||||||
return transition(c, state ?? { type: 'start' }, fd, { type: 'rate_limit' });
|
return transition(c, state ?? { type: 'start' }, fd, { type: 'rate_limit' });
|
||||||
}
|
}
|
||||||
|
|
||||||
const code = generate();
|
const code = generate();
|
||||||
const err = await config.sendCode(claims, code);
|
const err = await config.sendCode(claims as Claims, code);
|
||||||
if (err) return transition(c, { type: 'start' }, fd, err);
|
if (err) return transition(c, { type: 'start' }, fd, err);
|
||||||
|
|
||||||
// The code that was live until a moment ago stops being
|
// The code that was live until a moment ago stops being
|
||||||
@@ -310,13 +382,17 @@ export function CodeProvider<Claims extends Record<string, string> = Record<stri
|
|||||||
await Storage.set(
|
await Storage.set(
|
||||||
ctx.storage,
|
ctx.storage,
|
||||||
claimKey(claims),
|
claimKey(claims),
|
||||||
{
|
counted(sent, now, sendWindow),
|
||||||
at: now,
|
|
||||||
count: open ? sent.count + 1 : 1,
|
|
||||||
since: open ? sent.since : now
|
|
||||||
},
|
|
||||||
sendWindow
|
sendWindow
|
||||||
);
|
);
|
||||||
|
if (caller) {
|
||||||
|
await Storage.set(
|
||||||
|
ctx.storage,
|
||||||
|
addressKey(caller),
|
||||||
|
counted(fromCaller, now, addressWindow),
|
||||||
|
addressWindow
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
return transition(
|
return transition(
|
||||||
c,
|
c,
|
||||||
|
|||||||
@@ -45,6 +45,23 @@ export function isDomainMatch(a: string, b: string): boolean {
|
|||||||
return tailA === tailB;
|
return tailA === tailB;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The address a request came from, as the proxy in front of this server
|
||||||
|
* reports it.
|
||||||
|
*
|
||||||
|
* Only as trustworthy as that proxy: these headers are whatever the caller
|
||||||
|
* sent unless something in front overwrites them, so a deployment reachable
|
||||||
|
* directly should pass its own function instead.
|
||||||
|
*/
|
||||||
|
export function requestAddress(req: Request): string | undefined {
|
||||||
|
return (
|
||||||
|
req.headers.get('cf-connecting-ip') ??
|
||||||
|
req.headers.get('x-forwarded-for')?.split(',')[0]?.trim() ??
|
||||||
|
req.headers.get('x-real-ip') ??
|
||||||
|
undefined
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
export function lazy<T>(fn: () => T): () => T {
|
export function lazy<T>(fn: () => T): () => T {
|
||||||
let value: T | undefined;
|
let value: T | undefined;
|
||||||
return () => {
|
return () => {
|
||||||
|
|||||||
@@ -23,6 +23,7 @@ const auth = issuer({
|
|||||||
code: CodeProvider({
|
code: CodeProvider({
|
||||||
maxAttempts: 3,
|
maxAttempts: 3,
|
||||||
maxSends: 2,
|
maxSends: 2,
|
||||||
|
maxSendsPerAddress: 3,
|
||||||
sendWindow: 3600,
|
sendWindow: 3600,
|
||||||
resendInterval: 0,
|
resendInterval: 0,
|
||||||
request: async (_req, _state, _form, error) => ({
|
request: async (_req, _state, _form, error) => ({
|
||||||
@@ -60,10 +61,18 @@ function jar() {
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
async function post(cookies: ReturnType<typeof jar>, body: Record<string, string>) {
|
async function post(
|
||||||
|
cookies: ReturnType<typeof jar>,
|
||||||
|
body: Record<string, string>,
|
||||||
|
headers: Record<string, string> = {}
|
||||||
|
) {
|
||||||
const response = await auth.request(`${ORIGIN}/code/authorize`, {
|
const response = await auth.request(`${ORIGIN}/code/authorize`, {
|
||||||
method: 'POST',
|
method: 'POST',
|
||||||
headers: { cookie: cookies.header(), 'content-type': 'application/x-www-form-urlencoded' },
|
headers: {
|
||||||
|
...headers,
|
||||||
|
cookie: cookies.header(),
|
||||||
|
'content-type': 'application/x-www-form-urlencoded'
|
||||||
|
},
|
||||||
body: new URLSearchParams(body)
|
body: new URLSearchParams(body)
|
||||||
});
|
});
|
||||||
cookies.absorb(response);
|
cookies.absorb(response);
|
||||||
@@ -261,6 +270,45 @@ describe('asking for codes', () => {
|
|||||||
expect(await errorOf(withOldCode)).toBe('rate_limit');
|
expect(await errorOf(withOldCode)).toBe('rate_limit');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// The budget is keyed by the claim. When the claim was every field in the
|
||||||
|
// form, a field with a new value each time was a new key, and so a new
|
||||||
|
// budget for the same mailbox.
|
||||||
|
test('an extra form field does not buy more codes', async () => {
|
||||||
|
const email = 'padded@example.com';
|
||||||
|
for (let i = 0; i < 4; i++) {
|
||||||
|
const cookies = await begin();
|
||||||
|
await post(cookies, { action: 'request', email, pad: String(i) });
|
||||||
|
}
|
||||||
|
expect(sent).toHaveLength(2);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('an extra form field does not reach the claims', async () => {
|
||||||
|
const cookies = await begin();
|
||||||
|
await post(cookies, { action: 'request', email: 'clean@example.com', role: 'admin' });
|
||||||
|
const response = await post(cookies, { action: 'verify', code: sent.at(-1)! });
|
||||||
|
expect(response.status).toBe(302);
|
||||||
|
});
|
||||||
|
|
||||||
|
// A list of addresses walks around a budget held per mailbox. This is the
|
||||||
|
// one that bounds what a single caller can spend.
|
||||||
|
test('one caller cannot mail a list of strangers', async () => {
|
||||||
|
const from = { 'cf-connecting-ip': '198.51.100.20' };
|
||||||
|
for (let i = 0; i < 5; i++) {
|
||||||
|
const cookies = await begin();
|
||||||
|
await post(cookies, { action: 'request', email: `stranger-${i}@example.com` }, from);
|
||||||
|
}
|
||||||
|
expect(sent).toHaveLength(3);
|
||||||
|
|
||||||
|
// Somebody else is not caught by it.
|
||||||
|
const other = await begin();
|
||||||
|
await post(
|
||||||
|
other,
|
||||||
|
{ action: 'request', email: 'neighbour@example.com' },
|
||||||
|
{ 'cf-connecting-ip': '198.51.100.21' }
|
||||||
|
);
|
||||||
|
expect(sent).toHaveLength(4);
|
||||||
|
});
|
||||||
|
|
||||||
test('a bad address still gets told it is a bad address', async () => {
|
test('a bad address still gets told it is a bad address', async () => {
|
||||||
const cookies = await begin();
|
const cookies = await begin();
|
||||||
const response = await post(cookies, { action: 'request', email: 'not-an-address' });
|
const response = await post(cookies, { action: 'request', email: 'not-an-address' });
|
||||||
|
|||||||
Reference in New Issue
Block a user