Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTTP WebHook Specification #155

Merged
merged 13 commits into from
May 24, 2018
Merged

HTTP WebHook Specification #155

merged 13 commits into from
May 24, 2018

Conversation

clemensv
Copy link
Contributor

@clemensv clemensv commented Apr 12, 2018

This PR complements #148

This specification is a generic specification that formalizes delivery of notifications over HTTP, generally known as "Web hooks". The HTTP transport binding for Cloud Events composes with this specification for one-way event delivery.

The specification defines that notifications are delivered by HTTP POST, should use a token-based authNZ scheme either carried in the Authorization header or the query string (whereby "token" can be a simple key), and strongly suggests implementation of a pre-flight handshake (similar to W3C CORS) that protects a sender from being abused as a distributed denial of service machine gun.

This specification would probably be best proposed to IETF, but here's a good place to start with the draft.

This is not relevant to the 0.1 of the core spec, but is relevant to the interop group

Signed-off-by: Clemens Vasters [email protected]

Clemens Vasters added 9 commits April 11, 2018 18:37
Signed-off-by: Clemens Vasters <[email protected]>
Signed-off-by: Clemens Vasters <[email protected]>
Signed-off-by: Clemens Vasters <[email protected]>
Signed-off-by: Clemens Vasters <[email protected]>
Signed-off-by: Clemens Vasters <[email protected]>
Signed-off-by: Clemens Vasters <[email protected]>
Signed-off-by: Clemens Vasters <[email protected]>
Signed-off-by: Clemens Vasters <[email protected]>
Signed-off-by: Clemens Vasters <[email protected]>
information in the case of handling errors. This specification does not attempt
to define such a payload.

The response MUST NOT use any of the [3xx HTTP Redirect status codes][3xx] and
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious what's the rationale for this constraint? To reduce scope, make spec initially more focused?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Protection for the sender. I assume that externally supplied targets may be malicious and may want to send the sender on a neverending journey through redirects, so I disallow those outright. Also, the abuse protection only probes this site and not a site that is being redirected to. With redirects, you can basically sidestep the abuse mechanism with a static site that simply redirects all traffic to a DoS target. So this is security related from both angles.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense to me. I imagine there's other status codes that don't make sense, and headers. For example does it makes sense to return an ETag? If we assume the webhook use case is for a response to some event, then ETags would serve no purpose.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For that matter caching headers in general wouldn't make sense.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I understand the concerns, isn't this kind of harsh? I come across websites all the time that issue redirects for mindless reasons, like a missing "/" at the end of the URL, and most http clients just blindly handle it for you. Any reason we can't let the sender decide whether they want to support redirects or not?

@sslavic
Copy link
Contributor

sslavic commented Apr 13, 2018

Would it make sense, in addition to the explaining it with lots of text, to include also OpenAPI based contract stub of such notification endpoint, where HTTP request/message is CloudEvents with JSON binding compatible one?

http-webhook.md Outdated
[`Content-Type`][Content-Type] header.

If the delivery has been accepted and processed, but carries no payload, the
response MUST have the [204 No Content][204] status code.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be a problem to be more permissive? E.g. Google PubSub Push treats 200, 201, 202, 204, or 102 as ack that notification was successfully processed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to put a foot down here and make one profile that is "right". Our services don't implement all the rules here, yet, and we will have to make adjustments so I hope everyone else is also willing to. I allow 200, 202, and 204 here. 102 is not the end of the request, so I don't find that proper. 201 is debatable for adding alongside 200.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding 201.

@tomkerkhove
Copy link
Contributor

Apperently my PR breaks the DCO. How can I fix this for you guys?

@duglin
Copy link
Collaborator

duglin commented Apr 13, 2018

@tomkerkhove when you use "git commit" make sure you use the "-s" flag so its signed. You'll need to make sure your git config has your name and email correct too. See: https://github.com/cloudevents/spec/blob/master/CONTRIBUTING.md

to fix this... it might be easiest if @clemensv just squashed this down to one commit. The other option is to "--amend" your commit.

@tomkerkhove
Copy link
Contributor

@duglin I only found out after the fact, sorry for the inconvenience..

http-webhook.md Outdated

## 3. Authorization

The delivery request MUST use one of the following two methods, both of which
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"following two methods" --> "following two authorization methods" ?

More importantly, is this "MUST" requirement for delivery request and (in next sentence) for delivery target, with practically one option only, really necessary? This alone would eliminate (make non-compliant) many current OSS FaaS solutions, and any notification system which uses k8s+Istio RBAC for implementing security aspect.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that this isn't good wording. "It MUST use at least one of the two following options". That doens't preclude others. Also, the "Authorization" header with an arbitrary token is compatible with OAuth 2.0 and with practically all other models usig a bearer token. What does k8s RBAC use?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

App/workload logic is agnostic of the security aspect. Aspect like security is requested declaratively and enforced by Istio, where side-car container injected into protected workloads intercepts incoming and outgoing traffic to uniformly, typically across the k8s cluster, apply cross-cutting concerns like security (identity, authorization, mutual TLS), traffic management (weighted routing), tracing, monitoring. See https://istio.io/docs/concepts/security/rbac.html

Copy link
Contributor Author

