-
-
Notifications
You must be signed in to change notification settings - Fork 531
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
Comments
@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 |
Thanks for the hint, it seems everything was caused by using 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? |
@capital-G let's keep it open, we should possibly add a note about CORS in that document |
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.
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),
}
)
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
)
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
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() |
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 ;)) |
@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 |
Just ran into this issue, and want to give it a bump! @jhnnsrs Your solution worked, I just had to remove the singleton tuple |
For anyone having problems with Cross referenced comment: django/channels#1558 (comment) |
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) tolocalhost:8000
(django server)) the browser will verify theAccess-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 anOPTIONS
request, known as an preflight requestThis will respond with headers such as
Access-Control-Allow-Origin
so the client/browser knows if its OK toPOST
to the server.For some reason the
AsyncGraphQLView
will respond on the preflightOPTION
request with a405 - Method Not Allowed
response which will lead to an CORS error.The same issue also occured in the base library with FastAPI: #1942
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
strawberry/strawberry/django/views.py
Lines 223 to 227 in 609ebc7
and redirecting to a simple/empty
HttpResonse
in case of anOPTIONS
request coming in did not resolve the issue.Here is some inspections using curl.
It works using
django-cors-headers
on admin/but fails on
graphql/
to set these headersUsing the GET method for the API (which does not make use of a
OPTIONS
request) also ignores set the headers viadjango-cors-headers
.Maybe the root problem is here that Strawberry is using its own HttpResponse class
strawberry/strawberry/http/__init__.py
Lines 13 to 16 in 609ebc7
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 functionstrawberry/strawberry/django/views.py
Lines 223 to 224 in 609ebc7
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
The text was updated successfully, but these errors were encountered: