[passkeys] Fixes unhandled callback, double response errors & wrong return type

[a2kolbasov]

Fixed errors in processing error messages when passkeys are missing or a request is rejected.

1. The setTimeout callback was returning an error that was silently discarded. As a result, the page context never received the error, and the Promise remained pending forever.

2. When kpxcPasskeysUtils.passkeysFallback === true, kpxcPasskeysUtils.sendPasskeysResponse was being called (and sending a response) twice because of a missing return ;.

3. According to the WebAuthn specification, cancelled or rejected operations always return errors, not null. Some websites don’t expect this and throw a script error on the page, which also leads to an endless wait for UI updates. (https://www.w3.org/TR/webauthn-2/#sctn-getAssertion)

4. Fix lost callback on DB connection failure

---

• Fixes Promise not rejected, NotAllowedError cannot be caught #3045
• Fixes the "Error! Credential is excluded." error is swallowed and not propagated back to the browser/client code #2501

Screenshots or videos

Testing strategy

On https://webauthn.io/

1. Try to authenticate without passkeys. Wait for the timeout (1 min).
2. Register.
3. Try to authenticate, but reject the passkey request. Wait for the timeout.

Additional information, resources etc.

• Promise.withResolvers browser compatibility, Firefox strict_min_version

Type of change

• ✅ Bug fix (non-breaking change that fixes an issue)
• ✅ Refactor (significant modification to existing code)

[varjolintu]

Quickly tested this and noticed that when canceling the authentication request, the extension now immediately returns NotAllowedError. This goes against: https://www.w3.org/TR/webauthn-2/#sctn-assertion-privacy

Also explained at https://www.w3.org/TR/webauthn-2/#sctn-getAssertion step 18:

Return a DOMException whose name is "NotAllowedError". In order to prevent information leak that could identify the user without consent, this step MUST NOT be executed before lifetimeTimer has expired.

[a2kolbasov]

Quickly tested this and noticed that when canceling the authentication request, the extension now immediately returns NotAllowedError.

At first I thought it was very strange, because there is setTimeout(resolve, timeout). But now I realized that the PASSKEYS_REQUEST_CANCELED condition is missing in

keepassxc-browser/keepassxc-browser/content/passkeys-inject.js

Lines 36 to 42 in 379dfaf

	const letTimerRunOut = function (errorCode) {
	return (
	errorCode === PASSKEYS_WAIT_FOR_LIFETIMER ||
	errorCode === PASSKEYS_CREDENTIAL_IS_EXCLUDED ||
	errorCode === PASSKEYS_NO_LOGINS_FOUND
	);
	};

This goes against: https://www.w3.org/TR/webauthn-2/#sctn-assertion-privacy

Also explained at https://www.w3.org/TR/webauthn-2/#sctn-getAssertion step 18

On the other hand, in WebAuthn-3 privacy is implemented differently

• Update obsolete privacy concerns about throwing errors early w3c/webauthn#2134
• https://www.w3.org/TR/webauthn-3/#sctn-getAssertion:~:text=Throw%20a%20%22NotAllowedError%22%20DOMException%2E,-5%2E1%2E4%2E2
• https://www.w3.org/TR/webauthn-3/#sctn-assertion-privacy

[varjolintu]

On the other hand, in WebAuthn-3 privacy is implemented differently

* [Update obsolete privacy concerns about throwing errors early w3c/webauthn#2134](https://github.com/w3c/webauthn/pull/2134)

* https://www.w3.org/TR/webauthn-3/#sctn-getAssertion:~:text=Throw%20a%20%22NotAllowedError%22%20DOMException%2E,-5%2E1%2E4%2E2

* https://www.w3.org/TR/webauthn-3/#sctn-assertion-privacy

The latest released version is still 2. It's not recommended to implement changes from 3, because those might still change: https://www.w3.org/standards/types/#CR. I agree it will simplify a lot of things just to return an error immediately instead of waiting for the lifetimer.

[a2kolbasov]

Okay, got it. I've made changes in a044542.

[a2kolbasov]

When I was recording the video demonstration, I found another error. If the database is locked, the content script does not handle this situation, and the page script will never receive a response with an error.

[varjolintu]

Why not use const instead of let inside the function?

[a2kolbasov]

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/withResolvers#description

reject uses inside abort

[a2kolbasov]

let promise & let timerId can be replaced with const ...

[varjolintu]

Here's a chance for a small refactor:
const errorCode = ret === null ? PASSKEYS_REQUEST_CANCELED : ret.response.errorCode;

And then just use the errorCode for checks and send message etc. This way we can keep the ret as const and the code stays a bit cleaner.

[varjolintu]

Otherwise this seems to work great now!