@clemensv clemensv Apr 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked at this in the spec context again and now stand by the existing wording. The request MUST use either of the two options shown. If you have some magic interceptor thing, then that magic interceptor thng must make the request in aggregate compliant with the rule here before it gets to the target. This spec aims to define interop between a sender and a target that use open standards without coupling to some shared security framework.

I see that ISTIO provides a model to establish mutual TLS auth, but I do not understand how that established identity surfaces in an HTTP request other than through whatever proprietary context mechanism a web server gives you. If it's completely transparent also for the receiving endpoint we're effectively talking about unauthenticated HTTP plaintext at both ends that's aided by interceptor magic, and I don't know how to build a secure standard for the endpoints on top of that.

Crossreading ISTIO, it looks like a VPN-like container bridge. I don't think having that lets you off the hook from having a proper end-to-end security model.

http-webhook.md Outdated

The `WebHook-Request-Origin` header requests permission to send notifications
from this sender, and contains a DNS expression that identifies the sending
system, for example "eventemitter.example.com". The value is meant to summarily
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Who/what guarantees this identity really belongs to the request sender?

Would it make sense to build on top of SPIFFE/Spire/Istio?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am trying to explain here that Origin is not about identity. Authorization takes care of establishing identity where needed.

There are three roles here: (a) sender, (b) target, and (c) the 3rd party that tells the sender to send to the target. The threat vector here is that (c) tells (a) to send to (b) without (b) being ok to receive events from (a).

If (b) is an plain website running on nginx without authentication turned on, it's not going to benefit from any fancy identity model. What protects it from unwanted push traffic is that it simply doesn't respond to the abuse handshake with a positive acknowledgement. "Origin" allows a site to differentiate between different senders within the scope of this mechanism. Since the mechanism is designed to mitigate the threat explained above, Origin doesn't need further protection, because (c) can't manipulate it.

If I am operating (a) as "push.fabrikam.com" and were to steal someone else's identity, say "push.contoso.com" for the origin, because (b) has that name whitelisted but not mine, I'm not gaining any better access because I still need proper credentials. But I just allowed (c) to abuse my service to bombard (b) with invalid requests and therefore risk blacklisting of my originating IP address. So making a false statement about "Origin" is actually self-defeating with what this mechanism is aiming at.

If Origin needs to be further authenticated, that should be done using the existing authNZ model, e.g. you could use TLS client auth to establish the identity, or you could include the Origin value into the bearer token.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit torn here because this specifies a perfectly valid model and I like the idea of dynamic rate limiting. A few questions though:

  1. Why not actually reuse the CORS headers here?
  2. Many clouds implement this by requiring (c) to prove ownership of (b) before allowing (a) to point to (c). Should such a cloud have a separate auth mechanism just for cloud events?

@clemensv
Copy link
Contributor Author

@sslavic regarding the OpenAPI spec - yes, I think that would make sense to have. Do you want to take a stab at creating an OpenAPI schema that reflects the doc?

http-webhook.md Outdated
The response MUST NOT use any of the [3xx HTTP Redirect status codes][3xx] and
the client MUST NOT follow any such redirection.

If a delivery target has been retired, but the HTTP site still exists, the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious how a sender should process a 404 differently from a 410

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

410 is "there was something here but it's gone now". That is different from 404 "there's nothing here". 404 is a likely config error, 410 is outdated config.


If the delivery target is unable to process the request due to exceeding a
request rate limit, it SHOULD return a [429 Too Many Requests][429] status code
and MUST include the [`Retry-After`][Retry-After] header. The sender MUST
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be nitty, though I'm curious whether a distributed sender MUST coordinate/interlock so that whenever one shard gets a 429, all shards must respect that immediately.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's realistic or necessary to expect perfect consistency across multiple nodes that act as if they were one sender. If there are 4 concurrent requests in flight from 4 nodes, each of them is presumably going to get a 429 each and will subsequently back off. For the protocol, multiple senders acting as one doesn't really exist. You'll be smart to have an eventually consistent gossip (or other mechanism) to tell your push agents about a target currently throttling, so that you can move on to other work while the condition persists. That is ultimately what Retry-After helps with.

[`Content-Type`][Content-Type] header.

If the delivery has been accepted and processed, but carries no payload, the
response MUST have the [201 Created][201] or [204 No Content][204] status code.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A number of new web developers send 200 with Content-Length 0. Is there a reason to explicitly call these non-compliant?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because they are not compliant. HTTP reserves 204 for when you have no payload. I can't relax the rules of HTTP here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@clemensv according to the HTTP spec any Content-Length greater than or equal to 0 is a valid value. See here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@inlined @glennblock the normative spec for that is now RFC7230 which has superseded RFC2616. The relevant section is here. I agree that the wording distinguishes between an absent and an empty body in the context of allowing for POST with no payload. There is also this SHOULD rule:

"A user agent SHOULD NOT send a Content-Length header field when the request message does not contain a payload body and the method semantics do not anticipate such a body."

The HTTP spec isn't clear here about the recommendation for responses where there is no payload.

My position is that Content-Length:0 with 200 OK is therefore an edge case I'd like to resolve cleanly for this spec as not being permitted and fof 204 to be used instead.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing to the updated spec. I agree that it is an edge case. As this is a new standard, there's no problem with setting some firmer ground rules.

Copy link
Contributor

@sslavic sslavic May 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In his webinar about callback support in OpenAPI 3.0, @fehguy suggests to use 204 response as one way to signal that receiving party is no longer interested in further updates, that it that callback to be unregistered from web hook (as alternative to explicit API for unregistering callbacks/subscriptions).

