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

Add support for Digest authentication #176

Merged
merged 23 commits into from
Nov 14, 2021
Merged

Add support for Digest authentication #176

merged 23 commits into from
Nov 14, 2021

Conversation

ducaale
Copy link
Owner

@ducaale ducaale commented Sep 12, 2021

Also introduces a middleware system "inspired" by https://truelayer.com/blog/adding-middleware-support-to-rust-reqwest.

Ticks another item from #4.

@ducaale ducaale mentioned this pull request Sep 12, 2021
28 tasks
also handle digest auth in session + a little bit of refactoring for
auth.rs
@ducaale ducaale marked this pull request as ready for review September 14, 2021 19:47
@ducaale
Copy link
Owner Author

ducaale commented Sep 14, 2021

Now that --auth can take either a user[:pass] or a token, how its docs should be updated? and do we want to deprecate the --bearer flag?

src/main.rs Outdated Show resolved Hide resolved
@blyxxyz blyxxyz self-requested a review September 14, 2021 20:22
src/session.rs Show resolved Hide resolved
src/auth.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@blyxxyz blyxxyz left a comment

Choose a reason for hiding this comment

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

This looks great! I like that unlike HTTPie it can actually print the intermediary requests.

Sorry for taking so long to review it.

@@ -2197,6 +2244,122 @@ fn warns_if_config_is_invalid() {
.success();
}

#[test]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you additionally write a test against httpbin's digest endpoint, to make sure it works with a real implementation?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Done

@@ -135,11 +135,9 @@ pub struct Cli {
#[structopt(skip)]
pub is_session_read_only: bool,

// Currently deprecated in favor of --bearer, un-hide if new auth types are introduced
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should probably hide --bearer now

src/middleware.rs Outdated Show resolved Hide resolved
@ducaale ducaale merged commit bc744ec into develop Nov 14, 2021
@ducaale ducaale deleted the digest-auth branch November 14, 2021 19:00
Comment on lines -161 to +166
/// Authenticate as USER with PASS. PASS will be prompted if missing.
/// Authenticate as USER with PASS or with token.
///
/// Use a trailing colon (i.e. `USER:`) to authenticate with just a username.
/// PASS will be prompted if missing. Use a trailing colon (i.e. `USER:`)
/// to authenticate with just a username.
///
/// if --auth-type=bearer then --auth expects a token
/// {n}{n}{n}
#[structopt(short = "a", long, value_name = "USER[:PASS]")]
#[structopt(short = "a", long, value_name = "USER[:PASS] | token")]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd capitalize TOKEN here, and write the last line as TOKEN is expected if `--auth-type=bearer`..

Copy link
Owner Author

@ducaale ducaale Nov 14, 2021

Choose a reason for hiding this comment

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

Done 45af7be

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.

2 participants