Skip to content

Add support to "buf curl" for --user and --netrc credentials flags#2010

Merged
jhump merged 5 commits intomainfrom
jh/buf-curl-creds
Apr 21, 2023
Merged

Add support to "buf curl" for --user and --netrc credentials flags#2010
jhump merged 5 commits intomainfrom
jh/buf-curl-creds

Conversation

@jhump
Copy link
Member

@jhump jhump commented Apr 18, 2023

This adds support for -u/--user, -n/--netrc, and --netrc-file flags to buf curl. These flags work the same way in this context as the flags of the same name in curl.

Resolves #1997.

@jhump jhump requested a review from bufdev April 18, 2023 22:11
@bufdev
Copy link
Member

bufdev commented Apr 18, 2023

Thanks for doing this

Copy link
Member

@bufdev bufdev left a comment

Choose a reason for hiding this comment

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

Mostly nits, sorry for the double review

@jhump
Copy link
Member Author

jhump commented Apr 21, 2023

@bufdev, FYI, I just pushed a commit to add a blurb about this change to the changelog.

@jhump jhump merged commit 2bb4623 into main Apr 21, 2023
@jhump jhump deleted the jh/buf-curl-creds branch April 21, 2023 21:46
@mattrobenolt
Copy link
Contributor

mattrobenolt commented May 17, 2023

I just realized this doesn't work as expected. It tags the Authorization header, but it's not sending Basic since this is basic auth. Not sure how this interacts with --netrc or what expectations are there, but strictly for --user, the intention is to send along as basic auth.

Right now, if I do:

$ buf curl -v -u foo:bar ...

I end up with a request such as:

buf: > (#1) Authorization: Zm9vOmJhcg==

And the expectation is this would be:

Authorization: Basic Zm9vOmJhcg==

Otherwise, the auth is invalid.

Edit: It seems based on my very limited understanding and my basic testing of a ~/.netrc file with curl, it also sends the credentials as Basic auth.

@mattrobenolt
Copy link
Contributor

PR to fix: #2096

Feel free to implement this however, but this gets it working. :)

@jhump
Copy link
Member Author

jhump commented May 17, 2023

@mattrobenolt 🤦 , not sure how I missed this. I thought I had tried this out with real credentials to a BSR to verify, but maybe I imagined that. I know I did quite a lot of buf curl -v and inspecting the headers. I am embarrassed a bug this bad made it into a release. Thank you for the PR.

@mattrobenolt
Copy link
Contributor

haha @jhump, no worries. Happens to us all. 🫶

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.

curl: support -u/--user

3 participants