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

Allow for reduced scopes when issuing new refresh tokens #47

Closed
alexbilbie opened this issue May 9, 2013 · 6 comments
Closed

Allow for reduced scopes when issuing new refresh tokens #47

alexbilbie opened this issue May 9, 2013 · 6 comments
Milestone

Comments

@alexbilbie
Copy link
Contributor

As discussed in point 7 in #41

@cziegenberg
Copy link
Contributor

I already mentioned this and checked the RFC for the correct behaviour some time before (and this is also important for allowing multiple access tokens). Here again what I wrote:

The Refresh Token Grant must differ between two cases - issue a new refresh token when a new access token is created, or not. This defines the further steps in the flow and the possibilities on the client side. Therefore it must be possible to define the preferred behavior for this Grant:

RefreshToken::useRefreshTokenRotation(true|false)

If TRUE:

  • Return error, if scope parameter set (or if it's not identical to the prior scope).
  • Check credentials and refresh token.
  • Create a new access token.
  • Revoke the old refresh token (as the client MUST replace it anyway in this case).
  • Create a new refresh token.
  • Return the access token and the refresh token.

If FALSE:

  • The scope parameter is optional and can be used to limit the prior scope. If not set, the prior scope is used.
  • Check credentials and refresh token.
  • Create a new access token.
  • The old refresh token remains valid.
  • Return the access token only.

@alexbilbie
Copy link
Contributor Author

Surely RefreshToken::useRefreshTokenRotation(true|false) would just define whether or not the old token would be deleted or kept.

The authorization server MAY issue a new refresh token, in which case
   the client MUST discard the old refresh token and replace it with the
   new refresh token.  The authorization server MAY revoke the old
   refresh token after issuing a new refresh token to the client.  If a
   new refresh token is issued, the refresh token scope MUST be
   identical to that of the refresh token included by the client in the
   request.

So the flow goes:

  • Check credentials and refresh token.
  • Create a new access token.
  • The scope parameter is optional and can be used to limit the prior scope. If not set, the prior scope is used. If the scope parameter is set then check that there are no new scopes being requested.

IF TRUE:

  • Revoke the old refresh token (as the client MUST replace it anyway in this case).
  • Create a new refresh token.
  • Return the access token and the refresh token.

IF FALSE:

  • Create a new access token.
  • The old refresh token remains valid.
  • Return the access token only.

I don't understand why you think it should error if the scope param is set. In section 6 it says:

   scope
         OPTIONAL.  The scope of the access request as described by
         Section 3.3.  The requested scope MUST NOT include any scope
         not originally granted by the resource owner, and if omitted is
         treated as equal to the scope originally granted by the
         resource owner.

Therefore it should only error if the request includes additional scopes that weren't originally used in the request.

@cziegenberg
Copy link
Contributor

That's basically right, but there is one special case, when token rotation is activated and the scope is limited at the same time - this leads to the following problem:

  1. Initial access token with scope 1, 2 and 3.
  2. New access token using the refresh token, with new scope 1 and 2.
  3. The refresh token MUST be replaced on server side and on client side.
  4. If you now want to get a third access token, you can only get one with the scopes 1 and 2 - scope 3 is lost forever. And this violated the RFC, because the "scope originally granted" cannot be returned anymore.

I think this can be solved by

  • ignoring the scope completely, when token rotation is activated,
  • deactivate token rotation for requests with scopes set,
  • or returning an error

@alexbilbie
Copy link
Contributor Author

After some drawing this out on a piece of paper I agree.

I'll implement it.

@alexbilbie
Copy link
Contributor Author

Implemented in e591fbb

@cziegenberg
Copy link
Contributor

I also thought about it and I think this is a problem of the dtabase structure:

The scopes depend of the access token, which is correct, But you can hve multiple access tokens and I think it is also possible, that an old access token is deleted, while the session itselt and the refresh tokens are still valid. This could lead to a sitiuation where you cannot get the original scope anymore.

I think we need to save the original scope for the session in an additional table, just for re-creating them (so only if we create a refresh token). This would make the scopes independent from the access token an you would be able to always rote tokens and limit scopes, also at the same time.

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

No branches or pull requests

2 participants