From fcaedbf4ba7163938b84909bd7ad415cc4333e87 Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Mon, 6 Nov 2023 16:45:33 -0500 Subject: [PATCH] FIX: Broken login with security key when passkeys enabled (#24249) In some browsers, 2FA login was failing with a "request is already pending" error. This only applied when `experimental_passkeys` was enabled and on Chrome only. This was due to the fact that the webauthn API only supports one auth attempt at a time, so the security key call needs to abort the passkey's conditional UI request before starting. I am not sure we can test this. We have system specs that simulate webauthn credentials and they didn't catch this (probably because the simulation covers the whole flow). --- .../javascripts/discourse/app/lib/webauthn.js | 43 ++++++++++--------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/app/assets/javascripts/discourse/app/lib/webauthn.js b/app/assets/javascripts/discourse/app/lib/webauthn.js index 94e47de4e94..f6ed1613be1 100644 --- a/app/assets/javascripts/discourse/app/lib/webauthn.js +++ b/app/assets/javascripts/discourse/app/lib/webauthn.js @@ -17,6 +17,27 @@ export function isWebauthnSupported() { return typeof PublicKeyCredential !== "undefined"; } +// The webauthn API only supports one auth attempt at a time +// We need this service to cancel the previous attempt when a new one is started +class WebauthnAbortService { + controller = undefined; + + signal() { + if (this.controller) { + const abortError = new Error("Cancelling pending webauthn call"); + abortError.name = "AbortError"; + this.controller.abort(abortError); + } + + this.controller = new AbortController(); + return this.controller.signal; + } +} + +// Need to use a singleton here to reset the active webauthn ceremony +// Inspired by the BaseWebAuthnAbortService in https://github.com/MasterKale/SimpleWebAuthn +const WebauthnAbortHandler = new WebauthnAbortService(); + export function getWebauthnCredential( challenge, allowedCredentialIds, @@ -49,6 +70,7 @@ export function getWebauthnCredential( // (this is only a hint, though, browser may still prompt) userVerification: "discouraged", }, + signal: WebauthnAbortHandler.signal(), }) .then((credential) => { // 3. If credential.response is not an instance of AuthenticatorAssertionResponse, abort the ceremony. @@ -94,27 +116,6 @@ export function getWebauthnCredential( }); } -// The webauthn API only supports one auth attempt at a time -// We need this service to cancel the previous attempt when a new one is started -class WebauthnAbortService { - controller = undefined; - - signal() { - if (this.controller) { - const abortError = new Error("Cancelling pending webauthn call"); - abortError.name = "AbortError"; - this.controller.abort(abortError); - } - - this.controller = new AbortController(); - return this.controller.signal; - } -} - -// Need to use a singleton here to reset the active webauthn ceremony -// Inspired by the BaseWebAuthnAbortService in https://github.com/MasterKale/SimpleWebAuthn -const WebauthnAbortHandler = new WebauthnAbortService(); - export async function getPasskeyCredential( challenge, errorCallback,