Refresh token grant not allowed to public clients

[ojhaujjwal]

According to the spec, refresh token grant is not allowed to public clients. See http://tools.ietf.org/html/rfc6749#section-1.5 and http://stackoverflow.com/questions/3487991/why-does-oauth-v2-have-both-access-and-refresh-tokens

If somebody is already using this grant, I think they are in big trouble.

[coveralls]

Coverage remained the same when pulling 15e07cdf6fe3147e6f4940d5b5e24864dc62a6fa on ojhaujjwal:patch-1 into 66cbf211878a358c044e336be69895b5fd3e0c9b on zf-fr:master.

[ojhaujjwal]

The other problem is that if there is someone new to oauth2 and they are using refresh token grant, they are probably in big trouble. The readme.md of zfr-oauth2-server-module also contains an example config of grants which shows using refresh token grant.

[bakura10]

I'm not sure about this. The main use case of refresh token is to avoid asking the user his credentials each time, by storing a short access token (that is sent over the wire at each request) AND a longer lived refresh token in the client (in the JavaScript app, for instance).

If your access token is expired, you can send the refresh token to get a new access token, without redirecting the user back to the login page.

Actually, if you want to retrieve a refresh token, your access token is already "implicitly approved" (as you need a valid access token to generate a refresh token). Imagine the following use case, if I merge your PR:

1. I retrieve an access token with passowrd/email and get an access token and refresh token.
2. My access token expires and I send the refresh token to get a new one. Obviously, I do not need to authenticate again because I send a refresh token that was already get with valid credentials.

If we do not allow public clients for refresh token, this won't work because of this: https://github.com/zf-fr/zfr-oauth2-server/blob/master/src/ZfrOAuth2/Server/AuthorizationServer.php#L181

[ojhaujjwal]

Read this. http://stackoverflow.com/questions/3487991/why-does-oauth-v2-have-both-access-and-refresh-tokens#answer-7209263

[bakura10]

I read it but I'm really unsure about it. The reason is that because refresh token are stored in the browser and NOT sent (unless you explicitly need it). If an attacker manages to retrieve the access token, because it's short lived, it will soon expires, however the refresh token will likely be stored in user's local storage, so it's safe!

If I merge your PR, refresh token are basically make useless, tbh.

[ojhaujjwal]

If I merge your PR, refresh token are basically make useless, tbh.

If you merge my PR, refresh token are basically useless for your use case(sorry to say that). See how google uses refresh token. https://developers.google.com/accounts/docs/OAuth2WebServer#refresh

[ojhaujjwal]

I think for your use case, you should probably create a custom grant which allows public clients. :)

[bakura10]

Google has never been a reference about how they use OAuth iirc... I can give you other examples that use refresh token like I do: https://stripe.com/docs/connect/reference#post-token

And actually, most JavaScript clients expect refresh token to work like this.

I won't merge this PR because it's too restricted and make refresh token useless for 99% of use cases which is a bit silly. You can check most other PHP implementations, they all do like this too.

Once again, there is not really security issue because the refresh token IS NOT sent with each request, so it cannot be retrieved by the attacker.

[ojhaujjwal]

Okay. I looked at oauth2-server from thephpleague. See this. https://github.com/thephpleague/oauth2-server/blob/master/src/League/OAuth2/Server/Grant/RefreshToken.php#L104-110. This is self-explanatory.

[bakura10]

Anyway, this is a major bc break that could not have been merged (I'm surprised that tests pass, we should add more coverage :D).

I will read that but one can argue that we respect the spec. Currently refresh token cannot be generated for public, because they need an access token, that imply that thr identity has ALREADY been validated by authorization server ;)

This is really subject to interpretation and I'm not ready to remove all use cases :)

[ojhaujjwal]

I am well aware of the fact that this PR make refresh token useless for most of use cases.

[bakura10]

This one https://github.com/bshaffer/oauth2-server-php/blob/develop/src/OAuth2/GrantType/RefreshToken.php

Which is the major oauth used (and used by apigility) seems to do it like me.

[ojhaujjwal]

I have already looked at that one. :)

[bakura10]

I mean, the goal of the refresh token is to be able to get a new access token easily. If you need the client id and secret then... Why not use the client credentials grant? That would completely defeat whole purpose of refresh token actually :).

Because the client credentials grant exactly does that (creating an access token from client id and client secret)

[bakura10]

However I'd be glad if you could add tests that would make sure that such changes would not make tests pass :D

[ojhaujjwal]

I have the same use cases(till now) as yours. But, I am worried about the spec.

[bakura10]

I would not ;). If Stripe is using it this way... Then it's good :D.

Envoyé de mon iPhone

Le 5 sept. 2014 à 13:52, Ujjwal Ojha notifications@github.com a écrit :

I have the same use cases(till now) as yours. But, I am worried about the spec.

— Reply to this email directly or view it on GitHub.

[ojhaujjwal]

Not related to this issue. I have created a module. See https://github.com/hrevert/ht-oauth-server-client-module. The flow is inspired from server-side implementation examples of satellizer. I would be glad if you can provide me feedbacks.

[bakura10]

Hmm I didn't dig the code but I'm not sure to understand what it does :D. It sounds a lot like the password credentials no?

[ojhaujjwal]

Exactly. In password credentials, clients send username and password. In that grant, clients send the authorization code of facebook, google etc. That's it.

Thanks for looking at it. :+1: