webhooks.net
webhooks.net copied to clipboard
Support hot-swapping of webhok secrets
Resolves #261 Resolves #24
(In draft until I finish implementing tests)
Behavior
Before the change?
- In both ASP.NET Core and Azure Functions, the webhook secret for the webhook processor must be specified when the processor is registered, and cannot be changed afterward. In addition, there can be only one secret specified.
After the change?
- A client now has the option of specifying a callback delegate to retrieve the list of secrets to validate against. This callback will be invoked for each request, so the results can change dynamically to allow rotation of the secret. In addition, the callback can return multiple secrets, with the request's signature required to match at least one of those secrets.
Other information
Since the request validation code that I was modifying was duplicated between the ASP.NET Core and Azure Functions packages, I took the opportunity to refactor the validation code to allow sharing (see #24). I chose to add the new ValidateWebhookRequestAsync()
method to the existing WebhookEventProcessor
class. It could go in a separate class, but I figured that most clients that process webhooks will also need to validate them. I'm tempted to also add a new ValidateAndProcessWebhookAsync()
method that does both the validation and the processing, since they take essentially the same parameters and will almost always be used together. I didn't go that far in this PR, though.
The parameter types for the new ValidateWebhookRequestAsync()
method include the request headers (as the same type used for ProcessWebhookAsync()
), the body Stream
, and the list of secrets to use. Allowing multiple secrets allows secret rotation to happen more smoothly, because the consumer can continue to process webhooks signed with the old secret for a while until everything starts using the new secret.
Passing the body as a Stream
seems to be the least common denominator between the ASP.NET Core and Azure Functions packages, and is likely useful outside of those two scenarios. I considered just passing the body as a string
, but that would make it more difficult to implement a defensive check against a maliciously large payload. That check was suggested in #24, but I have not implemented it here.
Validation errors are reported by throwing a WebhookValidationException
. This class contains a WebHookValidationError
enum that allows clients to determine exactly what went wrong. The two existing clients use this to determine what log message to emit, and what response text, if any, to return.
Additional info
Pull request checklist
- [ ] Tests for the changes have been added (for bug fixes / features)
- [ ] Docs have been reviewed and added / updated if needed (for bug fixes / features)
- [ ] Added the appropriate label for the given change
Does this introduce a breaking change?
Please see our docs on breaking changes to help!
- [ ] Yes (Please add the
Type: Breaking change
label) - [ ] No
If Yes
, what's the impact:
- N/A
Pull request type
Please add the corresponding label for change this PR introduces:
- Bugfix:
Type: Bug
- Feature/model/API additions:
Type: Feature
- Updates to docs or samples:
Type: Documentation
- Dependencies/code cleanup:
Type: Maintenance