fix(gatsby): Switching from devcert-san to devcert to fix HTTPS issues

[rmorabia]

• Upgrade devcert-san to devcert
• Update calls from package to match the new changes

Description

I changed devcert-san to devcert as described by @Js-Brecht in this comment: #16212 (comment)

This should work, however, I'm running on Linux (unix line endings?) and can't confirm it. Can someone on Windows run the repo before the PR is merged and confirm it's fixed?

Related Issues

Fixes #16212

[Js-Brecht]

Did you have a moment to investigate the key/cert paths shown here?

https://github.com/gatsbyjs/gatsby/pull/16726/files#diff-b7d3f367223033fe45bc0ed8f2915244R28-R29

Since they're not returned by certificateFor(), it could be misleading having the paths returned from one branch when the user specifies the cert, but not the other when returned by certificateFor. Somebody down the line might want to try using those paths, only to encounter some unexpected behavior when they come back undefined.

I guess if they're needed, somebody could issue a PR over @ devcerts to include them in the return value for that function. 🤷‍♂️

[Js-Brecht]

AFAICT, this fixes the issue (with paths) on Windows. 👍

However, it causes other issues. It installs the certificate in the Certificate Authority store, but when it tries to open Firefox to run the wizard for trusting the cert, the page it opens results in an endless refresh loop. And that CA cert is basically useless. No trust... "SEC_ERROR_BAD_SIGNATURE". Site still shows security errors.

I've opened issue davewasmer/devcert#35

This is definitely a regression from the previous version. On a path with no spaces, the previous version works correctly.

[wardpeet]

I'm getting UNHANDLED REJECTION request to https://localhost:8000/___graphql failed, reason: unable to verify the first certificate I do have the certificate installed

[Js-Brecht]

@wardpeet I noticed that same thing, but it's an existing issue (#15947, #15441, #14990). I had the same problem using the old version. The work around here sems to work.

[rmorabia]

Yep, had the same issues with the cert stuff. I had assumed that it was the key cert path issues that was causing this so I didn't change this. However, I had the same issue even without my changes, so I just decided to submit the PR anyway.

Right now, the rest of this is above my level of understanding, so I guess let me know if there's anything I can do.

[wardpeet]

i'll see what I can do :)

[Js-Brecht]

Is it just me, or is there something weird going on with gatsby-graphiql-explorer in this repo? Every time I run gatsby-dev, I have to manually install gatsby-graphiql-explorer in the target site in order to run gatsby develop. Gets rather frustrating, especially because gatsby-dev kept crashing when running yarn build in rmorabia/gatsby. So, it turned into yarn build -> gatsby-dev -> yarn add gatsby-graphiql-explorer -> cross-env NODE_TLS_REJECT_UNAUTHORIZED=0 gatsby develop --https. I don't think it would affect the master; it just slows down testing.

Really, it could just be me. I didn't try wiping it all out and starting over. Could be the yarn cache, too...

[rmorabia]

Nope, had the same issue! I thought that was a general gatsby-dev issue, happened to me no matter what I tried. I just ran yarn --force to fix it. Asked about it in the Discord channel as well.

I have no clue how my 5 lines of code change could mess up that much, lol.

EDIT: Perhaps it's an issue with the yarn.lock?

[Js-Brecht]

@rmorabia have you tried updating your repo? It looks like you're on 2.13.52, and the master is 2.13.71. Could pull the changes from the master into a new branch, and see if it fixes the problem 🤷‍♂.

[rmorabia]

Let me try messing around with that and see if that fixes the issue there. Not a huge deal either way I think, the certs issue is still way above my head, but i can try and get the gatsby-dev stuff working a bit better.

[Js-Brecht]

@wardpeet I'm pretty sure this can be merged, yeah? I could run another test, just to make sure it doesn't cause any issues... the other problems that were noticed before are kind of a separate issue.

Merging this would at least close #16212. The other issues with certificates should be fixed by davewasmer/devcert#41 if/when it gets merged (and #18703, by extension).

One caveat:
Issues will begin to crop up for users of MacOS Catalina: davewasmer/devcert#39
Fix is here davewasmer/devcert#45

[rmorabia]

Sorry I ghosted on this -- I couldn't find the issue, I assume it's the package.json/yarn.lock stuff. Up to y'all what you want to do on this.

[Js-Brecht]

Oh... I forgot... This upgrade would break the NODE_EXTRA_CA_CERTS workaround for #14990. The new version locks down the certificate on the disk, so it can't be read 😤

Just trying to get some of these things off my list, and since this PR conflicts with #18703, it stays.

[Js-Brecht]

@rmorabia This should fix the issue with the tests 👇

Diff output... see attached if you want to do a patch

