fix(api): accept Pireon webhooks from the password client too
Pireon signs each webhook with the secret of the client that registered it, and the API has two clients (the web app's and the password grant's). Only PIREON_WEBHOOK_SECRET was accepted, so a token revoked for someone signed in with a password went unheard. A delivery now verifies with PIREON_WEBHOOK_SECRET or PIREON_PASSWORD_WEBHOOK_SECRET; the same event arriving from both clients changes nothing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,48 @@
|
||||
import { UnauthorizedException } from '@nestjs/common';
|
||||
import { ConfigService } from '@nestjs/config';
|
||||
import { IdentityService } from '@task-stream/api/identity/application';
|
||||
import { createHmac } from 'node:crypto';
|
||||
import { PireonWebhookController } from './pireon-webhook.controller';
|
||||
|
||||
describe('PireonWebhookController', () => {
|
||||
const handled: string[] = [];
|
||||
const controller = (secrets: Record<string, string>) =>
|
||||
new PireonWebhookController(
|
||||
{
|
||||
handleWebhook: async (event: string) => {
|
||||
handled.push(event);
|
||||
},
|
||||
} as unknown as IdentityService,
|
||||
{ get: (key: string) => secrets[key] } as unknown as ConfigService,
|
||||
);
|
||||
const body = { event: 'token.revoked', data: { token_id: 't-1' } };
|
||||
const raw = Buffer.from(JSON.stringify(body));
|
||||
const signed = (secret: string) =>
|
||||
createHmac('sha256', secret).update(raw).digest('hex');
|
||||
const request = { rawBody: raw, body } as never;
|
||||
|
||||
beforeEach(() => (handled.length = 0));
|
||||
|
||||
it("accepts a delivery signed with either Pireon client's secret", async () => {
|
||||
const both = controller({
|
||||
PIREON_WEBHOOK_SECRET: 'web-secret',
|
||||
PIREON_PASSWORD_WEBHOOK_SECRET: 'password-secret',
|
||||
});
|
||||
await both.receive(request, signed('web-secret'));
|
||||
await both.receive(request, `sha256=${signed('password-secret')}`);
|
||||
expect(handled).toEqual(['token.revoked', 'token.revoked']);
|
||||
});
|
||||
|
||||
it('refuses any other signature, and everything when no secret is set', async () => {
|
||||
await expect(
|
||||
controller({ PIREON_WEBHOOK_SECRET: 'web-secret' }).receive(
|
||||
request,
|
||||
signed('password-secret'),
|
||||
),
|
||||
).rejects.toThrow(UnauthorizedException);
|
||||
await expect(
|
||||
controller({}).receive(request, signed('web-secret')),
|
||||
).rejects.toThrow(/disabled/);
|
||||
expect(handled).toEqual([]);
|
||||
});
|
||||
});
|
||||
@@ -21,7 +21,13 @@ import { Request } from 'express';
|
||||
import { createHmac, timingSafeEqual } from 'node:crypto';
|
||||
import { Public } from '../auth/auth.decorators';
|
||||
|
||||
/** Receives `user.deactivated` / `token.revoked` from Pireon. */
|
||||
/**
|
||||
* Receives `user.deactivated` / `token.revoked` from Pireon.
|
||||
*
|
||||
* Pireon signs each webhook with the secret of the client that registered
|
||||
* it, and this API has two (the web app's and the password grant's): a
|
||||
* delivery is good if it verifies with either.
|
||||
*/
|
||||
@ApiTags('Webhooks')
|
||||
@Controller('webhooks/pireon')
|
||||
export class PireonWebhookController {
|
||||
@@ -37,11 +43,12 @@ export class PireonWebhookController {
|
||||
summary: 'Pireon events (called by Pireon, not by apps)',
|
||||
description:
|
||||
"`user.deactivated` closes the user's sessions; `token.revoked` forgets the token. " +
|
||||
'Disabled unless PIREON_WEBHOOK_SECRET is set.',
|
||||
'Disabled unless PIREON_WEBHOOK_SECRET or PIREON_PASSWORD_WEBHOOK_SECRET is set.',
|
||||
})
|
||||
@ApiHeader({
|
||||
name: 'X-Webhook-Signature',
|
||||
description: 'HMAC-SHA256 of the raw body with PIREON_WEBHOOK_SECRET (hex)',
|
||||
description:
|
||||
'HMAC-SHA256 of the raw body with the secret of the Pireon client that registered the webhook (hex)',
|
||||
required: true,
|
||||
})
|
||||
@ApiNoContentResponse({ description: 'Processed' })
|
||||
@@ -63,13 +70,16 @@ export class PireonWebhookController {
|
||||
await this.identity.handleWebhook(event, data);
|
||||
}
|
||||
|
||||
/** X-Webhook-Signature = HMAC-SHA256(raw body, PIREON_WEBHOOK_SECRET). */
|
||||
/** X-Webhook-Signature = HMAC-SHA256(raw body, the client's webhook secret). */
|
||||
private verifySignature(
|
||||
rawBody: Buffer | undefined,
|
||||
signature: string | undefined,
|
||||
): void {
|
||||
const secret = this.config.get<string>('PIREON_WEBHOOK_SECRET');
|
||||
if (!secret) {
|
||||
const secrets = [
|
||||
this.config.get<string>('PIREON_WEBHOOK_SECRET'),
|
||||
this.config.get<string>('PIREON_PASSWORD_WEBHOOK_SECRET'),
|
||||
].filter((secret): secret is string => !!secret);
|
||||
if (!secrets.length) {
|
||||
throw new UnauthorizedException(
|
||||
'Webhooks are disabled (PIREON_WEBHOOK_SECRET not set)',
|
||||
);
|
||||
@@ -77,11 +87,16 @@ export class PireonWebhookController {
|
||||
if (!rawBody || !signature)
|
||||
throw new UnauthorizedException('Missing signature');
|
||||
|
||||
const expected = createHmac('sha256', secret).update(rawBody).digest('hex');
|
||||
const received = signature.replace(/^sha256=/, '');
|
||||
const valid =
|
||||
received.length === expected.length &&
|
||||
timingSafeEqual(Buffer.from(received), Buffer.from(expected));
|
||||
const received = Buffer.from(signature.replace(/^sha256=/, ''));
|
||||
const valid = secrets.some((secret) => {
|
||||
const expected = Buffer.from(
|
||||
createHmac('sha256', secret).update(rawBody).digest('hex'),
|
||||
);
|
||||
return (
|
||||
received.length === expected.length &&
|
||||
timingSafeEqual(received, expected)
|
||||
);
|
||||
});
|
||||
if (!valid) throw new UnauthorizedException('Invalid signature');
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user