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

Fixes #4483: Add support for Authorization: Bearer token Header #4502

Merged
merged 4 commits into from
Aug 17, 2018

Conversation

mbag
Copy link
Contributor

@mbag mbag commented Aug 8, 2018

Fixes: #4483

We tried to make Authorization Bearer header compliant with RFC6750 and other OAuth RFCs where appropriate. RFCs are referenced in comments.

Tests and documentation were added/updated as well.

mbag and others added 4 commits August 7, 2018 13:23
* appended Authorization header token parsing after X-Consul-Token
* added test cases
* updated website documentation to mention Authorization header
* improve tests
* improve Bearer parsing
Added Authorization Bearer token support as per RFC6750
Copy link
Member

@mkeeler mkeeler 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 really good @mbag. You even went ahead and updated the docs and tests.

My only request is that in the PR description you change it to say:

"Fixes: #4483" so that GitHub will automatically close the associated issue when its merged.

Copy link
Member

@mkeeler mkeeler left a comment

Choose a reason for hiding this comment

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

Not sure why GitHub duplicated my review.

This looks really good @mbag. You even went ahead and updated the docs and tests.

My only request is that in the PR description you change it to say:

"Fixes: #4483" so that GitHub will automatically close the associated issue when its merged.

@mbag mbag changed the title Add support for Authorization: Bearer token Header Fixes: #4483 Aug 8, 2018
@mbag mbag changed the title Fixes: #4483 Fixes #4483: Add support for Authorization: Bearer token Header Aug 8, 2018
@mbag
Copy link
Contributor Author

mbag commented Aug 8, 2018

@mkeeler thanks, I changed the description similar to PR #4437 Hope that's OK.

@mkeeler
Copy link
Member

mkeeler commented Aug 8, 2018

Thanks @mbag. By description I was talking about the block of text where you said that the PR was related to the issue. Turns out I can just edit the description myself so I did.

@mbag
Copy link
Contributor Author

mbag commented Aug 8, 2018

👍 OK. Now I know for the future PRs :)

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.

3 participants