self-signed certificates - polish the flow

desktop / desktop

Focus on what matters instead of fighting with Git.

https://desktop.github.com

MIT License

19.64k stars 9.38k forks source link

self-signed certificates - polish the flow #2622

Open shiftkey opened 7 years ago

shiftkey commented 7 years ago

Some feedback from https://github.com/desktop/desktop/pull/2581 around the certificate flow that we need to add in the app:

[x] support stacking dialogs so that we can push and pop dialogs within the app (done in #15496)
[ ] when the self-signed certificate is encountered, it should be pushed ontop of any existing dialog
[ ] once the callback from dialog.showCertificateTrustDialog is invoked, return the user to the previous dialog (if applicable)
[ ] stretch goal: get Electron to return a value indicating whether the certificate was added successfully

joshaber commented 7 years ago

support stacking dialogs so that we can push and pop dialogs within the app

@desktop/design if we all agree that this is desirable from a UX point of view, I think we could split this out into its own issue. What do you all think?

niik commented 7 years ago

@desktop/design if we all agree that this is desirable from a UX point of view, I think we could split this out into its own issue. What do you all think?

I'm in favor of having arbitrary dialog stacks at some point though I suspect it's not a trivial thing to implement well and I'm not certain that we need it in this particular case. Before we moved the display of unhandled errors (crashes) from being a modal dialog into its own process we used to render it on top of any content, including other modal dialogs.

We could implement a similar approach here. Render this particular dialog on its own layer on top of the dialog layer.

mattpollard commented 6 years ago

I think the following behavior from GitHub Desktop 1.4.0 falls within this issue's scope, but perhaps y'all can let me know if this would be better in a separate issue:

Configure a GitHub Enterprise appliance with the default self-signed certificate. Don't trust the certificate, or delete it from Keychain Access if you did during the initial configuration.
From a fresh GitHub Desktop installation—or after clearing the application's preferences from ~/Library/Application Support/GitHub Desktop—click Sign into GitHub Enterprise:
Enter the appliance's hostname and click Continue:
Undesired behavior: Instead of a warning about the self-signed certificate, you get a more generic authentication error:

You can work around this by clicking Cancel and then logging in via GitHub Desktop's preferences, or by explicitly trusting the appliance's self-signed certificate in Safari.

shiftkey commented 6 years ago

@mattpollard thanks for the heads up about the Welcome Wizard - I think that's failing because we don't have the ability to display the dialog for "validate the self-signed certificate" like we do when you sign in via Preferences.