Proposed 410 has some more desired meaning associated, still don't like part that it's a 4xx client error code - client made no error in attempting delivery.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sslavic I don't agree that 204 telegraphs any such intent, ever. If you want to terminate a relationship you need to reach into the 3xx bucket.


## 3. Authorization

The delivery request MUST use one of the following two methods, both of which
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is OAuth2 the only valid auth? E.g. GitHub uses content signatures in their webhooks. APNs uses connection-level auth by only allowing connections from known certificates.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"The client MAY use any token-based authorization scheme. Challenge-based
schemes MUST NOT be used."

The point of this spec is not to document all variations of WebHooks presently in use, but rather define a straightforward model with a core set of options. You can form a token with a content signature and flow that in the Authorization header under its own scheme and be compliant.

OAuth 2.0 provides a solid summary of how to do the token flow via Authorization header or via query string and that is why I lean on it.

The reason I'm not including client certificates is that they're notoriously terrible to manage. If/when the group thinks there is concrete need for client cert auth, then we should add it.

Copy link
Contributor

@rperelma rperelma May 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For Adobe I/O Events, we use the client secret, not a client certificate, for producing a content signature. We already have to manage client secrets created during clientID generation, so no added burden.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @inlined and made the same comment on the workgroup call. GitHub uses HMAC / symmetric secret and this is adopted by other platforms also like Patreon.

The delivery request MUST use one of the following two methods

What does this mean? Does it mean that if using auth we must implemented either - or that auth must be done and it must be either? @clemensv

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexellis I added the following clause yesterday:

"The client MAY use any token-based authorization scheme. The token can take any shape, and can be a standardized token format or a simple key expression."

That means (and meant) using any key or an HMAC using a key is completely legitimate. I don't define or mandate a specific token format here. Azure Functions generates a symmetric key and requires that key for its default implementation that you get from "file new", and that would be conforming. What I need to mandate for interop is that the query argument has a well-known name and that's mostly what I do here.

target needs to indicate that it agrees with notifications being delivered to it.

Reaching the delivery agreement is realized using the following validation
handshake. The handshake can either be executed immediately at registration
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: the language "either... or" is a false dichotomy. E.g. a K8S style API might call this endpoint both at resource creation and periodically out of band.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the requirement for doing the handshake twice?

