node-oauth
node-oauth copied to clipboard
ciaranj
Pass Basic Authorization header to OAuth2 access token request
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.
+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.
+1 for this PR. This is the default authentication mode in the RFC, some servers only support this.
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 @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.
@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.
@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?
@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.
