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

Pass Basic Authorization header to OAuth2 access token request #316

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lexi-lambda
Copy link

Fixes #143, #175, #205, and #300.

@boubou191911
Copy link

Hello,
I was facing the Basic Authorization issue and your commit solved my problem.
Just a little comment. I think that when using Basic Auth the client_id and client_secret should also be removed from the url params (it's the purpose that they are not sent in clear text in the URL).
But that means that an extra parameter must be passed in the strategy to decide how to pass the client_id and client_secret to the token endpoint.

@arryon
Copy link

arryon commented Jan 6, 2017

+1 For this PR. I'm using a custom header to indicate my API version for the endpoint, and my only option right now is to fork the entire repo. Custom headers should be a must.

@gierschv
Copy link

+1 for this PR. This is the default authentication mode in the RFC, some servers only support this.

@jordanbtucker
Copy link

Why hasn't this been merged? According to the OAuth 2.0 RFC, servers are only required to support HTTP Basic authentication with the client_credentials grant type. This client does not work with some OAuth 2.0 servers because of this.

@jdesboeufs
Copy link

Do we have to fork?
Some tests may help!

@corbfon
Copy link

corbfon commented Feb 13, 2019

What's the status on this??

@corbfon
Copy link

corbfon commented Feb 13, 2019

@jdesboeufs @jordanbtucker
So, after digging through the code and considering creating my own PR and such, I decided just to add my authorization as a custom header when I created my OAuth2 instance (and since I'm using passport-oauth2, which supports custom headers, I just passed it through that). See:

// just add your 'Authorization' header here to get the functionality - doesn't make it super smooth and easy, but it works
exports.OAuth2 = function (clientId, clientSecret, baseSite, authorizePath, accessTokenPath, customHeaders) {
...
}

customHeaders needs to be an object where the key is the header's name and the value is the header's value

Of course, it doesn't really solve the provider's use case, which is keeping your client id/secret pairing a true secret, since they're still sent in the url. You may be dealing with the risk of them logging that somewhere, but this PR doesn't solve that, anyway.

@lexi-lambda
Copy link
Author

@corbfon The last commit to this repository is over two years old. I think it’s time to admit that this project is unmaintained, and this is never getting merged. Use or create a fork if you have to.

@corbfon
Copy link

corbfon commented Feb 13, 2019

@lexi-lambda ah... good point. Sucks that this is a dependency of a library as large as passport. Do you recommend another lib for oauth2 flow?

@RomanMinkin
Copy link

Sad...

@newmanw
Copy link

newmanw commented Sep 21, 2021

@corbfon agreed. I guess at the very least it would be helpful if the previous maintainer of this project would mark it as unmaintained. This would at least flag others from using it in their projects or to migrate away from it if already in use.

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.

OAuth2 for twitter don't work
9 participants