fix(certificate): redesign untrusted certificate dialog

Signed-off-by: Grigorii K. Shartsev <me@shgk.me>

Author: ShGKme

src/app/certificate.service.ts

@@ -8,6 +8,12 @@ import type { BrowserWindow, Certificate } from 'electron'
 import { showCertificateTrustDialog } from '../certificate/certificate.window.ts'
 import { getAppConfig, setAppConfig } from './AppConfig.ts'
 
+export type UntrustedCertificateDetails = {
+	host: string
+	certificate: Certificate
+	error: string
+}
+
 /**
  * Pending showCertificateTrustDialog prompts per fingerprint to prevent duplicated dialogs
  */
@@ -20,31 +26,32 @@ const pendingCertificateTrustPrompts: Map<string, Promise<boolean>> = new Map()
  * since application level trusted dialog is needed, not system-level
  *
  * @param window - Parent window
- * @param certificate - Certificate
+ * @param details - Error details
  * @return Whether the certificate is accepted as trusted
  */
-export async function promptCertificateTrust(window: BrowserWindow, certificate: Certificate): Promise<boolean> {
+export async function promptCertificateTrust(window: BrowserWindow, details: UntrustedCertificateDetails): Promise<boolean> {
+	const fingerprint = details.certificate.fingerprint
 	const trustedFingerprints = getAppConfig('trustedFingerprints')
 
 	// Already accepted
-	if (trustedFingerprints.includes(certificate.fingerprint)) {
+	if (trustedFingerprints.includes(fingerprint)) {
 		return true
 	}
 
 	// Already in prompt in parallel
-	const existingPrompt = pendingCertificateTrustPrompts.get(certificate.fingerprint)
+	const existingPrompt = pendingCertificateTrustPrompts.get(fingerprint)
 	if (existingPrompt) {
 		return existingPrompt
 	}
 
 	// Prompt user acceptance
-	const pendingDialog = showCertificateTrustDialog(window, certificate)
-	pendingCertificateTrustPrompts.set(certificate.fingerprint, pendingDialog)
+	const pendingDialog = showCertificateTrustDialog(window, details)
+	pendingCertificateTrustPrompts.set(fingerprint, pendingDialog)
 	const isAccepted = await pendingDialog
-	pendingCertificateTrustPrompts.delete(certificate.fingerprint)
+	pendingCertificateTrustPrompts.delete(fingerprint)
 
 	if (isAccepted) {
-		setAppConfig('trustedFingerprints', [...trustedFingerprints, certificate.fingerprint])
+		setAppConfig('trustedFingerprints', [...trustedFingerprints, fingerprint])
 	}
 
 	return isAccepted

src/certificate/certificate.window.ts

@@ -4,6 +4,7 @@
  */
 
 import type { Certificate, IpcMainEvent } from 'electron'
+import type { UntrustedCertificateDetails } from '../app/certificate.service.ts'
 
 import { BrowserWindow, ipcMain } from 'electron'
 import { applyContextMenu } from '../app/applyContextMenu.js'
@@ -14,16 +15,16 @@ import { getBrowserWindowIcon } from '../shared/icons.utils.js'
  * Show untrusted certificate dialog window
  *
  * @param parentWindow - Parent browser window
- * @param certificate - Certificate
+ * @param details - Error details
  * @return Whether user accept the certificate
  */
-export function showCertificateTrustDialog(parentWindow: BrowserWindow, certificate: Certificate) {
-	const TITLE = buildTitle('Untrusted certificate')
+export function showCertificateTrustDialog(parentWindow: BrowserWindow, details: UntrustedCertificateDetails) {
+	const TITLE = buildTitle('Security warning')
 	const window = new BrowserWindow({
 		title: TITLE,
 		...getScaledWindowSize({
-			width: 650,
-			height: 900,
+			width: 600,
+			height: 600,
 		}),
 		...getScaledWindowMinSize({
 			minWidth: 320,
@@ -48,7 +49,7 @@ export function showCertificateTrustDialog(parentWindow: BrowserWindow, certific
 	window.removeMenu()
 	window.on('ready-to-show', () => window.show())
 
-	window.loadURL(getWindowUrl('certificate') + '#' + encodeURIComponent(JSON.stringify(certificate)))
+	window.loadURL(getWindowUrl('certificate') + '#' + encodeURIComponent(JSON.stringify(details)))
 
 	return new Promise<boolean>((resolve) => {
 		let isAccepted = false

src/certificate/renderer/CertificateApp.vue

@@ -4,41 +4,81 @@
 -->
 
 <script setup lang="ts">
-import type { Certificate } from 'electron'
+import type { UntrustedCertificateDetails } from '../../app/certificate.service.ts'
 
 import { t } from '@nextcloud/l10n'
 import { useHotKey } from '@nextcloud/vue/composables/useHotKey'
+import { ref } from 'vue'
 import NcButton from '@nextcloud/vue/components/NcButton'
 import NcNoteCard from '@nextcloud/vue/components/NcNoteCard'
+import IconAlert from 'vue-material-design-icons/Alert.vue'
+import IconShieldOffOutline from 'vue-material-design-icons/ShieldOffOutline.vue'
 import CertificateInfo from './components/CertificateInfo.vue'
 
 useHotKey('Escape', () => window.close())
 
-const certificate = JSON.parse(decodeURIComponent(location.hash.slice(1))) as Certificate
+const { host, certificate, error } = JSON.parse(decodeURIComponent(location.hash.slice(1))) as UntrustedCertificateDetails
+const urlParam = {
+	value: `<strong>${host}</strong>`,
+	escape: false,
+}
 
 const acceptCertificate = window.TALK_DESKTOP.acceptCertificate
+
+const isAdvanced = ref(false)
 </script>
 
 <template>
 	<div class="certificate">
 		<h2 class="certificate__heading">
-			{{ t('talk_desktop', 'Untrusted certificate') }}
+			<IconAlert :size="30" class="certificate__alert-icon" />
+			{{ t('talk_desktop', 'Warning: potential security risk') }}
 		</h2>
 
-		<div class="certificate__content">
-			<NcNoteCard type="warning" class="certificate__note">
-				{{ t('talk_desktop', 'The server\'s security certificate is not trusted. Your connection may not be secure. Proceed only if you trust this certificate.') }}
-			</NcNoteCard>
+		<NcNoteCard type="error" class="certificate__note">
+			<template #icon>
+				<IconShieldOffOutline :size="24" class="certificate__alert-icon" />
+			</template>
+			<!-- eslint-disable-next-line -->
+			<p v-html="t('talk_desktop', 'Connection to {url} is not private.', { url: urlParam })"/>
+			<p>{{ t('talk-desktop', 'If you are unsure to proceed contact your system administrator.') }}</p>
+		</NcNoteCard>
+
+		<NcButton
+			v-if="!isAdvanced"
+			aria-expanded="false"
+			variant="error"
+			@click="isAdvanced = true">
+			{{ t('talk_desktop', 'Advanced') }}
+		</NcButton>
 
-			<CertificateInfo :certificate="certificate" />
+		<div class="certificate__content">
+			<div v-if="isAdvanced" class="certificate__advanced">
+				<!-- eslint-disable-next-line -->
+				<p v-html="t('talk_desktop', 'This server could not prove that it is {url}.', { url: urlParam })"/>
+				<p>
+					{{ t('talk_desktop', 'It has untrusted SSL certificate. This might be caused by an attacker intercepting your connection or server misconfiguration.') }}
+				</p>
+				<p>
+					<code>{{ error }}</code>
+				</p>
+				<p>
+					<NcButton variant="error" @click="acceptCertificate(true)">
+						{{ t('talk_desktop', 'Proceed') }}
+					</NcButton>
+				</p>
+				<details>
+					<summary>
+						{{ t('talk_desktop', 'View certificate') }}
+					</summary>
+					<CertificateInfo :certificate="certificate" />
+				</details>
+			</div>
 		</div>
 
 		<div class="certificate__actions">
-			<NcButton variant="secondary" wide @click="acceptCertificate(false)">
-				{{ t('talk_desktop', 'Reject') }}
-			</NcButton>
-			<NcButton variant="primary" wide @click="acceptCertificate(true)">
-				{{ t('talk_desktop', 'Accept') }}
+			<NcButton variant="primary" wide @click="acceptCertificate(false)">
+				{{ t('talk_desktop', 'Cancel') }}
 			</NcButton>
 		</div>
 	</div>
@@ -63,11 +103,18 @@ const acceptCertificate = window.TALK_DESKTOP.acceptCertificate
 
 .certificate__heading {
 	margin-block: 0;
+	display: flex;
+	align-items: baseline;
+	justify-content: center;
+	gap: 1ch;
 	font-size: 1.5em;
-	text-align: center;
 	text-transform: capitalize;
 }
 
+.certificate__alert-icon {
+	color: var(--color-error);
+}
+
 .certificate__content {
 	flex: 1;
 	margin-inline: calc(-4 * var(--default-grid-baseline));
@@ -78,14 +125,24 @@ const acceptCertificate = window.TALK_DESKTOP.acceptCertificate
 	overflow: auto;
 }
 
+.certificate__advanced {
+	border: 1px solid var(--color-border);
+	border-radius: var(--border-radius-small);
+	padding: calc(2 * var(--default-grid-baseline));
+	display: flex;
+	flex-direction: column;
+	gap: 1em;
+	background-color: var(--color-background-dark);
+}
+
 .certificate__note {
 	/* Override default component styles */
 	margin: 0;
-	margin-block-end: calc(3 * var(--default-grid-baseline));
 }
 
 .certificate__actions {
 	display: flex;
 	gap: calc(2 * var(--default-grid-baseline));
+	align-items: flex-end;
 }
 </style>

src/main.js

@@ -234,7 +234,7 @@ app.whenReady().then(async () => {
 	// Note: the result of this verification is cached by domain in Electron
 	// There is no way to clean the cache except by restarting the app
 	session.defaultSession.setCertificateVerifyProc(async (request, callback) => {
-		const isAccepted = await promptCertificateTrust(mainWindow, request.certificate)
+		const isAccepted = await promptCertificateTrust(mainWindow, { host: request.hostname, certificate: request.certificate, error: request.verificationResult })
 		// See: https://source.chromium.org/chromium/chromium/src/+/main:net/base/net_error_list.h
 		const SSL_PROTOCOL_ERROR_CODE = -107
 		callback(isAccepted ? 0 : SSL_PROTOCOL_ERROR_CODE)
@@ -243,7 +243,7 @@ app.whenReady().then(async () => {
 	// Allow web-view with accepted untrusted certificate (Login Flow)
 	app.on('certificate-error', async (event, webContents, url, error, certificate, callback) => {
 		event.preventDefault()
-		const isAccepted = await promptCertificateTrust(mainWindow, certificate)
+		const isAccepted = await promptCertificateTrust(mainWindow, { host: new URL(url).host, certificate, error })
 		callback(isAccepted)
 	})