strawberry-graphql / strawberry

A GraphQL library for Python that leverages type annotations 🍓
https://strawberry.rocks
MIT License
4.03k stars 536 forks source link

Django `AsyncGraphQLView` ignores setting additional http response headers (CORS issue) #2290

Open capital-G opened 2 years ago

capital-G commented 2 years ago

As Graphql is used as an API it is crucial to access it from different origins, such as an JS frontend from the browser which runs on a different port (or even IP). When doing a request to another origin (e.g. from localhost:3000 (JS frontend) to localhost:8000(django server)) the browser will verify the Access-Control-Allow-Origin header in the response from the server and will verify if this matches our origin (as it is a security flaw if anybody can send us requests to which we respond) - if this header is not present or is missing our origin a CORS error will be raised.

In production you get around this by putting everything behind a reverse proxy like nginx, creating a unified origin, but during developing you can use https://github.com/adamchainz/django-cors-headers to add the Access-Control-Allow-Origin header to our django HTTP responses which works with DRF or django admin (I did not get a CORS error accessing the admin site).

But how does the client know what is allowed and what is not allowed? Before sending a POST request the client/browser will send an OPTIONS request, known as an preflight request This will respond with headers such as Access-Control-Allow-Origin so the client/browser knows if its OK to POST to the server. For some reason the AsyncGraphQLView will respond on the preflight OPTION request with a 405 - Method Not Allowed response which will lead to an CORS error. The same issue also occured in the base library with FastAPI: https://github.com/strawberry-graphql/strawberry/issues/1942

image

Interestingly enough before updating to Firefox 106.0.2 I did not have this problem - Chrome results in the same error.

Trying to modify this

https://github.com/strawberry-graphql/strawberry/blob/609ebc7ea6e210e4c7801361f345f345c84499ba/strawberry/django/views.py#L223-L227

and redirecting to a simple/empty HttpResonse in case of an OPTIONS request coming in did not resolve the issue.

Here is some inspections using curl.

It works using django-cors-headers on admin/

❯ curl -H "Access-Control-Request-Method: GET" -H "Origin: http://localhost:8081" --head http://localhost:8000/admin/
HTTP/1.1 302 Found
Content-Type: text/html; charset=utf-8
Location: /admin/login/?next=/admin/
Expires: Sun, 30 Oct 2022 12:40:34 GMT
Cache-Control: max-age=0, no-cache, no-store, must-revalidate, private
X-Frame-Options: DENY
Content-Length: 0
Vary: Cookie, Origin
X-Content-Type-Options: nosniff
Referrer-Policy: same-origin
Cross-Origin-Opener-Policy: same-origin
Access-Control-Allow-Credentials: true
Access-Control-Allow-Origin: http://localhost:8081

but fails on graphql/ to set these headers

~
❯ curl -H "Access-Control-Request-Method: GET" -H "Origin: http://localhost:8081" --head http://localhost:8000/graphql/
HTTP/1.1 405 Method Not Allowed
Allow: GET, POST

Using the GET method for the API (which does not make use of a OPTIONS request) also ignores set the headers via django-cors-headers.

Maybe the root problem is here that Strawberry is using its own HttpResponse class

https://github.com/strawberry-graphql/strawberry/blob/609ebc7ea6e210e4c7801361f345f345c84499ba/strawberry/http/__init__.py#L13-L16

which is not subclassed from the Django HttpResponse.

I tried to force a django HTTP response by injecting return HttpResponse("hello") into the first line of the function

https://github.com/strawberry-graphql/strawberry/blob/609ebc7ea6e210e4c7801361f345f345c84499ba/strawberry/django/views.py#L223-L224

but it was not picked up, although running an async dev server - right now I am out of ideas.

System Information

Additional Context

Upvote & Fund

Fund with Polar

jkimbo commented 2 years ago

@capital-G are you sure you've setup django-cors-headers correctly? The middleware should intercept all OPTION requests before they hit the Strawberry view: https://github.com/adamchainz/django-cors-headers/blob/c452118a208de50435cf55bb8e32be2f5e966aa8/src/corsheaders/middleware.py#L94-L100

capital-G commented 2 years ago

