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.

Sign up for GitHub

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

Jump to bottom

Fix webauthn regression and improve code #25113

Merged
merged 1 commit into from
Jun 7, 2023
Merged

Fix webauthn regression and improve code #25113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 17 additions & 16 deletions templates/user/auth/webauthn.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,22 @@
{{template "base/head" .}}
<div class="user signin webauthn-prompt">
<div class="ui middle centered very relaxed page grid">
<div role="main" aria-label="{{.Title}}" class="page-content user signin webauthn-prompt">
<div class="ui page grid">
<div class="column center aligned">
<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>
{{template "base/alert" .}}
<p>{{.locale.Tr "webauthn_sign_in"}}</p>
</div>
<div class="ui attached segment"><div class="ui active indeterminate inline loader"></div> {{.locale.Tr "webauthn_press_button"}} </div>
<div class="ui attached segment">
<a href="{{AppSubUrl}}/user/two_factor">{{.locale.Tr "webauthn_use_twofa"}}</a>
</div>
{{template "user/auth/webauthn_error" .}}
<h3 class="ui top attached header">{{.locale.Tr "twofa"}}</h3>
<div class="ui attached segment">
{{svg "octicon-key" 56}}
<h3>{{.locale.Tr "webauthn_insert_key"}}</h3>
{{template "base/alert" .}}
<p>{{.locale.Tr "webauthn_sign_in"}}</p>
</div>
<div class="ui attached segment">
<div class="ui active indeterminate inline loader"></div>
{{.locale.Tr "webauthn_press_button"}}
</div>
<div class="ui attached segment">
<a href="{{AppSubUrl}}/user/two_factor">{{.locale.Tr "webauthn_use_twofa"}}</a>
</div>
</div>
</div>
</div>
Expand Down
22 changes: 11 additions & 11 deletions templates/user/auth/webauthn_error.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
<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 id="webauthn-error" class="ui negative message gt-hidden">
<div class="header">{{.locale.Tr "webauthn_error"}}</div>
<div id="webauthn-error-msg" class="gt-pt-3"></div>
<div class="gt-hidden">
<div data-webauthn-error-msg="browser">{{.locale.Tr "webauthn_unsupported_browser"}}</div>
<div data-webauthn-error-msg="unknown">{{.locale.Tr "webauthn_error_unknown"}}</div>
<div data-webauthn-error-msg="insecure">{{.locale.Tr "webauthn_error_insecure"}}</div>
<div data-webauthn-error-msg="unable-to-process">{{.locale.Tr "webauthn_error_unable_to_process"}}</div>
<div data-webauthn-error-msg="duplicated">{{.locale.Tr "webauthn_error_duplicated"}}</div>
<div data-webauthn-error-msg="empty">{{.locale.Tr "webauthn_error_empty"}}</div>
<div data-webauthn-error-msg="timeout">{{.locale.Tr "webauthn_error_timeout"}}</div>
</div>
</div>
6 changes: 1 addition & 5 deletions templates/user/settings/security/webauthn.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
<h4 class="ui top attached header">
{{.locale.Tr "settings.webauthn"}}
</h4>
<h4 class="ui top attached header">{{.locale.Tr "settings.webauthn"}}</h4>
<div class="ui attached segment">
<p>{{.locale.Tr "settings.webauthn_desc" | Str2html}}</p>
{{template "user/auth/webauthn_error" .}}
Expand All @@ -20,7 +18,6 @@
{{end}}
</div>
<div class="ui form">
{{.CsrfTokenHtml}}
<div class="required field">
<label for="nickname">{{.locale.Tr "settings.webauthn_nickname"}}</label>
<input id="nickname" name="nickname" type="text" required>
Expand All @@ -29,7 +26,6 @@
</div>
</div>


<div class="ui g-modal-confirm delete modal" id="delete-registration">
<div class="header">
{{svg "octicon-trash"}}
Expand Down
4 changes: 0 additions & 4 deletions web_src/css/form.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,10 +394,6 @@ textarea:focus,
margin: 0;
}

.user.signin.webauthn-prompt {
margin-top: 15px;
}

