jazzband / django-oauth-toolkit

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

Error uploading a raw file with the DRF #1406

Open nnseva opened 3 months ago

nnseva commented 3 months ago

Describe the bug When using the DRF and creating an action view to upload a raw file in a POST body, the OAuth2 authenticator messes up this request raising either rest_framework.exceptions.UnsupportedMediaType, or django.core.exceptions.TooManyFieldsSent.

It happens because of the unconditional call of the request.POST.items() in the OAuthLibCore.extract_body() method.

To Reproduce Create some raw upload URL using the DRF. My code was approximately:

...
class BaseMapViewSet(viewsets.ModelViewSet):
    ....
    @action(detail=True, methods=['post'])
    def upload(self, request, pk=None):
        bm = self.get_object()
        ...
        bm.contents.save(filename, ContentFile(request.read()))

Register this view in the DRF as usual

...
router.register(r'base_maps', maps_views.BaseMapViewSet)
...

Setup the OAuth2 DRF authorization provided by the toolkit:

REST_FRAMEWORK = {
    'DEFAULT_FILTER_BACKENDS': ['django_filters.rest_framework.DjangoFilterBackend'],
    'DEFAULT_AUTHENTICATION_CLASSES': [
        'oauth2_provider.contrib.rest_framework.authentication.OAuth2Authentication',
        ...
    ],
}

Start the server.

Authorize yourself somehow (using OAuth2, f.e.)

Try to upload some binary file:

curl http://127.0.0.1:8000/api/base_maps/some_id/upload/ --data-binary @some-file-for-upload.bin -H "Authorization: ...." -H "Content-Type: application/stream"

The result is failed, with a response code=415 like

{"detail":"Unsupported media type \"application/stream\" in request."}

Try to upload a big binary file ignoring a Content Type header:

curl http://127.0.0.1:8000/api/base_maps/some_id/upload/ --data-binary @some-file-for-upload.bin -H "Authorization: ...."

The result is failed, with a response 400 Bad Request (the django.core.exceptions.TooManyFieldsSent is reported in the console).

Expected behavior Success processing and response as minimum in the first case.

Version django-oauth-toolkit==2.3.0

Additional context The change like the following fixes the issue:

diff --git a/oauth_app/utils.py b/oauth_app/utils.py
new file mode 100644
index 0000000..08b3cb1
--- /dev/null
+++ b/oauth_app/utils.py
@@ -0,0 +1,13 @@
+from oauth2_provider.oauth2_backends import OAuthLibCore as _OAuthLibCore
+
+
+class OAuthLibCore(_OAuthLibCore):
+    """Fixing some issues"""
+
+    def extract_body(self, request):
+        """Override to quick fix issues with unnecessary request.POST.items() call"""
+        try:
+            return super().extract_body(request)
+        except Exception:
+            pass
+        return {}
diff --git a/config/settings.py b/config/settings.py
index 793fcc9..e99f815 100644
--- a/config/settings.py
+++ b/config/settings.py
@@ -384,6 +384,7 @@ OAUTH2_PROVIDER = dict(
     REFRESH_TOKEN_ADMIN_CLASS='oauth_app.admin.RefreshTokenAdmin',
     ID_TOKEN_ADMIN_CLASS='oauth_app.admin.IDTokenAdmin',
     PKCE_REQUIRED=False,
+    OAUTH2_BACKEND_CLASS='oauth_app.utils.OAuthLibCore'
 )
n2ygk commented 1 month ago

I'm confused. Shouldn't only OAuth2-related stuff be hitting this endpoint, not arbitrary binary streams?

nnseva commented 3 weeks ago

@n2ygk actually this endpoint extract_body is called every time when the request is received trying to authorize it - the original class tries to find a token in the POST body. This is a reason why I've made this fix. For the proper processing it should probably check the Content-Type header to make a decision, whether it could search for the token in the body at all, and how to deserialize it for the purpose if could.

n2ygk commented 2 weeks ago

@nnseva it would help if you were to submit a PR to address this issue, starting with a failing test case. See https://django-oauth-toolkit.readthedocs.io/en/latest/contributing.html

I still don't understand why it's looking for a token in the POST body if there's an Authorization: Bearer <token> header....