Upstream/bitwarden-browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser.git synced 2025-02-12 00:41:29 +01:00
Code Issues Projects Releases Wiki Activity

[EC-598] feat: rearrange order of execution

Browse Source
This commit is contained in:
Andreas Coroiu 2023-03-22 10:49:05 +01:00
parent fbfaa06cbb
commit 376be67f28
No known key found for this signature in database
GPG Key ID: E70B5FFC81DFEC1A
2 changed files with 99 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

112
libs/common/src/webauthn/services/fido2-authenticator.service.spec.ts
View File

@ -34,43 +34,6 @@ describe("FidoAuthenticatorService", () => {
}); });
describe("authenticatorMakeCredential", () => { describe("authenticatorMakeCredential", () => {
describe("when vault contains excluded credential", () => {
let excludedCipherView: CipherView;
let params: Fido2AuthenticatorMakeCredentialsParams;
beforeEach(async () => {
const excludedCipher = createCipher();
excludedCipherView = await excludedCipher.decrypt();
params = await createCredentialParams({
excludeList: [{ id: Fido2Utils.stringToBuffer(excludedCipher.id), type: "public-key" }],
});
cipherService.get.mockImplementation(async (id) =>
id === excludedCipher.id ? excludedCipher : undefined
);
cipherService.getAllDecrypted.mockResolvedValue([excludedCipherView]);
});
/** Spec: wait for user presence */
it("should request confirmation from user", async () => {
userInterface.confirmDuplicateCredential.mockResolvedValue(true);
await authenticator.makeCredential(params);
expect(userInterface.confirmDuplicateCredential).toHaveBeenCalled();
});
/** Spec: then terminate this procedure and return error code */
it("should throw error if user denies duplication", async () => {
userInterface.confirmDuplicateCredential.mockResolvedValue(false);
const result = async () => await authenticator.makeCredential(params);
await expect(result).rejects.toThrowError(
Fido2AutenticatorErrorCode[Fido2AutenticatorErrorCode.CTAP2_ERR_CREDENTIAL_EXCLUDED]
);
});
});
// Spec: If the pubKeyCredParams parameter does not contain a valid COSEAlgorithmIdentifier value that is supported by the authenticator, terminate this procedure and return error code // Spec: If the pubKeyCredParams parameter does not contain a valid COSEAlgorithmIdentifier value that is supported by the authenticator, terminate this procedure and return error code
it("should throw error when input does not contain any supported algorithms", async () => { it("should throw error when input does not contain any supported algorithms", async () => {
const params = await createCredentialParams({ const params = await createCredentialParams({
@ -127,6 +90,62 @@ describe("FidoAuthenticatorService", () => {
}); });
}); });
describe("when vault contains excluded credential", () => {
let excludedCipherView: CipherView;
let params: Fido2AuthenticatorMakeCredentialsParams;
beforeEach(async () => {
const excludedCipher = createCipher();
excludedCipherView = await excludedCipher.decrypt();
params = await createCredentialParams({
excludeList: [{ id: Fido2Utils.stringToBuffer(excludedCipher.id), type: "public-key" }],
});
cipherService.get.mockImplementation(async (id) =>
id === excludedCipher.id ? excludedCipher : undefined
);
cipherService.getAllDecrypted.mockResolvedValue([excludedCipherView]);
});
/** Spec: wait for user presence */
it("should request confirmation from user", async () => {
userInterface.confirmDuplicateCredential.mockResolvedValue(true);
await authenticator.makeCredential(params);
expect(userInterface.confirmDuplicateCredential).toHaveBeenCalled();
});
/** Spec: then terminate this procedure and return error code */
it("should throw error if user denies duplication", async () => {
userInterface.confirmDuplicateCredential.mockResolvedValue(false);
const result = async () => await authenticator.makeCredential(params);
await expect(result).rejects.toThrowError(
Fido2AutenticatorErrorCode[Fido2AutenticatorErrorCode.CTAP2_ERR_CREDENTIAL_EXCLUDED]
);
});
/** Departure from spec: Check duplication last instead of first */
it("should not request confirmation from user when input data does not pass checks", async () => {
userInterface.confirmDuplicateCredential.mockResolvedValue(true);
const paramsList: Fido2AuthenticatorMakeCredentialsParams[] = [
{ ...params, options: { rk: "invalid-value" as any } },
{ ...params, options: { uv: "invalid-value" as any } },
{ ...params, pinAuth: { key: "value" } },
{ ...params, pubKeyCredParams: [{ alg: 9001, type: "public-key" }] },
];
for (const p of paramsList) {
try {
await authenticator.makeCredential(p);
// eslint-disable-next-line no-empty
} catch {}
}
expect(userInterface.confirmDuplicateCredential).not.toHaveBeenCalled();
});
});
describe("when input passes all initial checks", () => { describe("when input passes all initial checks", () => {
/** Spec: show the items contained within the user and rp parameter structures to the user. */ /** Spec: show the items contained within the user and rp parameter structures to the user. */
it("should request confirmation from user", async () => { it("should request confirmation from user", async () => {
@ -153,6 +172,25 @@ describe("FidoAuthenticatorService", () => {
); );
}); });
}); });
it("should not request confirmation from user when input data does not pass checks", async () => {
userInterface.confirmDuplicateCredential.mockResolvedValue(true);
const params = await createCredentialParams();
const paramsList: Fido2AuthenticatorMakeCredentialsParams[] = [
{ ...params, options: { rk: "invalid-value" as any } },
{ ...params, options: { uv: "invalid-value" as any } },
{ ...params, pinAuth: { key: "value" } },
{ ...params, pubKeyCredParams: [{ alg: 9001, type: "public-key" }] },
];
for (const p of paramsList) {
try {
await authenticator.makeCredential(p);
// eslint-disable-next-line no-empty
} catch {}
}
expect(userInterface.confirmNewCredential).not.toHaveBeenCalled();
});
}); });
}); });

