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

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

Open
capital-G opened this issue Oct 30, 2022 · 8 comments
Labels
bug Something isn't working

Comments

@capital-G
Copy link

capital-G commented Oct 30, 2022

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: #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

async def dispatch(self, request, *args, **kwargs):
if not self.is_request_allowed(request):
return HttpResponseNotAllowed(
["GET", "POST"], "GraphQL only supports GET and POST requests."
)

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

class GraphQLHTTPResponse(TypedDict, total=False):
data: Optional[Dict[str, Any]]
errors: Optional[List[Any]]
extensions: Optional[Dict[str, Any]]

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

async def dispatch(self, request, *args, **kwargs):
if not self.is_request_allowed(request):

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

System Information

  • Operating system:
  • Strawberry version (if applicable):
django==4.0.2
uvicorn[standard]==0.17.5
gunicorn==20.1.0
strawberry-graphql-django==0.5.1
strawberry-graphql[channels]<=0.132.0
strawberry-django-plus==1.27
django-cors-headers==3.13.0

Additional Context

Upvote & Fund

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@capital-G capital-G added the bug Something isn't working label Oct 30, 2022
@jkimbo
Copy link
Member

jkimbo commented Oct 31, 2022

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

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

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

@capital-G
Copy link
Author

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

jhnnsrs commented Aug 1, 2023

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

rossmeredith commented Mar 12, 2024

@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: django/channels#2081

@jamietdavidson
Copy link

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

jamietdavidson commented Aug 21, 2024

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: django/channels#1558 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants