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

Support "Authorization: Bearer <token>" #4483

Closed
splashx opened this issue Aug 2, 2018 · 4 comments
Closed

Support "Authorization: Bearer <token>" #4483

splashx opened this issue Aug 2, 2018 · 4 comments
Labels
good first issue A well-defined bug or improvement with sufficient context which should be approachable for new contr type/enhancement Proposed improvement or new feature

Comments

@splashx
Copy link
Contributor

splashx commented Aug 2, 2018

Feature Description

It would be great if consul could support Authorization: Bearer <token>.

Use Case(s)

There are some tools which won't support custom headers (see prometheus/prometheus#2346 and prometheus/prometheus#1724).

One should not expect the world would interface with Consul using a non-standard X-Consul-Token header.

Authorization: Bearer <token> and X-Consul-Token: <token> could be interchangeable for long-lasting backwards compatibility.
@splashx splashx changed the title Add support for "Authorization: Bearer <token>" Support for "Authorization: Bearer <token>" Aug 2, 2018
@splashx splashx changed the title Support for "Authorization: Bearer <token>" Support "Authorization: Bearer <token>" Aug 2, 2018
@mkeeler mkeeler added needs-discussion Topic needs discussion with the larger Consul maintainers before committing to for a release and removed needs-discussion Topic needs discussion with the larger Consul maintainers before committing to for a release labels Aug 2, 2018
@mkeeler
Copy link
Member

mkeeler commented Aug 2, 2018

@splashx This seems like a reasonable request. The Authorization header with the Bearer auth scheme seems to be an exact match for how the tokens are used. I am not sure when the team could get around to implementing it but we would be open to a PR for this.

@mbag mbag mentioned this issue Aug 7, 2018
Merged
@pearkes pearkes added type/enhancement Proposed improvement or new feature good first issue A well-defined bug or improvement with sufficient context which should be approachable for new contr labels Aug 7, 2018
@mbag mbag mentioned this issue Aug 8, 2018
Merged
@mbag
Copy link
Contributor

mbag commented Aug 8, 2018

@mkeeler : @splashx and I worked on this. I've created PR #4502 with code we have prepareed. If there are some changes to be made, please let me know.

@shubheksha
Copy link
Contributor

I'd be happy to take this up! Can y'all can give me some pointers regarding the implementation since I'm very new to the code base? 😄

@splashx
Copy link
Contributor Author

splashx commented Aug 8, 2018

@shubheksha there is a pending PR #4502 for this already ... feel free to chip in.

mkeeler pushed a commit that referenced this issue Aug 17, 2018
@mbag @mkeeler
Fixes #4483: Add support for Authorization: Bearer token Header (#4502)
3c23979 
Added Authorization Bearer token support as per RFC6750

* appended Authorization header token parsing after X-Consul-Token
* added test cases
* updated website documentation to mention Authorization header

* improve tests, improve Bearer parsing
@uepoch uepoch mentioned this issue Sep 25, 2018
Closed
@kevinschoonover kevinschoonover mentioned this issue Apr 10, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue A well-defined bug or improvement with sufficient context which should be approachable for new contr type/enhancement Proposed improvement or new feature
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@pearkes @splashx @mkeeler @mbag @shubheksha