Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error uploading a raw file with the DRF #1406

Closed
nnseva opened this issue Mar 6, 2024 · 5 comments
Closed

Error uploading a raw file with the DRF #1406

nnseva opened this issue Mar 6, 2024 · 5 comments
Labels

Comments

@nnseva
Copy link

nnseva commented Mar 6, 2024

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

  • [ x] I have tested with the latest published release and it's still a problem.
  • [x ] I have tested with the master branch and it's still a problem.

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'
 )
@nnseva nnseva added the bug label Mar 6, 2024
@n2ygk
Copy link
Member

n2ygk commented May 20, 2024

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

@nnseva
Copy link
Author

nnseva commented Jun 10, 2024

@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
Copy link
Member

n2ygk commented Jun 17, 2024

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

@nnseva
Copy link
Author

nnseva commented Jul 23, 2024

Hi @n2ygk,

I've found some extra time to investigate the issue.

Actually, this is not an issue of the django-oauth-toolkit. The problem is that the DRF has stronger rules to receive the POST body, requiring explicit parsers for allowed Content-Types of the endpoint. When the parser is not found, it raises the UnsupportedMediaType DRF-specific exception, not immediately, but when the endpoint handler tries to get request.POST only. That's the reason why your code getting request.POST in the OAuthLibCore.extract_body() method finally raises this exception.

Such a behaviour of the DRF is not clean enough, because it changes native Django request behaviour which silently returns empty request.POST member in case of unrecognized Content-Type. Such a way, the DRF messes up the stack trace and programmer's mind.

Now I've found a proper solution and will resolve the issue in my code without a patch above. Sorry for disturb.

@nnseva
Copy link
Author

nnseva commented Jul 23, 2024

Closing the issue

@nnseva nnseva closed this as completed Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants