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

Update from 1.0.x to 1.1.x causes CORS to behave differently for GraphiQL #627

Open
koenpunt opened this issue Mar 8, 2023 · 8 comments
Labels
status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged

Comments

@koenpunt
Copy link
Contributor

koenpunt commented Mar 8, 2023

For some reason the update from 1.0.x to 1.1.x caused GraphiQL to no longer work. This turned out to be a CORS issue, which is weird, because GraphiQL and the GraphQL endpoint share the same origin.

I couldn't really find any changes regarding CORS, but maybe I'm not looking in the right places?

For now I managed to solve the issue by adding the local domain to the CORS allowed origins list, but that shouldn't be necessary.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Mar 8, 2023
@bclozel
Copy link
Member

bclozel commented Mar 8, 2023

Do you have a sample application we can take a look at? Something that works in 1.0.x and doesn't with 1.1.x?

@bclozel bclozel added the status: waiting-for-feedback We need additional information before we can continue label Mar 8, 2023
@koenpunt
Copy link
Contributor Author

koenpunt commented Mar 8, 2023

I've created a sample application, but upon creating that I think the issue might be with Spring's overall CORS implementation, and not specific to Spring GraphQL;

https://github.com/koenpunt/graphiql-cors-example

If you build and run that application, and then use something like ngrok (ngrok http 8080) to make it available on something else than localhost, you will notice that performing a query in the graphiql UI will result in a 403 error in the browser's network log.

If in that same application you change the spring-boot dependency from 3.0.4 to 2.7.3 and build and run again, you will see that performing a query in graphiql now works correctly.

I had a quick look at the spring-framework repo to see if I could find the change that would cause this change in behavior, but didn't find anything.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Mar 8, 2023
@rstoyanchev
Copy link
Contributor

The main place for CORS decisions is in the reactive variant of DefaultCorsProcessor and in CorsUtils if you want to have a quick look at what happens with a debugger. If not, it's okay, we'll use the sample.

@koenpunt
Copy link
Contributor Author

koenpunt commented Mar 9, 2023

I think the issue might not be isolated to CORS, but with how spring builds urls in general, or how that has changed when upgrading from spring-boot 2.7 to 3.0.

Because we now also experience redirects from https://our-domain.com/graphql to https://our-domain.com:80/graphiql?path=/graphql, where previously the port (:80) was not added on redirect.

And I suspect the CORS functionality to use the same information to build the current URL, and then fails to match that with the incoming origin header of the request, because of the added port.

We've looked into this https://docs.spring.io/spring-boot/docs/current/reference/htmlsingle/#howto.webserver.use-behind-a-proxy-server already, but at least setting server.forward-headers-strategy=NATIVE doesn't work. Still have to try the FRAMEWORK strategy, but the fact remains that this worked before without any configuration.

@koenpunt
Copy link
Contributor Author

koenpunt commented Mar 9, 2023

Setting the strategy has no effect, but defining a bean for ForwardedHeaderTransformer correctly processes the forwarded headers. This, however, also removes the headers, causing issues with a scenario where we need the x-forwarded-for header (or really the remote address, but we derive that from the header) in a WebGraphQlInterceptor.

@rstoyanchev
Copy link
Contributor

rstoyanchev commented Mar 9, 2023

This sounds like it is related to spring-projects/spring-framework#30033, a regression in Spring Framework 6.0.5, with fixes in Spring Framework 6.0.6 and in Reactor Netty 1.1.4. But you already have Boot 3.0.4 with those versions so you should have the fixes. Port 80 should be getting ignored in ReactorServerHttpRequest.

@koenpunt
Copy link
Contributor Author

koenpunt commented Mar 9, 2023

We're already on Boot 3.0.4 with Framework 6.0.6, so I guess the regression isn't completely resolved then?

@koenpunt
Copy link
Contributor Author

@rstoyanchev I'm not sure what's next; was you comment a confirmation about the bug still existing and that it should be fixed, or would this be an issue on our side, and thus something we have to fix?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged
Projects
None yet
Development

No branches or pull requests

4 participants