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

server: transcode auth webhook #1731

Merged
merged 5 commits into from
Feb 22, 2021
Merged

server: transcode auth webhook #1731

merged 5 commits into from
Feb 22, 2021

Conversation

kyriediculous
Copy link
Contributor

@kyriediculous kyriediculous commented Jan 7, 2021

What does this pull request do? Explain your changes. (required)
Allows Orchestrators to specify an -authWebhookUrl to authenticate broadcasters upon discovery (GetOrchestatorInfo).

Specific updates (required)

  • created an authenticateBroadcaster() function in server/rpc.go this will no-op if no -authWebhookUrl is specified

  • Add a call to authenticateBroadcaster() when calling getOrchestrator() in server/rpc.go

  • Added unit tests for this new behaviour for getOrchestrator() using the httptest package which spins up a one-off http server which we specify as server.AuthWebhookUrl

  • Updated documentation on auth webhooks in doc

How did you test each of these updates (required)
Added unit/integration tests

Does this pull request close any open issues?
Fixes #1569

Checklist:

  • README and other documentation updated
  • Node runs in OSX and devenv
  • All tests in ./test.sh pass

@@ -302,6 +309,34 @@ func verifyOrchestratorReq(orch Orchestrator, addr ethcommon.Address, sig []byte
return orch.CheckCapacity("")
}

// authenticateBroadcaster returns an error if authentication fails
func authenticateBroadcaster(id string) error {
if AuthWebhookURL == "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

AuthWebhookURL is set only for Broadcaster node

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in 9ce9e44


Webhooks can authenticate streams supported by the RTMP and HTTP push ingest protocols. See the [ingest documentation](ingest.md) for details on how to use these protocols.
The webhook server should respond with HTTP status code `200` in order to authenticate / authorize the stream. A response with a HTTP status code other than `200` will cause the Livepeer node to disconnect the stream.
Copy link
Contributor

Choose a reason for hiding this comment

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

This says that code should be strictly 200 for success, but actual code checks for whole 2XX range. Should we make them match?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in 4090a12

doc/rtmpwebhookauth.md Outdated Show resolved Hide resolved
server/rpc.go Outdated Show resolved Hide resolved
Copy link
Member

@yondonfu yondonfu left a comment

Choose a reason for hiding this comment

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

LGTM

@kyriediculous kyriediculous merged commit c170547 into master Feb 22, 2021
@kyriediculous kyriediculous deleted the nv/transcode-auth branch February 22, 2021 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Transcoding Protocol Authentication
3 participants