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

[PM-4280] MP reprompt not respected on passkey creation and retrieval #6550

Merged
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
25 changes: 16 additions & 9 deletions apps/browser/src/vault/popup/components/fido2/fido2.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,10 +220,7 @@ export class Fido2Component implements OnInit, OnDestroy {
async submit() {
const data = this.message$.value;
if (data?.type === "PickCredentialRequest") {
let userVerified = false;
if (data.userVerification) {
userVerified = await this.passwordRepromptService.showPasswordPrompt();
}
const userVerified = await this.handleUserVerification(data.userVerification, this.cipher);

this.send({
sessionId: this.sessionId,
Expand All @@ -232,8 +229,6 @@ export class Fido2Component implements OnInit, OnDestroy {
userVerified,
});
} else if (data?.type === "ConfirmNewCredentialRequest") {
let userVerified = false;

if (this.cipher.login.hasFido2Credentials) {
const confirmed = await this.dialogService.openSimpleDialog({
title: { key: "overwritePasskey" },
Expand All @@ -246,9 +241,7 @@ export class Fido2Component implements OnInit, OnDestroy {
}
}

if (data.userVerification) {
userVerified = await this.passwordRepromptService.showPasswordPrompt();
}
const userVerified = await this.handleUserVerification(data.userVerification, this.cipher);

this.send({
sessionId: this.sessionId,
Expand Down Expand Up @@ -411,6 +404,20 @@ export class Fido2Component implements OnInit, OnDestroy {
}
}

private async handleUserVerification(
userVerification: boolean,
cipher: CipherView
): Promise<boolean> {
const masterPasswordRepromptRequiered = cipher && cipher.reprompt !== 0;
const verificationRequired = userVerification || masterPasswordRepromptRequiered;

if (!verificationRequired) {
return false;
}

return await this.passwordRepromptService.showPasswordPrompt();
}

private send(msg: BrowserFido2Message) {
BrowserFido2UserInterfaceSession.sendMessage({
sessionId: this.sessionId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
NewCredentialParams,
} from "../../abstractions/fido2/fido2-user-interface.service.abstraction";
import { SyncService } from "../../abstractions/sync/sync.service.abstraction";
import { CipherRepromptType } from "../../enums/cipher-reprompt-type";
import { CipherType } from "../../enums/cipher-type";
import { Cipher } from "../../models/domain/cipher";
import { CipherView } from "../../models/view/cipher.view";
Expand Down Expand Up @@ -269,6 +270,20 @@ describe("FidoAuthenticatorService", () => {
await expect(result).rejects.toThrowError(Fido2AutenticatorErrorCode.NotAllowed);
});

it("should throw error if user verification fails and cipher requires reprompt", async () => {
params.requireUserVerification = false;
userInterfaceSession.confirmNewCredential.mockResolvedValue({
cipherId: existingCipher.id,
userVerified: false,
});
const encryptedCipher = { ...existingCipher, reprompt: CipherRepromptType.Password };
cipherService.get.mockResolvedValue(encryptedCipher as unknown as Cipher);

const result = async () => await authenticator.makeCredential(params, tab);

await expect(result).rejects.toThrowError(Fido2AutenticatorErrorCode.Unknown);
});

/** Spec: If any error occurred while creating the new credential object, return an error code equivalent to "UnknownError" and terminate the operation. */
it("should throw unkown error if creation fails", async () => {
const encryptedCipher = Symbol();
Expand Down Expand Up @@ -587,6 +602,18 @@ describe("FidoAuthenticatorService", () => {

await expect(result).rejects.toThrowError(Fido2AutenticatorErrorCode.NotAllowed);
});

it("should throw error if user verification fails and cipher requires reprompt", async () => {
ciphers[0].reprompt = CipherRepromptType.Password;
userInterfaceSession.pickCredential.mockResolvedValue({
cipherId: ciphers[0].id,
userVerified: false,
});

const result = async () => await authenticator.getAssertion(params, tab);

await expect(result).rejects.toThrowError(Fido2AutenticatorErrorCode.NotAllowed);
});
});

describe("assertion of credential", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
} from "../../abstractions/fido2/fido2-authenticator.service.abstraction";
import { Fido2UserInterfaceService } from "../../abstractions/fido2/fido2-user-interface.service.abstraction";
import { SyncService } from "../../abstractions/sync/sync.service.abstraction";
import { CipherRepromptType } from "../../enums/cipher-reprompt-type";
import { CipherType } from "../../enums/cipher-type";
import { CipherView } from "../../models/view/cipher.view";
import { Fido2CredentialView } from "../../models/view/fido2-credential.view";
Expand Down Expand Up @@ -122,20 +123,24 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr
throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.NotAllowed);
}

if (params.requireUserVerification && !userVerified) {
this.logService?.warning(
`[Fido2Authenticator] Aborting because user verification was unsuccessful.`
);
throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.NotAllowed);
}

try {
keyPair = await createKeyPair();

const encrypted = await this.cipherService.get(cipherId);
cipher = await encrypted.decrypt(
await this.cipherService.getKeyForCipherKeyDecryption(encrypted)
);

if (
!userVerified &&
(params.requireUserVerification || cipher.reprompt !== CipherRepromptType.None)
) {
this.logService?.warning(
`[Fido2Authenticator] Aborting because user verification was unsuccessful.`
);
throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.NotAllowed);
}

fido2Credential = await createKeyView(params, keyPair.privateKey);
cipher.login.fido2Credentials = [fido2Credential];
const reencrypted = await this.cipherService.encrypt(cipher);
Expand Down Expand Up @@ -235,7 +240,10 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr
throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.NotAllowed);
}

if (params.requireUserVerification && !userVerified) {
if (
!userVerified &&
(params.requireUserVerification || selectedCipher.reprompt !== CipherRepromptType.None)
) {
this.logService?.warning(
`[Fido2Authenticator] Aborting because user verification was unsuccessful.`
);
Expand Down
Loading