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

Add Apollo-Require-Preflight to Apollo client header #8258

Merged
merged 2 commits into from
Jan 23, 2023

Conversation

borisno2
Copy link
Member

Fixes #7925

Also fixes image upload on main now Apollo 4 has CSRF protection enabled by default

@changeset-bot

This comment was marked as resolved.

@vercel
Copy link

vercel bot commented Jan 18, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
keystone-next-docs ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Jan 23, 2023 at 1:45AM (UTC)

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jan 18, 2023

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 9d56827:

Sandbox Source
@keystone-6/sandbox Configuration

@dcousens
Copy link
Member

dcousens commented Jan 23, 2023

Reading https://www.apollographql.com/docs/apollo-server/security/cors/#preventing-cross-site-request-forgery-csrf (thanks @megafinz, #7925) - I think this is OK as-is.

Apollo-Require-Preflight is an arbitrary header to help the browser flag the request (which - when uploading - has Content-Type: multipart/form-data) as a non-simple request (which are now rejected by Apollo 4 in #8221)

Unfortunate that we are adding this to every request, but, it's an easy solution with no other side-effects.
The downside is that this pull request won't help browsers using other frontends.

Content-Type: multipart/form-data triggering the browser into sending a "simple request" will always be annoying, but at least with Apollo 4 (#8221), it is now a server error by default.

The CSRF prevention feature for Apollo 3 has been configurable by users since the dependency was updated in #6409.

I don't think we need to issue another CVE as the Apollo advisory GHSA-2p3c-p3qw-69r4 and documentation is appropriate.
While this is an improvement to our default configuration; and like #7848 I think we should continue moving in this direction of "strict" by default; this always came down to the security implications of understanding and opting into accepting CORS.
Happy to hear if anyone had feedback against that point of view.

@dcousens dcousens enabled auto-merge (squash) January 23, 2023 01:42
@vercel vercel bot temporarily deployed to Preview January 23, 2023 01:45 Inactive
@dcousens
Copy link
Member

dcousens commented Jan 23, 2023

Inline with my comment #8258 (comment), I have removed the changeset, relying only on the Upgrade Apollo 4 changeset.
We aren't actually fixing anything from the previous release.

If users upgrade, and they use uploads on their frontend, and they use COR's, they will need to work through this problem too. Maybe we can document that somewhere...

@dcousens dcousens merged commit 3dc3ec8 into main Jan 23, 2023
@dcousens dcousens deleted the enable-apollo-preflight- branch January 23, 2023 01:57
@DiesIrae
Copy link
Contributor

If users upgrade, and they use uploads on their frontend, and they use COR's, they will need to work through this problem too. Maybe we can document that somewhere...

Yup, that's me! 😅 I have this current code in cors, what should I do to make Apollo Server work again on my servers?

cors: {
    origin: "*",
    credentials: true,
  },

@borisno2
Copy link
Member Author

Yup, that's me! 😅 I have this current code in cors, what should I do to make Apollo Server work again on my servers?

cors: {
    origin: "*",
    credentials: true,
  },

If you are using the apollo-upload-client in your frontend, you will need to pass headers: {'Apollo-Require-Preflight': 'true'} to createUploadLink - For more info check out the Apollo docs for enabling file-upload on Apollo Server. If you are still having issues feel free to start a GitHub discussion.

@DiesIrae
Copy link
Contributor

I'm afraid I don't understand: I just want the Apollo interface at /api/graphql to work on my servers, since the update it shows this error:
"This operation has been blocked as a potential Cross-Site Request Forgery (CSRF). Please either specify a 'content-type' header (with a type that is not one of application/x-www-form-urlencoded, multipart/form-data, text/plain) or provide a non-empty value for one of the following headers: x-apollo-operation-name, apollo-require-preflight\n"

@borisno2
Copy link
Member Author

For the Apollo interface at /api/graphql to work with file uploads the client that is calling /api/graphql needs to send a header of apollo-require-preflight with a non-empty value, for example true, along with the request. So the fix needs to happen on the side that is calling /api/graphql - not in Keystone. Hope this helps.

@DiesIrae
Copy link
Contributor

Sorry I wasn't clear enough about the problem. I think it may be not linked with file upload: we don't use file upload in particular.

We can't access the page /api/graphql on the browser, plain and simple. It works in dev on localhost:3000/api/graphql, but not in our prod/preprod servers in https://our-ks-server.com/api/graphql

@dcousens
Copy link
Member

dcousens commented Mar 29, 2023

@DiesIrae using your browser, can you copy paste your problematic request headers for us to inspect? Please remove any sensitive information.

@DiesIrae
Copy link
Contributor

Sure! Here are all the data of request/response:

Request headers:

GET /api/graphql HTTP/2
Host: *****
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:109.0) Gecko/20100101 Firefox/111.0
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,*/*;q=0.8
Accept-Language: en-US,en;q=0.5
Accept-Encoding: gzip, deflate, br
DNT: 1
Connection: keep-alive
Cookie: keystonejs-session=*************
Upgrade-Insecure-Requests: 1
Sec-Fetch-Dest: document
Sec-Fetch-Mode: navigate
Sec-Fetch-Site: none
Sec-Fetch-User: ?1
Pragma: no-cache
Cache-Control: no-cache

Response headers:

HTTP/2 400 Bad Request
date: Wed, 29 Mar 2023 11:48:35 GMT
content-type: application/json; charset=utf-8
content-length: 2215
x-powered-by: Express
access-control-allow-origin: *
access-control-allow-credentials: true
etag: W/"8a7-5EMpcbCJA5sb4HNVO61vtIc9Wd0"
strict-transport-security: max-age=15724800; includeSubDomains
cf-cache-status: DYNAMIC
report-to: {"endpoints":[{"url":"https:\/\/a.nel.cloudflare.com\/report\/v3?s=fD6ALZR6UbRJnzTXbV%2Fj499j3NOuxeaCk7RK%2B9YF7NfqnVflRd2U%2BW8BSRI4phcMjHu%2BWQzlSc%2B1aEs77WzhnrquDEz8ZPbGggtQrm%2FsXugFYYTpeb1%2F5756yg7IpZCJQw0I4gnxl4M2LMPgYRZMhdLlpWYR"}],"group":"cf-nel","max_age":604800}
nel: {"success_fraction":0,"report_to":"cf-nel","max_age":604800}
server: cloudflare
cf-ray: 7af7e27a1a3f0373-CDG
X-Firefox-Spdy: h2

Response:

{
	"errors": [
		{
			"message": "This operation has been blocked as a potential Cross-Site Request Forgery (CSRF). Please either specify a 'content-type' header (with a type that is not one of application/x-www-form-urlencoded, multipart/form-data, text/plain) or provide a non-empty value for one of the following headers: x-apollo-operation-name, apollo-require-preflight\n",
			"extensions": {
				"code": "BAD_REQUEST",
				"stacktrace": [
					"BadRequestError: This operation has been blocked as a potential Cross-Site Request Forgery (CSRF). Please either specify a 'content-type' header (with a type that is not one of application/x-www-form-urlencoded, multipart/form-data, text/plain) or provide a non-empty value for one of the following headers: x-apollo-operation-name, apollo-require-preflight",
					"",
					"    at new GraphQLErrorWithCode (/app/node_modules/@apollo/server/dist/cjs/internalErrorClasses.js:10:9)",
					"    at new BadRequestError (/app/node_modules/@apollo/server/dist/cjs/internalErrorClasses.js:84:9)",
					"    at preventCsrf (/app/node_modules/@apollo/server/dist/cjs/preventCsrf.js:35:11)",
					"    at ApolloServer.executeHTTPGraphQLRequest (/app/node_modules/@apollo/server/dist/cjs/ApolloServer.js:507:50)",
					"    at runMicrotasks (<anonymous>)",
					"    at processTicksAndRejections (node:internal/process/task_queues:96:5)"
				]
			},
			"kind": "BAD_REQUEST",
			"stack": "BadRequestError: This operation has been blocked as a potential Cross-Site Request Forgery (CSRF). Please either specify a 'content-type' header (with a type that is not one of application/x-www-form-urlencoded, multipart/form-data, text/plain) or provide a non-empty value for one of the following headers: x-apollo-operation-name, apollo-require-preflight\n\n    at new GraphQLErrorWithCode (/app/node_modules/@apollo/server/dist/cjs/internalErrorClasses.js:10:9)\n    at new BadRequestError (/app/node_modules/@apollo/server/dist/cjs/internalErrorClasses.js:84:9)\n    at preventCsrf (/app/node_modules/@apollo/server/dist/cjs/preventCsrf.js:35:11)\n    at ApolloServer.executeHTTPGraphQLRequest (/app/node_modules/@apollo/server/dist/cjs/ApolloServer.js:507:50)\n    at runMicrotasks (<anonymous>)\n    at processTicksAndRejections (node:internal/process/task_queues:96:5)"
		}
	]
}

@dcousens
Copy link
Member

@DiesIrae there is no Content-Type for your /api/graphql request?

@DiesIrae
Copy link
Contributor

DiesIrae commented Mar 30, 2023

not in the request, no. Neither in Chrome nor Firefox. Our Keystone is in a container and we use Cloudflare to point our domain to it, do you think the problem is in this direction?

@dcousens
Copy link
Member

dcousens commented Mar 30, 2023

@DiesIrae I might ask you to open a GitHub discussion question so as not to misuse this pull request comments, but yes, I think you might be using the /api/graphql endpoint in some unexpected way

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Apollo CSRF Prevention breaks image uploads
3 participants