http_stream: support HTTP authentication (AUD-4994) #1103
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
changed the title
stintel
added a commit
to toverainc/esp-adf
that referenced
this pull request
Nov 8, 2023
We've submitted a PR to Espressif to support enabling HTTP Basic Auth in ESP-ADF's HTTP stream: espressif#1103. Unfortunately we'd also require a change to ESP Audio to support enabling HTTP Basic Auth. As we can't seem to find the ESP Audio source, we'll just enable it unconditionally in ESP-ADF's HTTP stream. ESP Audio uses ESP-ADF's HTTP Stream so this is enough to support HTTP Basic Auth to WIS by using a URL in the form of http://user:[email protected]:19001/. ESP-IDF's http_parser component will detect the username and password in the URL, and esp_http_client will set the Authorization header if the auth type is set to basic, and the parser found a username and password in the URL. If the parser didn't find a username and password, esp_http_client will just skip setting the header entirely, so having auth type hardcoded to basic is not a problem even if users configure a URL without username and password.
stintel
added a commit
to toverainc/esp-adf
that referenced
this pull request
Nov 8, 2023
We've submitted a PR to Espressif to support enabling HTTP Basic Auth in ESP-ADF's HTTP stream: espressif#1103. Unfortunately we'd also require a change to ESP Audio to support enabling HTTP Basic Auth. As we can't seem to find the ESP Audio source, we'll just enable it unconditionally in ESP-ADF's HTTP stream. ESP Audio uses ESP-ADF's HTTP Stream so this is enough to support HTTP Basic Auth to WIS by using a URL in the form of http://user:[email protected]:19001/. ESP-IDF's http_parser component will detect the username and password in the URL, and esp_http_client will set the Authorization header if the auth type is set to basic, and the parser found a username and password in the URL. If the parser didn't find a username and password, esp_http_client will just skip setting the header entirely, so having auth type hardcoded to basic is not a problem even if users configure a URL without username and password.
|
@stintel Thank you for your contributions. It will be merged. |
|
Hi @stintel |
|
After submitting this PR, I discovered I could just use esp_http_client_set_authtype() and in the HTTP_STREAM_PRE_REQUEST event of the http_stream event_handle. Username and password is then automatically set by http_parser if included in the URL. This is slightly less efficient, as it has to be done on every new stream, while with this PR it would have to be set once, if I understand correctly, but I'm not sure if that's worth all the additional code. If you still think this change is valuable, let me know, and I'll make the requested changes. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
To support authentication in Willow Inference Server to allow people to run public, shared WIS servers, we need HTTP authentication support in http_stream. This PR adds support for it.