Thanks for the hint, it seems everything was caused by using GraphQLProtocolTypeRouter which I setup as described in https://strawberry.rocks/docs/integrations/channels#creating-the-consumers

This seems to remove headers if run in async mode (gunicorn/uvicorn) but not when run with the django debug server - took some time to figure this one out.

So, shall this remain open or is this more a channels issue?

patrick91 commented 2 years ago

@capital-G let's keep it open, we should possibly add a note about CORS in that document

capital-G commented 1 year ago

If anyone is interested, my current setup looks like this - it is really hacky but resolves my cors issues by sharing the cookie under the same domain.

asgi.py

import os

from channels.routing import ProtocolTypeRouter, URLRouter
from django.core.asgi import get_asgi_application
from django.urls import re_path
from strawberry.channels import GraphQLWSConsumer

os.environ.setdefault("DJANGO_SETTINGS_MODULE", "###.settings.dev")

django_asgi_app = get_asgi_application()

from .schema import schema

websocket_urlpatterns = [
    re_path(
        r"graphql",
        GraphQLWSConsumer.as_asgi(
            schema=schema,
        ),
    ),
]

application = ProtocolTypeRouter(
    {
        "http": URLRouter([re_path("^", django_asgi_app)]),  # type: ignore
        "websocket": URLRouter(websocket_urlpatterns),
    }
)

urls.py

from dataclasses import dataclass

from channels.layers import BaseChannelLayer, get_channel_layer
from django.conf import settings
from django.conf.urls.static import static
from django.contrib import admin
from django.http import HttpRequest
from django.shortcuts import HttpResponse
from django.urls import path
from django.utils.decorators import method_decorator
from django.views.decorators.csrf import csrf_exempt
from strawberry.django.views import AsyncGraphQLView

from .schema import schema

admin.site.site_header = "### admin"

@dataclass
class Context:
    channel_layer: BaseChannelLayer

class CorsAsyncGraphQLView(AsyncGraphQLView):
    """A hack to add CORS headers to our GraphQL endpoint."""

    def _create_response(self, response_data, sub_response):
        r = super()._create_response(response_data, sub_response)
        return r

    @method_decorator(csrf_exempt)
    async def dispatch(self, request, *args, **kwargs):
        if request.method.lower() == "options":
            return HttpResponse()
        return await super().dispatch(request, *args, **kwargs)

    async def get_context(
        self, request: HttpRequest, response: HttpResponse
    ) -> Context:
        context: Context = await super().get_context(request, response)
        context.channel_layer = get_channel_layer()  # type: ignore
        return context

urlpatterns = (
    [
        path("admin/", admin.site.urls),
        path(
            "graphql",
            CorsAsyncGraphQLView.as_view(
                schema=schema,
                subscriptions_enabled=True,
                graphiql=settings.DEBUG,
            ),
        ),
    ]
    + static(settings.STATIC_URL, document_root=settings.STATIC_ROOT)  # type: ignore
    + static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT)  # type: ignore
)

local.dev.settings.py

from .base import *  # noqa

DEBUG = True

CSRF_TRUSTED_ORIGINS = [
    "http://localhost:3001",
    "http://127.0.0.1:3001",
    "http://localhost:3000",
    "http://127.0.0.1:3000",
]

CORS_ALLOWED_ORIGINS = CSRF_TRUSTED_ORIGINS

CORS_ALLOW_CREDENTIALS = True

SESSION_COOKIE_SECURE = False
SESSION_COOKIE_DOMAIN = None

# forces us to use 127.0.0.1 instead of localhost
# b/c otherwise the browser
# will not share our cookie with the editor/frontend
SESSION_COOKIE_DOMAIN = "127.0.0.1"

Auth can be implemented this way

schema.py

class AuthStrawberryDjangoField(StrawberryDjangoField):
    """Allows us to restrict certain actions to logged in users."""

    def resolver(self, info: Info, source, **kwargs):
        request: HttpRequest = info.context.request
        if not request.user.is_authenticated:
            raise PermissionDenied()
        return super().resolver(info, source, **kwargs)

async def graphql_check_authenticated(info: Info):
    """Helper function to determine if we are loggin in an async manner.

    This would be better a decorator but strawberry is strange in these regards, see
    `Stack Overflow <https://stackoverflow.com/a/72796313/3475778>`_.
    """
    auth = await sync_to_async(lambda: info.context.request.user.is_authenticated)()
    if auth is False:
        raise PermissionDenied()

