From 2b65daa6e0110e74b03d373ef8453d32be000de2 Mon Sep 17 00:00:00 2001 From: Philip de Nier Date: Thu, 2 May 2024 13:18:55 +0100 Subject: [PATCH 1/6] api: change webhooks API Replace the "secret" with an "api_key_name" and "api_key_value". The "api_key_name" is the name of the HTTP header that is added when POSTing to the webhook URL. The header value is "api_key_value". --- api/TimeAddressableMediaStore.yaml | 17 +++++++++-------- api/schemas/webhook-post.json | 30 ++++++++++++++++++++++++++++++ api/schemas/webhook.json | 8 ++++++-- 3 files changed, 45 insertions(+), 10 deletions(-) create mode 100644 api/schemas/webhook-post.json diff --git a/api/TimeAddressableMediaStore.yaml b/api/TimeAddressableMediaStore.yaml index 3453b8d..eec87ec 100644 --- a/api/TimeAddressableMediaStore.yaml +++ b/api/TimeAddressableMediaStore.yaml @@ -141,11 +141,12 @@ paths: - Webhooks responses: "200": - description: Return the list of known webhook URLs. Note that the `secret` will be omitted. + description: Return the list of known webhook URLs. Note that the `api_key_value` will be omitted. content: application/json: example: - url: https://hook.example.com + api_key_name: Authorization events: - flows/created - flows/updated @@ -165,12 +166,11 @@ paths: Availability of this endpoint is indicated by the name "webhooks" appearing in the `event_stream_mechanisms` list on the service endpoint. - Making a POST request to this endpoint with the same URL and secret but a different list of `events` SHOULD - update the existing registration. POSTing an empty list of events SHOULD remove the registration. + Making a POST request to this endpoint with the same URL, API key name and value but a different list + of `events` SHOULD update the existing registration. POSTing an empty list of events SHOULD remove the + registration. - HTTP requests from the service SHOULD include a `X-Hook-Signature` header containing the HMAC hex digest of the - webhook body, hashed using SHA-256 using the shared secret as the key. Clients SHOULD verify each message using - the shared secret. + HTTP requests from the service SHOULD include a `api_key_name` header with the 'api_key_value' value. API implementations SHOULD consider the security implementations of providing webhooks, and include appropriate mitigations against Server Side Request Forgery (SSRF) attacks and similar. @@ -182,12 +182,13 @@ paths: application/json: example: url: https://hook.example.com - secret: super-secret-string + api_key_name: Authorization + api_key_value: Bearer 21238dksdjqwpqscj9 events: - flows/created - flows/updated schema: - $ref: schemas/webhook.json + $ref: schemas/webhook-post.json required: true responses: "200": diff --git a/api/schemas/webhook-post.json b/api/schemas/webhook-post.json new file mode 100644 index 0000000..ca60de8 --- /dev/null +++ b/api/schemas/webhook-post.json @@ -0,0 +1,30 @@ +{ + "title": "Register Webhook", + "description": "Register to receive updates via webhook", + "type": "object", + "required": [ + "url", + "events" + ], + "properties": { + "url": { + "description": "The URL to which the API should make HTTP POST requests with event data", + "type": "string" + }, + "api_key_name": { + "description": "The HTTP header name that is added to the event POST with value 'api_key_value'", + "type": "string" + }, + "api_key_value": { + "description": "The value that the HTTP header 'api_key_name' will be set to", + "type": "string" + }, + "events": { + "description": "List of event types to receive", + "type": "array", + "items": { + "type": "string" + } + } + } +} \ No newline at end of file diff --git a/api/schemas/webhook.json b/api/schemas/webhook.json index 41f3dad..1dd3638 100644 --- a/api/schemas/webhook.json +++ b/api/schemas/webhook.json @@ -11,8 +11,12 @@ "description": "The URL to which the API should make HTTP POST requests with event data", "type": "string" }, - "secret": { - "description": "A shared secret, used to sign deliveries to clients so they can validate the source", + "api_key_name": { + "description": "The HTTP header name that is added to the event POST", + "type": "string" + }, + "api_key_value": { + "description": "The value that the HTTP header 'api_key_name' will be set to", "type": "string" }, "events": { From 5e47ba1349e1ba3a5a40bd85c67d3e7e878eac05 Mon Sep 17 00:00:00 2001 From: Philip de Nier Date: Thu, 2 May 2024 13:21:13 +0100 Subject: [PATCH 2/6] api: fix the event mechanism name in example to match the spec --- api/examples/service-get-200.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/examples/service-get-200.json b/api/examples/service-get-200.json index 0cbe5b6..22030cd 100644 --- a/api/examples/service-get-200.json +++ b/api/examples/service-get-200.json @@ -8,7 +8,7 @@ }, "event_stream_mechanisms": [ { - "name": "webhook", + "name": "webhooks", "docs": "https://bbc.github.io/tams/#/operations/POST_webhooks" } ] From e4ce6a19b52d1fa815dac7dfb00dae57d8c5606d Mon Sep 17 00:00:00 2001 From: Philip de Nier Date: Thu, 2 May 2024 16:03:56 +0100 Subject: [PATCH 3/6] api: add event object in webhook event message Makes it easier to construct the TAMS event --- api/TimeAddressableMediaStore.yaml | 100 ++++++++++++++++++++--------- 1 file changed, 70 insertions(+), 30 deletions(-) diff --git a/api/TimeAddressableMediaStore.yaml b/api/TimeAddressableMediaStore.yaml index eec87ec..f8e2b85 100644 --- a/api/TimeAddressableMediaStore.yaml +++ b/api/TimeAddressableMediaStore.yaml @@ -1305,7 +1305,7 @@ webhooks: required: - event_timestamp - event_type - - flow + - event properties: event_timestamp: description: Timestamp at which the new Flow was created @@ -1314,8 +1314,13 @@ webhooks: event_type: type: string const: flows/created - flow: - $ref: "schemas/flow-core.json" + event: + type: object + required: + - flow + properties: + flow: + $ref: "schemas/flow-core.json" flows/updated: post: @@ -1330,7 +1335,7 @@ webhooks: required: - event_timestamp - event_type - - flow + - event properties: event_timestamp: description: Timestamp at which the Flow was modified @@ -1339,8 +1344,13 @@ webhooks: event_type: type: string const: flows/updated - flow: - $ref: "schemas/flow-core.json" + event: + type: object + required: + - flow + properties: + flow: + $ref: "schemas/flow-core.json" flows/deleted: post: @@ -1355,7 +1365,7 @@ webhooks: required: - event_timestamp - event_type - - flow_id + - event properties: event_timestamp: description: Timestamp at which the Flow was modified @@ -1364,8 +1374,13 @@ webhooks: event_type: type: string const: flows/deleted - flow_id: - $ref: "#/components/schemas/uuid" + event: + type: object + required: + - flow_id + properties: + flow_id: + $ref: "#/components/schemas/uuid" flows/segments_added: post: @@ -1384,8 +1399,7 @@ webhooks: required: - event_timestamp - event_type - - flow_id - - timerange + - event properties: event_timestamp: description: Timestamp at which the most recent segment in the timerange was added (and the message generated) @@ -1394,10 +1408,16 @@ webhooks: event_type: type: string const: flows/segments_added - flow_id: - $ref: "#/components/schemas/uuid" - timerange: - $ref: 'schemas/timerange.json' + event: + type: object + required: + - flow_id + - timerange + properties: + flow_id: + $ref: "#/components/schemas/uuid" + timerange: + $ref: 'schemas/timerange.json' flows/segments_deleted: post: @@ -1416,8 +1436,7 @@ webhooks: required: - event_timestamp - event_type - - flow_id - - timerange + - event properties: event_timestamp: description: Timestamp at which the most recent segment in the timerange was added (and the message generated) @@ -1426,10 +1445,16 @@ webhooks: event_type: type: string const: flows/segments_added - flow_id: - $ref: "#/components/schemas/uuid" - timerange: - $ref: 'schemas/timerange.json' + event: + type: object + required: + - flow_id + - timerange + properties: + flow_id: + $ref: "#/components/schemas/uuid" + timerange: + $ref: 'schemas/timerange.json' sources/created: post: @@ -1444,7 +1469,7 @@ webhooks: required: - event_timestamp - event_type - - source + - event properties: event_timestamp: description: Timestamp at which the new Source was created @@ -1453,8 +1478,13 @@ webhooks: event_type: type: string const: sources/created - source: - $ref: "schemas/source.json" + event: + type: object + required: + - source + properties: + source: + $ref: "schemas/source.json" sources/updated: post: @@ -1469,7 +1499,7 @@ webhooks: required: - event_timestamp - event_type - - source + - event properties: event_timestamp: description: Timestamp at which the Source was modified @@ -1478,8 +1508,13 @@ webhooks: event_type: type: string const: sources/updated - source: - $ref: "schemas/source.json" + event: + type: object + required: + - source + properties: + source: + $ref: "schemas/source.json" sources/deleted: post: @@ -1494,7 +1529,7 @@ webhooks: required: - event_timestamp - event_type - - source_id + - event properties: event_timestamp: description: Timestamp at which the Source was modified @@ -1503,8 +1538,13 @@ webhooks: event_type: type: string const: sources/deleted - source_id: - $ref: "#/components/schemas/uuid" + event: + type: object + required: + - source_id + properties: + source_id: + $ref: "#/components/schemas/uuid" components: schemas: From db3f928f5466cacf39516ada010ea933b9e25c47 Mon Sep 17 00:00:00 2001 From: Philip de Nier Date: Thu, 2 May 2024 16:20:08 +0100 Subject: [PATCH 4/6] api: fix response codes for webhook post --- api/TimeAddressableMediaStore.yaml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/api/TimeAddressableMediaStore.yaml b/api/TimeAddressableMediaStore.yaml index f8e2b85..08ec239 100644 --- a/api/TimeAddressableMediaStore.yaml +++ b/api/TimeAddressableMediaStore.yaml @@ -191,8 +191,10 @@ paths: $ref: schemas/webhook-post.json required: true responses: - "200": - description: Success. The webhook has been registered + "201": + description: Success. The webhook has been registered or updated + "204": + description: Success. The webhook has been removed "404": description: "Webhooks are not supported by this API implementation" From 761f2e024823036c74dff47364ee818c5bfd9d4a Mon Sep 17 00:00:00 2001 From: Philip de Nier Date: Thu, 2 May 2024 16:45:59 +0100 Subject: [PATCH 5/6] api: allow full flow in webhook event --- api/TimeAddressableMediaStore.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/TimeAddressableMediaStore.yaml b/api/TimeAddressableMediaStore.yaml index 08ec239..bfcaecd 100644 --- a/api/TimeAddressableMediaStore.yaml +++ b/api/TimeAddressableMediaStore.yaml @@ -1322,7 +1322,7 @@ webhooks: - flow properties: flow: - $ref: "schemas/flow-core.json" + $ref: "schemas/flow.json" flows/updated: post: @@ -1352,7 +1352,7 @@ webhooks: - flow properties: flow: - $ref: "schemas/flow-core.json" + $ref: "schemas/flow.json" flows/deleted: post: From 8e531fba54e0e0c011fba8581ad63797d4385751 Mon Sep 17 00:00:00 2001 From: Philip de Nier Date: Thu, 2 May 2024 17:04:47 +0100 Subject: [PATCH 6/6] reviewcomment: add webhook spoofing comment Co-authored-by: Sam Mesterton-Gibbons --- api/TimeAddressableMediaStore.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/TimeAddressableMediaStore.yaml b/api/TimeAddressableMediaStore.yaml index bfcaecd..e1f9712 100644 --- a/api/TimeAddressableMediaStore.yaml +++ b/api/TimeAddressableMediaStore.yaml @@ -170,7 +170,7 @@ paths: of `events` SHOULD update the existing registration. POSTing an empty list of events SHOULD remove the registration. - HTTP requests from the service SHOULD include a `api_key_name` header with the 'api_key_value' value. + HTTP requests from the service SHOULD include a `api_key_name` header with the 'api_key_value' value. Clients SHOULD verify this against the value they provided when registering the webhook. API implementations SHOULD consider the security implementations of providing webhooks, and include appropriate mitigations against Server Side Request Forgery (SSRF) attacks and similar.