http-webhook.md Outdated
WebHook-Request-Origin: eventemitter.example.com
```

#### 4.1.1.2. WebHook-Request-Rate
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rate limiting might apply to the endpoint (e.g. the Tweet analyzer subsystem can only support 1500QPS) or to a client as a whole (each twitter user can only send 50req/hr). I wonder if we need an additional header to indicate the scope on which the rate limit applies.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The request rate applies to this exact target resource. If the resource has some composite rules, it ought to do the math and give the sender a simple answer rather than complicating matters by giving two answers that may be conflicting. Keep it simple.

http-webhook.md Outdated

The `WebHook-Request-Origin` header requests permission to send notifications
from this sender, and contains a DNS expression that identifies the sending
system, for example "eventemitter.example.com". The value is meant to summarily
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit torn here because this specifies a perfectly valid model and I like the idea of dynamic rate limiting. A few questions though:

  1. Why not actually reuse the CORS headers here?
  2. Many clouds implement this by requiring (c) to prove ownership of (b) before allowing (a) to point to (c). Should such a cloud have a separate auth mechanism just for cloud events?


``` text
WebHook-Request-Rate: 120
```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This webhook seems like a reasonable place to negotiate all kinds of features:

  1. Valid timeout for processing?
  2. Valid data types (e.g. schemas, versioning)
  3. Accepted encodings

In general I've held back on these features since we've put the control plane APIs out of scope for now. This PR suddenly puts these (very important) features back in scope.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressing both comments at once:

First:

  1. I cannot use the CORS headers, because this is a very different problem to solve, and I can't interfere with the actual CORS use-case
  2. The point of this mechanism is to provide a common path and not document everything everyone does now. Microsoft will have to make changes for this as well.

Second:

Further negotiations could indeed take place in this handshake, and they should lean on HTTP constructs. We could, for instance, have a reversing HTTP Accept handshake here, meaning that the target gets to declare what format it would like. I don't think that's critical for a first stab, but it would be good to play through and think about what to add. We have the CloudEvents versioning strategy and we do have preliminary media types.

@glennblock
Copy link
Contributor

glennblock commented Apr 21, 2018

Good to see this @clemensv! Webhooks are a central part of the products I work on. Today whenever the term Webhook comes up it is understood as a general pattern. There are rules of the road, but they haven't really been written (there's no standard), thus this is nice to see. I'll add some comments to the draft as appropriate.

As to the products, Webtask.io is a free developer tool / environment for creating Serverless endpoints. Webhooks are one of the most common use cases. Webtask is generally the backend for handing notifications from a Service that fires off events, Github, Slack, and Stripe are three of the very common use case both for receiving and invoking.

Second we have a premium product Extend which is a Serverless Webhook offering which can be "embedded" within a SaaS product to allow users to have a native experience within the product for handling application events. Our Auth0 Identity product depends on it, as do many other SaaS companies.

http-webhook.md Outdated

## Abstract

"Web hooks" are a popular pattern to deliver notifications between applications
Copy link
Contributor

@glennblock glennblock Apr 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would just go with webhook over Web Hook or WebHook. It is the most common usage these days i.e. Wikipedia, Github, Stripe, Slack

http-webhook.md Outdated

"Web hooks" are a popular pattern to deliver notifications between applications
and via HTTP endpoints. In spite of pattern usage being widespread, there is no
formal definition for Web Hooks. This specification aims to provide such a
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it is meant to be generic, why not split it out as its own draft?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is its own draft. Microsoft would like to take this specific spec to IETF.

http-webhook.md Outdated

## 1. Introduction

"Web hooks" are a popular pattern to deliver notifications between applications
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest mentioning Jeff Lindsay's post on this: http://progrium.com/blog/2007/05/03/web-hooks-to-revolutionize-the-web/. He coined it as "Web Hooks" then but since has also adopted the term "Webhook".


The HTTP method for the delivery request MUST be [POST][POST].

The [`Content-Type`][Content-Type] header MUST be carried and the request MUST
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if data is specified via the query string and there is no payload? Are you saying every webhook MUST send a payload? If so, why?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@glennblock you've pointed to the Content-Length rule yourself, Glenn. POST requires a payload body, but that body may be empty and even if it's empty you probably still need to declare it. That said, in the interest of trading choice for interop, I do indeed believe that a payload body can be mandated for a webhook request.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My reason for asking was for simplicity, but I have no fundamental issue with requiring it. Most of the established Webhooks I am familiar with do send a payload. I have seen some that do not, but they are much less common.

If the delivery has been accepted, but has not yet been processed or if the
processing status is unknown, the response MUST have the [202 Accepted][202]
status code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For 202, maybe go further and require a Location header specifying where the new resource will live OR a status monitor.

Copy link
Contributor

@glennblock glennblock Apr 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I am suggesting this is to provide a standard way to handle async webhooks, where the client expects a response, but can handle it out of band. This would be a pattern that can easily be encapsulated within SDKs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@glennblock For the status monitor case, I would rather loosen the redirect rule and permit 302. You'll be redirected to a status resource with "Retry-After", possiby repeatedly, until the request is done and then you get 200/201/204.

202 were "fire and forget".

http-webhook.md Outdated
"Web hooks" are a popular pattern to deliver notifications between applications
and via HTTP endpoints. Applications that make notifications available, allow
for other applications to register an HTTP endpoint to which notifications are
delivered once available.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does the "once available" relate to the endpoint or the notification? I wonder if we could drop the "once available" entirely.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

http-webhook.md Outdated
#### 4.1.1.2. WebHook-Allowed-Rate

The `WebHook-Allowed-Rate` header MUST be returned if the request contained
teh `WebHook-Request-Rate`, otherwise it SHOULD be returned.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: teh

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks

http-webhook.md Outdated
no rate limitation. An integer number expresses the permitted request rate in
"requests per minute". For request rates exceeding the granted notification
rate, the sender ought to expect request throttling. Throttling is indicated
by requests being rejected using HTTP status code [429][429].
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HTTP 429 Too Many Requests

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really see what is wrong here, it's referring to the spec here so looks ok?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done (adding the reason phrase for clarity)

http-webhook.md Outdated

The following header fields MUST be included in the validation request:

#### 4.1.1.2. WebHook-Request-Origin
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

section number: 4.1.1.1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Numbering is off. Fixing.

http-webhook.md Outdated
Host: server.example.com
```

The HTTP request URI query can include other request-specific parameters, in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replace "can" with "MAY"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

http-webhook.md Outdated
status code.

If the delivery cannot be accepted because the notification format has not
been understood, the service MUST respond with status code [415][415].
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

415 Unsupported Media Type

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

To protect the sender from being abused in such a way, a legitimate delivery
target needs to indicate that it agrees with notifications being delivered to it.

Reaching the delivery agreement is realized using the following validation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a section: 4.1 Registration/Validation Handshake

Copy link
Contributor

@cneijenhuis cneijenhuis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work 👍

I think a detailed specification for webhooks is a step forward, and I am in support of submitting this draft to the IETF.

I appreciate that you've thought of several edge cases, and specify standard solutions for authentication and abuse protection.

What makes webhooks attractive is that they are quick to implement. I like that the abuse protection isn't required, and I think the same could be considered for the Authorization. At least, the URI Query Parameter could be made optional (or removed altogether).

There is another set of edge cases that I haven't seen common best practices on when I looked at popular (sender) implementations of webhooks. I'm coming from a commerce background. At-least-once delivery is extremely important, timely delivery can be important, debuggability is important. (As an example, take food delivery: The sender is the web shop and notifies the delivery target about a food order, which informs the restaurant that it has to prepare a meal)
In such a scenario, you want to be able to quickly notice if the sender can not deliver at all or at a reduced rate (the sender could offer a health or metrics endpoint), or if a specific event has been tried multiple times, but repeatedly resulted in a 4xx or 5xx error (aka a dead letter queue). These are standard features for message queues. (I blogged about it here in more detail)

I can completely understand if you want to keep this spec lightweight and not address these edge cases - however, if you are interested, I'm happy to collaborate and submit a proposal.


If the delivery has been accepted, but has not yet been processed or if the
processing status is unknown, the response MUST have the [202 Accepted][202]
status code.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two paragraphs above, especially 204 and 202, could explain what the expectation is for the sender (like it was done for 410 and 429).

