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

[adityakrgupta25]

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': ...}

3. 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:
#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.

[phroiland]

Is that causing this?

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

[adityakrgupta25]

Is that causing this?

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

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

[SamSchelfhout]

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?

[MattBlack85]

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?

[SamSchelfhout]

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?

[n2ygk]

@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.

[MattBlack85]

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 <notifications@github.com> 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 <#839 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA7YNFV5DPTUQZQDUOVCA6TSNVS6FANCNFSM4M6WDBMA> .

[krzysieqq]

still in 1.4.0, any updates?

[petros94]

Any news? still in 2.2.0