.repository.new.repo form,
.repository.new.migrate form,
.repository.new.fork form {
Expand Down
43 changes: 16 additions & 27 deletions web_src/js/features/user-auth-webauthn.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
import {encodeURLEncodedBase64, decodeURLEncodedBase64} from '../utils.js';
import {showElem, hideElem} from '../utils/dom.js';
import {showElem} from '../utils/dom.js';

const {appSubUrl, csrfToken} = window.config;

export async function initUserAuthWebAuthn() {
hideElem('#webauthn-error');

const elPrompt = document.querySelector('.user.signin.webauthn-prompt');
if (!elPrompt) {
return;
Expand All @@ -25,21 +23,21 @@ export async function initUserAuthWebAuthn() {
for (const cred of options.publicKey.allowCredentials) {
cred.id = decodeURLEncodedBase64(cred.id);
}
const credential = await navigator.credentials.get({
publicKey: options.publicKey
});
try {
const credential = await navigator.credentials.get({
publicKey: options.publicKey
});
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 {
const credential = await navigator.credentials.get({
publicKey: options.publicKey
});
await verifyAssertion(credential);
} catch (err) {
webAuthnError('general', err.message);
Expand All @@ -48,7 +46,7 @@ export async function initUserAuthWebAuthn() {
}

async function verifyAssertion(assertedCredential) {
// Move data into Arrays incase it is super long
// Move data into Arrays in case it is super long
const authData = new Uint8Array(assertedCredential.response.authenticatorData);
const clientDataJSON = new Uint8Array(assertedCredential.response.clientDataJSON);
const rawId = new Uint8Array(assertedCredential.rawId);
Expand Down Expand Up @@ -137,15 +135,11 @@ function webAuthnError(errorType, message) {

function detectWebAuthnSupport() {
if (!window.isSecureContext) {
document.getElementById('register-button').disabled = true;
document.getElementById('login-button').disabled = true;
webAuthnError('insecure');
return false;
}

if (typeof window.PublicKeyCredential !== 'function') {
document.getElementById('register-button').disabled = true;
document.getElementById('login-button').disabled = true;
webAuthnError('browser');
return false;
}
Expand All @@ -158,15 +152,13 @@ export function initUserAuthWebAuthnRegister() {
if (!elRegister) {
return;
}

hideElem('#webauthn-error');

elRegister.addEventListener('click', (e) => {
if (!detectWebAuthnSupport()) {
elRegister.disabled = true;
return;
}
elRegister.addEventListener('click', async (e) => {
e.preventDefault();
if (!detectWebAuthnSupport()) {
return;
}
webAuthnRegisterRequest();
await webAuthnRegisterRequest();
});
}

Expand Down Expand Up @@ -203,15 +195,12 @@ async function webAuthnRegisterRequest() {
}
}

let credential;
try {
credential = await navigator.credentials.create({
const credential = await navigator.credentials.create({
publicKey: options.publicKey
});
await webauthnRegistered(credential);
} catch (err) {
webAuthnError('unknown', err);
return;
}

webauthnRegistered(credential);
}
15 changes: 12 additions & 3 deletions web_src/js/utils.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,17 @@ test('toAbsoluteUrl', () => {
expect(() => toAbsoluteUrl('path')).toThrowError('unsupported');
});

const uint8array = (s) => new TextEncoder().encode(s);
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');
expect(encodeURLEncodedBase64(uint8array('AA?'))).toEqual('QUE_'); // standard base64: "QUE/"
expect(encodeURLEncodedBase64(uint8array('AA~'))).toEqual('QUF-'); // standard base64: "QUF+"

expect(decodeURLEncodedBase64('QUE/')).toEqual(uint8array('AA?'));
expect(decodeURLEncodedBase64('QUF+')).toEqual(uint8array('AA~'));
expect(decodeURLEncodedBase64('QUE_')).toEqual(uint8array('AA?'));
expect(decodeURLEncodedBase64('QUF-')).toEqual(uint8array('AA~'));

expect(encodeURLEncodedBase64(uint8array('a'))).toEqual('YQ'); // standard base64: "YQ=="
expect(decodeURLEncodedBase64('YQ')).toEqual(uint8array('a'));
expect(decodeURLEncodedBase64('YQ==')).toEqual(uint8array('a'));
});