There are use cases where the sender expects a response. Since it is not specified, one could assume that the sender MUST accept 202 or 204. I would prefer to explicitly state that the sender MAY accept them, and SHOULD accept them if it is a "fire and forget" type of request.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After submitting, I saw that the PR description says the spec is for "one-way event delivery".

I didn't find such a claim in the spec itself (unless you interpret "notification" as only applicable to past events). I understand how focusing on one-way delivery makes sense in the context of CloudEvents, but - if this spec is to be used for webhooks in general - I think you should consider the case of the delivery target sending a proper response that the sender uses to change its behaviour.

FWIW, the wikipedia definition of a webhook includes it as well: "A webhook [...] is a method of augmenting or altering the behaviour of a web page, or web application [...]."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cneijenhuis If you can get a proper response, that's arguably just another HTTP request. RFC7231 covers that exhaustively. The point if this spec is to constrain RFC7231 to a profile that everyone agrees to for one-way event delivery, i.e. callbacks.

Here's the place where we all make the spec and then I'm glad if someone summarizes that on Wikipedia - not the other way around :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're selling yourself short here :) Section 3. and 4. are as relevant for a webhook that returns a response, as they are for a webhook for one-way event delivery.

The changes needed are small, I'm happy to prepare something!

I quoted Wikipedia because it captures my understanding of the term webhook - it's not about one-way event delivery per se. It is about being able to connect two systems via the web, but instead of the usual "client calls API", it's the other way around: "API calls client".

One-way event delivery is the more common way to use a webhook, so if you look at e.g. Stripe or GitHub, it'll appear as if webhook is synonymous with it. That is why the Wikipedia quote is good: It doesn't try to define webhook as used by a single vendor, but how it is used across the industry.

Some examples where webhooks are used for responses:

This Twilio tutorial has a great use case where a synchronous response is needed: If you have a caller on the phone, you can't keep him waiting forever. I guess @glennblock has many more use cases 😉

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes @cneijenhuis, it is not uncommon to have synchronous responses. Auth0 rules, for example, are synchronous. At the time of login, custom logic runs in rules (which are webhooks) which could prevent the login. We have many other synchronous use cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cneijenhuis @glennblock I understand that synchronous responses are being used, but CloudEvents defines a unidirectional and routable event flow (on top of which you can model async responses). The examples you cite are extensibility points where the web hook call performs a command execution in context. Those are not an events; events are facts, commands are intents.

Mind the headline here: "HTTP 1.1 Web Hooks for Event Delivery"

There might quite well be a derivative or parallel spec "HTTP 1.1 Web Hooks for Extensiblity Commands", but I would like to keep this one scoped to the one-way case that we keep symmetric to what we can easily achieve in AMQP, MQTT, NATS, etc.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to keep this one scoped to the one-way case that we keep symmetric to what we can easily achieve in AMQP, MQTT, NATS, etc.

While I think it wouldn't be too hard to include the two-way cases, I fully understand that you want to keep the scope as small as possible.

If you allow me to make a final request, though, it would be to have the scope more explicitly in the title, e.g. "Web Hooks for Asynchronous Event Delivery", or "Web Hooks for One-Way Event Delivery". Let me try to explain:

The examples you cite are extensibility points where the web hook call performs a command execution in context. Those are not an events; events are facts, commands are intents.

I have to disagree. All three examples actually send events, or facts, via the webhook.

  • Twilio: Caller with ID foobar has called
  • Auth0: User foobar has made a login attempt
  • Dialogflow: User foobar has said: "Book a flight from LA to London"

These are arguably facts - events that have happened. If the use cases wouldn't require an immediate response, it would also be fine to implement them as asynchronous webhooks.

E.g. for a chatbot, both options work well:

  • Chatbot receives event via asynchronous webhook. Chatbot responds by calling API.
  • Chatbot receives event via synchronous webhook. Chatbot responds directly.

The event data would be exactly the same in both cases! Therefore, I think it is worth pointing out in the title that the spec is scoped to asynchronous/one-way event delivery.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cneijenhuis I'm going to note that as an issue. I'm not yet sure whether clarifying the spec intent or cracking the door open for response is really the right solution. For an IETF spec, the latter might be better, for CloudEvents I'd like to keep the scope small.

http-webhook.md Outdated
observe the value of the Retry-After header and refrain from sending further
requests until the indicated time.

If the delivery has been accepted and processed, and if the response carries a
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I think this section would read nicer if it starts with the happy cases (2xx responses) instead of the failure cases (3xx and 4xx responses). (Basically, move line 94 to 104 up to line 84)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. Fixing.

http-webhook.md Outdated
method being permitted. Other methods MAY be permitted on the resource, but
their function is outside the scope of this specification.

#### 4.1.1.2. WebHook-Allowed-Origin
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4.1.1.1 here as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The numbering is off. Fixing.

Because of the security weaknesses associated with the URI method (see
[RFC6750, Section 5][RFC6750]), including the high likelihood that the URL
containing the access token will be logged, it SHOULD NOT be used unless it is
impossible to transport the access token in the "Authorization" request header
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a use case in mind where it is impossible to use the Authorization header?

In several places, the spec is close to requiring the delivery target to understand (custom) HTTP headers, e.g. in 4.1.1.2: "The WebHook-Allowed-Rate header MUST be returned if the request contained the WebHook-Request-Rate, otherwise it SHOULD be returned."

