ai/live: use webhookUrl query param to override auth webhook url

[junhyr]

Add support for webhookUrl query parameter to override LiveAIAuthWebhookURL.
Sign the auth requests rather than passing an API key to avoid exposing this to any 3rd party putting in their own endpoints as an override.

[mjh1]

@junhyr Damn, just realised a problem with this, it's open to people putting in their own webhook URL and grabbing the admin auth token when the auth call is made.
Maybe if we restrict the hostname to *.preview.livepeer.monster that'll be ok?

[codecov]

Codecov Report

❌ Patch coverage is 0% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 31.66367%. Comparing base (4a2774a) to head (3bcc44a).
⚠️ Report is 3 commits behind head on master.

Files with missing lines	Patch %	Lines
server/ai_mediaserver.go	0.00000%	14 Missing ⚠️

Additional details and impacted files

@@                 Coverage Diff                 @@
##              master       #3732         +/-   ##
===================================================
- Coverage   31.66820%   31.66367%   -0.00453%     
===================================================
  Files            158         158                 
  Lines          47764       47774         +10     
===================================================
+ Hits           15126       15127          +1     
- Misses         31738       31747          +9     
  Partials         900         900                 

Files with missing lines	Coverage Δ
server/ai_mediaserver.go	6.78261% <0.00000%> (-0.05950%)	⬇️

... and 2 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4a2774a...3bcc44a. Read the comment docs.

Files with missing lines	Coverage Δ
server/ai_mediaserver.go	6.78261% <0.00000%> (-0.05950%)	⬇️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[junhyr]

@junhyr Damn, just realised a problem with this, it's open to people putting in their own webhook URL and grabbing the admin auth token when the auth call is made. Maybe if we restrict the hostname to *.preview.livepeer.monster that'll be ok?

yeah that will do, but it seems like we're not using x-api-key on the Daydream API side anyway 🥲

[mjh1]

@junhyr Damn, just realised a problem with this, it's open to people putting in their own webhook URL and grabbing the admin auth token when the auth call is made. Maybe if we restrict the hostname to *.preview.livepeer.monster that'll be ok?

yeah that will do, but it seems like we're not using x-api-key on the Daydream API side anyway 🥲

Yeah that's ok, I've added the authorization header in another PR

[mjh1]

@junhyr i was going to make the change to check the hostname but then remembered the fly preview urls don't use any of our livepeer domains

[mjh1]

Looks good

[j0sh]

If we pass in the auth URL as part of the query string, we can pass in additional request headers as part of the query string, why not. And keep them coupled - if the auth URL is overriden then don't pass in any default headers (like the auth token), only the headers that were specified by the user. Something like ?authWebhookURL=https://host/path&authWebhookHeaders=Authorization: token

Implementation-wise that would be easier for folks to manage if they want to integrate a webhook endpoint, rather than have to roll their own HMAC signature checking and it's a little more standard as far as API authentication goes.

Leaking secrets is one thing but IMO the bigger problem is that by controlling the URL, the attacker can also control the response. So the auth webhook wouldn't actually authenticate anything anymore and may actually become an attack vector if it enables things that the user shouldn't have access to.

To fix this we should have a static shared secret somewhere in the response - either in the response header or the response object, and check that, like an API key in reverse. This should be optional (and separate from the main API key, of course).

[junhyr]

@j0sh We still want to verify whether the response is really coming from the gateway. but yes, we could add an additional Authorization header to the request if this information is considered sensitive. (Some additional webhookUrl validation as well)

[j0sh]

We still want to verify whether the response is really coming from the gateway

@junhyr Sure, the original API key (in the request) does that, and we should keep that in as it's a standard and easy-to-use method for API authentication outside of Inc.

The issue with allowing a user submitted webhook URL is that it side-steps auth completely. There needs to be another key to verify the legitimacy of the response.