jazzband / django-oauth-toolkit

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

Refresh Token 500 Internal Server Error instead of the expected 401 Unauthorized #1318

Closed discobeta closed 8 months ago

discobeta commented 9 months ago

Describe the bug While trying to refresh an OAuth token using an older refresh token, the server returns a 500 Internal Server Error instead of the expected 401 Unauthorized response. This occurs when an attempt is made to refresh the token with an older refresh token after it has been invalidated, resulting in a oauth2_provider.models.AccessToken.DoesNotExist error message.

To Reproduce

  1. Obtain a new token and a corresponding refresh token.
  2. Invalidate the new token by removing it (i.e., leave the "Authorization: Bearer" line empty) and attempt to refresh the token using the refresh token obtained in step 1.
  3. Use the new token obtained in step 2 to refresh the token again, successfully this time.
  4. Now, try to refresh the token using the original token obtained in step 1.
  5. At this point, a 500 Internal Server Error is encountered, with the error message oauth2_provider.models.AccessToken.DoesNotExist: AccessToken matching query does not exist.

Expected behavior Upon attempting to refresh using an older or non-existent token, the server should return a 401 Unauthorized response. The system's current behavior of querying the database to find an AccessToken entry at this stage seems unnecessary and unclear.

Version Django OAuth Toolkit Version: 2.2.0

Additional context When attempting to refresh an non-existing token the response should be Unauthorized

dopry commented 9 months ago

@discobeta, Do you have a stack trace so we can see what is raising the error?

discobeta commented 9 months ago

hi @dopry thanks for following up here.

The error is 500, see the complete traceback below.



  File "/usr/local/lib/python3.11/site-packages/django/core/handlers/exception.py", line 55, in inner

    response = get_response(request)

               ^^^^^^^^^^^^^^^^^^^^^

  File "/usr/local/lib/python3.11/site-packages/django/core/handlers/base.py", line 197, in _get_response

    response = wrapped_callback(request, *callback_args, **callback_kwargs)

               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

  File "/usr/local/lib/python3.11/site-packages/sentry_sdk/integrations/django/views.py", line 84, in sentry_wrapped_callback

    return callback(request, *args, **kwargs)

           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

  File "/usr/local/lib/python3.11/site-packages/django/views/generic/base.py", line 104, in view

    return self.dispatch(request, *args, **kwargs)

           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

  File "/usr/local/lib/python3.11/site-packages/django/utils/decorators.py", line 46, in _wrapper

    return bound_method(*args, **kwargs)

           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

  File "/usr/local/lib/python3.11/site-packages/django/views/decorators/csrf.py", line 56, in wrapper_view

    return view_func(*args, **kwargs)

           ^^^^^^^^^^^^^^^^^^^^^^^^^^

  File "/usr/local/lib/python3.11/site-packages/django/views/generic/base.py", line 143, in dispatch

    return handler(request, *args, **kwargs)

           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

  File "/usr/local/lib/python3.11/site-packages/django/utils/decorators.py", line 46, in _wrapper

    return bound_method(*args, **kwargs)

           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

  File "/usr/local/lib/python3.11/site-packages/django/views/decorators/debug.py", line 92, in sensitive_post_parameters_wrapper

    return view(request, *args, **kwargs)

           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

  File "/usr/local/lib/python3.11/site-packages/oauth2_provider/views/base.py", line 261, in post

    url, headers, body, status = self.create_token_response(request)

                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

  File "/usr/local/lib/python3.11/site-packages/oauth2_provider/views/mixins.py", line 124, in create_token_response

    return core.create_token_response(request)

           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

  File "/usr/local/lib/python3.11/site-packages/oauth2_provider/oauth2_backends.py", line 156, in create_token_response

    headers, body, status = self.server.create_token_response(

                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

  File "/usr/local/lib/python3.11/site-packages/oauthlib/oauth2/rfc6749/endpoints/base.py", line 112, in wrapper

    return f(endpoint, uri, *args, **kwargs)

           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

  File "/usr/local/lib/python3.11/site-packages/oauthlib/oauth2/rfc6749/endpoints/token.py", line 114, in create_token_response

    return grant_type_handler.create_token_response(

           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

  File "/usr/local/lib/python3.11/site-packages/oauthlib/oauth2/rfc6749/grant_types/refresh_token.py", line 56, in create_token_response

    self.validate_token_request(request)

  File "/usr/local/lib/python3.11/site-packages/oauthlib/oauth2/rfc6749/grant_types/refresh_token.py", line 121, in validate_token_request

    self.request_validator.get_original_scopes(

  File "/usr/local/lib/python3.11/site-packages/oauth2_provider/oauth2_validators.py", line 717, in get_original_scopes

    return AccessToken.objects.get(source_refresh_token_id=[rt.id](http://rt.id/)).scope

           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

  File "/usr/local/lib/python3.11/site-packages/django/db/models/manager.py", line 87, in manager_method

    return getattr(self.get_queryset(), name)(*args, **kwargs)

           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

  File "/usr/local/lib/python3.11/site-packages/django/db/models/query.py", line 637, in get

    raise self.model.DoesNotExist(

oauth2_provider.models.AccessToken.DoesNotExist: AccessToken matching query does not exist.```
dopry commented 8 months ago

So looking at the blame for https://github.com/jazzband/django-oauth-toolkit/blob/b39ec01333ecee0278cdfb87554fa082bb4e1071/oauth2_provider/oauth2_validators.py#L728 is looks like this token lookup was added in https://github.com/jazzband/django-oauth-toolkit/commit/2e4d15ee9372eb65289382dae0ee3c82a54cac06, which may provide some context for why the lookup is happening. It looks like we might need a try/catch here, but I don't have much time to dig into the details. @discobeta do you have time to reason through this issue and propose a fix?

discobeta commented 8 months ago

It looks like if we replace that .get() with the following code, it'll fix the issue gracefully.

            try:
                return AccessToken.objects.get(source_refresh_token_id=rt.id).scope
            except AccessToken.DoesNotExist:
                return None

I am happy to push a fix for this.

dopry commented 8 months ago

please go ahead and open a PR. We can pick up the discussion there.

discobeta commented 8 months ago

@dopry when you get chance please review the open PR