Skip to content
This repository was archived by the owner on Mar 20, 2023. It is now read-only.

📚 Update docs with Helmet CSP support #701

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,41 @@ getGraphQLParams(request).then((params) => {
});
```

### Integration with other express middlewares

#### [Helmet](https://helmetjs.github.io/)

When using Helmet the default CSP of helmet blocks graphiql's inline scripts support. To enable graphiql with helmet you need to update the contentSecurityPolicies with graphiql's needs.
The code snippet below makes use of Helmet's default CSP and adds what graphiql needs.

```js
app.use(
helmet({
/**
* Default helmet policy + own customizations - graphiql support
* - https://helmetjs.github.io/
*/
contentSecurityPolicy: {
directives: {
...helmet.contentSecurityPolicy.getDefaultDirectives(),
['default-src']: [
...helmet.contentSecurityPolicy.getDefaultDirectives()['default-src'],
/** @by-us - adds graphiql support over helmet's default CSP */
"'unsafe-inline'",
],
['script-src']: [
...helmet.contentSecurityPolicy.getDefaultDirectives()['script-src'],
/** @by-us - adds graphiql support over helmet's default CSP */
"'unsafe-inline'",
/** @by-us - adds graphiql support over helmet's default CSP */
"'unsafe-eval'",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we need eval here?
The same goes for inline?
Maybe it's something we can address in GraphiQL?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@IvanGoncharov For some reason chrome threw errors saying smth like "script violates security policy, please add unsafe-eval"

You can reproduce easily by removing these 2 lines from an express/graphiql server and opening graphiql -- a blank screen shows, and the above errors are logged

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will try to fix our code to be CSP compliant in the next release.
It looks like unsafe-eval is required by GraphiQL you can help by submitting an issue there so the GraphiQL team can start working on it.

Copy link
Member

@acao acao Dec 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably has to do with our unpkg example and the script tag attributes we use? are we using cdn assets in this case?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we sure it's not caused by this?
https://github.com/graphql/express-graphql/blob/master/resources/load-statically-from-npm.js

I do not see this issue with chrome when visiting https://graphiql-test.netlify.com

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@acao It's probably because you not setting CSP for graphiql-test.netlify.com
Can you please try it with this snippet https://github.com/graphql/express-graphql/pull/701/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R359 ?

],
},
},
}),
);
```

## Debugging Tips

During development, it's useful to get more information from errors, such as
Expand Down
3 changes: 3 additions & 0 deletions cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@
"tsconfig.json"
],
"words": [
"eval",
"graphiql",
"graphiql's",
"middlewares",
"unfetch",
"noindex",
"codecov",
Expand Down