@strawberry.type
class Query:
    """Queries for ###."""

    stream_point: StreamPoint = AuthStrawberryDjangoField()
jhnnsrs commented 1 year ago

faced the same problem but decided to implement a custom CORS middleware that can be used to wrap the app (still using the GraphQL Consumer), putting it here:


class CorsMiddleware:
    def __init__(self, app):
        self.app = app

    async def __call__(self, scope, receive, send):
        print(scope)
        if scope['type'] == 'http' and scope['method'] == 'OPTIONS':

            print(scope)
            # preflight request. reply successfully:
            headers = [
                (b'Access-Control-Allow-Origin', b'*'),
                (b'Access-Control-Allow-Headers', b'Authorization, Content-Type'),
                (b'Access-Control-Allow-Methods', b'GET, POST, PUT, DELETE, OPTIONS'),
                (b'Access-Control-Max-Age', b'86400'),
                (b"")
            ]
            await send({
                'type': 'http.response.start',
                'status': 200,
                'headers': headers
            })
            await send({
                'type': 'http.response.body',
                'body': b'',
            })
        else:
            async def wrapped_send(event):
                if event["type"] == "http.response.start":
                    original_headers = event.get("headers") or []
                    access_control_allow_origin = b"*"

                    if access_control_allow_origin is not None:
                        # Construct a new event with new headers
                        event = {
                            "type": "http.response.start",
                            "status": event["status"],
                            "headers": [
                                p
                                for p in original_headers
                                if p[0] != b"access-control-allow-origin"
                            ]
                            + [
                                [
                                    b"access-control-allow-origin",
                                    access_control_allow_origin,
                                ]
                            ],
                        }
                await send(event)

            await self.app(scope, receive, wrapped_send)

import os
from channels.auth import AuthMiddlewareStack
from channels.routing import ProtocolTypeRouter, URLRouter
from django.core.asgi import get_asgi_application
from django.urls import re_path
from strawberry.channels import GraphQLHTTPConsumer, GraphQLWSConsumer

os.environ.setdefault("DJANGO_SETTINGS_MODULE", "berry.settings")
django_asgi_app = get_asgi_application()

# Import your Strawberry schema after creating the django ASGI application
# This ensures django.setup() has been called before any ORM models are imported
# for the schema.

from chat import routing
from mysite.graphql import schema

websocket_urlpatterns = routing.websocket_urlpatterns + [
    re_path(r"graphql", GraphQLWSConsumer.as_asgi(schema=schema)),
]

# Order is important
gql_http_consumer = CorsMiddleware(AuthMiddlewareStack(GraphQLHTTPConsumer.as_asgi(schema=schema)))
gql_ws_consumer = GraphQLWSConsumer.as_asgi(schema=schema)
application = ProtocolTypeRouter(
    {
        "http": URLRouter(
            [
                re_path("^graphql", gql_http_consumer),
                re_path(
                    "^", django_asgi_app
                ),  # This might be another endpoint in your app
            ]
        ),
        "websocket": CorsMiddleware(AuthMiddlewareStack(URLRouter(websocket_urlpatterns))),
    }
)

Cheers. Could add a PR (with a bit more config if needed ;))

rossmeredith commented 8 months ago

@jhnnsrs Didn't work for me.

This did though: https://github.com/hot666666/asgi-cors-strawberry/blob/main/asgi_cors_strawberry/middleware.py

I've also logged a ticket with django-channels to see if they'll add this middleware: https://github.com/django/channels/issues/2081

jamietdavidson commented 3 months ago

Just ran into this issue, and want to give it a bump!

@jhnnsrs Your solution worked, I just had to remove the singleton tuple (b"") in the headers list to stop an exception that was being thrown! 🙏🏼

jamietdavidson commented 3 months ago

For anyone having problems with "corsheaders" when routing to the app provided by get_asgi_application, removing "daphne" from INSTALLED_APPS fixed the problem for me. For whatever reason that server causes problems and django runs ASGI apps fine without it!

Cross referenced comment: https://github.com/django/channels/issues/1558#issuecomment-2302624560