django-oauth-toolkit icon indicating copy to clipboard operation
django-oauth-toolkit copied to clipboard

revoking AccessToken only and then using RefreshToken token to generate new AccessToken breaks.

Open adityakrgupta25 opened this issue 4 years ago • 9 comments

Describe the bug Revoking AccessTokens does not revoke the corresponding RefreshTokens. So in case, AccessToken is revoked, its deleted from the dB. If a request for a new token on TokenView comes with the existing RefreshToken, it would cause the flow to break at OAuth2Validator.get_original_scopes() due to lack of existence of AccessToken.

  File "/usr/local/lib/python3.7/site-packages/oauth2_provider/oauth2_backends.py", line 138, in create_token_response
     headers, extra_credentials)
   File "/usr/local/lib/python3.7/site-packages/oauthlib/oauth2/rfc6749/endpoints/base.py", line 64, in wrapper
     return f(endpoint, uri, *args, **kwargs)
   File "/usr/local/lib/python3.7/site-packages/oauthlib/oauth2/rfc6749/endpoints/token.py", line 117, in create_token_response
     request, self.default_token_type)
  File "/usr/local/lib/python3.7/site-packages/oauthlib/oauth2/rfc6749/grant_types/refresh_token.py", line 59, in create_token_response
     self.validate_token_request(request)
   File "/usr/local/lib/python3.7/site-packages/oauthlib/oauth2/rfc6749/grant_types/refresh_token.py", line 117, in validate_token_request
 request.refresh_token, request))
   File "/usr/local/lib/python3.7/site-packages/oauth2_provider/oauth2_validators.py", line 605, in get_original_scopes
    return AccessToken.objects.get(source_refresh_token_id=rt.id).scope
   File "/usr/local/lib/python3.7/site-packages/django/db/models/manager.py", line 82, in manager_method
  return getattr(self.get_queryset(), name)(*args, **kwargs)
line 408, in get
 self.model._meta.object_name
oauth2_provider.models.AccessToken.DoesNotExist: AccessToken matching query does not exist.

To Reproduce

  1. Generate access token/ refresh token pair.
  2. Revoke only access token by hitting: /o/revoke_token/ endpoint with a body that would look like:
{ 'token': <your access token>,
            'token_type_hint': 'access_token',
            'client_id': ...,
            'client_secret': ...}
  1. Try regenerating access/refresh token pair using refresh_token as your grant_type.

Expected behaviour If revoking (aka deleting) only AccessTokens is allowed without revoking refresh tokens, this should not crash when generating new token with existing valid refresh token.

Version django-oauth-toolkit==1.2.0

  • [ ] I have tested with the latest published release and it's still a problem.
  • [ ] I have tested with the master branch and it's still a problem.

Should be reproducible on the newer version.

Additional context

It came with this merging of this issue: https://github.com/jazzband/django-oauth-toolkit/issues/497 wherein it was intended to provide grace period window by not deleting the Refresh token if the Access Token. This has lead to above unintended consequences.

adityakrgupta25 avatar May 12 '20 10:05 adityakrgupta25

Is that causing this?

Screen Shot 2020-05-16 at 4 56 04 PM

Version django-oauth-toolkit==1.2.0 Django==1.11.29

phroiland avatar May 16 '20 20:05 phroiland

Is that causing this?

Screen Shot 2020-05-16 at 4 56 04 PM

Version django-oauth-toolkit==1.2.0 Django==1.11.29

No, this seems very different from what we have been experiencing.

adityakrgupta25 avatar May 16 '20 21:05 adityakrgupta25

This issue still exists in the latest version. I`m not sure if it is an oauthlib or django-oauth-toolkit issue. In the oauthlib refreh token function the following function is called:

File: oauth2_validators.py

def get_original_scopes(self, refresh_token, request, *args, **kwargs):
        # Avoid second query for RefreshToken since this method is invoked *after*
        # validate_refresh_token.
        rt = request.refresh_token_instance
        if not rt.access_token_id:
            return AccessToken.objects.get(source_refresh_token_id=rt.id).scope

        return rt.access_token.scope

When the access code was revoked, it was removed from the database. Thus it is impossible to get the original scope based on the original access code.

This issue seems to break a crucial part of the OAuth2 process. Is there a solution coming?

SamSchelfhout avatar Oct 31 '20 20:10 SamSchelfhout

Revoking seems to me like something very destructive and somethingvvery different from an expiration,like you don't want anybody else to use your app no matter what. I have to read more, but to me it looks like this code provide some guard when the access token is expired,if the access one is revoked, it seems to me that both access and refresh one should go?

MattBlack85 avatar Oct 31 '20 21:10 MattBlack85

This issue causes a 500 HTTP error. The RFC https://tools.ietf.org/html/rfc7009 states the following:

Depending on the authorization server's revocation policy, the revocation of a particular token may cause the revocation of related tokens and the underlying authorization grant. If the particular token is a refresh token and the authorization server supports the revocation of access tokens, then the authorization server SHOULD also invalidate all access tokens based on the same authorization grant (see Implementation Note). If the token passed to the request is an access token, the server MAY revoke the respective refresh token as well.

How did the django-oauth-toolkit intended this to work? Should the refresh token be invalidated too or should it still be usable?

SamSchelfhout avatar Nov 01 '20 10:11 SamSchelfhout

@SamSchelfhout I think that the more conservative approach is to revoke the respective refresh token as well. What does it mean to revoke access and then expect the refresh token -- which exists solely to renew access -- to let one use it to get access again? This feels like a "pass the hash" exposure. I think the code as implemented is right but the 500 error should be fixed.

n2ygk avatar Nov 01 '20 14:11 n2ygk

The problem is here https://github.com/jazzband/django-oauth-toolkit/blob/master/oauth2_provider/models.py#L281

We do not delete the refresh token on cascade on revocation but we set the relation to Null, that's why end in that situation

Il dom 1 nov 2020, 15:02 Alan Crosswell [email protected] ha scritto:

@SamSchelfhout https://github.com/SamSchelfhout I think that the more conservative approach is to revoke the respective refresh token as well. What does it mean to revoke access and then expect the refresh token -- which exists solely to renew access -- to let one use it to get access again? This feels like a "pass the hash" exposure. I think the code as implemented is right but the 500 error should be fixed.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/jazzband/django-oauth-toolkit/issues/839#issuecomment-720093331, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA7YNFV5DPTUQZQDUOVCA6TSNVS6FANCNFSM4M6WDBMA .

MattBlack85 avatar Nov 01 '20 14:11 MattBlack85

still in 1.4.0, any updates?

krzysieqq avatar Mar 07 '21 18:03 krzysieqq

Any news? still in 2.2.0

petros94 avatar Nov 17 '22 13:11 petros94