Personally, I'd prefer to not allow the URI Query Parameter, or at least make it optional.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are plenty of sites where you can configure web hooks that will allow you to paste a URL and nothing else. Having a token right in the URL is simply the easiest way to give someone a reference with embedded access permission. So it's not neccessarily about not having access to the header per-se (which still is an occasional problem w/ some APIs), but about the information being split across two places. The feature is already optional for the client, since that's where those concerns arise. The server shouldn't have a opt-out choice given that it is a client consideration and might be a constraint for which the client has no alternative.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The feature is already optional for the client, since that's where those concerns arise.

I don't agree it is only a concern for the client, and it's been an issue for me several times (e.g. with IronMQ urls, and with Azure Functions - both default to tokens in the URI).

Our clients entrust us with their access tokens, and they should be assured we keep them safe (similar to e.g. passwords, or personal data of our clients customers).
We have policies to never log passwords, or personal data, or similar. Access to our logs is not heavily restricted, allowing many people within our company to quickly gain insights if things go wrong (e.g. support).
If my code logs the access token, I am a) basically violating company policy if you assume access tokens are a form of a password, and b) make the access token available to a broad range of people within my company, that are not allowed to access critical data.

(As a commerce system, stealing an access token may allow the attacker to e.g. trigger a refund, or trigger the shipment of an order before the payment is confirmed - so the threat isn't purely theoretical)

Fair enough, then don't log the URI, right? Turns out it is virtually impossible to assure if you are using an HTTP lib on the JVM (I'll spare you the details...).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having a token right in the URL is simply the easiest way to give someone a reference with embedded access permission.

I totally agree.

Here is a compromise: The delivery target is allowed to register the webhook with the access token in the URI. The sender MUST send the access token, but it SHOULD use the Authorization Request Header Field instead of the URI Query Parameter.

(I guess this is technically already fine via The delivery target MUST support both methods., but it would be surprising for the client)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cneijenhuis In our service, we have "Connection Strings" that have URLs and AuthNZ material in them, and we then split that information across query string and headers as needed.

