Disable multipart uploads by default (#3645)

* Disable multipart uploads by default * Document the new option * Stop disabling Django's CSRF protection by default * Document breaking changes * Add release file * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Bump date * Test * Add tweet file * Shorter tweet --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Patrick Arminio <[email protected]>
strawberry-graphql · Sep 25, 2024 · 37265b2 · 37265b2
1 parent 18f0f5d
commit 37265b2
Show file tree

Hide file tree

Showing 40 changed files with 207 additions and 44 deletions.
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -59,20 +59,6 @@ jobs:
             3.12
             3.13-dev
 
-      - name: Pip and nox cache
-        id: cache
-        uses: actions/cache@v4
-        with:
-          path: |
-            ~/.cache
-            ~/.nox
-            .nox
-          key:
-            ${{ runner.os }}-nox-${{ matrix.session.session }}-${{ env.pythonLocation }}-${{
-            hashFiles('**/poetry.lock') }}-${{ hashFiles('**/noxfile.py') }}
-          restore-keys: |
-            ${{ runner.os }}-nox-${{ matrix.session.session }}-${{ env.pythonLocation }}
-
       - run: pip install poetry nox nox-poetry uv
       - run: nox -r -t tests -s "${{ matrix.session.session }}"
       - uses: actions/upload-artifact@v4

diff --git a/RELEASE.md b/RELEASE.md
@@ -0,0 +1,7 @@
+Release type: minor
+
+Starting with this release, multipart uploads are disabled by default and Strawberry Django view is no longer implicitly exempted from Django's CSRF protection.
+Both changes relieve users from implicit security implications inherited from the GraphQL multipart request specification which was enabled in Strawberry by default.
+
+These are breaking changes if you are using multipart uploads OR the Strawberry Django view.
+Migrations guides including further information are available on the Strawberry website.
diff --git a/TWEET.md b/TWEET.md
@@ -0,0 +1,8 @@
+🆕 Release $version is out! Thanks to $contributor 👏
+
+We've made some important security changes regarding file uploads and CSRF in
+Django.
+
+Check out our migration guides if you're using multipart or Django view.
+
+👇 $release_url
diff --git a/docs/breaking-changes.md b/docs/breaking-changes.md
@@ -4,6 +4,7 @@ title: List of breaking changes and deprecations
 
 # List of breaking changes and deprecations
 
+- [Version 0.243.0 - 25 September 2024](./breaking-changes/0.243.0.md)
 - [Version 0.240.0 - 10 September 2024](./breaking-changes/0.240.0.md)
 - [Version 0.236.0 - 17 July 2024](./breaking-changes/0.236.0.md)
 - [Version 0.233.0 - 29 May 2024](./breaking-changes/0.233.0.md)

diff --git a/docs/breaking-changes/0.243.0.md b/docs/breaking-changes/0.243.0.md
@@ -0,0 +1,53 @@
+---
+title: 0.243.0 Breaking Changes
+slug: breaking-changes/0.243.0
+---
+
+# v0.240.0 Breaking Changes
+
+Release v0.240.0 comes with two breaking changes regarding multipart file
+uploads and Django CSRF protection.
+
+## Multipart uploads disabled by default
+
+Previously, support for uploads via the
+[GraphQL multipart request specification](https://github.com/jaydenseric/graphql-multipart-request-spec)
+was enabled by default. This implicitly required Strawberry users to consider
+the
+[security implications outlined in the GraphQL Multipart Request Specification](https://github.com/jaydenseric/graphql-multipart-request-spec/blob/master/readme.md#security).
+Given that most Strawberry users were likely not aware of this, we're making
+multipart file upload support stictly opt-in via a new
+`multipart_uploads_enabled` view settings.
+
+To enable multipart upload support for your Strawberry view integration, please
+follow the updated integration guides and enable appropriate security
+measurements for your server.
+
+## Django CSRF protection enabled
+
+Previously, the Strawberry Django view integration was internally exempted from
+Django's built-in CSRF protection (i.e, the `CsrfViewMiddleware` middleware).
+While this is how many GraphQL APIs operate, implicitly addded exemptions can
+lead to security vulnerabilities. Instead, we delegate the decision of adding an
+CSRF exemption to users now.
+
+Note that having the CSRF protection enabled on your Strawberry Django view
+potentially requires all your clients to send an CSRF token with every request.
+You can learn more about this in the official Django
+[Cross Site Request Forgery protection documentation](https://docs.djangoproject.com/en/dev/ref/csrf/).
+
+To restore the behaviour of the integration before this release, you can add the
+`csrf_exempt` decorator provided by Django yourself:
+
+```python
+from django.urls import path
+from django.views.decorators.csrf import csrf_exempt
+
+from strawberry.django.views import GraphQLView
+
+from api.schema import schema
+
+urlpatterns = [
+    path("graphql/", csrf_exempt(GraphQLView.as_view(schema=schema))),
+]
+```
diff --git a/docs/integrations/aiohttp.md b/docs/integrations/aiohttp.md
@@ -29,14 +29,18 @@ app.router.add_route("*", "/graphql", GraphQLView(schema=schema))
 
 ## Options
 
-The `GraphQLView` accepts two options at the moment:
+The `GraphQLView` accepts the following options at the moment:
 
 - `schema`: mandatory, the schema created by `strawberry.Schema`.
 - `graphql_ide`: optional, defaults to `"graphiql"`, allows to choose the
   GraphQL IDE interface (one of `graphiql`, `apollo-sandbox` or `pathfinder`) or
   to disable it by passing `None`.
 - `allow_queries_via_get`: optional, defaults to `True`, whether to enable
   queries via `GET` requests
+- `multipart_uploads_enabled`: optional, defaults to `False`, controls whether
+  to enable multipart uploads. Please make sure to consider the
+  [security implications mentioned in the GraphQL Multipart Request Specification](https://github.com/jaydenseric/graphql-multipart-request-spec/blob/master/readme.md#security)
+  when enabling this feature.
 
 ## Extending the view
 

diff --git a/docs/integrations/asgi.md b/docs/integrations/asgi.md
@@ -29,14 +29,18 @@ app with `uvicorn server:app`
 
 ## Options
 
-The `GraphQL` app accepts two options at the moment:
+The `GraphQL` app accepts the following options at the moment:
 
 - `schema`: mandatory, the schema created by `strawberry.Schema`.
 - `graphql_ide`: optional, defaults to `"graphiql"`, allows to choose the
   GraphQL IDE interface (one of `graphiql`, `apollo-sandbox` or `pathfinder`) or
   to disable it by passing `None`.
 - `allow_queries_via_get`: optional, defaults to `True`, whether to enable
   queries via `GET` requests
+- `multipart_uploads_enabled`: optional, defaults to `False`, controls whether
+  to enable multipart uploads. Please make sure to consider the
+  [security implications mentioned in the GraphQL Multipart Request Specification](https://github.com/jaydenseric/graphql-multipart-request-spec/blob/master/readme.md#security)
+  when enabling this feature.
 
 ## Extending the view
 

diff --git a/docs/integrations/channels.md b/docs/integrations/channels.md
@@ -524,6 +524,10 @@ GraphQLWebsocketCommunicator(
   queries via `GET` requests
 - `subscriptions_enabled`: optional boolean paramenter enabling subscriptions in
   the GraphiQL interface, defaults to `True`
+- `multipart_uploads_enabled`: optional, defaults to `False`, controls whether
+  to enable multipart uploads. Please make sure to consider the
+  [security implications mentioned in the GraphQL Multipart Request Specification](https://github.com/jaydenseric/graphql-multipart-request-spec/blob/master/readme.md#security)
+  when enabling this feature.
 
 ### Extending the consumer
 

diff --git a/docs/integrations/django.md b/docs/integrations/django.md
@@ -10,13 +10,14 @@ It provides a view that you can use to serve your GraphQL schema:
 
 ```python
 from django.urls import path
+from django.views.decorators.csrf import csrf_exempt
 
 from strawberry.django.views import GraphQLView
 
 from api.schema import schema
 
 urlpatterns = [
-    path("graphql/", GraphQLView.as_view(schema=schema)),
+    path("graphql/", csrf_exempt(GraphQLView.as_view(schema=schema))),
 ]
 ```
 
@@ -40,6 +41,10 @@ The `GraphQLView` accepts the following arguments:
   queries via `GET` requests
 - `subscriptions_enabled`: optional boolean paramenter enabling subscriptions in
   the GraphiQL interface, defaults to `False`.
+- `multipart_uploads_enabled`: optional, defaults to `False`, controls whether
+  to enable multipart uploads. Please make sure to consider the
+  [security implications mentioned in the GraphQL Multipart Request Specification](https://github.com/jaydenseric/graphql-multipart-request-spec/blob/master/readme.md#security)
+  when enabling this feature.
 
 ## Deprecated options
 

diff --git a/docs/integrations/fastapi.md b/docs/integrations/fastapi.md
@@ -54,6 +54,10 @@ The `GraphQLRouter` accepts the following options:
   value.
 - `root_value_getter`: optional FastAPI dependency for providing custom root
   value.
+- `multipart_uploads_enabled`: optional, defaults to `False`, controls whether
+  to enable multipart uploads. Please make sure to consider the
+  [security implications mentioned in the GraphQL Multipart Request Specification](https://github.com/jaydenseric/graphql-multipart-request-spec/blob/master/readme.md#security)
+  when enabling this feature.
 
 ## context_getter
 

diff --git a/docs/integrations/flask.md b/docs/integrations/flask.md
@@ -34,13 +34,17 @@ from strawberry.flask.views import AsyncGraphQLView
 
 ## Options
 
-The `GraphQLView` accepts two options at the moment:
+The `GraphQLView` accepts the following options at the moment:
 
 - `schema`: mandatory, the schema created by `strawberry.Schema`.
 - `graphiql:` optional, defaults to `True`, whether to enable the GraphiQL
   interface.
 - `allow_queries_via_get`: optional, defaults to `True`, whether to enable
   queries via `GET` requests
+- `multipart_uploads_enabled`: optional, defaults to `False`, controls whether
+  to enable multipart uploads. Please make sure to consider the
+  [security implications mentioned in the GraphQL Multipart Request Specification](https://github.com/jaydenseric/graphql-multipart-request-spec/blob/master/readme.md#security)
+  when enabling this feature.
 
 ## Extending the view
 

diff --git a/docs/integrations/litestar.md b/docs/integrations/litestar.md
@@ -61,6 +61,10 @@ The `make_graphql_controller` function accepts the following options:
   the maximum time to wait for the connection initialization message when using
   `graphql-transport-ws`
   [protocol](https://github.com/enisdenjo/graphql-ws/blob/master/PROTOCOL.md#connectioninit)
+- `multipart_uploads_enabled`: optional, defaults to `False`, controls whether
+  to enable multipart uploads. Please make sure to consider the
+  [security implications mentioned in the GraphQL Multipart Request Specification](https://github.com/jaydenseric/graphql-multipart-request-spec/blob/master/readme.md#security)
+  when enabling this feature.
 
 ## context_getter
 

diff --git a/docs/integrations/quart.md b/docs/integrations/quart.md
@@ -26,13 +26,17 @@ if __name__ == "__main__":
 
 ## Options
 
-The `GraphQLView` accepts two options at the moment:
+The `GraphQLView` accepts the following options at the moment:
 
 - `schema`: mandatory, the schema created by `strawberry.Schema`.
 - `graphiql:` optional, defaults to `True`, whether to enable the GraphiQL
   interface.
 - `allow_queries_via_get`: optional, defaults to `True`, whether to enable
   queries via `GET` requests
+- `multipart_uploads_enabled`: optional, defaults to `False`, controls whether
+  to enable multipart uploads. Please make sure to consider the
+  [security implications mentioned in the GraphQL Multipart Request Specification](https://github.com/jaydenseric/graphql-multipart-request-spec/blob/master/readme.md#security)
+  when enabling this feature.
 
 ## Extending the view
 

diff --git a/docs/integrations/sanic.md b/docs/integrations/sanic.md
@@ -22,15 +22,18 @@ app.add_route(
 
 ## Options
 
-The `GraphQLView` accepts two options at the moment:
+The `GraphQLView` accepts the following options at the moment:
 
 - `schema`: mandatory, the schema created by `strawberry.Schema`.
 - `graphql_ide`: optional, defaults to `"graphiql"`, allows to choose the
   GraphQL IDE interface (one of `graphiql`, `apollo-sandbox` or `pathfinder`) or
   to disable it by passing `None`.
 - `allow_queries_via_get`: optional, defaults to `True`, whether to enable
   queries via `GET` requests
-- `def encode_json(self, data: GraphQLHTTPResponse) -> str`
+- `multipart_uploads_enabled`: optional, defaults to `False`, controls whether
+  to enable multipart uploads. Please make sure to consider the
+  [security implications mentioned in the GraphQL Multipart Request Specification](https://github.com/jaydenseric/graphql-multipart-request-spec/blob/master/readme.md#security)
+  when enabling this feature.
 
 ## Extending the view
 

diff --git a/strawberry/aiohttp/views.py b/strawberry/aiohttp/views.py
@@ -111,6 +111,7 @@ def __init__(
             GRAPHQL_WS_PROTOCOL,
         ),
         connection_init_wait_timeout: timedelta = timedelta(minutes=1),
+        multipart_uploads_enabled: bool = False,
     ) -> None:
         self.schema = schema
         self.allow_queries_via_get = allow_queries_via_get
@@ -119,6 +120,7 @@ def __init__(
         self.debug = debug
         self.subscription_protocols = subscription_protocols
         self.connection_init_wait_timeout = connection_init_wait_timeout
+        self.multipart_uploads_enabled = multipart_uploads_enabled
 
         if graphiql is not None:
             warnings.warn(

diff --git a/strawberry/asgi/__init__.py b/strawberry/asgi/__init__.py
@@ -106,6 +106,7 @@ def __init__(
             GRAPHQL_WS_PROTOCOL,
         ),
         connection_init_wait_timeout: timedelta = timedelta(minutes=1),
+        multipart_uploads_enabled: bool = False,
     ) -> None:
         self.schema = schema
         self.allow_queries_via_get = allow_queries_via_get
@@ -114,6 +115,7 @@ def __init__(
         self.debug = debug
         self.protocols = subscription_protocols
         self.connection_init_wait_timeout = connection_init_wait_timeout
+        self.multipart_uploads_enabled = multipart_uploads_enabled
 
         if graphiql is not None:
             warnings.warn(

diff --git a/strawberry/channels/handlers/http_handler.py b/strawberry/channels/handlers/http_handler.py
@@ -168,12 +168,14 @@ def __init__(
         graphql_ide: Optional[GraphQL_IDE] = "graphiql",
         allow_queries_via_get: bool = True,
         subscriptions_enabled: bool = True,
+        multipart_uploads_enabled: bool = False,
         **kwargs: Any,
     ) -> None:
         self.schema = schema
         self.allow_queries_via_get = allow_queries_via_get
         self.subscriptions_enabled = subscriptions_enabled
         self._ide_subscriptions_enabled = subscriptions_enabled
+        self.multipart_uploads_enabled = multipart_uploads_enabled
 
         if graphiql is not None:
             warnings.warn(

diff --git a/strawberry/django/views.py b/strawberry/django/views.py
@@ -28,8 +28,7 @@
 from django.template.exceptions import TemplateDoesNotExist
 from django.template.loader import render_to_string
 from django.template.response import TemplateResponse
-from django.utils.decorators import classonlymethod, method_decorator
-from django.views.decorators.csrf import csrf_exempt
+from django.utils.decorators import classonlymethod
 from django.views.generic import View
 
 from strawberry.http.async_base_view import AsyncBaseHTTPView, AsyncHTTPRequestAdapter
@@ -147,11 +146,13 @@ def __init__(
         graphql_ide: Optional[GraphQL_IDE] = "graphiql",
         allow_queries_via_get: bool = True,
         subscriptions_enabled: bool = False,
+        multipart_uploads_enabled: bool = False,
         **kwargs: Any,
     ) -> None:
         self.schema = schema
         self.allow_queries_via_get = allow_queries_via_get
         self.subscriptions_enabled = subscriptions_enabled
+        self.multipart_uploads_enabled = multipart_uploads_enabled
 
         if graphiql is not None:
             warnings.warn(
@@ -229,7 +230,6 @@ def get_context(self, request: HttpRequest, response: HttpResponse) -> Any:
     def get_sub_response(self, request: HttpRequest) -> TemporalHttpResponse:
         return TemporalHttpResponse()
 
-    @method_decorator(csrf_exempt)
     def dispatch(
         self, request: HttpRequest, *args: Any, **kwargs: Any
     ) -> Union[HttpResponseNotAllowed, TemplateResponse, HttpResponseBase]:
@@ -288,7 +288,6 @@ async def get_context(self, request: HttpRequest, response: HttpResponse) -> Any
     async def get_sub_response(self, request: HttpRequest) -> TemporalHttpResponse:
         return TemporalHttpResponse()
 
-    @method_decorator(csrf_exempt)
     async def dispatch(  # pyright: ignore
         self, request: HttpRequest, *args: Any, **kwargs: Any
     ) -> Union[HttpResponseNotAllowed, TemplateResponse, HttpResponseBase]:

diff --git a/strawberry/fastapi/router.py b/strawberry/fastapi/router.py
@@ -156,6 +156,7 @@ def __init__(
         generate_unique_id_function: Callable[[APIRoute], str] = Default(
             generate_unique_id
         ),
+        multipart_uploads_enabled: bool = False,
         **kwargs: Any,
     ) -> None:
         super().__init__(
@@ -190,6 +191,7 @@ def __init__(
         )
         self.protocols = subscription_protocols
         self.connection_init_wait_timeout = connection_init_wait_timeout
+        self.multipart_uploads_enabled = multipart_uploads_enabled
 
         if graphiql is not None:
             warnings.warn(

diff --git a/strawberry/flask/views.py b/strawberry/flask/views.py
@@ -71,10 +71,12 @@ def __init__(
         graphiql: Optional[bool] = None,
         graphql_ide: Optional[GraphQL_IDE] = "graphiql",
         allow_queries_via_get: bool = True,
+        multipart_uploads_enabled: bool = False,
     ) -> None:
         self.schema = schema
         self.graphiql = graphiql
         self.allow_queries_via_get = allow_queries_via_get
+        self.multipart_uploads_enabled = multipart_uploads_enabled
 
         if graphiql is not None:
             warnings.warn(

diff --git a/strawberry/http/async_base_view.py b/strawberry/http/async_base_view.py
@@ -333,7 +333,7 @@ async def parse_http_body(
             data = self.parse_query_params(request.query_params)
         elif "application/json" in content_type:
             data = self.parse_json(await request.get_body())
-        elif content_type == "multipart/form-data":
+        elif self.multipart_uploads_enabled and content_type == "multipart/form-data":
             data = await self.parse_multipart(request)
         else:
             raise HTTPException(400, "Unsupported content type")

diff --git a/strawberry/http/base.py b/strawberry/http/base.py
@@ -23,6 +23,7 @@ def headers(self) -> Mapping[str, str]: ...
 
 class BaseView(Generic[Request]):
     graphql_ide: Optional[GraphQL_IDE]
+    multipart_uploads_enabled: bool = False
 
     # TODO: we might remove this in future :)
     _ide_replace_variables: bool = True