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.

Sign up for GitHub

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

Jump to bottom

Add auth::{Authorization, AuthenticationScheme, BasicAuth, WwwAuthenticate, ProxyAuthorization, ProxyAuthenticate} #252

Merged
merged 11 commits into from
Oct 25, 2020
Merged

Add auth::{Authorization, AuthenticationScheme, BasicAuth, WwwAuthenticate, ProxyAuthorization, ProxyAuthenticate} #252

merged 11 commits into from
Oct 25, 2020

Conversation

yoshuawuyts
Copy link
Member

@yoshuawuyts yoshuawuyts commented Oct 1, 2020 •
edited
Loading

Ref #99

Adds HTTP basic auth and related structs for authentication.

BasicAuth is a specialization of Authorization; because schemes like oauth are rather elaborate, we've chosen to put them into separate structs.

Thanks!

Screenshots

Screenshot_2020-10-01 http_types auth - Rust

@yoshuawuyts yoshuawuyts changed the title Add auth::{Authorization, AuthenticationScheme, BasicAuth} Add auth::{Authorization, AuthenticationScheme, BasicAuth, WwwAuthenticate} Oct 2, 2020
@yoshuawuyts
Copy link
Member Author

I've added support for WWW-Authenticate, but need to change it up slightly before merging. Right now we support at most 1 authentication scheme, but the spec says this should be up to n. Also since Proxy-Authenticate parses exactly the same way as WWW-Authenticate we can copy-paste the impl once it works. That should provide the majority of common authentication things.

@yoshuawuyts yoshuawuyts mentioned this pull request Oct 2, 2020
Closed
@yoshuawuyts yoshuawuyts changed the title Add auth::{Authorization, AuthenticationScheme, BasicAuth, WwwAuthenticate} Add auth::{Authorization, AuthenticationScheme, BasicAuth, WwwAuthenticate, ProxyAuthorization, ProxyAuthenticate} Oct 6, 2020
@yoshuawuyts
Copy link
Member Author

Okay; all implemented!

@Fishrock123
Copy link
Member

BasicAuth seems to work fine, as I am using it here: https://github.com/squamishaccess/squamishaccess-signup-function-rs/blob/6c445916760a4db21da80c732fbe8e9c704c1bd1/src/main.rs#L168-L172

However I did run into a UX issue with typed headers and surf: http-rs/surf#251

Fishrock123
Fishrock123 approved these changes Oct 20, 2020
View reviewed changes
Copy link
Member

@Fishrock123 Fishrock123 left a comment

Choose a reason for hiding this comment

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

For some reason I had the impression this covered more than basic auth. Looks good, although I am not familiar with WWW-Authenticate.

Basic auth certainly works when I make client requests to mailchimp.

One question I have is how to make Authorization and Proxy-Authorization work seamlessly.

src/auth/basic_auth.rs Outdated Show resolved Hide resolved
src/macros.rs Outdated Show resolved Hide resolved
@yoshuawuyts yoshuawuyts merged commit 9127b5f into main Oct 25, 2020
@delete-merged-branch delete-merged-branch bot deleted the basic-auth branch October 25, 2020 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Fishrock123 Fishrock123 Fishrock123 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@yoshuawuyts @Fishrock123