jazzband / django-oauth-toolkit

OAuth2 goodies for the Djangonauts!
https://django-oauth-toolkit.readthedocs.io
Other
3.13k stars 792 forks source link

AccessTokens are automatically removed during the refresh #360

Closed H0neyBadger closed 2 weeks ago

H0neyBadger commented 8 years ago

Hello, What happened to these ‘features’?

With the latest version of DOT, AccessTokens are automatically removed during the refresh (using a RefreshToken).

I tried to follow the execution thread And I think it comes from lines: https://github.com/evonove/django-oauth-toolkit/blob/master/oauth2_provider/oauth2_validators.py#L303

    def save_bearer_token(self, token, request, *args, **kwargs):
        """
        Save access and refresh token, If refresh token is issued, remove old refresh tokens as
        in rfc:`6`
        """
        if request.refresh_token:
            # remove used refresh token
            try:
                RefreshToken.objects.get(token=request.refresh_token).revoke()
            except RefreshToken.DoesNotExist:
                assert()  # TODO though being here would be very strange, at least log the error

and inside the revoke function, the access token is removed.

    def revoke(self):
        """
        Delete this refresh token along with related access token
        """
        AccessToken.objects.get(id=self.access_token.id).revoke()
        self.delete()

To fix this issue, I think we should create a dedicated function or simply call .delete() at line 303. this should do the trick. related commit 03004028cbd93e6723b2f3c13aaff6f8783bc24f

Regarding the issue :

The https://tools.ietf.org/html/rfc7009#section-2.1 section only concerns the revoke functions and not the token renewal process.

Eventually, this change may solve the race condition exception due to token that DoesNotExist.

kevin-brown commented 8 years ago

When migrating things over the DOT from DOAC, we noticed this issue as well. The specification does not limit it to a single access token per refresh token (and in practice, most places allow for multiple), but for some reason DOT limits it to a single access token per refresh token.