47
libs/common/src/webauthn/services/fido2-authenticator.service.ts
View File

@ -20,24 +20,6 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr
) {} ) {}
async makeCredential(params: Fido2AuthenticatorMakeCredentialsParams): Promise<void> { async makeCredential(params: Fido2AuthenticatorMakeCredentialsParams): Promise<void> {
const duplicateExists = await this.vaultContainsId(
params.excludeList.map((key) => Fido2Utils.bufferToString(key.id))
);
if (duplicateExists) {
const userConfirmation = await this.userInterface.confirmDuplicateCredential(
[Fido2Utils.bufferToString(params.excludeList[0].id)],
{
credentialName: params.rp.name,
userName: params.user.name,
}
);
if (!userConfirmation) {
throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.CTAP2_ERR_CREDENTIAL_EXCLUDED);
}
}
if (params.pubKeyCredParams.every((p) => p.alg !== Fido2AlgorithmIdentifier.ES256)) { if (params.pubKeyCredParams.every((p) => p.alg !== Fido2AlgorithmIdentifier.ES256)) {
throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.CTAP2_ERR_UNSUPPORTED_ALGORITHM); throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.CTAP2_ERR_UNSUPPORTED_ALGORITHM);
} }
@ -54,15 +36,34 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr
throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.CTAP2_ERR_PIN_AUTH_INVALID); throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.CTAP2_ERR_PIN_AUTH_INVALID);
} }
if (!duplicateExists) { // In the spec the `excludeList` is checked first.
const userVerification = await this.userInterface.confirmNewCredential({ // We deviate from this because we allow duplicates to be created if the user confirms it,
// and we don't want to ask the user for confirmation if the input params haven't already
// been verified.
const duplicateExists = await this.vaultContainsId(
params.excludeList.map((key) => Fido2Utils.bufferToString(key.id))
);
let userVerification = false;
if (duplicateExists) {
userVerification = await this.userInterface.confirmDuplicateCredential(
[Fido2Utils.bufferToString(params.excludeList[0].id)],
{
credentialName: params.rp.name,
userName: params.user.name,
}
);
} else {
userVerification = await this.userInterface.confirmNewCredential({
credentialName: params.rp.name, credentialName: params.rp.name,
userName: params.user.name, userName: params.user.name,
}); });
}
if (!userVerification) { if (!userVerification && duplicateExists) {
throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.CTAP2_ERR_OPERATION_DENIED); throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.CTAP2_ERR_CREDENTIAL_EXCLUDED);
} } else if (!userVerification && !duplicateExists) {
throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.CTAP2_ERR_OPERATION_DENIED);
} }
} }