feat(intake): sign the two actions an email may take (#224)
Signed over the item, the action and the expiry together. Signing any subset would let a link be replayed against a different item or upgraded to a different action, and leaving the expiry out of the payload would let anyone holding an expired link extend it by editing the timestamp in the URL. Each of those is a test. Compared through a second digest rather than directly, because timingSafeEqual throws when the buffers differ in length, and a truncated link is an ordinary thing to receive rather than an exception. Same idiom as the admin gate. actionUrl returns null rather than throwing when there is no secret or no PUBLIC_URL. An unconfigured environment still sends the notification with its review link — being told an item arrived matters far more than the shortcuts do — and a link that could not be verified must never be offered in the first place. Uses the shared trimTrailingSlashes rather than a trailing-slash regex, which is what utils.ts exports it for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,75 @@
|
|||||||
|
import crypto from 'crypto';
|
||||||
|
import { trimTrailingSlashes } from '../utils';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Links in the notification email that act without a login.
|
||||||
|
*
|
||||||
|
* Only two actions are signable, and neither can publish. The worst case of a
|
||||||
|
* leaked link is a wasted API call or a hide the review queue can undo — which
|
||||||
|
* is what makes it acceptable to put them in an inbox at all.
|
||||||
|
*
|
||||||
|
* Signed over the item, the action and the expiry together. Signing any subset
|
||||||
|
* would let a link be replayed against a different item or upgraded to a
|
||||||
|
* different action, and leaving the expiry out of the payload would let anyone
|
||||||
|
* holding an expired link extend it by editing the timestamp in the URL.
|
||||||
|
*/
|
||||||
|
export type IntakeAction = 'regenerate' | 'discard';
|
||||||
|
|
||||||
|
/** Thirty days. Long enough to survive a holiday, short enough to lapse. */
|
||||||
|
export const ACTION_TTL_MS = 30 * 24 * 60 * 60 * 1000;
|
||||||
|
|
||||||
|
function secret(): string | null {
|
||||||
|
const value = process.env.INTAKE_ACTION_SECRET;
|
||||||
|
return value && value.trim() !== '' ? value : null;
|
||||||
|
}
|
||||||
|
|
||||||
|
export function signAction(itemId: number, action: IntakeAction, expiresAt: number): string {
|
||||||
|
const key = secret();
|
||||||
|
if (!key) throw new Error('INTAKE_ACTION_SECRET is not set');
|
||||||
|
return crypto
|
||||||
|
.createHmac('sha256', key)
|
||||||
|
.update(`${itemId}:${action}:${expiresAt}`)
|
||||||
|
.digest('base64url');
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Compared through a second digest rather than directly, because
|
||||||
|
* timingSafeEqual throws when the two buffers differ in length — and a
|
||||||
|
* malformed signature from a truncated link is an ordinary thing to receive
|
||||||
|
* rather than an exception. Same idiom as middleware/adminGate.ts.
|
||||||
|
*/
|
||||||
|
function digest(value: string): Buffer {
|
||||||
|
return crypto.createHash('sha256').update(value).digest();
|
||||||
|
}
|
||||||
|
|
||||||
|
export function verifyAction(
|
||||||
|
itemId: number,
|
||||||
|
action: IntakeAction,
|
||||||
|
expiresAt: number,
|
||||||
|
signature: string,
|
||||||
|
now: number = Date.now()
|
||||||
|
): boolean {
|
||||||
|
if (!secret()) return false;
|
||||||
|
if (!Number.isFinite(expiresAt) || now > expiresAt) return false;
|
||||||
|
|
||||||
|
const expected = signAction(itemId, action, expiresAt);
|
||||||
|
return crypto.timingSafeEqual(digest(expected), digest(signature));
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The absolute link, or null when one cannot be made.
|
||||||
|
*
|
||||||
|
* Null rather than a throw or a relative path. An unconfigured environment
|
||||||
|
* still sends the notification with its review link — being told an item
|
||||||
|
* arrived matters far more than the shortcuts — and a link that could not be
|
||||||
|
* verified must never be offered in the first place.
|
||||||
|
*/
|
||||||
|
export function actionUrl(itemId: number, action: IntakeAction): string | null {
|
||||||
|
const base = process.env.PUBLIC_URL;
|
||||||
|
if (!secret() || !base || base.trim() === '') return null;
|
||||||
|
|
||||||
|
const expiresAt = Date.now() + ACTION_TTL_MS;
|
||||||
|
const sig = signAction(itemId, action, expiresAt);
|
||||||
|
const origin = trimTrailingSlashes(base);
|
||||||
|
return `${origin}/api/intake-actions/${itemId}/${action}?expires=${expiresAt}&sig=${sig}`;
|
||||||
|
}
|
||||||
@@ -0,0 +1,85 @@
|
|||||||
|
import { signAction, verifyAction, actionUrl, ACTION_TTL_MS } from '../../src/intake/actionLinks';
|
||||||
|
|
||||||
|
const SECRET = 'test-intake-secret';
|
||||||
|
const NOW = 1_800_000_000_000;
|
||||||
|
const EXPIRY = NOW + ACTION_TTL_MS;
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
process.env.INTAKE_ACTION_SECRET = SECRET;
|
||||||
|
process.env.PUBLIC_URL = 'https://shop.example.com';
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('signAction / verifyAction', () => {
|
||||||
|
it('accepts a signature it produced', () => {
|
||||||
|
const sig = signAction(7, 'discard', EXPIRY);
|
||||||
|
expect(verifyAction(7, 'discard', EXPIRY, sig, NOW)).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
// Each of these is a different link. A signature that survives any of these
|
||||||
|
// swaps is a signature that authorises more than it names.
|
||||||
|
it('refuses a signature reused for another item', () => {
|
||||||
|
const sig = signAction(7, 'discard', EXPIRY);
|
||||||
|
expect(verifyAction(8, 'discard', EXPIRY, sig, NOW)).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('refuses a signature reused for another action', () => {
|
||||||
|
const sig = signAction(7, 'discard', EXPIRY);
|
||||||
|
expect(verifyAction(7, 'regenerate', EXPIRY, sig, NOW)).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
// Otherwise the expiry is decoration: anyone holding an expired link could
|
||||||
|
// extend it themselves by editing the timestamp.
|
||||||
|
it('refuses a signature whose expiry was altered', () => {
|
||||||
|
const sig = signAction(7, 'discard', EXPIRY);
|
||||||
|
expect(verifyAction(7, 'discard', EXPIRY + 1000, sig, NOW)).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('refuses an expired link even with a valid signature', () => {
|
||||||
|
const sig = signAction(7, 'discard', EXPIRY);
|
||||||
|
expect(verifyAction(7, 'discard', EXPIRY, sig, EXPIRY + 1)).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('refuses a malformed signature without throwing', () => {
|
||||||
|
expect(verifyAction(7, 'discard', EXPIRY, 'not-a-signature', NOW)).toBe(false);
|
||||||
|
expect(verifyAction(7, 'discard', EXPIRY, '', NOW)).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
// This is what makes rotating the secret revoke every outstanding link.
|
||||||
|
it('refuses a signature made with a different secret', () => {
|
||||||
|
const sig = signAction(7, 'discard', EXPIRY);
|
||||||
|
process.env.INTAKE_ACTION_SECRET = 'a-different-secret';
|
||||||
|
expect(verifyAction(7, 'discard', EXPIRY, sig, NOW)).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('refuses everything when there is no secret at all', () => {
|
||||||
|
const sig = signAction(7, 'discard', EXPIRY);
|
||||||
|
delete process.env.INTAKE_ACTION_SECRET;
|
||||||
|
expect(verifyAction(7, 'discard', EXPIRY, sig, NOW)).toBe(false);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('actionUrl', () => {
|
||||||
|
it('builds an absolute url carrying the expiry and signature', () => {
|
||||||
|
const url = actionUrl(7, 'discard');
|
||||||
|
expect(url).toContain('https://shop.example.com/api/intake-actions/7/discard');
|
||||||
|
expect(url).toMatch(/expires=\d+/);
|
||||||
|
expect(url).toMatch(/sig=[A-Za-z0-9_-]+/);
|
||||||
|
});
|
||||||
|
|
||||||
|
// Absent secret is a working configuration: the notification still sends with
|
||||||
|
// its review link. A link that cannot be verified must never be offered.
|
||||||
|
it('returns null when there is no secret', () => {
|
||||||
|
delete process.env.INTAKE_ACTION_SECRET;
|
||||||
|
expect(actionUrl(7, 'discard')).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns null when there is no public url to build against', () => {
|
||||||
|
delete process.env.PUBLIC_URL;
|
||||||
|
expect(actionUrl(7, 'discard')).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not double the slash when PUBLIC_URL has a trailing one', () => {
|
||||||
|
process.env.PUBLIC_URL = 'https://shop.example.com/';
|
||||||
|
expect(actionUrl(7, 'discard')).toContain('https://shop.example.com/api/intake-actions/');
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user