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 HTTPS to GraphQL and NextJS Server #376

Closed
wants to merge 1 commit into from

Conversation

willwill96
Copy link
Contributor

@willwill96 willwill96 commented May 18, 2020

fixes #256

graphql/.gitignore Outdated Show resolved Hide resolved
@bdeining
Copy link
Contributor

verified ssl works

@willwill96 willwill96 changed the title Add HTTPS to GraphQL Server Add HTTPS to GraphQL and NextJS Server May 18, 2020
app/package.json Outdated Show resolved Hide resolved
@bdeining
Copy link
Contributor

verified communication between app and graphql work over ssl

const app = next({ dev })
const handle = app.getRequestHandler()
// @ts-ignore
process.env.NODE_TLS_REJECT_UNAUTHORIZED = 0
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't the point of https so we can now remove this line?

Copy link
Contributor

Choose a reason for hiding this comment

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

since the ddf cert is self signed, we still need this to get past that auth rejection

@@ -1,6 +1,6 @@
import url from 'url'
const defaultDdfUrl = 'https://localhost:8993'
const defaultWebappUrl = 'http://localhost:3000'
const defaultWebappUrl = 'https://localhost:3000'
Copy link
Contributor

@Bdthomson Bdthomson May 18, 2020

Choose a reason for hiding this comment

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

Just realized, does the graphql server need to know about the webapp at all? This key should be able to be removed unless I'm missing something.

Copy link
Contributor Author

@willwill96 willwill96 May 19, 2020

Choose a reason for hiding this comment

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

This is used for the CORS policy. Would it be better to allow from all origins?

cors: {
origin: config('WEBAPP_LOCATION'),
credentials: true,
},

handle(req, res, parsedUrl)
}).listen(3000, err => {
if (err) throw err
console.log('> Ready on https://localhost:3000')
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't CI have failed with this console.log are we no longer enforcing that? Or are you ignoring it for this case and I'm just not seeing it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i was able to add this to the eslint config for the app directory but not graphql. for some reason the no-console rule won't play nicely with typescript files

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.

Enable HTTPS
4 participants