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.

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

Exempt additional WebAuthn error logging as expected #11577

Merged
merged 1 commit into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
30 changes: 30 additions & 0 deletions app/javascript/packages/webauthn/is-expected-error.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,34 @@ describe('isExpectedWebauthnError', () => {

expect(result).to.be.true();
});

it('returns true for a NotReadableError Android credential manager incompatibility', () => {
const error = new DOMException(
'An unknown error occurred while talking to the credential manager.',
'NotReadableError',
);
const result = isExpectedWebauthnError(error);

expect(result).to.be.true();
});

it('returns false for NotSupportedError when not during verification', () => {
const error = new DOMException(
'The user agent does not support public key credentials.',
'NotSupportedError',
);
const result = isExpectedWebauthnError(error);

expect(result).to.be.false();
});

it('returns true for NotSupportedError during verification', () => {
const error = new DOMException(
'The user agent does not support public key credentials.',
'NotSupportedError',
);
const result = isExpectedWebauthnError(error, { isVerifying: true });

expect(result).to.be.true();
});
});
50 changes: 30 additions & 20 deletions app/javascript/packages/webauthn/is-expected-error.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,34 @@
import isUserVerificationScreenLockError from './is-user-verification-screen-lock-error';

/**
* Set of expected DOM exceptions, which occur based on some user behavior that is not noteworthy:
*
* - Declining permissions
* - Timeout due to inactivity
* - Invalid state such as duplicate key enrollment
*
* @see https://webidl.spec.whatwg.org/#idl-DOMException
* Functions to test whether an error is expected and should not be logged for further analysis.
*/
const EXPECTED_DOM_EXCEPTIONS: Set<string> = new Set([
'NotAllowedError',
'TimeoutError',
'InvalidStateError',
]);
const EXPECTED_ERRORS: Array<(error: Error, options: IsExpectedErrorOptions) => boolean> = [
// A user who is unable to complete due to following DOMException reasons is not noteworthy:
//
// - Declining permissions
// - Timeout due to inactivity
// - Invalid state such as duplicate key enrollment
(error) =>
error.name === 'NotAllowedError' ||
error.name === 'TimeoutError' ||
error.name === 'InvalidStateError',
// Some indication of incompatibilities on specific Android devices, either phone itself or
// through credential manager.
//
// See: https://community.bitwarden.com/t/android-mobile-yubikey-5-nfc-webauth/51732
// See: https://www.reddit.com/r/GooglePixel/comments/17enqf3/pixel_7_pro_unable_to_setup_passkeys/
(error) =>
error.name === 'NotReadableError' &&
error.message === 'An unknown error occurred while talking to the credential manager.',
// A user can choose to authenticate with Face or Touch Unlock from another device from what
// they set up from, which may not necessarily support platform authenticators.
(error, { isVerifying }) => isVerifying && isUserVerificationScreenLockError(error),
(error, { isVerifying }) =>
isVerifying &&
error.name === 'NotSupportedError' &&
error.message === 'The user agent does not support public key credentials.',
];

interface IsExpectedErrorOptions {
/**
Expand All @@ -22,14 +37,9 @@ interface IsExpectedErrorOptions {
isVerifying: boolean;
}

function isExpectedWebauthnError(
const isExpectedWebauthnError = (
error: Error,
{ isVerifying }: Partial<IsExpectedErrorOptions> = {},
): boolean {
return (
(error instanceof DOMException && EXPECTED_DOM_EXCEPTIONS.has(error.name)) ||
(!!isVerifying && isUserVerificationScreenLockError(error))
);
}
{ isVerifying = false }: Partial<IsExpectedErrorOptions> = {},
): boolean => EXPECTED_ERRORS.some((isExpected) => isExpected(error, { isVerifying }));

export default isExpectedWebauthnError;