diff --git a/packages/gatsby/src/utils/__tests__/get-ssl-cert.js b/packages/gatsby/src/utils/__tests__/get-ssl-cert.js
index 7191c7496..8ef7651b6 100644
--- a/packages/gatsby/src/utils/__tests__/get-ssl-cert.js
+++ b/packages/gatsby/src/utils/__tests__/get-ssl-cert.js
@@ -11,13 +11,13 @@ jest.mock(`gatsby-cli/lib/reporter`, () => {
     info: jest.fn(),
   }
 })
-jest.mock(`devcert-san`, () => {
+jest.mock(`devcert`, () => {
   return {
-    default: jest.fn(),
+    certificateFor: jest.fn(),
   }
 })

-const devcertSan = require(`devcert-san`).default
+const { certificateFor } = require(`devcert`)
 const reporter = require(`gatsby-cli/lib/reporter`)
 const getSslCert = require(`../get-ssl-cert`)

@@ -25,7 +25,7 @@ describe(`gets ssl certs`, () => {
   beforeEach(() => {
     reporter.panic.mockClear()
     reporter.info.mockClear()
-    devcertSan.mockClear()
+    certificateFor.mockClear()
   })
   describe(`Custom SSL certificate`, () => {
     it.each([[{ certFile: `foo` }], [{ keyFile: `bar` }]])(
@@ -60,11 +60,11 @@ describe(`gets ssl certs`, () => {
   describe(`automatic SSL certificate`, () => {
     it(`sets up dev cert`, () => {
       getSslCert({ name: `mock-cert` })
-      expect(devcertSan).toBeCalledWith(`mock-cert`, { installCertutil: true })
+      expect(certificateFor).toBeCalledWith(`mock-cert`, { installCertutil: true })
       expect(reporter.info.mock.calls).toMatchSnapshot()
     })
     it(`panics if certificate can't be created`, () => {
-      devcertSan.mockImplementation(() => {
+      certificateFor.mockImplementation(() => {
         throw new Error(`mock error message`)
       })
       getSslCert({ name: `mock-cert` })

get-ssl-cert.diff.txt

[rmorabia]

I'll fix the tests tonight/tomorrow night.

[rmorabia]

Ahh, still getting failing tests because devcert isn't installed. I thought I ...would've done that for this PR to exist in the first place. I've lost track of this since I wrote it so long ago. Not sure what I'm supposed to do here.

[Js-Brecht]

Just a couple notes to try to get the build on track

[Js-Brecht]

There's other things that might need to happen for this update to go smoothly.

For one, the common name of the certificate generated will match the project name; meanwhile, gatsby develop --https will report that the server is running on https://localhost:<port>. This means that if the user opens the url that gatsby develop reports, then an error will be shown indicating that the certificate is invalid for that domain.

@rmorabia This is something that I address in PR #18703, but if you wanted to fix it here, I will make sure mine aligns with yours.

https://github.com/gatsbyjs/gatsby/pull/18703/files#diff-346c3005d97c1ca0b5efb170af1b43f6R358-R367

Another problem: the password prompt that's generated for Windows users doesn't work very well. Not a very big deal, and a fix for it is already part of #18703. It will probably hold until then.

---

Progress is being made on devcert, so it shouldn't be much longer before all of the certificate issues can be fixed.

[LekoArts]

Hi @rmorabia 👋 Do you have time to rebase your PR onto master and fix the merge conflict?

[rmorabia]

Hi @rmorabia 👋 Do you have time to rebase your PR onto master and fix the merge conflict?

Done!

[wardpeet]

Looks great! I did a few modifications to bring back old support so it gets issued to the host we're running develop on.

[gatsbot]

Holy buckets, @rmorabia — we just merged your PR to Gatsby! 💪💜

Gatsby is built by awesome people like you. Let us say “thanks” in two ways:

1. We’d like to send you some Gatsby swag. As a token of our appreciation, you can go to the Gatsby Swag Store and log in with your GitHub account to get a coupon code good for one free piece of swag. We’ve got Gatsby t-shirts, stickers, hats, scrunchies, and much more. (You can also unlock even more free swag with 5 contributions — wink wink nudge nudge.) See gatsby.dev/swag for details.
2. We just invited you to join the Gatsby organization on GitHub. This will add you to our team of maintainers. Accept the invite by visiting https://github.com/orgs/gatsbyjs/invitation. By joining the team, you’ll be able to label issues, review pull requests, and merge approved pull requests.

If there’s anything we can do to help, please don’t hesitate to reach out to us: tweet at @gatsbyjs and we’ll come a-runnin’.

Thanks again!

[wardpeet]

Fixed in [email protected]