From 036fb7861fb8fb12f1dcabbd8edab9b322b7c96c Mon Sep 17 00:00:00 2001
From: zeripath <art27@cantab.net>
Date: Tue, 6 Jun 2023 06:29:37 +0100
Subject: [PATCH] Clean up WebAuthn javascript code and remove JQuery code
(#22697)
There were several issues with the WebAuthn registration and testing
code and the style
was very old javascript with jquery callbacks.
This PR uses async and fetch to replace the JQuery code.
Ref #22651
Signed-off-by: Andrew Thornton <art27@cantab.net>
---------
Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: delvh <dev.lh@web.de>
Co-authored-by: silverwind <me@silverwind.io>
---
routers/web/user/setting/security/webauthn.go | 6 +-
templates/user/auth/webauthn.tmpl | 2 +-
templates/user/auth/webauthn_error.tmpl | 31 +--
.../user/settings/security/webauthn.tmpl | 2 +-
web_src/css/base.css | 22 ++
web_src/css/repo.css | 5 -
web_src/js/features/user-auth-webauthn.js | 263 +++++++++---------
web_src/js/utils.js | 16 ++
web_src/js/utils.test.js | 8 +-
9 files changed, 191 insertions(+), 164 deletions(-)
diff --git a/routers/web/user/setting/security/webauthn.go b/routers/web/user/setting/security/webauthn.go
index 005431886746..826562f15748 100644
--- a/routers/web/user/setting/security/webauthn.go
+++ b/routers/web/user/setting/security/webauthn.go
@@ -6,6 +6,8 @@ package security
import (
"errors"
"net/http"
+ "strconv"
+ "time"
"code.gitea.io/gitea/models/auth"
wa "code.gitea.io/gitea/modules/auth/webauthn"
@@ -23,8 +25,8 @@ import (
func WebAuthnRegister(ctx *context.Context) {
form := web.GetForm(ctx).(*forms.WebauthnRegistrationForm)
if form.Name == "" {
- ctx.Error(http.StatusConflict)
- return
+ // Set name to the hexadecimal of the current time
+ form.Name = strconv.FormatInt(time.Now().UnixNano(), 16)
}
cred, err := auth.GetWebAuthnCredentialByName(ctx.Doer.ID, form.Name)
diff --git a/templates/user/auth/webauthn.tmpl b/templates/user/auth/webauthn.tmpl
index ea773d2566c5..f1c4f29fd903 100644
--- a/templates/user/auth/webauthn.tmpl
+++ b/templates/user/auth/webauthn.tmpl
@@ -5,6 +5,7 @@
<h3 class="ui top attached header">
{{.locale.Tr "twofa"}}
</h3>
+ {{template "user/auth/webauthn_error" .}}
<div class="ui attached segment">
{{svg "octicon-key" 56}}
<h3>{{.locale.Tr "webauthn_insert_key"}}</h3>
@@ -18,5 +19,4 @@
</div>
</div>
</div>
-{{template "user/auth/webauthn_error" .}}
{{template "base/footer" .}}
diff --git a/templates/user/auth/webauthn_error.tmpl b/templates/user/auth/webauthn_error.tmpl
index b6467de1aad8..f90882ef121f 100644
--- a/templates/user/auth/webauthn_error.tmpl
+++ b/templates/user/auth/webauthn_error.tmpl
@@ -1,22 +1,13 @@
-<div class="ui small modal" id="webauthn-error">
- <div class="header">{{.locale.Tr "webauthn_error"}}</div>
- <div class="content">
- <div class="ui negative message">
- <div class="header">
- {{.locale.Tr "webauthn_error"}}
- </div>
- <div class="gt-hidden" data-webauthn-error-msg="browser"><p>{{.locale.Tr "webauthn_unsupported_browser"}}</div>
- <div class="gt-hidden" data-webauthn-error-msg="unknown"><p>{{.locale.Tr "webauthn_error_unknown"}}</div>
- <div class="gt-hidden" data-webauthn-error-msg="insecure"><p>{{.locale.Tr "webauthn_error_insecure"}}</div>
- <div class="gt-hidden" data-webauthn-error-msg="unable-to-process"><p>{{.locale.Tr "webauthn_error_unable_to_process"}}</div>
- <div class="gt-hidden" data-webauthn-error-msg="duplicated"><p>{{.locale.Tr "webauthn_error_duplicated"}}</div>
- <div class="gt-hidden" data-webauthn-error-msg="empty"><p>{{.locale.Tr "webauthn_error_empty"}}</div>
- <div class="gt-hidden" data-webauthn-error-msg="timeout"><p>{{.locale.Tr "webauthn_error_timeout"}}</div>
- <div class="gt-hidden" data-webauthn-error-msg="general"></div>
- </div>
- </div>
- <div class="actions">
- <button onclick="window.location.reload()" class="success ui button gt-hidden webauthn_error_timeout">{{.locale.Tr "webauthn_reload"}}</button>
- <button class="ui cancel button">{{.locale.Tr "cancel"}}</button>
+<div id="webauthn-error" class="ui small gt-hidden">
+ <div class="content ui negative message gt-df gt-fc gt-gap-3">
+ <div class="header">{{.locale.Tr "webauthn_error"}}</div>
+ <div id="webauthn-error-msg"></div>
+ <div class="gt-hidden" data-webauthn-error-msg="browser">{{.locale.Tr "webauthn_unsupported_browser"}}</div>
+ <div class="gt-hidden" data-webauthn-error-msg="unknown">{{.locale.Tr "webauthn_error_unknown"}}</div>
+ <div class="gt-hidden" data-webauthn-error-msg="insecure">{{.locale.Tr "webauthn_error_insecure"}}</div>
+ <div class="gt-hidden" data-webauthn-error-msg="unable-to-process">{{.locale.Tr "webauthn_error_unable_to_process"}}</div>
+ <div class="gt-hidden" data-webauthn-error-msg="duplicated">{{.locale.Tr "webauthn_error_duplicated"}}</div>
+ <div class="gt-hidden" data-webauthn-error-msg="empty">{{.locale.Tr "webauthn_error_empty"}}</div>
+ <div class="gt-hidden" data-webauthn-error-msg="timeout">{{.locale.Tr "webauthn_error_timeout"}}</div>
</div>
</div>
diff --git a/templates/user/settings/security/webauthn.tmpl b/templates/user/settings/security/webauthn.tmpl
index 59022eb1c976..e541f764bc13 100644
--- a/templates/user/settings/security/webauthn.tmpl
+++ b/templates/user/settings/security/webauthn.tmpl
@@ -3,6 +3,7 @@
</h4>
<div class="ui attached segment">
<p>{{.locale.Tr "settings.webauthn_desc" | Str2html}}</p>
+ {{template "user/auth/webauthn_error" .}}
<div class="ui key list">
{{range .WebAuthnCredentials}}
<div class="item">
@@ -28,7 +29,6 @@
</div>
</div>
-{{template "user/auth/webauthn_error" .}}
<div class="ui g-modal-confirm delete modal" id="delete-registration">
<div class="header">
diff --git a/web_src/css/base.css b/web_src/css/base.css
index 49bdfed1b74d..b352e6d98e18 100644
--- a/web_src/css/base.css
+++ b/web_src/css/base.css
@@ -699,6 +699,11 @@ a.label,
border: 1px solid var(--color-secondary);
}
+.ui.info.message .header,
+.ui.blue.message .header {
+ color: var(--color-blue);
+}
+
.ui.info.message,
.ui.attached.info.message,
.ui.blue.message,
@@ -708,6 +713,12 @@ a.label,
border-color: var(--color-info-border);
}
+.ui.success.message .header,
+.ui.positive.message .header,
+.ui.green.message .header {
+ color: var(--color-green);
+}
+
.ui.success.message,
.ui.attached.success.message,
.ui.positive.message,
@@ -717,6 +728,12 @@ a.label,
border-color: var(--color-success-border);
}
+.ui.error.message .header,
+.ui.negative.message .header,
+.ui.red.message .header {
+ color: var(--color-red);
+}
+
.ui.error.message,
.ui.attached.error.message,
.ui.red.message,
@@ -728,6 +745,11 @@ a.label,
border-color: var(--color-error-border);
}
+.ui.warning.message .header,
+.ui.yellow.message .header {
+ color: var(--color-yellow);
+}
+
.ui.warning.message,
.ui.attached.warning.message,
.ui.yellow.message,
diff --git a/web_src/css/repo.css b/web_src/css/repo.css
index b2b544a7ea1a..4e300be87879 100644
--- a/web_src/css/repo.css
+++ b/web_src/css/repo.css
@@ -2405,11 +2405,6 @@
padding-bottom: 0 !important;
}
-.settings .content > .header,
-.settings .content .segment {
- box-shadow: 0 1px 2px 0 var(--color-box-header);
-}
-
.settings.webhooks .list > .item:not(:first-child),
.settings.githooks .list > .item:not(:first-child),
.settings.actions .list > .item:not(:first-child) {
diff --git a/web_src/js/features/user-auth-webauthn.js b/web_src/js/features/user-auth-webauthn.js
index f30017afc61d..8085313d223e 100644
--- a/web_src/js/features/user-auth-webauthn.js
+++ b/web_src/js/features/user-auth-webauthn.js
@@ -1,11 +1,13 @@
-import $ from 'jquery';
-import {encode, decode} from 'uint8-to-base64';
-import {hideElem, showElem} from '../utils/dom.js';
+import {encodeURLEncodedBase64, decodeURLEncodedBase64} from '../utils.js';
+import {showElem, hideElem} from '../utils/dom.js';
const {appSubUrl, csrfToken} = window.config;
-export function initUserAuthWebAuthn() {
- if ($('.user.signin.webauthn-prompt').length === 0) {
+export async function initUserAuthWebAuthn() {
+ hideElem('#webauthn-error');
+
+ const elPrompt = document.querySelector('.user.signin.webauthn-prompt');
+ if (!elPrompt) {
return;
}
@@ -13,49 +15,52 @@ export function initUserAuthWebAuthn() {
return;
}
- $.getJSON(`${appSubUrl}/user/webauthn/assertion`, {})
- .done((makeAssertionOptions) => {
- makeAssertionOptions.publicKey.challenge = decodeURLEncodedBase64(makeAssertionOptions.publicKey.challenge);
- for (let i = 0; i < makeAssertionOptions.publicKey.allowCredentials.length; i++) {
- makeAssertionOptions.publicKey.allowCredentials[i].id = decodeURLEncodedBase64(makeAssertionOptions.publicKey.allowCredentials[i].id);
- }
- navigator.credentials.get({
- publicKey: makeAssertionOptions.publicKey
- })
- .then((credential) => {
- verifyAssertion(credential);
- }).catch((err) => {
- // Try again... without the appid
- if (makeAssertionOptions.publicKey.extensions && makeAssertionOptions.publicKey.extensions.appid) {
- delete makeAssertionOptions.publicKey.extensions['appid'];
- navigator.credentials.get({
- publicKey: makeAssertionOptions.publicKey
- })
- .then((credential) => {
- verifyAssertion(credential);
- }).catch((err) => {
- webAuthnError('general', err.message);
- });
- return;
- }
- webAuthnError('general', err.message);
- });
- }).fail(() => {
- webAuthnError('unknown');
+ const res = await fetch(`${appSubUrl}/user/webauthn/assertion`);
+ if (res.status !== 200) {
+ webAuthnError('unknown');
+ return;
+ }
+ const options = await res.json();
+ options.publicKey.challenge = decodeURLEncodedBase64(options.publicKey.challenge);
+ for (const cred of options.publicKey.allowCredentials) {
+ cred.id = decodeURLEncodedBase64(cred.id);
+ }
+ const credential = await navigator.credentials.get({
+ publicKey: options.publicKey
+ });
+ try {
+ await verifyAssertion(credential);
+ } catch (err) {
+ if (!options.publicKey.extensions?.appid) {
+ webAuthnError('general', err.message);
+ return;
+ }
+ delete options.publicKey.extensions.appid;
+ const credential = await navigator.credentials.get({
+ publicKey: options.publicKey
});
+ try {
+ await verifyAssertion(credential);
+ } catch (err) {
+ webAuthnError('general', err.message);
+ }
+ }
}
-function verifyAssertion(assertedCredential) {
+async function verifyAssertion(assertedCredential) {
// Move data into Arrays incase it is super long
const authData = new Uint8Array(assertedCredential.response.authenticatorData);
const clientDataJSON = new Uint8Array(assertedCredential.response.clientDataJSON);
const rawId = new Uint8Array(assertedCredential.rawId);
const sig = new Uint8Array(assertedCredential.response.signature);
const userHandle = new Uint8Array(assertedCredential.response.userHandle);
- $.ajax({
- url: `${appSubUrl}/user/webauthn/assertion`,
- type: 'POST',
- data: JSON.stringify({
+
+ const res = await fetch(`${appSubUrl}/user/webauthn/assertion`, {
+ method: 'POST',
+ headers: {
+ 'Content-Type': 'application/json; charset=utf-8'
+ },
+ body: JSON.stringify({
id: assertedCredential.id,
rawId: encodeURLEncodedBase64(rawId),
type: assertedCredential.type,
@@ -67,50 +72,31 @@ function verifyAssertion(assertedCredential) {
userHandle: encodeURLEncodedBase64(userHandle),
},
}),
- contentType: 'application/json; charset=utf-8',
- dataType: 'json',
- success: (resp) => {
- if (resp && resp['redirect']) {
- window.location.href = resp['redirect'];
- } else {
- window.location.href = '/';
- }
- },
- error: (xhr) => {
- if (xhr.status === 500) {
- webAuthnError('unknown');
- return;
- }
- webAuthnError('unable-to-process');
- }
});
+ if (res.status === 500) {
+ webAuthnError('unknown');
+ return;
+ } else if (res.status !== 200) {
+ webAuthnError('unable-to-process');
+ return;
+ }
+ const reply = await res.json();
+
+ window.location.href = reply?.redirect ?? `${appSubUrl}/`;
}
-// Encode an ArrayBuffer into a URLEncoded base64 string.
-function encodeURLEncodedBase64(value) {
- return encode(value)
- .replace(/\+/g, '-')
- .replace(/\//g, '_')
- .replace(/=/g, '');
-}
-
-// Dccode a URLEncoded base64 to an ArrayBuffer string.
-function decodeURLEncodedBase64(value) {
- return decode(value
- .replace(/_/g, '/')
- .replace(/-/g, '+'));
-}
-
-function webauthnRegistered(newCredential) {
+async function webauthnRegistered(newCredential) {
const attestationObject = new Uint8Array(newCredential.response.attestationObject);
const clientDataJSON = new Uint8Array(newCredential.response.clientDataJSON);
const rawId = new Uint8Array(newCredential.rawId);
- return $.ajax({
- url: `${appSubUrl}/user/settings/security/webauthn/register`,
- type: 'POST',
- headers: {'X-Csrf-Token': csrfToken},
- data: JSON.stringify({
+ const res = await fetch(`${appSubUrl}/user/settings/security/webauthn/register`, {
+ method: 'POST',
+ headers: {
+ 'X-Csrf-Token': csrfToken,
+ 'Content-Type': 'application/json; charset=utf-8',
+ },
+ body: JSON.stringify({
id: newCredential.id,
rawId: encodeURLEncodedBase64(rawId),
type: newCredential.type,
@@ -119,48 +105,47 @@ function webauthnRegistered(newCredential) {
clientDataJSON: encodeURLEncodedBase64(clientDataJSON),
},
}),
- dataType: 'json',
- contentType: 'application/json; charset=utf-8',
- }).then(() => {
- window.location.reload();
- }).fail((xhr) => {
- if (xhr.status === 409) {
- webAuthnError('duplicated');
- return;
- }
- webAuthnError('unknown');
});
+
+ if (res.status === 409) {
+ webAuthnError('duplicated');
+ return;
+ } else if (res.status !== 201) {
+ webAuthnError('unknown');
+ return;
+ }
+
+ window.location.reload();
}
function webAuthnError(errorType, message) {
- hideElem($('#webauthn-error [data-webauthn-error-msg]'));
- const $errorGeneral = $(`#webauthn-error [data-webauthn-error-msg=general]`);
+ const elErrorMsg = document.getElementById(`webauthn-error-msg`);
+
if (errorType === 'general') {
- showElem($errorGeneral);
- $errorGeneral.text(message || 'unknown error');
+ elErrorMsg.textContent = message || 'unknown error';
} else {
- const $errorTyped = $(`#webauthn-error [data-webauthn-error-msg=${errorType}]`);
- if ($errorTyped.length) {
- showElem($errorTyped);
+ const elTypedError = document.querySelector(`#webauthn-error [data-webauthn-error-msg=${errorType}]`);
+ if (elTypedError) {
+ elErrorMsg.textContent = `${elTypedError.textContent}${message ? ` ${message}` : ''}`;
} else {
- showElem($errorGeneral);
- $errorGeneral.text(`unknown error type: ${errorType}`);
+ elErrorMsg.textContent = `unknown error type: ${errorType}${message ? ` ${message}` : ''}`;
}
}
- $('#webauthn-error').modal('show');
+
+ showElem('#webauthn-error');
}
function detectWebAuthnSupport() {
if (!window.isSecureContext) {
- $('#register-button').prop('disabled', true);
- $('#login-button').prop('disabled', true);
+ document.getElementById('register-button').disabled = true;
+ document.getElementById('login-button').disabled = true;
webAuthnError('insecure');
return false;
}
if (typeof window.PublicKeyCredential !== 'function') {
- $('#register-button').prop('disabled', true);
- $('#login-button').prop('disabled', true);
+ document.getElementById('register-button').disabled = true;
+ document.getElementById('login-button').disabled = true;
webAuthnError('browser');
return false;
}
@@ -169,12 +154,14 @@ function detectWebAuthnSupport() {
}
export function initUserAuthWebAuthnRegister() {
- if ($('#register-webauthn').length === 0) {
+ const elRegister = document.getElementById('register-webauthn');
+ if (!elRegister) {
return;
}
- $('#webauthn-error').modal({allowMultiple: false});
- $('#register-webauthn').on('click', (e) => {
+ hideElem('#webauthn-error');
+
+ elRegister.addEventListener('click', (e) => {
e.preventDefault();
if (!detectWebAuthnSupport()) {
return;
@@ -183,40 +170,48 @@ export function initUserAuthWebAuthnRegister() {
});
}
-function webAuthnRegisterRequest() {
- if ($('#nickname').val() === '') {
- webAuthnError('empty');
+async function webAuthnRegisterRequest() {
+ const elNickname = document.getElementById('nickname');
+
+ const body = new FormData();
+ body.append('name', elNickname.value);
+
+ const res = await fetch(`${appSubUrl}/user/settings/security/webauthn/request_register`, {
+ method: 'POST',
+ headers: {
+ 'X-Csrf-Token': csrfToken,
+ },
+ body,
+ });
+
+ if (res.status === 409) {
+ webAuthnError('duplicated');
+ return;
+ } else if (res.status !== 200) {
+ webAuthnError('unknown');
return;
}
- $.post(`${appSubUrl}/user/settings/security/webauthn/request_register`, {
- _csrf: csrfToken,
- name: $('#nickname').val(),
- }).done((makeCredentialOptions) => {
- $('#nickname').closest('div.field').removeClass('error');
- makeCredentialOptions.publicKey.challenge = decodeURLEncodedBase64(makeCredentialOptions.publicKey.challenge);
- makeCredentialOptions.publicKey.user.id = decodeURLEncodedBase64(makeCredentialOptions.publicKey.user.id);
- if (makeCredentialOptions.publicKey.excludeCredentials) {
- for (let i = 0; i < makeCredentialOptions.publicKey.excludeCredentials.length; i++) {
- makeCredentialOptions.publicKey.excludeCredentials[i].id = decodeURLEncodedBase64(makeCredentialOptions.publicKey.excludeCredentials[i].id);
- }
- }
+ const options = await res.json();
+ elNickname.closest('div.field').classList.remove('error');
- navigator.credentials.create({
- publicKey: makeCredentialOptions.publicKey
- }).then(webauthnRegistered)
- .catch((err) => {
- if (!err) {
- webAuthnError('unknown');
- return;
- }
- webAuthnError('general', err.message);
- });
- }).fail((xhr) => {
- if (xhr.status === 409) {
- webAuthnError('duplicated');
- return;
+ options.publicKey.challenge = decodeURLEncodedBase64(options.publicKey.challenge);
+ options.publicKey.user.id = decodeURLEncodedBase64(options.publicKey.user.id);
+ if (options.publicKey.excludeCredentials) {
+ for (const cred of options.publicKey.excludeCredentials) {
+ cred.id = decodeURLEncodedBase64(cred.id);
}
- webAuthnError('unknown');
- });
+ }
+
+ let credential;
+ try {
+ credential = await navigator.credentials.create({
+ publicKey: options.publicKey
+ });
+ } catch (err) {
+ webAuthnError('unknown', err);
+ return;
+ }
+
+ webauthnRegistered(credential);
}
diff --git a/web_src/js/utils.js b/web_src/js/utils.js
index 6bee4f083630..4f9ad452f6f4 100644
--- a/web_src/js/utils.js
+++ b/web_src/js/utils.js
@@ -1,3 +1,5 @@
+import {encode, decode} from 'uint8-to-base64';
+
// transform /path/to/file.ext to file.ext
export function basename(path = '') {
return path ? path.replace(/^.*\//, '') : '';
@@ -135,3 +137,17 @@ export function toAbsoluteUrl(url) {
return `${window.location.origin}${url}`;
}
+// Encode an ArrayBuffer into a URLEncoded base64 string.
+export function encodeURLEncodedBase64(arrayBuffer) {
+ return encode(arrayBuffer)
+ .replace(/\+/g, '-')
+ .replace(/\//g, '_')
+ .replace(/=/g, '');
+}
+
+// Decode a URLEncoded base64 to an ArrayBuffer string.
+export function decodeURLEncodedBase64(base64url) {
+ return decode(base64url
+ .replace(/_/g, '/')
+ .replace(/-/g, '+'));
+}
diff --git a/web_src/js/utils.test.js b/web_src/js/utils.test.js
index 2f9e5fb47d5e..cf73b63b9901 100644
--- a/web_src/js/utils.test.js
+++ b/web_src/js/utils.test.js
@@ -2,7 +2,7 @@ import {expect, test} from 'vitest';
import {
basename, extname, isObject, stripTags, joinPaths, parseIssueHref,
parseUrl, translateMonth, translateDay, blobToDataURI,
- toAbsoluteUrl,
+ toAbsoluteUrl, encodeURLEncodedBase64, decodeURLEncodedBase64,
} from './utils.js';
test('basename', () => {
@@ -132,3 +132,9 @@ test('toAbsoluteUrl', () => {
expect(() => toAbsoluteUrl('path')).toThrowError('unsupported');
});
+
+test('encodeURLEncodedBase64, decodeURLEncodedBase64', () => {
+ expect(encodeURLEncodedBase64(decodeURLEncodedBase64('foo'))).toEqual('foo'); // No = padding
+ expect(encodeURLEncodedBase64(decodeURLEncodedBase64('a-minus'))).toEqual('a-minus');
+ expect(encodeURLEncodedBase64(decodeURLEncodedBase64('_underscorc'))).toEqual('_underscorc');
+});