diff --git a/api/TimeAddressableMediaStore.yaml b/api/TimeAddressableMediaStore.yaml index 0146237c..cc5904e6 100644 --- a/api/TimeAddressableMediaStore.yaml +++ b/api/TimeAddressableMediaStore.yaml @@ -153,9 +153,29 @@ paths: operationId: HEAD_webhooks tags: - Webhooks + parameters: + - $ref: '#/components/parameters/trait_resource_paged_key' + - $ref: '#/components/parameters/trait_paged_limit' responses: "200": - $ref: '#/components/responses/trait_resource_listing_head_200' + description: "" + headers: + Link: + description: Provides references to cursors for paging. Only the 'rel' attribute with value 'next' and a link to the next page is currently supported. If 'next' is not present then it is the last page. + schema: + type: string + X-Paging-Limit: + description: Identifies the current limit being used for paging. This may not match the requested value if the requested value was too high for the implementation + schema: + type: integer + X-Paging-NextKey: + description: Opaque string that can be supplied to the `page` query parameter to get the next page of results. + schema: + type: string + content: + application/json: + schema: + type: string "404": description: "Webhooks are not supported by this service implementation" get: @@ -167,33 +187,118 @@ paths: operationId: GET_webhooks tags: - Webhooks + parameters: + - $ref: '#/components/parameters/trait_resource_paged_key' + - $ref: '#/components/parameters/trait_paged_limit' responses: "200": - description: Return the list of known webhook URLs. Note that the `api_key_value` will be omitted. + description: Return the list of known webhook URLs. + headers: + Link: + description: Provides references to cursors for paging. Only the 'rel' attribute with value 'next' and a link to the next page is currently supported. If 'next' is not present then it is the last page. + schema: + type: string + X-Paging-Limit: + description: Identifies the current limit being used for paging. This may not match the requested value if the requested value was too high for the implementation + schema: + type: integer + X-Paging-NextKey: + description: Opaque string that can be supplied to the `page` query parameter to get the next page of results. + schema: + type: string content: application/json: example: - - url: https://hook.example.com - api_key_name: Authorization - events: - - flows/created - - flows/updated - - flows/deleted + $ref: "examples/webhook-get-200-list.json" schema: type: array items: - $ref: "schemas/webhook.json" + $ref: "schemas/webhook-get.json" "404": - description: "Webhooks are not supported by this service implementation" + description: "Webhooks are not supported by this API implementation" post: - summary: Register Webhook URL + summary: Register webhook details description: | - Register to receive event notifications as webhooks on a specified URL. - Webhook messages will conform to the format in the `webhooks` section of the API docs, depending on the event type (as defined in the same section). - Availability of this endpoint is indicated by the name "webhooks" appearing in the `event_stream_mechanisms` list on the [/service](#/operations/GET_service) endpoint. + Register to receive event notifications as webhooks on a specified URL. Webhook messages will conform to the + format in the `webhooks` section of the API docs, depending on the event type (as defined in the same section). + Availability of this endpoint is indicated by the name "webhooks" appearing in the `event_stream_mechanisms` + list on the service endpoint. + + 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. - 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. + API implementations MAY partially support event filtering and transformations. + API implementations SHALL return a 400 response code if the filtering or transformation specified in the request is not supported. + + API implementations SHOULD consider the security implementations of providing webhooks, and include appropriate mitigations against Server Side Request Forgery (SSRF) attacks and similar. API implementations SHOULD take appropriate steps to authorize the modification of existing webhooks. This may take the form of RBAC, or ABAC. + operationId: POST_webhooks + tags: + - Webhooks + requestBody: + content: + application/json: + example: + $ref: "examples/webhook-post.json" + schema: + $ref: schemas/webhook-post.json + required: true + responses: + "201": + description: Success. The webhook has been registered. + content: + application/json: + example: + $ref: "examples/webhook-get-200.json" + schema: + $ref: schemas/webhook-get.json + "400": + description: Bad request. Invalid parameters or unsupported event filtering or transformation. + "404": + description: Webhooks are not supported by this service implementation + /service/webhooks/{webhookId}: + parameters: + - name: webhookId + in: path + required: true + schema: + $ref: 'schemas/uuid.json' + description: The Webhook identifier. + head: + summary: Webhook details + description: Return webhook path headers + operationId: HEAD_webhooks-webhookId + tags: + - Webhooks + responses: + "200": + $ref: '#/components/responses/trait_resource_listing_head_200' + "404": + description: The requested Webhook ID in the path is invalid, or Webhooks are not supported by this service implementation + get: + summary: Webhook details + description: | + Get the details of a webhook. Service implementations SHOULD take steps to avoid displaying URLs to users other than those who have suitable permissions (e.g. the owning user). + Availability of this endpoint is indicated by the name "webhooks" appearing in the `event_stream_mechanisms` list on the [`/service`](#/operations/GET_service) endpoint. + operationId: GET_webhooks-webhookId + tags: + - Webhooks + responses: + "200": + description: Return the webhook details. + content: + application/json: + example: + $ref: "examples/webhook-get-200.json" + schema: + $ref: "schemas/webhook-get.json" + "404": + description: The requested Webhook ID in the path is invalid, or Webhooks are not supported by this service implementation + put: + summary: Update webhook details + description: | + Update the configuration of an existing webhook. + + Webhook messages will conform to the format in the `webhooks` section of the API docs, depending on the event type (as defined in the same section). + Availability of this endpoint is indicated by the name "webhooks" appearing in the `event_stream_mechanisms` list on the [`/service`](#/operations/GET_service) endpoint. HTTP events sent by the service to a client webhook's endpoint 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. @@ -202,33 +307,53 @@ paths: Service implementations SHALL return a 400 response code if the filtering or transformation specified in the request is not supported. Service implementations SHOULD consider the security implications of providing webhooks, and include appropriate mitigations against Server Side Request Forgery (SSRF) attacks and similar. - operationId: POST_webhooks + Service implementations SHOULD take appropriate steps to authorize the modification of existing webhooks. + This may take the form of RBAC, or ABAC. + operationId: PUT_webhooks tags: - Webhooks requestBody: content: application/json: example: - url: https://hook.example.com - api_key_name: Authorization - api_key_value: Bearer 21238dksdjqwpqscj9 - events: - - flows/created - - flows/updated + $ref: "examples/webhook-put.json" schema: - $ref: schemas/webhook-post.json + $ref: schemas/webhook-put.json required: true responses: "201": - description: Success. The webhook has been registered or updated - "204": - description: Success. The webhook has been removed + description: Success. The webhook has been updated + content: + application/json: + example: + $ref: "examples/webhook-get-200.json" + schema: + $ref: schemas/webhook-get.json "400": description: Bad request. Invalid parameters or unsupported event filtering or transformation. "403": description: Forbidden. You do not have permission to modify this resource. "404": - description: "Webhooks are not supported by this service implementation" + description: The requested Webhook ID in the path is invalid, or Webhooks are not supported by this service implementation + delete: + summary: Delete Webhook + description: | + Deletes the webhook. + Availability of this endpoint is indicated by the name "webhooks" appearing in the `event_stream_mechanisms` list on the service endpoint. + + Service implementations SHOULD consider the security implementations of providing webhooks, and include appropriate mitigations against Server Side Request Forgery (SSRF) attacks and similar. + Service implementations SHOULD take appropriate steps to authorize the deleting of webhooks. + This may take the form of RBAC, or ABAC. + operationId: DELETE_webhooks-webhookId + tags: + - Webhooks + responses: + "204": + description: No content. The webhook has been deleted. + "403": + description: Forbidden. You do not have permission to modify this webhook. + "404": + description: The requested Webhook ID in the path is invalid, or Webhooks are not supported by this service implementation /sources: head: summary: List Sources @@ -2445,7 +2570,7 @@ webhooks: format: date-time event_type: type: string - const: flows/segments_added + const: flows/segments_deleted event: type: object required: diff --git a/api/examples/webhook-get-200-list.json b/api/examples/webhook-get-200-list.json new file mode 100644 index 00000000..eb0e963d --- /dev/null +++ b/api/examples/webhook-get-200-list.json @@ -0,0 +1,12 @@ +[ + { + "id": "e85efab4-993b-4ad6-9af3-4cd8d0d38860", + "url": "https://hook.example.com", + "api_key_name": "Authorization", + "events": [ + "flows/created", + "flows/updated" + ], + "status": "started" + } +] \ No newline at end of file diff --git a/api/examples/webhook-get-200.json b/api/examples/webhook-get-200.json new file mode 100644 index 00000000..065723c1 --- /dev/null +++ b/api/examples/webhook-get-200.json @@ -0,0 +1,10 @@ +{ + "id": "e85efab4-993b-4ad6-9af3-4cd8d0d38860", + "url": "https://hook.example.com", + "api_key_name": "Authorization", + "events": [ + "flows/created", + "flows/updated" + ], + "status": "started" +} \ No newline at end of file diff --git a/api/examples/webhook-post.json b/api/examples/webhook-post.json new file mode 100644 index 00000000..f470d195 --- /dev/null +++ b/api/examples/webhook-post.json @@ -0,0 +1,9 @@ +{ + "url": "https://hook.example.com", + "api_key_name": "Authorization", + "api_key_value": "Bearer 21238dksdjqwpqscj9", + "events": [ + "flows/created", + "flows/updated" + ] +} \ No newline at end of file diff --git a/api/examples/webhook-put.json b/api/examples/webhook-put.json new file mode 100644 index 00000000..f7fb7fa8 --- /dev/null +++ b/api/examples/webhook-put.json @@ -0,0 +1,11 @@ +{ + "id": "e85efab4-993b-4ad6-9af3-4cd8d0d38860", + "url": "https://hook.example.com", + "api_key_name": "Authorization", + "api_key_value": "Bearer 21238dksdjqwpqscj9", + "events": [ + "flows/created", + "flows/updated" + ], + "status": "created" +} \ No newline at end of file diff --git a/api/schemas/webhook-get.json b/api/schemas/webhook-get.json new file mode 100644 index 00000000..5ed059fc --- /dev/null +++ b/api/schemas/webhook-get.json @@ -0,0 +1,33 @@ +{ + "title": "Webhook Detail", + "description": "Describes a Webhook", + "type": "object", + "allOf": [ + { + "$ref": "webhook-with-id.json" + }, + { + "type": "object", + "required": + [ + "status" + ], + "properties": { + "error": { + "description": "Provides more information for the error status, as described by the [Error](../schemas/error#top) type", + "$ref": "error.json" + }, + "status": { + "description": "Status of the Webhook. `created` indicates the webhook has been successfully registered but is yet to begin sending events or, depending on the service implementation, the worker responsible for sending the events has yet to start. `started` indicates the webhook is active and sending events. `disabled` indicates the webhook has been disabled by a client and is not currently sending events. `error` indicates an error condition has been encountered and the webhook has been disabled by the service instance. More information about the error condition will be indicated by the service instance in the `error` parameter. Service implementations SHOULD implement appropriate retries and only enter the `error` state when absolutely necesary. A webhook in the `error` or `disabled` state may be re-enabled by a client by setting the status to `created`. A webhook in the `created` or `started` state may be disabled by a client by setting the status to `disabled`. Attempting to transition an `error` status to `disabled` SHOULD be rejected.", + "type": "string", + "enum": [ + "created", + "started", + "disabled", + "error" + ] + } + } + } + ] +} \ No newline at end of file diff --git a/api/schemas/webhook-post.json b/api/schemas/webhook-post.json index 38f28f35..242b72f4 100644 --- a/api/schemas/webhook-post.json +++ b/api/schemas/webhook-post.json @@ -2,79 +2,26 @@ "title": "Register Webhook", "description": "Register to receive updates via webhook", "type": "object", - "required": [ - "url", - "events" - ], - "properties": { - "url": { - "description": "The URL to which the service instance 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" - } - }, - "flow_ids": { - "description": "Limit Flow and Flow Segment events to Flows in the given list of Flow IDs", - "type": "array", - "items": { - "$ref": "uuid.json" - } - }, - "source_ids": { - "description": "Limit Flow, Flow Segment and Source events to Sources in the given list of Source IDs", - "type": "array", - "items": { - "$ref": "uuid.json" - } - }, - "flow_collected_by_ids": { - "description": "Limit Flow and Flow Segment events to those with Flow that is collected by a Flow Collection in the given list of Flow Collection IDs", - "type": "array", - "items": { - "$ref": "uuid.json" + "allOf": [ + { + "$ref": "webhook.json" + }, + { + "type": "object", + "properties": { + "api_key_value": { + "description": "The value that the HTTP header 'api_key_name' will be set to", + "type": "string" + }, + "status": { + "description": "Status of the Webhook. `created` will register the webhook in the created state and the service instance will attempt to start sending events. `disabled` will register the webhook in a disabled state and will not send events. Assumed to be `created` if not set.", + "type": "string", + "enum": [ + "created", + "disabled" + ] + } } - }, - "source_collected_by_ids": { - "description": "Limit Flow, Flow Segment and Source events to those with Source that is collected by a Source Collection in the given list of Source Collection IDs", - "type": "array", - "items": { - "$ref": "uuid.json" - } - }, - "accept_get_urls": { - "description": "List of labels of URLs to include in the `get_urls` property in `flows/segments_added` events. Where multiple `get_urls` filter query parameters are provided, the included `get_urls` will match all filters. This option is the same as the `accept_get_urls` query parameter for the [/flows/{flowId}/segments](#/operations/GET_flows-flowId-segments) API endpoint, except that the labels are represented using a JSON array rather than a (comma separated list) string.", - "type": "array", - "items": { - "type": "string" - } - }, - "accept_storage_ids": { - "description": "List of labels of `storage_id`s to include in the `get_urls` property in `flows/segments_added` events. Where multiple `get_urls` filter query parameters are provided, the included `get_urls` will match all filters. This option is the same as the `accept_storage_ids` query parameter for the [/flows/{flowId}/segments](#/operations/GET_flows-flowId-segments) API endpoint, except that the IDs are represented using a JSON array rather than a (comma separated list) string.", - "type": "array", - "items": { - "$ref": "uuid.json" - } - }, - "presigned": { - "description": "Whether to include presigned/non-presigned URLs in the `get_urls` property in `flows/segments_added` events. Where multiple `get_urls` filter query parameters are provided, the included `get_urls` will match all filters. This option is the same as the `presigned` query parameter for the [/flows/{flowId}/segments](#/operations/GET_flows-flowId-segments) API endpoint.", - "type": "boolean" - }, - "verbose_storage": { - "description": "Whether to include storage metadata in the `get_urls` property in `flows/segments_added` events. This option is the same as the `verbose_storage` query parameter for the [/flows/{flowId}/segments](#/operations/GET_flows-flowId-segments) API endpoint.", - "type": "boolean" } - } -} \ No newline at end of file + ] +} diff --git a/api/schemas/webhook-put.json b/api/schemas/webhook-put.json new file mode 100644 index 00000000..ce33db0d --- /dev/null +++ b/api/schemas/webhook-put.json @@ -0,0 +1,31 @@ +{ + "title": "Modify Webhook", + "description": "Modify existing webhook", + "type": "object", + "allOf": [ + { + "$ref": "webhook-with-id.json" + }, + { + "type": "object", + "required": + [ + "status" + ], + "properties": { + "api_key_value": { + "description": "The value that the HTTP header 'api_key_name' will be set to", + "type": "string" + }, + "status": { + "description": "Status of the Webhook. `created` indicates the webhook has been successfully registered but is yet to begin sending events or, depending on the service implementation, the worker responsible for sending the events has yet to start. `started` indicates the webhook is active and sending events. `disabled` indicates the webhook has been disabled by a client and is not currently sending events. `error` indicates an error condition has been encountered and the webhook has been disabled by the service instance. More information about the error condition will be indicated by the service instance in the `error` parameter. Service implementations SHOULD implement appropriate retries and only enter the `error` state when absolutely necesary. A webhook in the `error` or `disabled` state may be re-enabled by a client by setting the status to `created`. A webhook in the `created` or `started` state may be disabled by a client by setting the status to `disabled`. Attempting to transition an `error` status to `disabled` SHOULD be rejected.", + "type": "string", + "enum": [ + "created", + "disabled" + ] + } + } + } + ] +} \ No newline at end of file diff --git a/api/schemas/webhook-with-id.json b/api/schemas/webhook-with-id.json new file mode 100644 index 00000000..4ada98cc --- /dev/null +++ b/api/schemas/webhook-with-id.json @@ -0,0 +1,22 @@ +{ + "title": "Webhook Details", + "description": "Details of an existing registered webhook", + "type": "object", + "allOf": [ + { + "$ref": "webhook.json" + }, + { + "type": "object", + "required": [ + "id" + ], + "properties": { + "id": { + "description": "Webhook identifier", + "$ref": "uuid.json" + } + } + } + ] +} \ No newline at end of file diff --git a/api/schemas/webhook.json b/api/schemas/webhook.json index 152474c3..6caa2364 100644 --- a/api/schemas/webhook.json +++ b/api/schemas/webhook.json @@ -15,15 +15,21 @@ "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": { "description": "List of event types to receive", "type": "array", "items": { - "type": "string" + "type": "string", + "enum": [ + "flows/created", + "flows/updated", + "flows/deleted", + "flows/segments_added", + "flows/segments_deleted", + "sources/created", + "sources/updated", + "sources/deleted" + ] } }, "flow_ids": { @@ -77,4 +83,4 @@ "type": "boolean" } } -} \ No newline at end of file +} diff --git a/docs/README.md b/docs/README.md index 61a20e4f..b18e27c1 100644 --- a/docs/README.md +++ b/docs/README.md @@ -68,6 +68,7 @@ For more information on how we use ADRs, see [here](./adr/README.md). | [0031](./adr/0031-flow-image-support.md) | Add new flow type to support still images | | [0032](./adr/0032-specifying-storage-backend.md) | Specifying storage backend when requesting storage allocation | | [0034](./adr/0034-storage-allow-object_ids.md) | Add object_ids option to Flow Storage request | +| [0037](./adr/0037-improve-webhooks.md) | Proposal for improvements to the Webhooks endpoints | | [0038](./adr/0038-improved-storage-management.md) | Improved Storage Management | | [0039](./adr/0039-remove-pre-actions.md) | Proposal to remove pre-actions from storage allocation response | diff --git a/docs/adr/0037-improve-webhooks.md b/docs/adr/0037-improve-webhooks.md new file mode 100644 index 00000000..747feebc --- /dev/null +++ b/docs/adr/0037-improve-webhooks.md @@ -0,0 +1,298 @@ +--- +status: "accepted" +--- +# Proposal for improvements to the Webhooks endpoints + +## Context and Problem Statement + +Webhooks were added to the TAMS API shortly before it was open-sources. +They have seen some refinement over time, but have only recently started to see significant use. + +While carrying out recent work on fine-grained auth it was noticed that AWS' open-source implementation of the API, and BBC R&D's experimental implementation did not match in their interpretation of the primary key for Webhooks. +AWS have used the `url` as the sole primary key. +R&D have used a tuple of `url`, `api_key_name`, and `api_key_value`. +This arrives from a strict reading of the specification on when implementations should update existing Webhooks. +It was noticed that this later interpretation, combined with the hiding of the secret `api_key_value`, could result in multiple Webhooks existing which look otherwise identical. +It was also noticed that as currently defined, the loss of the secret `api_key_value` would prevent the editing or deleting of Webhooks. +Worse, the use of an existing `url` and `api_key_name`, but a different `api_key_value` could unintentionally and opaquely result in the creation of a new Webhook. +Furthermore, it was noted that the current CRUD workflows for Webhooks do not match those elsewhere in the API. +A compelling use case was also identified that is not currently easily implemented. +A client may wish to filter different events in different ways. +For example, they may wish to receive all `flows/created` events, but only `flows/segments_added` for specific flows. + +## Decision Drivers + +* Decision driver 1: The current specification is ambiguous towards what is the primary key for Webhooks +* Decision driver 2: The current approach can result in Webhooks which are impossible to edit/delete +* Decision driver 3: A strict interpretation of the current specification can result in multiple Webhooks that look identical +* Decision driver 4: The current specification for Webhooks does not follow CRUD workflow patterns elsewhere in the API +* Decision driver 5: The current approach does not easily allow for more complex matrixing of events and filters +* Decision driver 6: The current approach does not allow for direct retrieval of individual Webhooks +* Decision driver 7: Editing/rotating of `api_key_value` currently requires deleting of the existing webhook using the old key, and re-creation with the new key +* Decision driver 8: This part of the API is starting to see increased use + +## Considered Options + +* 1a Primary key: `url` +* 1b Primary key: Tuple of `url`, `api_key_name`, and `api_key_value` +* 1c Primary key: Tuple of `url` and `api_key_name` +* 1d Primary key: Add a UUID +* 2a Authorization method: Data as a secret +* 2b Authorization method: Use the same auth methods as the rest of the API +* 3a HTTP endpoint structure: Single endpoint +* 3b HTTP endpoint structure: Add a `/service/webhooks/{id}` endpoint +* 4a Registering of Webhooks: Existing POST to `/service/webhooks` +* 4b Registering of Webhooks: POST with `ID` to `/service/webhooks` +* 4c Registering of Webhooks: PUT/POST to `/service/webhooks/{id}` +* 5a Editing of Webhooks: Existing POST to `/service/webhooks` +* 5b Editing of Webhooks: POST with `ID` to `/service/webhooks` +* 5c Editing of Webhooks: PUT/POST to `/service/webhooks/{id}` +* 6a Deleting of Webhooks: Existing POST to `/service/webhooks` +* 6b Deleting of Webhooks: POST with `ID` to `/service/webhooks` +* 6c Deleting of Webhooks: PUT/POST to `/service/webhooks/{id}` +* 6d Deleting of Webhooks: DELETE to `/service/webhooks/{id}` +* 7a Backwards compatibility: Maintain backwards compatibility +* 7b Backwards compatibility: Do not maintain backwards compatibility + +## Decision Outcome + +Chosen options: + +* 1d Primary key: Add a UUID +* 2b Authorization method: Use the same auth methods as the rest of the API +* 3b HTTP endpoint structure: Add a `/service/webhooks/{id}` endpoint +* 4a Registering of Webhooks: Existing POST to `/service/webhooks` +* 5c Editing of Webhooks: PUT/POST to `/service/webhooks/{id}` +* 6d Deleting of Webhooks: DELETE to `/service/webhooks/{id}` +* 7b Backwards compatibility: Do not maintain backwards compatibility + +### Consequences + +* Good, because it brings CRUD workflows for Webhooks into line with other parts of the API +* Good, because it removes ambiguities around managing Webhooks +* Good, because it removes ambiguities from the specification that have been interpreted in multiple ways +* Good, because it prevents Webhooks ending up in a state where they cannot be deleted +* Good, because it prevents accidental creation of duplicate Webhooks +* Good, because it enables more complex Webhooks use cases +* Neutral, because the issues identified necessitate breaking changes anyway, we should take the opportunity to "do things right" while usage is relatively low + +### Implementation + +See the API specification changes in PR [#142](https://github.com/bbc/tams/pull/142). + +## Pros and Cons of the Options + +### 1a Primary key: `url` + +Use the `url` parameter as the unique primary key for Webhooks. + +* Neutral, because this matches one of the most commonly used TAMS server implementations +* Bad, because this this prevents setting of different filters for different event types +* Bad, because combining this with options that add endpoints for specific Webhooks would require escaping of the URL within the parameter + +### 1b Primary key: Tuple of `url`, `api_key_name`, and `api_key_value` + +The description of the Webhook endpoint currently states the following: + +```text +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. +``` + +The `api_key_name` is a header to be included in events sent to the URL, and that will be set to the secret `api_key_value`. +The API Key serves as a secret that receiving clients may use to authenticate event payloads. +The secret `api_key_value` is never returned in responses from the TAMS instance. + +A strict reading of this could be that the tuple of URL, `api_key_name`, and `api_key_value` constitutes the primary key for Webhooks. + +* Good, because this matches a strict reading of the spec +* Bad, because the primary key relies on a secret + * If that secret is lost, the Webhook cannot be edited or deleted +* Bad, because multiple Webhooks could use the same URL and `api_key_name` + * These Webhooks would look identical in the listing, but be different +* Bad, because this this prevents setting of different filters for different event types + * While different `api_key_name`'s could be used, but this may clash with the fields intended purpose of providing a consistent means to authenticate events +* Bad, because if the incorrect `api_key_value` is inadvertently used when updating Webhooks, a new one maybe created + +### 1c Primary key: Tuple of `url` and `api_key_name` + +A modification to prevent accidental duplication of Webhooks would be to only use the URL and `api_key_name` as the identifying tuple. + +* Good, because this would not violate the existing wording around when an existing Webhook should be updated +* Good, because it prevents inadvertent creation of duplicate Webhooks +* Good, because the primary key consists only of readable data +* Neutral, because it may be considered a non-breaking change +* Neutral, because if that secret is lost, the wWbhook cannot be edited or deleted without further API changes +* Bad, because this this prevents setting of different filters for different event types + * While different `api_key_name`'s could be used, but this may clash with the fields intended purpose of providing a consistent means to authenticate events + +### 1d Primary key: Add a UUID + +This option would add a UUID `id` property which would serve as the unique identifier for Webhooks. + +* Good, because it prevents inadvertent creation of duplicate Webhooks +* Good, because the primary key consists only of readable data +* Good, because it is consistent with patterns used elsewhere in the API +* Good, because it allows for multiple Webhooks using the same URL and API Key + * This would allow for full matrixing of filters and event types +* Neutral, because if that secret is lost, the Webhook cannot be edited or deleted without further API changes +* Bad, because this likely requires breaking changes + +### 2a Authorization method: Data as a secret + +The description of the Webhook endpoint currently states the following: + +```text +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. +``` + +The secret `api_key_value` is never returned in responses from the TAMS instance. +In practice, this results in the `api_key_value` being used as a means to authorize modification of existing Webhooks. + +* Good, because it protects a part of the API which interacts with other systems without requiring full ABAC/RBAC +* Neutral, because the same authorization secret is used in multiple parts of the architecture +* Bad, because loss of the secret `api_key_value` prevents updating/deleting of Webhooks + +### 2b Authorization method: Use the same auth methods as the rest of the API + +This option would see authorization on the webhooks match the rest of the API. +The specific auth method used will depend on the deployment. +RBAC (Role Based Access Control) is widely used in existing TAMS implementations and fine-grained ABAC (Attribute Based Access Control) approaches are currently in development. + +* Good, because it avoids authorisation on the Webhooks endpoint being a special case +* Good, because RBAC is widely used in existing TAMS implementations +* Good, because proposed ABAC approaches would allow for fine-grained access control to individual Webhooks +* Neutral, because fine-grained ABAC with TAMS is still experimental + +### 3a HTTP endpoint structure: Single endpoint + +This would see the existing Webhook endpoint structure maintained with no changes. + +* Good, because it requires no changes +* Bad, because it doesn't follow patterns used elsewhere in the API +* Bad, because it requires overloading of POST for creation, edit, and deletion of Webhooks +* Bad, because it requires the reading of a potentially large listing to read a single Webhook even if it's ID is known + +### 3b HTTP endpoint structure: Add a `/service/webhooks/{id}` endpoint + +This would add an endpoint for accessing individual Webhooks, akin to `/flows/{flowID}` and `/sources/{sourceID}`. + +* Good, because it follows patterns used elsewhere in the API +* Good, because it allows for splitting out of DELETE methods for Webhooks +* Good, because it allows immediate access to specific Webhooks +* Neutral, because it requires a non-breaking change + +### 4a Registering of Webhooks: Existing POST to `/service/webhooks` + +This would maintain the existing approach to registering Webhooks. +But may result in the API generating an ID and adding it to the return data, if Option 1d is chosen. + +* Good, because it doesn't require any changes to the POST request body +* Neutral, because clients may have to handle the new `ID` in the return data +* Bad, because it doesn't match patterns used elsewhere in the API +* Bad, because the current approach has some ambiguities that can lead to a poor user experience + +### 4b Registering of Webhooks: POST with `ID` to `/service/webhooks` + +This would maintain the existing endpoint for registering Webhooks, but require the specifcation of an ID. + +* Good, because it removes ambiguities in the endpoint +* Bad, because its a breaking change +* Bad, because it doesn't match patterns used elsewhere in the API + +### 4c Registering of Webhooks: PUT/POST to `/service/webhooks/{id}` + +This would use a new endpoint for the creation of Webhooks. + +* Good, because it removes ambiguities in the existing endpoint +* Good, because it matches patterns used elsewhere in the API +* Bad, because its a breaking change + +### 5a Editing of Webhooks: Existing POST to `/service/webhooks` + +This would maintain the existing approach to editing Webhooks without any changes to the request body. + +* Good, because it doesn't require any changes +* Bad, because which Webhook a user wishes to edit may be ambiguous + * This may make this option impossible to implement +* Bad, because it may be ambiguous if a user wishes to create or edit a Webhook +* Bad, because it doesn't match patterns used elsewhere in the API + +### 5b Editing of Webhooks: POST with `ID` to `/service/webhooks` + +This would maintain the existing endpoint for editing Webhooks, but require the specifcation of an ID. + +* Good, because it removes ambiguities over which Webhook is to be edited +* Good, because it removes ambiguities over whether the Webhook is to be edited or created +* Bad, because its a breaking change +* Bad, because it doesn't match patterns used elsewhere in the API + +### 5c Editing of Webhooks: PUT/POST to `/service/webhooks/{id}` + +This would use a new endpoint for the editing of Webhooks. + +* Good, because it removes ambiguities over which Webhook is to be edited +* Good, because it removes ambiguities over whether the Webhook is to be edited or created +* Good, because it matches patterns used elsewhere in the API +* Bad, because its a breaking change + +### 6a Deleting of Webhooks: Existing POST to `/service/webhooks` + +This would maintain the existing approach to deleting Webhooks without any changes to the request body. + +* Good, because it doesn't require any changes +* Bad, because which Webhook a user wishes to delete may be ambiguous + * This may make this option impossible to implement +* Bad, because it overloads the POST method to enact a delete operation +* Bad, because it doesn't match patterns used elsewhere in the API + +### 6b Deleting of Webhooks: POST with `ID` to `/service/webhooks` + +This would maintain the existing endpoint for deleting Webhooks, but require the specifcation of an ID. + +* Good, because it removes ambiguities over which Webhook is to be deleted +* Bad, because its a breaking change +* Bad, because it overloads the POST method to enact a delete operation +* Bad, because it doesn't match patterns used elsewhere in the API + +### 6c Deleting of Webhooks: POST to `/service/webhooks/{id}` + +This would use a new endpoint for the deleting of Webhooks, but use the current approach of deleting by removing all events from the Webhook. +This option would require this method to be a POST due to the result of the request not matching the data sent in it. + +* Good, because it removes ambiguities over which Webhook is to be deleted +* Bad, because its a breaking change +* Bad, because it prevents users from retaining webhooks (e.g. to retain the keys) but suppressing events +* Bad, because it overloads the PUT/POST method to enact a delete operation +* Bad, because it doesn't match patterns used elsewhere in the API + +### 6d Deleting of Webhooks: DELETE to `/service/webhooks/{id}` + +This would use a DELETE method on a new endpoint for the deleting of Webhooks. + +* Good, because it removes ambiguities over which Webhook is to be deleted +* Good, because it uses an unambiguous DELETE method to enact a delete operation +* Good, because it matches patterns used elsewhere in the API +* Bad, because its a breaking change + +### 7a Backwards compatibility: Maintain backwards compatibility + +This option would see backwards compatibility maintained with the existing implementation, possibly with deprecation warnings, even if new workflows are added. + +* Good, because it maintains backwards compatibility +* Neutral, because breaking changes may be required anyway +* Bad, because it maintains patterns which don't match the rest of the API +* Bad, because some operations may become ambiguous and impossible to fulfil +* Bad, because some existing operations may not actually be possible to implement going forward +* Bad, because it perpetuates workflows that may have unintended results + * e.g. using the wrong `api_key_value` results in the creation of a new Webhook, instead of editing of an existing one +* Bad, because it may result in multiple possible workflows to achieve the same result + +### 7b Backwards compatibility: Do not maintain backwards compatibility + +This option would see backwards compatibility broken. + +* Good, because it makes Webhooks follow patterns elsewhere in the API +* Good, because it removes all possibilities for ambiguous operations +* Good, because it minimises the number of possible workflows to achieve a given result +* Neutral, because breaking changes may be required anyway +* Bad, because it breaks backwards compatibility