Add CSRF protection by Shane32 · Pull Request #1138 · graphql-dotnet/server

Skip to content

Navigation Menu

Sign in

Appearance settings

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up

Appearance settings

Merged

Shane32 merged 11 commits intodevelopfrom

csrf

Aug 3, 2024

Merged

Add CSRF protection#1138

Shane32 merged 11 commits intodevelopfrom

csrf

Conversation

@Shane32

Copy link

Member

@Shane32 Shane32 commented

Aug 3, 2024

edited

Loading

Requires CORS preflight requests by ensuring that the requests are not 'simple' - e.g. GET request or form-POST requests - or that a specific header has been added to the request.

See similar logic: https://www.apollographql.com/docs/apollo-server/security/cors/#preventing-cross-site-request-forgery-csrf

The chosen default header was based on the current proposal by the GraphQL working group. See:

@Shane32 Shane32 added the enhancement

New feature or request

label

Aug 3, 2024

@Shane32 Shane32 added this to the 8.0 milestone

Aug 3, 2024

@Shane32 Shane32 self-assigned this

Aug 3, 2024

@Shane32 Shane32 linked an issue

Aug 3, 2024

that may be closed by this pull request

@Shane32 Shane32 requested a review from gao-artur

August 3, 2024 19:16

Comment on lines +510 to +514

foreach (var header in _options.CsrfProtectionHeaders)
{
if (context.Request.Headers.TryGetValue(header, out var values) && values.Count > 0 && values[0]?.Length > 0)
return false;
}

Check notice

Code scanning / CodeQL

Missed opportunity to use Where

This foreach loop [implicitly filters its target sequence](1) - consider filtering the sequence explicitly using '.Where(...)'.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gao-artur gao-artur gao-artur approved these changes

Assignees

@Shane32 Shane32

Labels

enhancement

New feature or request

Projects

None yet

Milestone

8.0

Development

Successfully merging this pull request may close these issues.

Add CSRF protection

2 participants

@Shane32 @gao-artur