bitwarden/browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser synced 2025-12-17 00:33:44 +00:00
Code Issues Releases Wiki Activity

[EC-598] feat: remove ability to duplicate excluded credentials

Browse Source
This commit is contained in:
Andreas Coroiu
2023-03-22 16:07:13 +01:00
parent 4926278fb9
commit 260bcc9f9e
3 changed files with 24 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
libs/common/src/webauthn/abstractions/fido2-user-interface.service.abstraction.ts
View File

@@ -10,9 +10,9 @@ export abstract class Fido2UserInterfaceService {
params: NewCredentialParams, params: NewCredentialParams,
abortController?: AbortController abortController?: AbortController
) => Promise<boolean>; ) => Promise<boolean>;
confirmDuplicateCredential: ( informExcludedCredential: (
existingCipherIds: string[], existingCipherIds: string[],
newCredential: NewCredentialParams, newCredential: NewCredentialParams,
abortController?: AbortController abortController?: AbortController
) => Promise<boolean>; ) => Promise<void>;
} }

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

@@ -79,7 +79,7 @@ describe("FidoAuthenticatorService", () => {
}); });
it("should not request confirmation from user", async () => { it("should not request confirmation from user", async () => {
userInterface.confirmDuplicateCredential.mockResolvedValue(true); userInterface.confirmNewCredential.mockResolvedValue(true);
const invalidParams = await createInvalidParams(); const invalidParams = await createInvalidParams();
for (const p of Object.values(invalidParams)) { for (const p of Object.values(invalidParams)) {
@@ -115,17 +115,20 @@ describe("FidoAuthenticatorService", () => {
}); });
/** Spec: wait for user presence */ /** Spec: wait for user presence */
it("should request confirmation from user", async () => { it("should inform user", async () => {
userInterface.confirmDuplicateCredential.mockResolvedValue(true); userInterface.informExcludedCredential.mockResolvedValue();
try {
await authenticator.makeCredential(params); await authenticator.makeCredential(params);
// eslint-disable-next-line no-empty
} catch {}
expect(userInterface.confirmDuplicateCredential).toHaveBeenCalled(); expect(userInterface.informExcludedCredential).toHaveBeenCalled();
}); });
/** Spec: then terminate this procedure and return error code */ /** Spec: then terminate this procedure and return error code */
it("should throw error if user denies duplication", async () => { it("should throw error", async () => {
userInterface.confirmDuplicateCredential.mockResolvedValue(false); userInterface.informExcludedCredential.mockResolvedValue();
const result = async () => await authenticator.makeCredential(params); const result = async () => await authenticator.makeCredential(params);
@@ -135,8 +138,8 @@ describe("FidoAuthenticatorService", () => {
}); });
/** Departure from spec: Check duplication last instead of first */ /** Departure from spec: Check duplication last instead of first */
it("should not request confirmation from user when input data does not pass checks", async () => { it("should not inform user of duplication when input data does not pass checks", async () => {
userInterface.confirmDuplicateCredential.mockResolvedValue(true); userInterface.informExcludedCredential.mockResolvedValue();
const invalidParams = await createInvalidParams(); const invalidParams = await createInvalidParams();
for (const p of Object.values(invalidParams)) { for (const p of Object.values(invalidParams)) {
@@ -145,7 +148,7 @@ describe("FidoAuthenticatorService", () => {
// eslint-disable-next-line no-empty // eslint-disable-next-line no-empty
} catch {} } catch {}
} }
expect(userInterface.confirmDuplicateCredential).not.toHaveBeenCalled(); expect(userInterface.informExcludedCredential).not.toHaveBeenCalled();
}); });
}); });

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

@@ -41,33 +41,28 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr
throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.CTAP2_ERR_PIN_AUTH_INVALID); throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.CTAP2_ERR_PIN_AUTH_INVALID);
} }
// In the spec the `excludeList` is checked first.
// 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 isExcluded = await this.vaultContainsId( const isExcluded = await this.vaultContainsId(
params.excludeList.map((key) => Fido2Utils.bufferToString(key.id)) params.excludeList.map((key) => Fido2Utils.bufferToString(key.id))
); );
let userVerification = false;
if (isExcluded) { if (isExcluded) {
userVerification = await this.userInterface.confirmDuplicateCredential( await this.userInterface.informExcludedCredential(
[Fido2Utils.bufferToString(params.excludeList[0].id)], [Fido2Utils.bufferToString(params.excludeList[0].id)],
{ {
credentialName: params.rp.name, credentialName: params.rp.name,
userName: params.user.name, userName: params.user.name,
} }
); );
} else {
userVerification = await this.userInterface.confirmNewCredential({ throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.CTAP2_ERR_CREDENTIAL_EXCLUDED);
}
const userVerification = await this.userInterface.confirmNewCredential({
credentialName: params.rp.name, credentialName: params.rp.name,
userName: params.user.name, userName: params.user.name,
}); });
}
if (!userVerification && isExcluded) { if (!userVerification) {
throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.CTAP2_ERR_CREDENTIAL_EXCLUDED);
} else if (!userVerification && !isExcluded) {
throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.CTAP2_ERR_OPERATION_DENIED); throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.CTAP2_ERR_OPERATION_DENIED);
} }