Adding webhook response on transcribing success and failure

[anderser]

Tests not run here as I had trouble getting VS code run the dev container. Please review and check code. This is fast and maybe sloppy work :)

[anderser]

We might consider adding some kind of signing and security to these webhooks. I.e. https://rednafi.github.io/reflections/verifying-webhook-origin-via-payload-hash-signing.html or https://github.com/cloudevents/spec/blob/main/cloudevents/http-webhook.md

This will increase the complexity and require the webhook receivers to do more work validating and responding in the right way, but will increase security and/or integrity.

[jgorset]

🤩

[anderser]

I suggest we add some more security/integrity features to this before merging.

• Allowlist of approved webhook urls with corresponding id and token
• On transcribing-request, we check if webhook id given in parameter is in allowlist. None valid webhook ids should result in a Not Allowed response or similar to the client.
• The client can also send an externalRef parameter which is posted back with the webhook.
• When transcribing is done, fetch the webhook url and token based on the webhook id
• We add a HMAC hash of the payload to the request header based on the token and post to the webhook url (https://prismatic.io/blog/how-secure-webhook-endpoints-hmac/)
• The client needs to have the secret token to verify the content integrity (if they wish to do so)

[olekenneth]

I was thinking about just adding a JWT and sending it as a Authorization-header that way the client can check/validate the JWT if we just post the public key on jojo.schibsted.com/jwks like SchAcc does here: https://payment.schibsted.no/oauth/jwks. By doing this the client doesn't need to know about a shared secret.

Read more about local validation of JWTs: https://techdocs.spid.no/token-introspection/#local-token-introspection

[jgorset]

Considering that most people will probably run this in a controlled environment (e.g. behind a firewall or similar), I think we should just go ahead and merge this even without the security features, because it's an awesome and much needed feature to use WAAS as an API. What do you think @olekenneth @anderser?

[anderser]

The security features have already been added to this PR. In this version it is the HMAC hash version with a shared secret (per webhook receiver), that the receiver need to know if they want to verify the content/payload. It is of course possible to receive the webhook without verifying the hash/payload. But all webhooks need to be defined in allow list as things are now. At least that stops the server from posting to random web pages/servers. I see there are some errors when building, so that has to be fixed first.

If we want to implement an RSA solution with public keys etc as @olekenneth suggested, we would need to set up an endpoint for the public keys, rotate keys etc. I am afraid I do not have time/knowledge to do that as of now. But feel free to help out there. I definitely see the upside of using public keys and not having to share a secret.

As for the allow list, it is a bit cumbersome to add new receivers now (add to a json file on the server), so we might need to figure out a better way. Env variable with a list of server/url patterns that are allowed and deny all internal ips or something like that?

[jgorset]

Alright, cool! I'm on vacation this week, and since this is a considerable (and awesome) change, let's merge next week and deploy and test everything thoroughly in production, too!