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

Disable multipart uploads by default #3645

Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions RELEASE.md
Original file line number Diff line number Diff line change
@@ -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.
1 change: 1 addition & 0 deletions docs/breaking-changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ title: List of breaking changes and deprecations

# List of breaking changes and deprecations

- [Version 0.243.0 - 24 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)
Expand Down
40 changes: 40 additions & 0 deletions docs/breaking-changes/0.243.0.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
---
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))),
]
```
4 changes: 3 additions & 1 deletion docs/integrations/aiohttp.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,16 @@ 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

Expand Down
4 changes: 3 additions & 1 deletion docs/integrations/asgi.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,16 @@ 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

Expand Down
2 changes: 2 additions & 0 deletions docs/integrations/channels.md
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,8 @@ 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

Expand Down
5 changes: 4 additions & 1 deletion docs/integrations/django.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))),
]
```

Expand All @@ -40,6 +41,8 @@ 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

Expand Down
2 changes: 2 additions & 0 deletions docs/integrations/fastapi.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ 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

Expand Down
4 changes: 3 additions & 1 deletion docs/integrations/flask.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,15 @@ 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

Expand Down
2 changes: 2 additions & 0 deletions docs/integrations/litestar.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ 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

Expand Down
4 changes: 3 additions & 1 deletion docs/integrations/quart.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,15 @@ 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

Expand Down
5 changes: 3 additions & 2 deletions docs/integrations/sanic.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,16 @@ 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

Expand Down
2 changes: 2 additions & 0 deletions strawberry/aiohttp/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(
Expand Down
2 changes: 2 additions & 0 deletions strawberry/asgi/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(
Expand Down
2 changes: 2 additions & 0 deletions strawberry/channels/handlers/http_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
7 changes: 3 additions & 4 deletions strawberry/django/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
DoctorJohn marked this conversation as resolved.
Show resolved Hide resolved
**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(
Expand Down Expand Up @@ -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(
DoctorJohn marked this conversation as resolved.
Show resolved Hide resolved
self, request: HttpRequest, *args: Any, **kwargs: Any
) -> Union[HttpResponseNotAllowed, TemplateResponse, HttpResponseBase]:
Expand Down Expand Up @@ -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]:
Expand Down
2 changes: 2 additions & 0 deletions strawberry/fastapi/router.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__(
Expand Down Expand Up @@ -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(
Expand Down
2 changes: 2 additions & 0 deletions strawberry/flask/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion strawberry/http/async_base_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
1 change: 1 addition & 0 deletions strawberry/http/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion strawberry/http/sync_base_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ def parse_http_body(self, request: SyncHTTPRequestAdapter) -> GraphQLRequestData
elif "application/json" in content_type:
data = self.parse_json(request.body)
# TODO: multipart via get?
elif content_type == "multipart/form-data":
elif self.multipart_uploads_enabled and content_type == "multipart/form-data":
data = self.parse_multipart(request)
elif self._is_multipart_subscriptions(content_type, params):
raise HTTPException(
Expand Down
2 changes: 2 additions & 0 deletions strawberry/litestar/controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,7 @@ def make_graphql_controller(
GRAPHQL_WS_PROTOCOL,
),
connection_init_wait_timeout: timedelta = timedelta(minutes=1),
multipart_uploads_enabled: bool = False,
) -> Type[GraphQLController]: # sourcery skip: move-assign
if context_getter is None:
custom_context_getter_ = _none_custom_context_getter
Expand Down Expand Up @@ -456,6 +457,7 @@ class _GraphQLController(GraphQLController):
_GraphQLController.schema = schema_
_GraphQLController.allow_queries_via_get = allow_queries_via_get_
_GraphQLController.graphql_ide = graphql_ide_
_GraphQLController.multipart_uploads_enabled = multipart_uploads_enabled

return _GraphQLController

Expand Down
2 changes: 2 additions & 0 deletions strawberry/quart/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,11 @@ 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.allow_queries_via_get = allow_queries_via_get
self.multipart_uploads_enabled = multipart_uploads_enabled

if graphiql is not None:
warnings.warn(
Expand Down
2 changes: 2 additions & 0 deletions strawberry/sanic/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,13 @@ def __init__(
allow_queries_via_get: bool = True,
json_encoder: Optional[Type[json.JSONEncoder]] = None,
json_dumps_params: Optional[Dict[str, Any]] = None,
multipart_uploads_enabled: bool = False,
) -> None:
self.schema = schema
self.allow_queries_via_get = allow_queries_via_get
self.json_encoder = json_encoder
self.json_dumps_params = json_dumps_params
self.multipart_uploads_enabled = multipart_uploads_enabled

if self.json_encoder is not None: # pragma: no cover
warnings.warn(
Expand Down
2 changes: 2 additions & 0 deletions tests/http/clients/aiohttp.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,15 @@ def __init__(
graphql_ide: Optional[GraphQL_IDE] = "graphiql",
allow_queries_via_get: bool = True,
result_override: ResultOverrideFunction = None,
multipart_uploads_enabled: bool = False,
):
view = GraphQLView(
schema=schema,
graphiql=graphiql,
graphql_ide=graphql_ide,
allow_queries_via_get=allow_queries_via_get,
keep_alive=False,
multipart_uploads_enabled=multipart_uploads_enabled,
)
view.result_override = result_override

Expand Down
Loading
Loading