I like the rule you stated above, but it's right on the line of getting into the hair of how an implementation does its config. We don't mention the actual subscription gesture in any spec (and per earlier group discussions it'll be quite difficult to harmonize it), and therefore it's difficult to impose constraints on it.

I already make a pretty strong and normative SHOULD NOT statement against the URI method, including giving caching guidance, and that effectively says what you say here. How about we amend that with:

"Even if the client has been configured with a token embedded in the URI query string conforming with 3.2, the client SHOULD extract and remove the token from the query string and send it via the Authorization request header field whenever feasible."

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@clemensv Sounds great 👍

Minor: the client SHOULD extract should probably be the sender SHOULD extract?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cneijenhuis Noting as issue for the next edit pass.

http-webhook.md Outdated
WebHook-Request-Origin: *
```

#### 4.1.1.2. WebHook-Allowed-Rate
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am torn on this one. I can see where it is coming from, and there are certainly use cases where it can be a good idea. On the other hand, with 429 we already have a way for the delivery target to tell the sender to slow down.

And having a fixed limit comes with problems:

The delivery target may set 100 req / min. It is generally fine if there are peaks above that, the sender delays events for a few minutes or even an hour or two. But if the events are being produced constantly at a higher rate (e.g. 200 req / min), requests will pile up indefinitely.
There is no way (in the current spec) for the sender to inform the delivery target that something is wrong. Therefore, the delivery target can not (easily) monitor it's own system, and auto-scale it accordingly.
If the sender would increase the delivery rate, and the delivery target returns a 429, there is a common understanding of both parties that the delivery target is at fault. The maintainers of the delivery target can monitor their responses, and take action. If they don't, the sender can take action in a pre-agreed manner (e.g. suspend the webhook, or shed messages older than a day).

Even worse, if the delivery target realizes it has been too slow, and scales resources - how does it tell the sender to increase the request rate? Registering a new webhook would not affect the requests already "locked" in the current registration.

TLDR: Using 429 for rate limiting establishes a common understanding between both parties, enabling both sides to take appropriate actions if the delivery target is too slow for a longer period of time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I already call out usage of 429 in section 2.2.

The problem I am trying to address here is that the publisher doesn't suddenly show up with a mortal flood of events that takes out the target outright. 429 is reactive. This here aims to prevent the negotiation apriori.

@duglin duglin added the v1.0 label May 10, 2018
a registration. In extreme cases, a notification infrastructure could be abused
to launch denial-of-service attacks against an arbitrary web-site.

To protect the sender from being abused in such a way, a legitimate delivery
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Across the industry (facebook, dropbox, slack, etc.) the most common (and much simpler) approach for URL validation is to send a challenge. I realize you don't want to use a challenge for authorization, but I believe it is perfectly acceptable for validation. If the URL is invalid, or the delivery target doesn't want to accept a webhook call, it simply doesn't respond to the initial challenge, and in that case the sender MUST NOT deliver events to the target.

Copy link
Contributor Author

@clemensv clemensv May 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point here is that we need a standard way to express the opt-in. Everyone does these things, but each in their own way. The opt-in can be a reply or the opt-in can be a callback to a URL; either one is fine and we can also offer both.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rperelma I just added a callback option.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@clemensv thanks, but not quite what I had in mind :)
I was referring to a simple challenge sent to the target, and the target responding (or not) to that challenge, thereby accepting (or not) further deliveries. This is a very widely supported approach across the industry.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Facebook has this for example: https://developers.facebook.com/docs/graph-api/webhooks/getting-started see the section on verification.

Copy link
Contributor Author

@clemensv clemensv May 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gblock @rperelma that's exactly what this mechanism is, though. The client sends a string (WebHook-Request-Origin) and the server echoes that string (WebHook-Allowed-Origin) if and only if it's ok with that source pushing. I do that on OPTIONS, because that's what OPTIONS is for (also see W3C CORS). The rate negotiation is optional.

What I believe I need for a multi-party interop standard is a way for the target to not only echo back some arbitrary string, but for the target to also have a way to make semantic sense out of that string, because the webhook might not just be written for a single client system, but for multiple different client systems and the target may want to be able to differentiate between those.

The origin challenge might be "instance-a35f7d9e.service.example.com" and the target may have a rule to permit ".example.com" and then grants access by echoing the origin phrase. The implementation of the target might also be ok with anyone pushing to them and then just replies with "" out of a dumb response rule.

If I could mandate a tighter authorization model (which I would if this were a product and not a standards design), I would even tie the Origin into the access token. I can't do that here, because there's appears to be majority who work with simple symmetric keys.

Signed-off-by: Clemens Vasters <[email protected]>
http-webhook.md Outdated
The client MAY use any token-based authorization scheme. Challenge-based
schemes MUST NOT be used.

### 3.1. Authorization Request Header Field
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: Two white spaces between 3.1. and Authorization.

The delivery request MUST use one of the following two methods, both of which
lean on the OAuth 2.0 Bearer Token [RFC6750][RFC6750] model.

The delivery target MUST support both methods.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why must the target support both? This is imposing implementation detail on the FaaS receiver.

Copy link
Contributor Author

@clemensv clemensv May 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexellis Because only the client knows whether it can or cannot put a token into the header through its client framework or service platform. From a security hygiene perspective, headers are the preferred method because they're not dragged through all of the most common logs as query strings are. It's a client choice and therefore the server needs to support the client making that choice. Yes, it's imposing a rule on the FaaS receiver; that's what the whole spec is trying.

Signed-off-by: Clemens Vasters <[email protected]>
available, allow for other applications to register an HTTP endpoint to which
notifications are delivered.

This specification defines a HTTP method by how notifications are delivered by
Copy link
Contributor

@sslavic sslavic May 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delivery/receiving callback requests is just part of the Web hook mechanism. As web hook was originally covered in @progrium's blog

To support web hooks, you allow the user to specify a URL where your application will post to and on what events.

So far actual hook API part and related protocol like:

  • API how callbacks are to be registered
    ** how should callback URL attribute be named
    ** how one defines for which events should callback URL be called (potential CloudEvents attribute usage?)
    ** negotiating content type for web hook callback
    ** verifying callback URL is valid and owned by the calling party
    ** which QoS is expected (delivery guarantee, retry constraints, ordering, max number of events in parallel, delivery timeout, ...)
    ** have can all of this information be exchanged (query params, request body, via some 3rd party storage/API like k8s API CRD, ...)
  • how can receiving party communicate it's no longer interested in the events
  • how can delivery party share and interested party discover for which events hook can be registered, etc.
    are not covered, yet.

Would it make sense to explicitly mention that some aspects of Web hook mechanism for reasons have been deliberately left out for now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am indeed very deliberately leaving quite a few aspects mentioned above, except for the abuse protection handshake, because I don't think we can standardize them at this point.

For instance, in Azure, you are registering webhooks into Event Grid through the Azure Resource Manager API, as a subscription on a topic supplying filters, and that's a single interaction in a large framework of system-wide conventions applying to thousands of APIs.

It's presently not realistic to expect for that interaction to be superseded by an API standard that this WG comes up with. First, because of the greater platform convention context. Second, because we're far from ready to commit to constraints for such an API.

the given URL. The HTTP GET request can be performed manually using a browser
client.

The delivery target MAY include the `WebHook-Allowed-Rate` response in the callback.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is WebHook-Allowed-Rate optional (MAY) for async handshake confirm while in 4.2 it's mandatory (MUST)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't make that MUST, because then I can't use a browser to do the confirmation. There may be a clause missing here what the omission of the value means.

The delivery target MAY include the `WebHook-Allowed-Rate` response in the callback.

The URL is not formally constrained, but it SHOULD contain an identifier for the
delivery target along with a secret key that makes the URL difficult to guess so that
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe suggest it should be temporary secret key.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think "temporary" adds that would otherwise be misunderstood by an implementer?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO these tokens, just as whole subscription request if not confirmed through handshake, should be expiring after some time. Without mentioning these are temporary, it may be assumed that one can confirm at anytime in the future, subscription requests and tokens to be persisted endlessly until confirmed back.

instances that act on the behalf of a certain system, and not an individual
host.

After the handshake and if permission has been granted, the sender MUST use the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use WebHook-Request-Origin only for abuse protection handshake request? Would it make sense to use Origin both for the handshake and later on delivery? The two requests are already different enough, by HTTP method - handshake uses OPTIONS and delivery uses POST.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what the 'Origin' header would add for the handshake itself. It is mandated for the delivery request.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently it's written that:

The `WebHook-Request-Origin` header MUST be included in the validation request
and requests permission to send notifications from this sender, and contains a
DNS expression that identifies the sending system, for example
"eventemitter.example.com"

After the handshake and if permission has been granted, the sender MUST use the
`Origin` request header for each delivery request, with the value matching that
of this header.

If I understood correctly, in handshake/OPTIONS request event emitter has to send WebHook-Request-Origin header with value equal to the hostname of Origin header that it will be sending in delivery/POST requests.

Dilemma I have is why not send Origin during handshake with same value that will be used later in delivery, why new custom header when standard Origin header will be sent anyway during handshake too and probably with same value as during delivery and from it hostname can be extracted if just hostname is needed?

@darrelmiller
Copy link

darrelmiller commented May 24, 2018

Here is a first stab at capturing the essence of the spec as an OpenAPI V3 document. I have only done the auth header scenario just to stop it from getting any longer than it already is.

openapi: 3.0.0
info: 
  title: Example Web Hook based on Cloud Events spec
  version: 1.0.0
paths:
  '/SubscribeToEvent':  
    'post':    
      summary: Subscription operation. (Not defined by specification)
      parameters:
        - name: deliveryTarget
          in: query
          schema:
            type: string
            format: uri
      responses:
        '201':
          description: Successfully subscribed
      callbacks:
        exampleWithAuthHeader:
          '$request.query.deliveryTarget':
            'post':
              description: Delivery Request
              parameters:
                - name: Origin
                  in: header
                  required: true
              requestBody:
                required: true
              responses:
                '200':
                  description: Delivery Response with payload
                '201':
                  description: Created Notification with or without payload
                '202':
                  description: Accepted Notification Delivery
                '410':
                  description: Unsubscribe
                '415':
                  description: Delivery request payload media type not understood
                '429':
                  description: Too many requests
                  headers:
                    'retry-after':
                      required: true
                      schema:
                        type: integer
              security:
                - callbackToken : []
            'options':
              description: Validation Request
              parameters:
                - name: WebHook-Request-Origin
                  in: header
                  required: true
                - name: WebHook-Request-Callback
                  in: header
                  required: false
                  schema:
                    type: string
                    format: uri
                - name: WebHook-Request-Rate
                  in: header
                  required: false
                  schema:
                    type: integer
                - name: Allow
                  in: header
                  schema:
                    type: string
                    default: POST
              responses:
                200:
                  description: Validated
                  headers:
                    WebHook-Allowed-Origin:
                      required: true
                      schema:
                        type: string
                    WebHook-Allowed-Rate:
                      required: false
                      schema:
                        type: integer                   
components:
  securitySchemes:
    'callbackToken':
      type: http
      scheme: bearer

@sslavic
Copy link
Contributor

sslavic commented May 24, 2018

@darrelmiller thanks for sharing. I had a take as well and here's gist of what I have so far: https://gist.github.com/sslavic/b1f36a8fa4ac0a52568b7b0155c3f241

Using OpenAPI it's really hard to define reusable/standard part - it seems only thing possible is to express some examples, like one you shared of event emitter example API and there can be matching example of event consumer API.

It would be different story, if spec was extended to cover not only delivery part - it's already going to some extent into subscription API as well, with handshake.

@nicolaferraro
Copy link

I like the attempts to provide a OpenAPI V3 document for the spec. And I really love the idea of adding a subscription API, or better writing a new spec (it seems outside the scope of the webhook delivery).

A standard API to list the published event types and handle subscription (subscribe, unsubscribe, checkSubscription). The cloud will be a lot more connected if we reduce fragmentation 😄.

Notifications are delivered using a HTTP request. The response indicates the
resulting status of the delivery.

HTTP-over-TLS (HTTPS) [RFC2818][RFC2818] MUST be used for the connection.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While obviously https is a good, this implies that an in-house impl that doesn't use https is non-compliant. Is that the intent?

to define such a payload.

The response MUST NOT use any of the [3xx HTTP Redirect status codes][3xx] and
the client MUST NOT follow any such redirection.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we may want to be consistent in our terms, sender vs client for example. Obviously non-blocking comment :-)

The response MUST NOT use any of the [3xx HTTP Redirect status codes][3xx] and
the client MUST NOT follow any such redirection.

If the delivery has been accepted and processed, and if the response carries a
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section appears to be repeating the http specs. While my initial reaction was to suggest that we remove it because repeating specs is usually not a good idea, highlighting certain key aspects for people so they don't have to read the entire set of specs can be very useful. However, can we add a sentence that says any conflict between this spec and the http specs results in the http specs winning?

@duglin
Copy link
Collaborator

duglin commented May 24, 2018

I don't think this PR has been changed in about a week. And while there are still some questions I don't see any that are significant enough that the questioner is suggesting that the entire PR is headed in the wrong direction - they appear to be more targeted points of discussion. To that end, I'd like to suggest that we merge this PR and then people can open focused issues/PR to deal with any lingering concerns.

What do people think? If you can not make the call today and object to this suggestion please voice your concern here.

@duglin
Copy link
Collaborator

duglin commented May 24, 2018

Approved on the 5/24 call. Will deal with any open questions/concerns in follow-on PR/Issues.
If you'd like to see a change in this, or any doc, please open a PR with the proposed text you'd like to see.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.