Skip to content

Commit 8f805d9

Browse files
ShGKmeShGKme
committed
Update design + do not ask for confirmation for valid certs
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
1 parent 6a1c27c commit 8f805d9

File tree

4 files changed

+56
-62
lines changed

4 files changed

+56
-62
lines changed

‎src/app/certificate.service.ts‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,13 @@
33
* SPDX-License-Identifier: AGPL-3.0-or-later
44
*/
55

6-
import type { BrowserWindow, Certificate } from 'electron'
6+
import type { BrowserWindow, Request } from 'electron'
77

88
import { session } from 'electron'
99
import { showCertificateTrustDialog } from '../certificate/certificate.window.ts'
1010
import { getAppConfig, setAppConfig } from './AppConfig.ts'
1111

12-
export type UntrustedCertificateDetails = {
13-
host: string
14-
certificate: Certificate
15-
error: string
16-
}
12+
export type UntrustedCertificateDetails = Pick<Request, 'hostname' | 'certificate' | 'verificationResult'>
1713

1814
/**
1915
* Pending showCertificateTrustDialog prompts per fingerprint to prevent duplicated dialogs
@@ -83,11 +79,7 @@ export async function verifyCertificate(window: BrowserWindow, url: string): Pro
8379
// Use original result, failing the request
8480
callback(-3)
8581

86-
const isAccepted = await promptCertificateTrust(window, {
87-
host: request.hostname,
88-
certificate: request.certificate,
89-
error: request.verificationResult,
90-
})
82+
const isAccepted = request.errorCode === 0 || await promptCertificateTrust(window, request)
9183
verificationResolvers.resolve(isAccepted)
9284
})
9385

@@ -100,7 +92,7 @@ export async function verifyCertificate(window: BrowserWindow, url: string): Pro
10092
if (verificationResolvers) {
10193
return verificationResolvers.promise
10294
}
103-
// Some unexpected network error
104-
return false
95+
// Some unexpected network error - not a certificate error
96+
return true
10597
}
10698
}

‎src/authentication/renderer/AuthenticationApp.vue‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ async function login() {
8686
8787
// Check the certificate before actually sending a request
8888
if (!await window.TALK_DESKTOP.verifyCertificate(serverUrl.value)) {
89-
return setError(t('talk_desktop', 'Untrusted certificate'))
89+
return setError(t('talk_desktop', 'SSL certificate error'))
9090
}
9191
9292
// Prepare to request the server

‎src/certificate/renderer/CertificateApp.vue‎

Lines changed: 47 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@ import CertificateInfo from './components/CertificateInfo.vue'
1717
1818
useHotKey('Escape', () => window.close())
1919
20-
const { host, certificate, error } = JSON.parse(decodeURIComponent(location.hash.slice(1))) as UntrustedCertificateDetails
20+
const { hostname, certificate, verificationResult } = JSON.parse(decodeURIComponent(location.hash.slice(1))) as UntrustedCertificateDetails
2121
const urlParam = {
22-
value: `<strong>${host}</strong>`,
22+
value: `<strong>${hostname}</strong>`,
2323
escape: false,
2424
}
2525
@@ -31,49 +31,53 @@ const isAdvanced = ref(false)
3131
<template>
3232
<div class="certificate">
3333
<h2 class="certificate__heading">
34-
<IconAlert :size="30" class="certificate__alert-icon" />
3534
{{ t('talk_desktop', 'Warning: potential security risk') }}
3635
</h2>
3736

3837
<NcNoteCard type="error" class="certificate__note">
3938
<template #icon>
40-
<IconShieldOffOutline :size="24" class="certificate__alert-icon" />
39+
<IconShieldOffOutline :size="24" class="certificate__note-icon" />
4140
</template>
4241
<!-- eslint-disable-next-line -->
4342
<p v-html="t('talk_desktop', 'Connection to {url} is not private.', { url: urlParam })"/>
44-
<p>{{ t('talk-desktop', 'If you are unsure to proceed contact your system administrator.') }}</p>
4543
</NcNoteCard>
4644

47-
<NcButton
48-
v-if="!isAdvanced"
49-
aria-expanded="false"
50-
variant="error"
51-
@click="isAdvanced = true">
52-
{{ t('talk_desktop', 'Advanced') }}
53-
</NcButton>
45+
<div class="certificate__content" :class="{ 'certificate__content--empty': !isAdvanced }">
46+
<template v-if="!isAdvanced">
47+
<IconAlert :size="128" class="certificate__alert-icon" />
48+
<p id="advanced-description">
49+
{{ t('talk-desktop', 'If you are unsure how to proceed contact your system administrator.') }}
50+
</p>
51+
<NcButton
52+
aria-describedby="advanced-description"
53+
variant="secondary"
54+
@click="isAdvanced = true">
55+
{{ t('talk_desktop', 'Advanced') }}
56+
</NcButton>
57+
</template>
5458

55-
<div class="certificate__content">
56-
<div v-if="isAdvanced" class="certificate__advanced">
59+
<template v-else>
5760
<!-- eslint-disable-next-line -->
58-
<p v-html="t('talk_desktop', 'This server could not prove that it is {url}.', { url: urlParam })"/>
61+
<p v-html="t('talk_desktop', 'This server could not prove that it is {url}.', { url: urlParam })"/>
5962
<p>
6063
{{ t('talk_desktop', 'It has untrusted SSL certificate. This might be caused by an attacker intercepting your connection or server misconfiguration.') }}
6164
</p>
6265
<p>
63-
<code>{{ error }}</code>
66+
{{ t('talk-desktop', 'If you are unsure how to proceed contact your system administrator.') }}
6467
</p>
6568
<p>
66-
<NcButton variant="error" @click="acceptCertificate(true)">
67-
{{ t('talk_desktop', 'Proceed') }}
68-
</NcButton>
69+
<code>{{ verificationResult }}</code>
6970
</p>
71+
<NcButton variant="error" @click="acceptCertificate(true)">
72+
{{ t('talk_desktop', 'Proceed') }}
73+
</NcButton>
7074
<details>
7175
<summary>
7276
{{ t('talk_desktop', 'View certificate') }}
7377
</summary>
7478
<CertificateInfo :certificate="certificate" />
7579
</details>
76-
</div>
80+
</template>
7781
</div>
7882

7983
<div class="certificate__actions">
@@ -96,53 +100,53 @@ const isAdvanced = ref(false)
96100
flex-direction: column;
97101
align-items: stretch;
98102
gap: calc(4 * var(--default-grid-baseline));
99-
padding: calc(4 * var(--default-grid-baseline));
100103
height: 100%;
104+
padding: calc(4 * var(--default-grid-baseline));
101105
background: var(--color-main-background);
102106
}
103107
104108
.certificate__heading {
105-
margin-block: 0;
106109
display: flex;
107110
align-items: baseline;
108111
justify-content: center;
109112
gap: 1ch;
113+
margin-block: 0;
110114
font-size: 1.5em;
111115
text-transform: capitalize;
112116
}
113117
114118
.certificate__alert-icon {
115-
color: var(--color-error);
119+
color: var(--color-placeholder-dark);
116120
}
117121
118-
.certificate__content {
119-
flex: 1;
120-
margin-inline: calc(-4 * var(--default-grid-baseline));
121-
padding-inline: calc(4 * var(--default-grid-baseline));
122-
display: flex;
123-
flex-direction: column;
124-
gap: calc(4 * var(--default-grid-baseline));
125-
overflow: auto;
122+
.certificate__note {
123+
/* Override default component styles */
124+
margin: 0;
126125
}
127126
128-
.certificate__advanced {
129-
border: 1px solid var(--color-border);
130-
border-radius: var(--border-radius-small);
131-
padding: calc(2 * var(--default-grid-baseline));
132-
display: flex;
133-
flex-direction: column;
134-
gap: 1em;
135-
background-color: var(--color-background-dark);
127+
.certificate__note-icon {
128+
color: var(--color-error);
136129
}
137130
138-
.certificate__note {
139-
/* Override default component styles */
140-
margin: 0;
131+
.certificate__content {
132+
background-color: var(--color-background-dark);
133+
flex: 1;
134+
display: flex;
135+
flex-direction: column;
136+
gap: calc(2 * var(--default-grid-baseline));
137+
padding: calc(2 * var(--default-grid-baseline));
138+
border-radius: var(--border-radius-small);
139+
overflow: auto;
140+
}
141+
142+
.certificate__content--empty {
143+
align-items: center;
144+
justify-content: center;
141145
}
142146
143147
.certificate__actions {
144148
display: flex;
145-
gap: calc(2 * var(--default-grid-baseline));
146149
align-items: flex-end;
150+
gap: calc(2 * var(--default-grid-baseline));
147151
}
148152
</style>

‎src/main.js‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -234,16 +234,14 @@ app.whenReady().then(async () => {
234234
// Note: the result of this verification is cached by domain in Electron
235235
// There is no way to clean the cache except by restarting the app
236236
session.defaultSession.setCertificateVerifyProc(async (request, callback) => {
237-
const isAccepted = await promptCertificateTrust(mainWindow, { host: request.hostname, certificate: request.certificate, error: request.verificationResult })
238-
// See: https://source.chromium.org/chromium/chromium/src/+/main:net/base/net_error_list.h
239-
const SSL_PROTOCOL_ERROR_CODE = -107
240-
callback(isAccepted ? 0 : SSL_PROTOCOL_ERROR_CODE)
237+
const isAccepted = request.errorCode === 0 || await promptCertificateTrust(mainWindow, request)
238+
callback(isAccepted ? 0 : -3)
241239
})
242240

243241
// Allow web-view with accepted untrusted certificate (Login Flow)
244242
app.on('certificate-error', async (event, webContents, url, error, certificate, callback) => {
245243
event.preventDefault()
246-
const isAccepted = await promptCertificateTrust(mainWindow, { host: new URL(url).host, certificate, error })
244+
const isAccepted = await promptCertificateTrust(mainWindow, { hostname: new URL(url).hostname, certificate, verificationResult: error })
247245
callback(isAccepted)
248246
})
249247

0 commit comments

Comments
 (0)