From 562fde6eaab53dcf0fae5321f5b901c2a6c8d9ad Mon Sep 17 00:00:00 2001 From: Liam Phung Date: Fri, 16 Oct 2026 19:09:57 +0700 Subject: [PATCH] feat(webhooks): support multiple webhook endpoints per developer with ID-based deletion (#1241) --- src/routes/webhooks.test.ts | 80 ++++++++++ src/routes/webhooks.ts | 245 ++++++++++++++++++++++++++--- src/webhooks/webhook.store.test.ts | 52 ++++++ src/webhooks/webhook.store.ts | 117 ++++++++++++-- src/webhooks/webhook.types.ts | 1 + 5 files changed, 456 insertions(+), 39 deletions(-) diff --git a/src/routes/webhooks.test.ts b/src/routes/webhooks.test.ts index b5ea7402..72634567 100644 --- a/src/routes/webhooks.test.ts +++ b/src/routes/webhooks.test.ts @@ -420,4 +420,84 @@ describe('Webhook management routes — Authentication & Authorization (Issue #1 assert.equal(res.body.developerId, 'dev-bob'); }); }); + + describe('Multiple webhook endpoints per developer (Issue #1241)', () => { + it('registers two different URLs with independent secrets/retry policies, supports legacy GET, and deletes one without affecting the other', async () => { + const reg1 = await request(app) + .post('/api/webhooks') + .set(authHeader('dev-multi-route')) + .send({ + developerId: 'dev-multi-route', + url: 'https://example.com/finance-hook', + events: ['settlement_completed', 'new_api_call'], + secret: 'finance-secret-key-at-least-32-chars-long', + retryPolicy: { maxRetries: 5, baseDelayMs: 2000 }, + }); + assert.equal(reg1.status, 201); + const id1 = reg1.body.id; + assert.ok(id1); + + const reg2 = await request(app) + .post('/api/webhooks') + .set(authHeader('dev-multi-route')) + .send({ + developerId: 'dev-multi-route', + url: 'https://example.com/monitoring-hook', + events: ['new_api_call', 'low_balance_alert'], + secret: 'monitoring-secret-key-at-least-32-chars', + retryPolicy: { maxRetries: 2, baseDelayMs: 500 }, + }); + assert.equal(reg2.status, 201); + const id2 = reg2.body.id; + assert.ok(id2); + assert.notEqual(id1, id2); + + // Both registrations receive matching events via getByEvent + const matching = WebhookStore.getByEvent('new_api_call').filter( + (c) => c.developerId === 'dev-multi-route', + ); + assert.equal(matching.length, 2); + + // Legacy GET /api/webhooks/:developerId returns the first subscription + const legacyGet = await request(app) + .get('/api/webhooks/dev-multi-route') + .set(authHeader('dev-multi-route')); + assert.equal(legacyGet.status, 200); + assert.equal(legacyGet.body.id, id1); + assert.equal(legacyGet.body.url, 'https://example.com/finance-hook'); + + // Subscription-specific GET and PATCH /api/webhooks/:developerId/subscriptions/:id + const sub2Get = await request(app) + .get(`/api/webhooks/dev-multi-route/subscriptions/${id2}`) + .set(authHeader('dev-multi-route')); + assert.equal(sub2Get.status, 200); + assert.equal(sub2Get.body.url, 'https://example.com/monitoring-hook'); + assert.deepEqual(sub2Get.body.retryPolicy, { maxRetries: 2, baseDelayMs: 500 }); + + const sub2Patch = await request(app) + .patch(`/api/webhooks/dev-multi-route/subscriptions/${id2}/retry-policy`) + .set(authHeader('dev-multi-route')) + .send({ retryPolicy: { maxRetries: 4, baseDelayMs: 1500 } }); + assert.equal(sub2Patch.status, 200); + assert.deepEqual(sub2Patch.body.retryPolicy, { maxRetries: 4, baseDelayMs: 1500 }); + // First subscription retryPolicy is untouched + assert.deepEqual(WebhookStore.get('dev-multi-route', id1)?.retryPolicy, { + maxRetries: 5, + baseDelayMs: 2000, + }); + + // Deleting one subscription leaves the other intact + const del1 = await request(app) + .delete(`/api/webhooks/dev-multi-route/subscriptions/${id1}`) + .set(authHeader('dev-multi-route')); + assert.equal(del1.status, 200); + + const listRes = await request(app) + .get('/api/webhooks/dev-multi-route/subscriptions') + .set(authHeader('dev-multi-route')); + assert.equal(listRes.status, 200); + assert.equal(listRes.body.subscriptions.length, 1); + assert.equal(listRes.body.subscriptions[0].id, id2); + }); + }); }); \ No newline at end of file diff --git a/src/routes/webhooks.ts b/src/routes/webhooks.ts index 582abfd4..81277d00 100644 --- a/src/routes/webhooks.ts +++ b/src/routes/webhooks.ts @@ -219,9 +219,15 @@ router.post('/', webhookMgmtRateLimit, express.json(), async (req: Request, res: throw new AppError('URL validation failed.', 500, 'WEBHOOK_URL_VALIDATION_FAILED'); } - const before = WebhookStore.get(developerId) ? sanitizeConfig(WebhookStore.get(developerId) as unknown as Record) : null; - - WebhookStore.register({ + const existingForUrl = req.body?.id + ? WebhookStore.get(developerId, String(req.body.id)) + : WebhookStore.getByUrl(developerId, url); + const before = existingForUrl + ? sanitizeConfig(existingForUrl as unknown as Record) + : null; + + const registered = WebhookStore.register({ + id: typeof req.body?.id === 'string' ? req.body.id : undefined, developerId, url, events: events as WebhookEventType[], @@ -230,11 +236,13 @@ router.post('/', webhookMgmtRateLimit, express.json(), async (req: Request, res: createdAt: new Date(), }); - const after = sanitizeConfig(WebhookStore.get(developerId) as unknown as Record); + const after = sanitizeConfig(registered as unknown as Record); await auditStateChange(req, res, 'WEBHOOK_REGISTERED', before, after); res.status(201).json({ + id: registered.id, + subscriptionId: registered.id, message: 'Webhook registered successfully.', developerId, url, @@ -245,7 +253,205 @@ router.post('/', webhookMgmtRateLimit, express.json(), async (req: Request, res: } }); -// GET /api/webhooks/:developerId — Get webhook config +function toSafeWebhookConfig(config: Record & { secret?: unknown; secret_current?: unknown; secret_previous?: unknown }) { + const { + secret: _s, + secret_current: _sc, + secret_previous: _sp, + ...safeConfig + } = config; + return safeConfig; +} + +// GET /api/webhooks/:developerId/subscriptions — List all webhook subscriptions for a developer +router.get('/:developerId/subscriptions', webhookMgmtRateLimit, (req: Request, res: Response, next: NextFunction) => { + try { + const { developerId } = req.params; + if (!isAuthorizedForDeveloper(res, developerId)) { + throw new ForbiddenError( + 'Cannot view webhook for another developer.', + 'FORBIDDEN' + ); + } + + const subscriptions = WebhookStore.listByDeveloper(developerId).map((cfg) => + toSafeWebhookConfig(cfg as unknown as Record), + ); + return res.json({ developerId, subscriptions }); + } catch (error) { + next(error); + } +}); + +// GET /api/webhooks/:developerId/subscriptions/:id — Get a specific webhook subscription +router.get('/:developerId/subscriptions/:id', webhookMgmtRateLimit, (req: Request, res: Response, next: NextFunction) => { + try { + const { developerId, id } = req.params; + if (!isAuthorizedForDeveloper(res, developerId)) { + throw new ForbiddenError( + 'Cannot view webhook for another developer.', + 'FORBIDDEN' + ); + } + + const subscription = WebhookStore.get(developerId, id); + if (!subscription) { + throw new NotFoundError( + 'No webhook subscription found with this id.', + 'WEBHOOK_NOT_FOUND' + ); + } + + return res.json(toSafeWebhookConfig(subscription as unknown as Record)); + } catch (error) { + next(error); + } +}); + +// POST /api/webhooks/:developerId/subscriptions/:id/rotate-secret — Rotate secret for specific subscription +router.post('/:developerId/subscriptions/:id/rotate-secret', webhookMgmtRateLimit, (req: Request, res: Response, next: NextFunction) => { + try { + const { developerId, id } = req.params; + if (!isAuthorizedForDeveloper(res, developerId)) { + throw new ForbiddenError( + 'Cannot rotate secret for another developer.', + 'FORBIDDEN' + ); + } + + const existing = WebhookStore.get(developerId, id); + if (!existing) { + throw new NotFoundError( + 'No webhook subscription found with this id.', + 'WEBHOOK_NOT_FOUND' + ); + } + + const before = sanitizeConfig(existing as unknown as Record); + const newSecret = generateWebhookSecret(); + const previousExpiresAt = new Date(Date.now() + config.webhooks.secretRotationGraceMs); + const rotated = WebhookStore.rotateSecret(developerId, newSecret, previousExpiresAt, id); + + if (!rotated) { + throw new NotFoundError( + 'No webhook subscription found with this id.', + 'WEBHOOK_NOT_FOUND' + ); + } + + const after = sanitizeConfig(rotated as unknown as Record); + const actor = getAuthenticatedActor(res) ?? developerId; + + logger.audit('WEBHOOK_SECRET_ROTATED', actor, { + developerId, + subscriptionId: id, + actor, + previousExpiresAt: rotated.previous_expires_at?.toISOString(), + hadPreviousSecret: Boolean(existing.secret_current ?? existing.secret), + }); + + void auditStateChange(req, res, 'WEBHOOK_SECRET_ROTATED', before, after); + + return res.status(200).json({ + message: 'Webhook secret rotated successfully.', + id: rotated.id, + developerId, + secret: newSecret, + previous_expires_at: rotated.previous_expires_at?.toISOString(), + }); + } catch (error) { + next(error); + } +}); + +// PATCH /api/webhooks/:developerId/subscriptions/:id/retry-policy — Update retry policy for specific subscription +router.patch('/:developerId/subscriptions/:id/retry-policy', webhookMgmtRateLimit, express.json(), (req: Request, res: Response, next: NextFunction) => { + try { + const { developerId, id } = req.params; + if (!isAuthorizedForDeveloper(res, developerId)) { + throw new ForbiddenError( + 'Cannot update retry policy for another developer.', + 'FORBIDDEN' + ); + } + + const { retryPolicy } = req.body; + const validation = validateRetryPolicy(retryPolicy); + if (!validation.valid) { + throw new BadRequestError( + validation.error!, + 'INVALID_RETRY_POLICY' + ); + } + + const existing = WebhookStore.get(developerId, id); + const before = existing ? { retryPolicy: (existing as unknown as Record).retryPolicy } : null; + + const updated = WebhookStore.updateRetryPolicy( + developerId, + retryPolicy as RetryPolicy | undefined, + id, + ); + + if (!updated) { + throw new NotFoundError( + 'No webhook subscription found with this id.', + 'WEBHOOK_NOT_FOUND' + ); + } + + const after = { retryPolicy: updated.retryPolicy }; + const actor = getAuthenticatedActor(res) ?? developerId; + + logger.audit('WEBHOOK_RETRY_POLICY_UPDATED', actor, { + developerId, + subscriptionId: id, + actor, + retryPolicy: updated.retryPolicy, + }); + + void auditStateChange(req, res, 'WEBHOOK_RETRY_POLICY_UPDATED', before, after); + + return res.status(200).json({ + message: 'Webhook retry policy updated successfully.', + ...toSafeWebhookConfig(updated as unknown as Record), + }); + } catch (error) { + next(error); + } +}); + +// DELETE /api/webhooks/:developerId/subscriptions/:id — Remove a specific subscription +router.delete('/:developerId/subscriptions/:id', webhookMgmtRateLimit, async (req: Request, res: Response, next: NextFunction) => { + try { + const { developerId, id } = req.params; + if (!isAuthorizedForDeveloper(res, developerId)) { + throw new ForbiddenError( + 'Cannot delete webhook for another developer.', + 'FORBIDDEN' + ); + } + + const existing = WebhookStore.get(developerId, id); + if (!existing) { + throw new NotFoundError( + 'No webhook subscription found with this id.', + 'WEBHOOK_NOT_FOUND' + ); + } + + const before = sanitizeConfig(existing as unknown as Record); + WebhookStore.delete(developerId, id); + + await auditStateChange(req, res, 'WEBHOOK_DELETED', before, null); + + return res.json({ message: 'Webhook subscription removed.', id }); + } catch (error) { + next(error); + } +}); + +// GET /api/webhooks/:developerId — Legacy alias for the first webhook subscription router.get('/:developerId', webhookMgmtRateLimit, (req: Request, res: Response, next: NextFunction) => { try { const { developerId } = req.params; @@ -263,13 +469,7 @@ router.get('/:developerId', webhookMgmtRateLimit, (req: Request, res: Response, 'WEBHOOK_NOT_FOUND' ); } - const { - secret: _s, - secret_current: _sc, - secret_previous: _sp, - ...safeConfig - } = config; - return res.json(safeConfig); + return res.json(toSafeWebhookConfig(config as unknown as Record)); } catch (error) { next(error); } @@ -401,16 +601,9 @@ router.patch('/:developerId/retry-policy', webhookMgmtRateLimit, express.json(), void auditStateChange(req, res, 'WEBHOOK_RETRY_POLICY_UPDATED', before, after); - const { - secret: _s, - secret_current: _sc, - secret_previous: _sp, - ...safeConfig - } = updated; - return res.status(200).json({ message: 'Webhook retry policy updated successfully.', - ...safeConfig, + ...toSafeWebhookConfig(updated as unknown as Record), }); } catch (error) { next(error); @@ -421,15 +614,21 @@ router.post( '/deliver/:developerId', captureRawBody, (req: Request & { webhookSecrets?: string[] }, res: Response, next) => { - const config = WebhookStore.get(req.params.developerId); - if (!config) { + const subscriptions = WebhookStore.listByDeveloper(req.params.developerId); + if (subscriptions.length === 0) { next(new NotFoundError( 'No webhook registered for this developer.', 'WEBHOOK_NOT_FOUND' )); return; } - req.webhookSecrets = WebhookStore.getActiveSecrets(config); + const activeSecrets = new Set(); + for (const sub of subscriptions) { + for (const secret of WebhookStore.getActiveSecrets(sub)) { + activeSecrets.add(secret); + } + } + req.webhookSecrets = [...activeSecrets]; next(); }, verifyWebhookSignature, diff --git a/src/webhooks/webhook.store.test.ts b/src/webhooks/webhook.store.test.ts index b3b1452d..e1670766 100644 --- a/src/webhooks/webhook.store.test.ts +++ b/src/webhooks/webhook.store.test.ts @@ -78,4 +78,56 @@ describe('WebhookStore rotation + deletion', () => { ['s2', 's1'], ); }); + + it('supports multiple webhook subscriptions per developer with independent secrets, retry policies, and deletion (#1241)', () => { + const first = WebhookStore.register(baseConfig({ + developerId: 'dev-multi', + url: 'https://finance.example.com/webhook', + events: ['settlement_completed', 'new_api_call'], + secret_current: 'finance-secret', + retryPolicy: { maxRetries: 5, baseDelayMs: 2000 }, + })); + + const second = WebhookStore.register(baseConfig({ + developerId: 'dev-multi', + url: 'https://monitoring.example.com/webhook', + events: ['new_api_call', 'low_balance_alert'], + secret_current: 'monitoring-secret', + retryPolicy: { maxRetries: 2, baseDelayMs: 500 }, + })); + + assert.ok(first.id); + assert.ok(second.id); + assert.notEqual(first.id, second.id); + + // Both subscriptions receive matching event 'new_api_call' + const matchingNewCall = WebhookStore.getByEvent('new_api_call'); + assert.equal(matchingNewCall.length, 2); + assert.deepEqual( + matchingNewCall.map((c) => c.url), + ['https://finance.example.com/webhook', 'https://monitoring.example.com/webhook'], + ); + + // Legacy get(developerId) returns the first subscription + const legacyFirst = WebhookStore.get('dev-multi'); + assert.equal(legacyFirst?.id, first.id); + assert.equal(legacyFirst?.url, 'https://finance.example.com/webhook'); + + // Each subscription maintains its own secret and retry policy + const sub1 = WebhookStore.get('dev-multi', first.id); + const sub2 = WebhookStore.get('dev-multi', second.id); + assert.equal(sub1?.secret_current, 'finance-secret'); + assert.deepEqual(sub1?.retryPolicy, { maxRetries: 5, baseDelayMs: 2000 }); + assert.equal(sub2?.secret_current, 'monitoring-secret'); + assert.deepEqual(sub2?.retryPolicy, { maxRetries: 2, baseDelayMs: 500 }); + + // Deleting one subscription leaves the other intact + const deleted = WebhookStore.delete('dev-multi', first.id); + assert.equal(deleted, true); + assert.equal(WebhookStore.get('dev-multi', first.id), undefined); + const remaining = WebhookStore.listByDeveloper('dev-multi'); + assert.equal(remaining.length, 1); + assert.equal(remaining[0]?.id, second.id); + assert.equal(WebhookStore.get('dev-multi')?.id, second.id); + }); }); diff --git a/src/webhooks/webhook.store.ts b/src/webhooks/webhook.store.ts index 44c33aaf..60769981 100644 --- a/src/webhooks/webhook.store.ts +++ b/src/webhooks/webhook.store.ts @@ -1,7 +1,8 @@ +import crypto from 'crypto'; import { WebhookConfig, WebhookEventType, DeadLetterEntry, type RetryPolicy } from './webhook.types.js'; import { WebhookNonceStore } from './webhook.nonceStore.js'; -const store = new Map(); +const store = new Map(); const deadLetterStore = new Map(); /** @@ -34,36 +35,79 @@ const MAX_FAILED_LOG = 200; // keep 2× the read limit for ring-buffer headroom function normalizeConfig(config: WebhookConfig): WebhookConfig { const secret_current = config.secret_current ?? config.secret; + const id = config.id ?? crypto.randomUUID(); return { ...config, + id, secret: secret_current, secret_current, }; } export const WebhookStore = { - register(config: WebhookConfig): void { - store.set(config.developerId, normalizeConfig(config)); + register(config: WebhookConfig): WebhookConfig { + const normalized = normalizeConfig(config); + const existing = store.get(config.developerId) ?? []; + const matchIndex = existing.findIndex( + (item) => (config.id && item.id === config.id) || item.url === config.url, + ); + + if (matchIndex >= 0) { + const preservedId = config.id ?? existing[matchIndex]!.id; + const updated = { ...normalized, id: preservedId }; + const nextList = [...existing]; + nextList[matchIndex] = updated; + store.set(config.developerId, nextList); + return updated; + } + + store.set(config.developerId, [...existing, normalized]); + return normalized; + }, + + get(developerId: string, subscriptionId?: string): WebhookConfig | undefined { + const list = store.get(developerId); + if (!list || list.length === 0) return undefined; + if (subscriptionId !== undefined) { + return list.find((cfg) => cfg.id === subscriptionId); + } + return list[0]; }, - get(developerId: string): WebhookConfig | undefined { - return store.get(developerId); + getByUrl(developerId: string, url: string): WebhookConfig | undefined { + const list = store.get(developerId); + if (!list) return undefined; + return list.find((cfg) => cfg.url === url); + }, + + listByDeveloper(developerId: string): WebhookConfig[] { + return [...(store.get(developerId) ?? [])]; }, updateRetryPolicy( developerId: string, retryPolicy: RetryPolicy | undefined, + subscriptionId?: string, ): WebhookConfig | undefined { - const currentConfig = store.get(developerId); - if (!currentConfig) return undefined; + const list = store.get(developerId); + if (!list || list.length === 0) return undefined; + + const targetIndex = + subscriptionId !== undefined + ? list.findIndex((cfg) => cfg.id === subscriptionId) + : 0; + if (targetIndex < 0 || !list[targetIndex]) return undefined; + const currentConfig = list[targetIndex]!; const nextConfig = normalizeConfig({ ...currentConfig, retryPolicy, }); - store.set(developerId, nextConfig); + const nextList = [...list]; + nextList[targetIndex] = nextConfig; + store.set(developerId, nextList); return nextConfig; }, @@ -71,10 +115,18 @@ export const WebhookStore = { developerId: string, newSecret: string, previousExpiresAt: Date, + subscriptionId?: string, ): WebhookConfig | undefined { - const currentConfig = store.get(developerId); - if (!currentConfig) return undefined; + const list = store.get(developerId); + if (!list || list.length === 0) return undefined; + const targetIndex = + subscriptionId !== undefined + ? list.findIndex((cfg) => cfg.id === subscriptionId) + : 0; + if (targetIndex < 0 || !list[targetIndex]) return undefined; + + const currentConfig = list[targetIndex]!; const currentSecret = currentConfig.secret_current ?? currentConfig.secret; const nextConfig = normalizeConfig({ ...currentConfig, @@ -84,7 +136,9 @@ export const WebhookStore = { previous_expires_at: currentSecret ? previousExpiresAt : undefined, }); - store.set(developerId, nextConfig); + const nextList = [...list]; + nextList[targetIndex] = nextConfig; + store.set(developerId, nextList); return nextConfig; }, @@ -129,17 +183,48 @@ export const WebhookStore = { return [...secrets]; }, - delete(developerId: string): void { - store.delete(developerId); - WebhookNonceStore.purgeScope(developerId); + delete(developerId: string, subscriptionId?: string): boolean { + if (subscriptionId === undefined) { + const existed = store.delete(developerId); + WebhookNonceStore.purgeScope(developerId); + return existed; + } + + const list = store.get(developerId); + if (!list || list.length === 0) return false; + + const nextList = list.filter((cfg) => cfg.id !== subscriptionId); + if (nextList.length === list.length) { + return false; + } + + if (nextList.length === 0) { + store.delete(developerId); + WebhookNonceStore.purgeScope(developerId); + } else { + store.set(developerId, nextList); + } + return true; }, getByEvent(event: WebhookEventType): WebhookConfig[] { - return [...store.values()].filter((cfg) => cfg.events.includes(event)); + const matching: WebhookConfig[] = []; + for (const list of store.values()) { + for (const cfg of list) { + if (cfg.events.includes(event)) { + matching.push(cfg); + } + } + } + return matching; }, list(): WebhookConfig[] { - return [...store.values()]; + const all: WebhookConfig[] = []; + for (const list of store.values()) { + all.push(...list); + } + return all; }, /** Clear all webhook configurations - for testing only */ diff --git a/src/webhooks/webhook.types.ts b/src/webhooks/webhook.types.ts index 174f4c20..f25a70dc 100644 --- a/src/webhooks/webhook.types.ts +++ b/src/webhooks/webhook.types.ts @@ -16,6 +16,7 @@ export type WebhookEventType = // docs/webhook-retry-override.md). export interface WebhookConfig { + id?: string; developerId: string; url: string; events: string[];