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

Improve UX around certificate errors #118

Merged
merged 4 commits into from
Aug 9, 2023
Merged

Improve UX around certificate errors #118

merged 4 commits into from
Aug 9, 2023

Conversation

code-asher
Copy link
Member

@code-asher code-asher commented Aug 1, 2023

Unfortunately the insecure bypass does not seem to work so I removed the button for now. Interestingly, according to the tests I added it should actually work for all cases including the partial chain case, but for some reason in VS Code it appears to simply do nothing?

In addition to the tests, I manually ran the plugin in VS Code against domains provided by badssl.com. The only one I have not tested manually yet is the missing key usage one since badssl.com has no sample for that but there is a test for it.

Closes #116

Comment on lines +15 to +20
PARTIAL_CHAIN = "Your Coder deployment's certificate cannot be verified because a certificate is missing from its chain. To fix this your deployment's administrator should bundle the missing certificates or you can add the missing certificates directly to this system's trust store.",
// NON_SIGNING can be removed if BoringSSL is patched and the patch makes it
// into the version of Electron used by VS Code.
NON_SIGNING = "Your Coder deployment's certificate is not marked as being capable of signing. VS Code uses a version of Electron that does not support certificates like this even if they are self-issued. The certificate should be regenerated with the certificate signing capability.",
UNTRUSTED_LEAF = "Your Coder deployment's certificate does not appear to be trusted by this system. The certificate should be added to this system's trust store.",
UNTRUSTED_CHAIN = "Your Coder deployment's certificate chain does not appear to be trusted by this system. The root of the certificate chain should be added to this system's trust store. ",
Copy link
Member Author

Choose a reason for hiding this comment

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

@bpmct the issue #116 says to link to docs but do you think these instructions are sufficient? Or do you think we will also need a docs page that goes into more detail, like what exactly it means to bundle, how to serve a bundled certificate, how to add to the trust store to each operating system, how to add the key signing capability, that sort of thing?

Copy link
Member Author

@code-asher code-asher Aug 2, 2023

Choose a reason for hiding this comment

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

Alternatively we could offload all the work to docs, basically if we get any TLS error we link to the docs and then in the docs we describe how to figure out what the problem is with your certificate (rather than trying to detect it automatically here) and how to fix it. That might be a bit more flexible.

Copy link
Member Author

Choose a reason for hiding this comment

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

Merging for now, but happy to make changes later!

Copy link
Member

@kylecarbs kylecarbs left a comment

Choose a reason for hiding this comment

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

These tests are nuts and I love it.

It used to be inline before the formatter kicked in but the code is
self-apparent anyway.
@code-asher code-asher merged commit 7cec304 into main Aug 9, 2023
2 checks passed
@code-asher code-asher deleted the asher/cert-help branch August 9, 2023 22:43
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.

Link to docs when the server has a partial chain instead of "allow insecure" prompt loop
2 participants