panva / openid-client

OAuth 2 / OpenID Connect Client API for JavaScript Runtimes
MIT License
1.83k stars 392 forks source link

token_type received from /token endpoint is case sensitive #248

Closed vdechef closed 4 years ago

vdechef commented 4 years ago

I try to authenticate with Gluu Server 4.1, from a Node application using passeport.js and node-openid-client 3.14.2. My applications works fine with other OP (Auth0 for example), but it refuses to authenticate with Gluu Server.

The error is caused by the token_type field returned by Gluu /token endpoint : this token_type is set to bearer by Gluu, thus the following request to /userinfo endpoint does not recognize the Authorization header, because openid-client sets its value at Authorization: bearer xxxxxxxx. If I hardcode the token_type to Bearer in the node-openid-client library, I can authenticate with my Gluu Server successfully.

In the RFC 6749, the token_type field is described as case insensitive, so openid-client should not use it directly in requests header.
I guess that openid-client should rather check the token_type, and match it with a list of supported token types, then add the corresponding header in the following requests.

While searching for a related issue, I saw a comment containing a token_type: bearer in lowercase (from Yahoo OP)

panva commented 4 years ago

Gluu responds with a lowercase token_type value and then doesn’t recognize it at its own userinfo endpoint?

Take that plus the fact that it is meant to be a case insensitive string, you should raise this at Gluu as far as i can tell.

panva commented 4 years ago

Furthermore, you can force a certain tokenType to be used right from the existing userinfo method API. Dtto for arbitrary resource calls.

Either through the API or just change the token_type property on your TokenSet instance as needed?

nynymike commented 4 years ago

Thanks guys. We'll fix this in the Gluu Server.

panva commented 4 years ago

Thanks Mike 💪

vdechef commented 4 years ago

Wow ! Thanks for the reactivity !

Actually I opened the issue in openid-client project because strictly speaking Gluu follows the RFC specification. The token_type field and the Authorization header are 2 different things :

Personaly I don't really mind who fixes this, as long as things work smoothly.
I think it is great that Gluu implements a fix for this, but I was wondering if some other OP could also have this kind of behavior. If it is the case, maybe openid-client should be robust and implement the translation from token_type to Authorization header (consider the mac type, which is used as MAC in the header in this RFC example)

@panva calling the client's methods directly would defeat the purpose of using passeport and openid-client Strategy, as it would transfer the responsibility for oidc sequence implementation to my app. This library is great, and that's why I would like to rely on it for my implementation.

panva commented 4 years ago

@vdechef

I've been through this topic before. It boils down to https://tools.ietf.org/html/rfc2617#section-1.2 where the auth-scheme is defined as case insensitive.

It uses an extensible, case-insensitive token to identify the authentication scheme

So there, the client is compliant and as seen by various implementations, servers must be accepting mixed-case values in order to stand a chance of being interoperable. I very much doubt the authors expected client software to match a case insensitive string and then transform it into a fixed form from a known list. Don't you think? FYI we have more authentication schemes coming to OAuth 2.0 access tokens (e.g. DPoP), that is the reason why the client no longer uses Bearer as a fixed value - other schemes are going to be popping up and it is imho the correct behaviour that the signaled token_type is used in the authorization header's scheme.

Now it is my base expectation that all servers accept the value Bearer, no brainer there. But if an access_token response contains a value like bearer i would expect its userinfo endpoint implementation to also accept it.

If it is the case, maybe openid-client should be robust and implement the translation from token_type to Authorization header (consider the mac type, which is used as MAC in the header in this RFC example)

I do strongly believe the current behaviour is robust since it is future-proof for new schemes and types and simply echoes whatever the AS sent. As far as the RFC example goes - all RFC examples are non-normative.

passport

How are you finding passport? I don't mind having to implement the sequence in my app when it means i can drop passport altogether. ;)

vdechef commented 4 years ago

@panva

Ok, thanks for the details. I agree that it makes sense to have the OP send the token_type matching the Authorization header. I was just wondering if there was something strange, but it seems like everything is fine.

nynymike commented 4 years ago

If there was a good reason for the OP to be case sensitive, then fine... but this seems like it would be better not to drive people crazy.