From 0bfdaa9c02c761b3c24e83c57960e015c8796614 Mon Sep 17 00:00:00 2001 From: CamilleLegeron Date: Thu, 4 Jul 2024 14:17:10 +0200 Subject: [PATCH] Add authorization header in webhooks stored in secrets table (#941) Summary: Adding authorization header support for webhooks. Issue: https://github.com/gristlabs/grist-core/issues/827 --------- Co-authored-by: Florent --- app/client/ui/WebhookPage.ts | 17 ++++++++++++----- app/common/Triggers-ti.ts | 4 ++++ app/common/Triggers.ts | 4 ++++ app/gen-server/lib/HomeDBManager.ts | 24 +++++++++++++++++++++--- app/server/lib/DocApi.ts | 15 ++++++++------- app/server/lib/Triggers.ts | 12 ++++++++++-- static/locales/en.client.json | 3 ++- test/nbrowser/WebhookPage.ts | 28 +++++++++++++++++++++++++--- test/server/lib/DocApi.ts | 5 +++++ 9 files changed, 91 insertions(+), 21 deletions(-) diff --git a/app/client/ui/WebhookPage.ts b/app/client/ui/WebhookPage.ts index 9e263fc0..ff93061c 100644 --- a/app/client/ui/WebhookPage.ts +++ b/app/client/ui/WebhookPage.ts @@ -107,6 +107,12 @@ const WEBHOOK_COLUMNS = [ type: 'Text', label: t('Status'), }, + { + id: VirtualId(), + colId: 'authorization', + type: 'Text', + label: t('Header Authorization'), + }, ] as const; /** @@ -114,10 +120,11 @@ const WEBHOOK_COLUMNS = [ */ const WEBHOOK_VIEW_FIELDS: Array<(typeof WEBHOOK_COLUMNS)[number]['colId']> = [ 'name', 'memo', - 'eventTypes', 'url', - 'tableId', 'isReadyColumn', - 'watchedColIdsText', 'webhookId', - 'enabled', 'status' + 'eventTypes', 'tableId', + 'watchedColIdsText', 'isReadyColumn', + 'url', 'authorization', + 'webhookId', 'enabled', + 'status' ]; /** @@ -136,7 +143,7 @@ class WebhookExternalTable implements IExternalTable { public name = 'GristHidden_WebhookTable'; public initialActions = _prepareWebhookInitialActions(this.name); public saveableFields = [ - 'tableId', 'watchedColIdsText', 'url', 'eventTypes', 'enabled', 'name', 'memo', 'isReadyColumn', + 'tableId', 'watchedColIdsText', 'url', 'authorization', 'eventTypes', 'enabled', 'name', 'memo', 'isReadyColumn', ]; public webhooks: ObservableArray = observableArray([]); diff --git a/app/common/Triggers-ti.ts b/app/common/Triggers-ti.ts index bb04bbae..f93d12ae 100644 --- a/app/common/Triggers-ti.ts +++ b/app/common/Triggers-ti.ts @@ -14,6 +14,7 @@ export const Webhook = t.iface([], { export const WebhookFields = t.iface([], { "url": "string", + "authorization": t.opt("string"), "eventTypes": t.array(t.union(t.lit("add"), t.lit("update"))), "tableId": "string", "watchedColIds": t.opt(t.array("string")), @@ -29,6 +30,7 @@ export const WebhookStatus = t.union(t.lit('idle'), t.lit('sending'), t.lit('ret export const WebhookSubscribe = t.iface([], { "url": "string", + "authorization": t.opt("string"), "eventTypes": t.array(t.union(t.lit("add"), t.lit("update"))), "watchedColIds": t.opt(t.array("string")), "enabled": t.opt("boolean"), @@ -45,6 +47,7 @@ export const WebhookSummary = t.iface([], { "id": "string", "fields": t.iface([], { "url": "string", + "authorization": t.opt("string"), "unsubscribeKey": "string", "eventTypes": t.array("string"), "isReadyColumn": t.union("string", "null"), @@ -64,6 +67,7 @@ export const WebhookUpdate = t.iface([], { export const WebhookPatch = t.iface([], { "url": t.opt("string"), + "authorization": t.opt("string"), "eventTypes": t.opt(t.array(t.union(t.lit("add"), t.lit("update")))), "tableId": t.opt("string"), "watchedColIds": t.opt(t.array("string")), diff --git a/app/common/Triggers.ts b/app/common/Triggers.ts index d3b492d6..a53dd1fe 100644 --- a/app/common/Triggers.ts +++ b/app/common/Triggers.ts @@ -8,6 +8,7 @@ export interface Webhook { export interface WebhookFields { url: string; + authorization?: string; eventTypes: Array<"add"|"update">; tableId: string; watchedColIds?: string[]; @@ -26,6 +27,7 @@ export type WebhookStatus = 'idle'|'sending'|'retrying'|'postponed'|'error'|'inv // tableId from the url) but generics are not yet supported by ts-interface-builder export interface WebhookSubscribe { url: string; + authorization?: string; eventTypes: Array<"add"|"update">; watchedColIds?: string[]; enabled?: boolean; @@ -42,6 +44,7 @@ export interface WebhookSummary { id: string; fields: { url: string; + authorization?: string; unsubscribeKey: string; eventTypes: string[]; isReadyColumn: string|null; @@ -64,6 +67,7 @@ export interface WebhookUpdate { // ts-interface-builder export interface WebhookPatch { url?: string; + authorization?: string; eventTypes?: Array<"add"|"update">; tableId?: string; watchedColIds?: string[]; diff --git a/app/gen-server/lib/HomeDBManager.ts b/app/gen-server/lib/HomeDBManager.ts index 06409d1c..a4f89721 100644 --- a/app/gen-server/lib/HomeDBManager.ts +++ b/app/gen-server/lib/HomeDBManager.ts @@ -1608,7 +1608,7 @@ export class HomeDBManager extends EventEmitter { .where("id = :id AND doc_id = :docId", {id, docId}) .execute(); if (res.affected !== 1) { - throw new ApiError('secret with given id not found', 404); + throw new ApiError('secret with given id not found or nothing was updated', 404); } } @@ -1623,14 +1623,32 @@ export class HomeDBManager extends EventEmitter { // Update the webhook url in the webhook's corresponding secret (note: the webhook identifier is // its secret identifier). - public async updateWebhookUrl(id: string, docId: string, url: string, outerManager?: EntityManager) { + public async updateWebhookUrlAndAuth( + props: { + id: string, + docId: string, + url: string | undefined, + auth: string | undefined, + outerManager?: EntityManager} + ) { + const {id, docId, url, auth, outerManager} = props; return await this._runInTransaction(outerManager, async manager => { + if (url === undefined && auth === undefined) { + throw new ApiError('None of the Webhook url and auth are defined', 404); + } const value = await this.getSecret(id, docId, manager); if (!value) { throw new ApiError('Webhook with given id not found', 404); } const webhookSecret = JSON.parse(value); - webhookSecret.url = url; + // As we want to patch the webhookSecret object, only set the url and the authorization when they are defined. + // When the user wants to empty the value, we are expected to receive empty strings. + if (url !== undefined) { + webhookSecret.url = url; + } + if (auth !== undefined) { + webhookSecret.authorization = auth; + } await this.updateSecret(id, docId, JSON.stringify(webhookSecret), manager); }); } diff --git a/app/server/lib/DocApi.ts b/app/server/lib/DocApi.ts index e5f66df4..297cafff 100644 --- a/app/server/lib/DocApi.ts +++ b/app/server/lib/DocApi.ts @@ -324,7 +324,7 @@ export class DocWorkerApi { ); const registerWebhook = async (activeDoc: ActiveDoc, req: RequestWithLogin, webhook: WebhookFields) => { - const {fields, url} = await getWebhookSettings(activeDoc, req, null, webhook); + const {fields, url, authorization} = await getWebhookSettings(activeDoc, req, null, webhook); if (!fields.eventTypes?.length) { throw new ApiError(`eventTypes must be a non-empty array`, 400); } @@ -336,7 +336,7 @@ export class DocWorkerApi { } const unsubscribeKey = uuidv4(); - const webhookSecret: WebHookSecret = {unsubscribeKey, url}; + const webhookSecret: WebHookSecret = {unsubscribeKey, url, authorization}; const secretValue = JSON.stringify(webhookSecret); const webhookId = (await this._dbManager.addSecret(secretValue, activeDoc.docName)).id; @@ -392,7 +392,7 @@ export class DocWorkerApi { const tablesTable = activeDoc.docData!.getMetaTable("_grist_Tables"); const trigger = webhookId ? activeDoc.triggers.getWebhookTriggerRecord(webhookId) : undefined; let currentTableId = trigger ? tablesTable.getValue(trigger.tableRef, 'tableId')! : undefined; - const {url, eventTypes, watchedColIds, isReadyColumn, name} = webhook; + const {url, authorization, eventTypes, watchedColIds, isReadyColumn, name} = webhook; const tableId = await getRealTableId(req.params.tableId || webhook.tableId, {metaTables}); const fields: Partial = {}; @@ -454,6 +454,7 @@ export class DocWorkerApi { return { fields, url, + authorization, }; } @@ -926,16 +927,16 @@ export class DocWorkerApi { const docId = activeDoc.docName; const webhookId = req.params.webhookId; - const {fields, url} = await getWebhookSettings(activeDoc, req, webhookId, req.body); + const {fields, url, authorization} = await getWebhookSettings(activeDoc, req, webhookId, req.body); if (fields.enabled === false) { await activeDoc.triggers.clearSingleWebhookQueue(webhookId); } const triggerRowId = activeDoc.triggers.getWebhookTriggerRecord(webhookId).id; - // update url in homedb - if (url) { - await this._dbManager.updateWebhookUrl(webhookId, docId, url); + // update url and authorization header in homedb + if (url || authorization) { + await this._dbManager.updateWebhookUrlAndAuth({id: webhookId, docId, url, auth: authorization}); activeDoc.triggers.webhookDeleted(webhookId); // clear cache } diff --git a/app/server/lib/Triggers.ts b/app/server/lib/Triggers.ts index c90ee548..e9d51484 100644 --- a/app/server/lib/Triggers.ts +++ b/app/server/lib/Triggers.ts @@ -72,6 +72,7 @@ type Trigger = MetaRowRecord<"_grist_Triggers">; export interface WebHookSecret { url: string; unsubscribeKey: string; + authorization?: string; } // Work to do after fetching values from the document @@ -259,6 +260,7 @@ export class DocTriggers { const getTableId = docData.getMetaTable("_grist_Tables").getRowPropFunc("tableId"); const getColId = docData.getMetaTable("_grist_Tables_column").getRowPropFunc("colId"); const getUrl = async (id: string) => (await this._getWebHook(id))?.url ?? ''; + const getAuthorization = async (id: string) => (await this._getWebHook(id))?.authorization ?? ''; const getUnsubscribeKey = async (id: string) => (await this._getWebHook(id))?.unsubscribeKey ?? ''; const resultTable: WebhookSummary[] = []; @@ -271,6 +273,7 @@ export class DocTriggers { for (const act of webhookActions) { // Url, probably should be hidden for non-owners (but currently this API is owners only). const url = await getUrl(act.id); + const authorization = await getAuthorization(act.id); // Same story, should be hidden. const unsubscribeKey = await getUnsubscribeKey(act.id); if (!url || !unsubscribeKey) { @@ -285,6 +288,7 @@ export class DocTriggers { fields: { // Url, probably should be hidden for non-owners (but currently this API is owners only). url, + authorization, unsubscribeKey, // Other fields used to register this webhook. eventTypes: decodeObject(t.eventTypes) as string[], @@ -683,6 +687,7 @@ export class DocTriggers { const batch = _.takeWhile(this._webHookEventQueue.slice(0, 100), {id}); const body = JSON.stringify(batch.map(e => e.payload)); const url = await this._getWebHookUrl(id); + const authorization = (await this._getWebHook(id))?.authorization || ""; if (this._loopAbort.signal.aborted) { continue; } @@ -698,7 +703,8 @@ export class DocTriggers { this._activeDoc.logTelemetryEvent(null, 'sendingWebhooks', { limited: {numEvents: meta.numEvents}, }); - success = await this._sendWebhookWithRetries(id, url, body, batch.length, this._loopAbort.signal); + success = await this._sendWebhookWithRetries( + id, url, authorization, body, batch.length, this._loopAbort.signal); if (this._loopAbort.signal.aborted) { continue; } @@ -770,7 +776,8 @@ export class DocTriggers { return this._drainingQueue ? Math.min(5, TRIGGER_MAX_ATTEMPTS) : TRIGGER_MAX_ATTEMPTS; } - private async _sendWebhookWithRetries(id: string, url: string, body: string, size: number, signal: AbortSignal) { + private async _sendWebhookWithRetries( + id: string, url: string, authorization: string, body: string, size: number, signal: AbortSignal) { const maxWait = 64; let wait = 1; for (let attempt = 0; attempt < this._maxWebhookAttempts; attempt++) { @@ -786,6 +793,7 @@ export class DocTriggers { body, headers: { 'Content-Type': 'application/json', + ...(authorization ? {'Authorization': authorization} : {}), }, signal, agent: proxyAgent(new URL(url)), diff --git a/static/locales/en.client.json b/static/locales/en.client.json index 420e580e..76790bb1 100644 --- a/static/locales/en.client.json +++ b/static/locales/en.client.json @@ -1241,7 +1241,8 @@ "URL": "URL", "Webhook Id": "Webhook Id", "Table": "Table", - "Filter for changes in these columns (semicolon-separated ids)": "Filter for changes in these columns (semicolon-separated ids)" + "Filter for changes in these columns (semicolon-separated ids)": "Filter for changes in these columns (semicolon-separated ids)", + "Header Authorization": "Header Authorization" }, "FormulaAssistant": { "Ask the bot.": "Ask the bot.", diff --git a/test/nbrowser/WebhookPage.ts b/test/nbrowser/WebhookPage.ts index 5db3e972..d53f4ebe 100644 --- a/test/nbrowser/WebhookPage.ts +++ b/test/nbrowser/WebhookPage.ts @@ -52,10 +52,11 @@ describe('WebhookPage', function () { 'Name', 'Memo', 'Event Types', - 'URL', 'Table', - 'Ready Column', 'Filter for changes in these columns (semicolon-separated ids)', + 'Ready Column', + 'URL', + 'Header Authorization', 'Webhook Id', 'Enabled', 'Status', @@ -81,7 +82,7 @@ describe('WebhookPage', function () { await gu.waitToPass(async () => { assert.equal(await getField(1, 'Webhook Id'), id); }); - // Now other fields like name, memo and watchColIds are persisted. + // Now other fields like name, memo, watchColIds, and Header Auth are persisted. await setField(1, 'Name', 'Test Webhook'); await setField(1, 'Memo', 'Test Memo'); await setField(1, 'Filter for changes in these columns (semicolon-separated ids)', 'A; B'); @@ -115,6 +116,27 @@ describe('WebhookPage', function () { assert.lengthOf((await docApi.getRows('Table2')).A, 0); }); + it('can create webhook with persistant header authorization', async function () { + // The webhook won't work because the header auth doesn't match the api key of the current test user. + await openWebhookPage(); + await setField(1, 'Event Types', 'add\nupdate\n'); + await setField(1, 'URL', `http://${host}/api/docs/${doc.id}/tables/Table2/records?flat=1`); + await setField(1, 'Table', 'Table1'); + await gu.waitForServer(); + await driver.navigate().refresh(); + await waitForWebhookPage(); + await setField(1, 'Header Authorization', 'Bearer 1234'); + await gu.waitForServer(); + await driver.navigate().refresh(); + await waitForWebhookPage(); + await gu.waitToPass(async () => { + assert.equal(await getField(1, 'Header Authorization'), 'Bearer 1234'); + }); + await gu.getDetailCell({col:'Header Authorization', rowNum: 1}).click(); + await gu.enterCell(Key.DELETE, Key.ENTER); + await gu.waitForServer(); + }); + it('can create two webhooks', async function () { await openWebhookPage(); await setField(1, 'Event Types', 'add\nupdate\n'); diff --git a/test/server/lib/DocApi.ts b/test/server/lib/DocApi.ts index 86b515d3..a12f1756 100644 --- a/test/server/lib/DocApi.ts +++ b/test/server/lib/DocApi.ts @@ -4625,6 +4625,7 @@ function testDocApi() { id: first.webhookId, fields: { url: `${serving.url}/200`, + authorization: '', unsubscribeKey: first.unsubscribeKey, eventTypes: ['add', 'update'], enabled: true, @@ -4643,6 +4644,7 @@ function testDocApi() { id: second.webhookId, fields: { url: `${serving.url}/404`, + authorization: '', unsubscribeKey: second.unsubscribeKey, eventTypes: ['add', 'update'], enabled: true, @@ -5010,6 +5012,7 @@ function testDocApi() { const expectedFields = { url: `${serving.url}/foo`, + authorization: '', eventTypes: ['add'], isReadyColumn: 'B', tableId: 'Table1', @@ -5079,6 +5082,8 @@ function testDocApi() { await check({isReadyColumn: null}, 200); await check({isReadyColumn: "bar"}, 404, `Column not found "bar"`); + + await check({authorization: 'Bearer fake-token'}, 200); }); });