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

fix/352-not-allowed-error #353

Merged
merged 2 commits into from
Feb 9, 2023
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
3 changes: 2 additions & 1 deletion packages/browser/src/helpers/__jest__/generateCustomError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ type WebAuthnErrorName =
| 'SecurityError'
| 'UnknownError';

export function generateCustomError(name: WebAuthnErrorName): Error {
export function generateCustomError(name: WebAuthnErrorName, message = ''): Error {
const customError = new Error();
customError.name = name;
customError.message = message;
return customError;
}
19 changes: 4 additions & 15 deletions packages/browser/src/helpers/identifyAuthenticationError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,21 +23,10 @@ export function identifyAuthenticationError({
return new WebAuthnError('Authentication ceremony was sent an abort signal', 'AbortError');
}
} else if (error.name === 'NotAllowedError') {
if (publicKey.allowCredentials?.length) {
// https://www.w3.org/TR/webauthn-2/#sctn-discover-from-external-source (Step 17)
// https://www.w3.org/TR/webauthn-2/#sctn-op-get-assertion (Step 6)
return new WebAuthnError(
'No available authenticator recognized any of the allowed credentials',
'NotAllowedError',
);
}

// https://www.w3.org/TR/webauthn-2/#sctn-discover-from-external-source (Step 18)
// https://www.w3.org/TR/webauthn-2/#sctn-op-get-assertion (Step 7)
return new WebAuthnError(
'User clicked cancel, or the authentication ceremony timed out',
'NotAllowedError',
);
/**
* Pass the error directly through. Platforms are overloading this error beyond what the spec
* defines and we don't want to overwrite potentially useful error messages.
*/
} else if (error.name === 'SecurityError') {
const effectiveDomain = window.location.hostname;
if (!isValidDomain(effectiveDomain)) {
Expand Down
10 changes: 4 additions & 6 deletions packages/browser/src/helpers/identifyRegistrationError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,10 @@ export function identifyRegistrationError({
// https://www.w3.org/TR/webauthn-2/#sctn-op-make-cred (Step 3)
return new WebAuthnError('The authenticator was previously registered', 'InvalidStateError');
} else if (error.name === 'NotAllowedError') {
// https://www.w3.org/TR/webauthn-2/#sctn-createCredential (Step 20)
// https://www.w3.org/TR/webauthn-2/#sctn-createCredential (Step 21)
return new WebAuthnError(
'User clicked cancel, or the registration ceremony timed out',
'NotAllowedError',
);
/**
* Pass the error directly through. Platforms are overloading this error beyond what the spec
* defines and we don't want to overwrite potentially useful error messages.
*/
} else if (error.name === 'NotSupportedError') {
const validPubKeyCredParams = publicKey.pubKeyCredParams.filter(
param => param.type === 'public-key',
Expand Down
39 changes: 24 additions & 15 deletions packages/browser/src/methods/startAuthentication.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -332,29 +332,38 @@ describe('WebAuthnError', () => {
});

describe('NotAllowedError', () => {
const NotAllowedError = generateCustomError('NotAllowedError');

test('should identify unrecognized allowed credentials', async () => {
test('should pass through error message (iOS Safari - Operation failed)', async () => {
/**
* Thrown when biometric is not enrolled, or a Safari bug prevents conditional UI from being
* aborted properly between page reloads.
*
* See https://github.com/MasterKale/SimpleWebAuthn/discussions/350#discussioncomment-4896572
*/
const NotAllowedError = generateCustomError('NotAllowedError', 'Operation failed.');
mockNavigatorGet.mockRejectedValueOnce(NotAllowedError);

const rejected = await expect(startAuthentication(goodOpts1)).rejects;
rejected.toThrow(WebAuthnError);
rejected.toThrow(/allowed credentials/i);
rejected.toThrow(Error);
rejected.toThrow(/operation failed/i);
rejected.toHaveProperty('name', 'NotAllowedError');
});

test('should identify cancellation or timeout', async () => {
test('should pass through error message (Chrome M110 - Bad TLS Cert)', async () => {
/**
* Starting from Chrome M110, WebAuthn is blocked if the site is being displayed on a URL with
* TLS certificate issues. This includes during development.
*
* See https://github.com/MasterKale/SimpleWebAuthn/discussions/351#discussioncomment-4910458
*/
const NotAllowedError = generateCustomError(
'NotAllowedError',
'WebAuthn is not supported on sites with TLS certificate errors.'
);
mockNavigatorGet.mockRejectedValueOnce(NotAllowedError);

const opts = {
...goodOpts1,
allowCredentials: [],
};

const rejected = await expect(startAuthentication(opts)).rejects;
rejected.toThrow(WebAuthnError);
rejected.toThrow(/cancel/i);
rejected.toThrow(/timed out/i);
const rejected = await expect(startAuthentication(goodOpts1)).rejects;
rejected.toThrow(Error);
rejected.toThrow(/sites with TLS certificate errors/i);
rejected.toHaveProperty('name', 'NotAllowedError');
});
});
Expand Down
33 changes: 28 additions & 5 deletions packages/browser/src/methods/startRegistration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -324,15 +324,38 @@ describe('WebAuthnError', () => {
});

describe('NotAllowedError', () => {
const NotAllowedError = generateCustomError('NotAllowedError');
test('should pass through error message (iOS Safari - Operation failed)', async () => {
/**
* Thrown when biometric is not enrolled, or a Safari bug prevents conditional UI from being
* aborted properly between page reloads.
*
* See https://github.com/MasterKale/SimpleWebAuthn/discussions/350#discussioncomment-4896572
*/
const NotAllowedError = generateCustomError('NotAllowedError', 'Operation failed.');
mockNavigatorCreate.mockRejectedValueOnce(NotAllowedError);

const rejected = await expect(startRegistration(goodOpts1)).rejects;
rejected.toThrow(Error);
rejected.toThrow(/operation failed/i);
rejected.toHaveProperty('name', 'NotAllowedError');
});

test('should identify cancellation or timeout', async () => {
test('should pass through error message (Chrome M110 - Bad TLS Cert)', async () => {
/**
* Starting from Chrome M110, WebAuthn is blocked if the site is being displayed on a URL with
* TLS certificate issues. This includes during development.
*
* See https://github.com/MasterKale/SimpleWebAuthn/discussions/351#discussioncomment-4910458
*/
const NotAllowedError = generateCustomError(
'NotAllowedError',
'WebAuthn is not supported on sites with TLS certificate errors.'
);
mockNavigatorCreate.mockRejectedValueOnce(NotAllowedError);

const rejected = await expect(startRegistration(goodOpts1)).rejects;
rejected.toThrow(WebAuthnError);
rejected.toThrow(/cancel/i);
rejected.toThrow(/timed out/i);
rejected.toThrow(Error);
rejected.toThrow(/sites with TLS certificate errors/i);
rejected.toHaveProperty('name', 'NotAllowedError');
});
});
Expand Down