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

Support CSRF protection in GraphiQL with cookie-to-header strategy #758

Closed
Kaemmelot opened this issue Jul 18, 2023 · 6 comments
Closed
Assignees
Labels
in: web Issues related to web handling type: enhancement A general enhancement
Milestone

Comments

@Kaemmelot
Copy link

When using cookie-to-header token CSRF protection as it is documented for Spring Security when using SPAs GraphiQL always gets 403 errors for every request. The common solution that I found in examples seems to be to disable CSRF (which is not really a solution).
Other solutions would be to create a custom GraphiQL build or to use something like web filters to modify the existing GraphiQL index.html.

Instead, I would like to request out-of-the-box support for CSRF protection in Spring's GraphiQL. It could look like the example provided in this discussion: graphql/graphiql#3355

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

bclozel commented Jul 18, 2023

We're linking (so, not shipping) to a stock GraphiQL build hosted on a CDN.
As explained in the GraphiQL discussion, we could indeed update our integration to send the CSRF as an HTTP header. But how would we get the CSRF token in the first place? The HTML page is rendered statically and we can't write the CSRF token within the HTML.

According to the Spring Security documentation, using a specific configuration for single page apps (Angular-like) is a better choice, using cookies. Have you tried this approach?

In any case, as outlined in that GraphiQL discussion, there are many different approaches for security in that space so I'm not sure we should invest much there for an integration that's supposed to be mostly for development. Quite quickly, new use cases would appear and you would need to build your own anyway.

@bclozel bclozel added status: waiting-for-feedback We need additional information before we can continue in: web Issues related to web handling labels Jul 18, 2023
@Kaemmelot
Copy link
Author

The CSRF token is automatically provided as cookie by the CsrfCookieFilter that you can see in the Spring Security documentation for SPAs. The only problem is that this GraphiQL does not use the cookie value to send it back in a header field. So adding this header (if the cookie is present) would be enough to support this use case.

In the end I can only speak for myself, but I cannot imagine this to be a rare use case and I think the effort of "redirecting" the cookie value into a header isn't that high.

@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 Jul 18, 2023
@Kaemmelot
Copy link
Author

By the way: The cookie name defaults to XSRF-TOKEN and the header to X-XSRF-TOKEN which is also what Spring Security uses by default. This is not about supporting every possible combination but at least the main one.

@bclozel bclozel self-assigned this Jul 18, 2023
@bclozel
Copy link
Member

bclozel commented Jul 18, 2023

Makes sense. Did you try the code suggested in the GraphiQL in your application? Does it work as expected?

@Kaemmelot
Copy link
Author

Just tried this as a proof of concept and it works for me:

React.createElement(GraphiQL, {
            fetcher: gqlFetcher,
            headers: `{ "X-XSRF-TOKEN" : "${ document.cookie.match(new RegExp('(?:^| )XSRF-TOKEN=([^;]+)'))[1] }" }`,
            defaultVariableEditorOpen: true,
            headerEditorEnabled: true,
            shouldPersistHeaders: true
        }),

Of course the headers field would need to be optional based on the existence of the cookie.

@bclozel bclozel added this to the 1.2.3 milestone Aug 3, 2023
@bclozel bclozel added type: enhancement A general enhancement and removed status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged labels Aug 3, 2023
@bclozel bclozel changed the title GraphiQL does not support cookie-to-header CSRF protection Support CSRF protection in GraphiQL with cookie-to-header strategy Aug 18, 2023
@bclozel
Copy link
Member

bclozel commented Aug 18, 2023

I've checked a similar change with several Spring Security setups.

For example, with WebFlux and no BREACH protection:

@Configuration
public class SecurityConfig {

    @Bean
    public SecurityWebFilterChain springSecurityFilterChain(ServerHttpSecurity http) {
        http
                .authorizeExchange(exchanges -> exchanges.anyExchange().authenticated())
                .csrf(csrf -> csrf
                        .csrfTokenRepository(CookieServerCsrfTokenRepository.withHttpOnlyFalse())
                        .csrfTokenRequestHandler(new ServerCsrfTokenRequestAttributeHandler()))
                .httpBasic(withDefaults())
                .formLogin(withDefaults());
        return http.build();
    }
}

A more involved (with BREACH protection) setup with MVC can be found in the Spring Security reference documentation, but I don't think the current arrangement would work in this case as the cookie value would change for each request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues related to web handling type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants