Styria-Digital / django-rest-framework-jwt

JSON Web Token Authentication support for Django REST Framework
https://styria-digital.github.io/django-rest-framework-jwt/
MIT License
191 stars 57 forks source link

get_token_from_request can raise DjangoUnicodeDecodeError #70

Closed jayvdb closed 3 years ago

jayvdb commented 4 years ago

Python 3.6 and Django 2.2 , if there is some strange authentication header like Bearer \x9d

Traceback (most recent call last):
  File "/usr/local/lib/python3.6/site-packages/rest_framework/views.py", line 493, in dispatch
    self.initial(request, *args, **kwargs)
  File "/usr/local/lib/python3.6/site-packages/rest_framework/views.py", line 410, in initial
    self.perform_authentication(request)
  File "/usr/local/lib/python3.6/site-packages/rest_framework/views.py", line 324, in perform_authentication
    request.user
  File "/usr/local/lib/python3.6/site-packages/rest_framework/request.py", line 220, in user
    self._authenticate()
  File "/usr/local/lib/python3.6/site-packages/rest_framework/request.py", line 373, in _authenticate
    user_auth_tuple = authenticator.authenticate(self)
  File "/usr/local/lib/python3.6/site-packages/rest_framework_jwt/authentication.py", line 66, in authenticate
    token = self.get_token_from_request(request)
  File "/usr/local/lib/python3.6/site-packages/rest_framework_jwt/authentication.py", line 91, in get_token_from_request
    authorization_header = force_str(get_authorization_header(request))
  File "/usr/local/lib/python3.6/site-packages/django/utils/encoding.py", line 69, in force_text
    raise DjangoUnicodeDecodeError(s, *e.args)
django.utils.encoding.DjangoUnicodeDecodeError: 'utf-8' codec can't decode byte 0x9d in position 7: invalid start byte. You passed in b'Bearer \x9d' (<class 'bytes'>)
jayvdb commented 4 years ago

Note this was found using https://pypi.org/project/schemathesis/ , which tries to find 500s like this. The correct response is either to blame the client (not 500 - the server), or ignore it so some other part of django could respond to it if they understand it.

fitodic commented 4 years ago

@jayvdb Thanks for reporting this. If I understood the issue you've described in the listed issues, these tokens are malformed to begin with, so returning a 4xx response to the client sounds like the best way forward. Any thoughts on the matter?

jayvdb commented 4 years ago

For my own needs / use case, returning a 4xx response would be perfect.

However, should drf-jwt claim ownership of the auth? Or defer to other auth providers in case they can make sense of it ?

Sort of touched on that conundrum at

https://github.com/Styria-Digital/django-rest-framework-jwt/pull/71/files#r463141406

fitodic commented 4 years ago

Considering the amount of possible authentication mechanisms per-site, I'm in favor of deferring.

A DEBUG/INFO level logger could be added if necessary, but then again, that implies that it could be troubleshooted locally using a debugger. I don't see the need for using a higher level logger as it would potentially clutter the logging service with noise which could only mislead developers.

jayvdb commented 3 years ago

The PR already defers.

This library doesnt do any logging. It would be odd to introduce logging only for this obscure problem.

Could you please review my PR. It is a bit annoying that there was a release without my PR. I need to use a git+ pip install in production because of this.