1
0
mirror of https://github.com/bitwarden/browser synced 2026-02-05 11:13:44 +00:00

Refactor Fido2 Components (#15105)

* Refactor Fido2 Components

* Address error message and missing session

* Address remaining missing session

* Reset modals so subsequent creates work (#15145)

* Fix broken test

* Rename relevantCiphers to displayedCiphers

* Clean up heading settings, errors, and other concerns

* Address missing comments and throw error in try block

* fix type issue for SimpleDialogType

* fix type issue for SimpleDialogType

* Revert new type

* try using as null to satisfy type issue

* Remove use of firstValueFrom in create component
This commit is contained in:
Jeffrey Holland
2025-06-30 14:41:03 +02:00
committed by GitHub
parent 7d72b98863
commit cd4f5fbdb9
9 changed files with 214 additions and 175 deletions

View File

@@ -67,7 +67,6 @@
],
"CFBundleDevelopmentRegion": "en"
},
"provisioningProfile": "bitwarden_desktop_developer_id.provisionprofile",
"singleArchFiles": "node_modules/@bitwarden/desktop-napi/desktop_napi.darwin-*.node",
"extraFiles": [
{
@@ -142,8 +141,7 @@
"extendInfo": {
"LSMinimumSystemVersion": "12",
"ElectronTeamID": "LTZ2PFU5D6"
},
"provisioningProfile": "bitwarden_desktop_appstore.provisionprofile"
}
},
"nsisWeb": {
"oneClick": false,

View File

@@ -38,7 +38,7 @@
</div>
<ng-template #hasCiphers>
<bit-item *ngFor="let c of ciphers$ | async" class="">
<button type="button" bit-item-content (click)="addPasskeyToCipher(c)">
<button type="button" bit-item-content (click)="addCredentialToCipher(c)">
<app-vault-icon [cipher]="c" slot="start"></app-vault-icon>
<button bitLink [title]="c.name" type="button">
{{ c.name }}

View File

@@ -123,29 +123,27 @@ describe("Fido2CreateComponent", () => {
await component.ngOnInit();
expect(mockAccountService.setShowHeader).toHaveBeenCalledWith(false);
expect(mockFido2UserInterfaceService.getCurrentSession).toHaveBeenCalled();
expect(component.session).toBe(mockSession);
});
it("should throw error when no active session found", async () => {
it("should show error dialog when no active session found", async () => {
mockFido2UserInterfaceService.getCurrentSession.mockReturnValue(null);
mockDialogService.openSimpleDialog.mockResolvedValue(false);
await expect(component.ngOnInit()).rejects.toThrow(
"Cannot read properties of null (reading 'getRpId')",
);
await component.ngOnInit();
expect(mockDialogService.openSimpleDialog).toHaveBeenCalledWith({
title: { key: "unableToSavePasskey" },
content: { key: "closeThisBitwardenWindow" },
type: "danger",
acceptButtonText: { key: "closeBitwarden" },
cancelButtonText: null,
});
});
});
describe("ngOnDestroy", () => {
it("should restore header visibility", async () => {
await component.ngOnDestroy();
expect(mockAccountService.setShowHeader).toHaveBeenCalledWith(true);
});
});
describe("addPasskeyToCipher", () => {
describe("addCredentialToCipher", () => {
beforeEach(() => {
component.session = mockSession;
});
@@ -153,7 +151,7 @@ describe("Fido2CreateComponent", () => {
it("should add passkey to cipher", async () => {
const cipher = createMockCiphers()[0];
await component.addPasskeyToCipher(cipher);
await component.addCredentialToCipher(cipher);
expect(mockSession.notifyConfirmCreateCredential).toHaveBeenCalledWith(true, cipher);
});
@@ -163,7 +161,7 @@ describe("Fido2CreateComponent", () => {
cipher.reprompt = CipherRepromptType.Password;
mockPasswordRepromptService.showPasswordPrompt.mockResolvedValue(false);
await component.addPasskeyToCipher(cipher);
await component.addCredentialToCipher(cipher);
expect(mockSession.notifyConfirmCreateCredential).toHaveBeenCalledWith(false, cipher);
});
@@ -175,7 +173,7 @@ describe("Fido2CreateComponent", () => {
});
mockDialogService.openSimpleDialog.mockResolvedValue(true);
await component.addPasskeyToCipher(cipher);
await component.addCredentialToCipher(cipher);
expect(mockDialogService.openSimpleDialog).toHaveBeenCalledWith({
title: { key: "overwritePasskey" },
@@ -192,7 +190,7 @@ describe("Fido2CreateComponent", () => {
});
mockDialogService.openSimpleDialog.mockResolvedValue(false);
await component.addPasskeyToCipher(cipher);
await component.addCredentialToCipher(cipher);
expect(mockSession.notifyConfirmCreateCredential).toHaveBeenCalledWith(false, cipher);
});
@@ -207,17 +205,16 @@ describe("Fido2CreateComponent", () => {
await component.confirmPasskey();
expect(mockSession.notifyConfirmCreateCredential).toHaveBeenCalledWith(true);
expect(mockRouter.navigate).toHaveBeenCalledWith(["/"]);
expect(mockDesktopSettingsService.setModalMode).toHaveBeenCalledWith(false);
});
it("should call openSimpleDialog when session is null", async () => {
component.session = null;
mockDialogService.openSimpleDialog.mockResolvedValue(false);
await component.confirmPasskey();
expect(mockDialogService.openSimpleDialog).toHaveBeenCalledWith({
title: { key: "unexpectedErrorShort" },
title: { key: "unableToSavePasskey" },
content: { key: "closeThisBitwardenWindow" },
type: "danger",
acceptButtonText: { key: "closeBitwarden" },
@@ -232,8 +229,6 @@ describe("Fido2CreateComponent", () => {
await component.closeModal();
expect(mockRouter.navigate).toHaveBeenCalledWith(["/"]);
expect(mockDesktopSettingsService.setModalMode).toHaveBeenCalledWith(false);
expect(mockSession.notifyConfirmCreateCredential).toHaveBeenCalledWith(false);
expect(mockSession.confirmChosenCipher).toHaveBeenCalledWith(null);
});

View File

@@ -1,13 +1,12 @@
import { CommonModule } from "@angular/common";
import { Component, OnInit, OnDestroy } from "@angular/core";
import { RouterModule, Router } from "@angular/router";
import { BehaviorSubject, firstValueFrom, map, Observable } from "rxjs";
import { combineLatest, map, Observable, switchMap } from "rxjs";
import { JslibModule } from "@bitwarden/angular/jslib.module";
import { BitwardenShield } from "@bitwarden/auth/angular";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { DomainSettingsService } from "@bitwarden/common/autofill/services/domain-settings.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { Fido2Utils } from "@bitwarden/common/platform/services/fido2/fido2-utils";
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
@@ -22,6 +21,7 @@ import {
TableModule,
SectionHeaderComponent,
BitIconButtonComponent,
SimpleDialogOptions,
} from "@bitwarden/components";
import { PasswordRepromptService } from "@bitwarden/vault";
@@ -34,6 +34,28 @@ import {
import { Fido2PasskeyExistsIcon } from "./fido2-passkey-exists-icon";
const DIALOG_MESSAGES = {
unexpectedErrorShort: {
title: { key: "unexpectedErrorShort" },
content: { key: "closeThisBitwardenWindow" },
type: "danger",
acceptButtonText: { key: "closeBitwarden" },
cancelButtonText: null as null,
},
unableToSavePasskey: {
title: { key: "unableToSavePasskey" },
content: { key: "closeThisBitwardenWindow" },
type: "danger",
acceptButtonText: { key: "closeBitwarden" },
cancelButtonText: null as null,
},
overwritePasskey: {
title: { key: "overwritePasskey" },
content: { key: "alreadyContainsPasskey" },
type: "warning",
},
} as const satisfies Record<string, SimpleDialogOptions>;
@Component({
standalone: true,
imports: [
@@ -54,8 +76,7 @@ import { Fido2PasskeyExistsIcon } from "./fido2-passkey-exists-icon";
})
export class Fido2CreateComponent implements OnInit, OnDestroy {
session?: DesktopFido2UserInterfaceSession = null;
private ciphersSubject = new BehaviorSubject<CipherView[]>([]);
ciphers$: Observable<CipherView[]> = this.ciphersSubject.asObservable();
ciphers$: Observable<CipherView[]>;
readonly Icons = { BitwardenShield };
protected fido2PasskeyExistsIcon = Fido2PasskeyExistsIcon;
@@ -67,106 +88,122 @@ export class Fido2CreateComponent implements OnInit, OnDestroy {
private readonly desktopAutofillService: DesktopAutofillService,
private readonly dialogService: DialogService,
private readonly domainSettingsService: DomainSettingsService,
private readonly logService: LogService,
private readonly passwordRepromptService: PasswordRepromptService,
private readonly router: Router,
) {}
async ngOnInit() {
await this.accountService.setShowHeader(false);
async ngOnInit(): Promise<void> {
this.session = this.fido2UserInterfaceService.getCurrentSession();
const rpid = await this.session?.getRpId();
if (!this.session) {
await this.showErrorDialog(DIALOG_MESSAGES.unableToSavePasskey);
return;
}
this.initializeCiphersObservable(rpid);
}
async ngOnDestroy(): Promise<void> {
await this.closeModal();
}
async addCredentialToCipher(cipher: CipherView): Promise<void> {
const isConfirmed = await this.validateCipherAccess(cipher);
try {
if (!this.session) {
throw new Error("Missing session");
}
this.session.notifyConfirmCreateCredential(isConfirmed, cipher);
} catch {
await this.showErrorDialog(DIALOG_MESSAGES.unableToSavePasskey);
return;
}
await this.closeModal();
}
async confirmPasskey(): Promise<void> {
try {
if (!this.session) {
throw new Error("Missing session");
}
this.session.notifyConfirmCreateCredential(true);
} catch {
await this.showErrorDialog(DIALOG_MESSAGES.unableToSavePasskey);
}
await this.closeModal();
}
async closeModal(): Promise<void> {
await this.router.navigate(["/"]);
if (this.session) {
this.session.notifyConfirmCreateCredential(false);
this.session.confirmChosenCipher(null);
}
}
private initializeCiphersObservable(rpid: string): void {
const lastRegistrationRequest = this.desktopAutofillService.lastRegistrationRequest;
const rpid = await this.session.getRpId();
const equivalentDomains = await firstValueFrom(
this.domainSettingsService.getUrlEquivalentDomains(rpid),
if (!lastRegistrationRequest || !rpid) {
return;
}
const userHandle = Fido2Utils.bufferToString(
new Uint8Array(lastRegistrationRequest.userHandle),
);
const activeUserId = await firstValueFrom(
this.ciphers$ = combineLatest([
this.accountService.activeAccount$.pipe(map((a) => a?.id)),
this.domainSettingsService.getUrlEquivalentDomains(rpid),
]).pipe(
switchMap(async ([activeUserId, equivalentDomains]) => {
if (!activeUserId) {
return [];
}
try {
const allCiphers = await this.cipherService.getAllDecrypted(activeUserId);
return allCiphers.filter(
(cipher) =>
cipher.login?.matchesUri(rpid, equivalentDomains) &&
Fido2Utils.cipherHasNoOtherPasskeys(cipher, userHandle) &&
!cipher.deletedDate,
);
} catch {
await this.showErrorDialog(DIALOG_MESSAGES.unexpectedErrorShort);
return [];
}
}),
);
this.cipherService
.getAllDecrypted(activeUserId)
.then((ciphers) => {
const relevantCiphers = ciphers.filter((cipher) => {
const userHandle = Fido2Utils.bufferToString(
new Uint8Array(lastRegistrationRequest.userHandle),
);
return (
cipher.login.matchesUri(rpid, equivalentDomains) &&
Fido2Utils.cipherHasNoOtherPasskeys(cipher, userHandle) &&
!cipher.deletedDate
);
});
this.ciphersSubject.next(relevantCiphers);
})
.catch((error) => this.logService.error(error));
}
async ngOnDestroy() {
await this.accountService.setShowHeader(true);
}
async addPasskeyToCipher(cipher: CipherView) {
let isConfirmed = true;
private async validateCipherAccess(cipher: CipherView): Promise<boolean> {
if (cipher.login.hasFido2Credentials) {
isConfirmed = await this.dialogService.openSimpleDialog({
title: { key: "overwritePasskey" },
content: { key: "alreadyContainsPasskey" },
type: "warning",
});
const overwriteConfirmed = await this.dialogService.openSimpleDialog(
DIALOG_MESSAGES.overwritePasskey,
);
if (!overwriteConfirmed) {
return false;
}
}
if (cipher.reprompt) {
isConfirmed = await this.passwordRepromptService.showPasswordPrompt();
return this.passwordRepromptService.showPasswordPrompt();
}
this.session.notifyConfirmCreateCredential(isConfirmed, cipher);
return true;
}
async confirmPasskey() {
try {
// Retrieve the current UI session to control the flow
if (!this.session) {
const confirmed = await this.dialogService.openSimpleDialog({
title: { key: "unexpectedErrorShort" },
content: { key: "closeThisBitwardenWindow" },
type: "danger",
acceptButtonText: { key: "closeBitwarden" },
cancelButtonText: null,
});
if (confirmed) {
await this.closeModal();
}
} else {
this.session.notifyConfirmCreateCredential(true);
}
// Not sure this clean up should happen here or in session.
// The session currently toggles modal on and send us here
// But if this route is somehow opened outside of session we want to make sure we clean up?
await this.router.navigate(["/"]);
await this.desktopSettingsService.setModalMode(false);
} catch {
const confirmed = await this.dialogService.openSimpleDialog({
title: { key: "unableToSavePasskey" },
content: { key: "closeThisBitwardenWindow" },
type: "danger",
acceptButtonText: { key: "closeBitwarden" },
cancelButtonText: null,
});
if (confirmed) {
await this.closeModal();
}
}
}
async closeModal() {
await this.router.navigate(["/"]);
await this.desktopSettingsService.setModalMode(false);
this.session.notifyConfirmCreateCredential(false);
this.session.confirmChosenCipher(null);
private async showErrorDialog(config: SimpleDialogOptions): Promise<void> {
await this.dialogService.openSimpleDialog(config);
await this.closeModal();
}
}

View File

@@ -55,31 +55,20 @@ describe("Fido2ExcludedCiphersComponent", () => {
});
describe("ngOnInit", () => {
it("should initialize session and set show header to false", async () => {
it("should initialize session", async () => {
await component.ngOnInit();
expect(mockAccountService.setShowHeader).toHaveBeenCalledWith(false);
expect(mockFido2UserInterfaceService.getCurrentSession).toHaveBeenCalled();
expect(component.session).toBe(mockSession);
});
});
describe("ngOnDestroy", () => {
it("should restore header visibility", async () => {
await component.ngOnDestroy();
expect(mockAccountService.setShowHeader).toHaveBeenCalledWith(true);
});
});
describe("closeModal", () => {
it("should close modal and notify session when session exists", async () => {
component.session = mockSession;
await component.closeModal();
expect(mockRouter.navigate).toHaveBeenCalledWith(["/"]);
expect(mockDesktopSettingsService.setModalMode).toHaveBeenCalledWith(false);
expect(mockSession.notifyConfirmCreateCredential).toHaveBeenCalledWith(false);
expect(mockSession.confirmChosenCipher).toHaveBeenCalledWith(null);
});

View File

@@ -55,19 +55,20 @@ export class Fido2ExcludedCiphersComponent implements OnInit, OnDestroy {
private readonly router: Router,
) {}
async ngOnInit() {
await this.accountService.setShowHeader(false);
async ngOnInit(): Promise<void> {
this.session = this.fido2UserInterfaceService.getCurrentSession();
}
async ngOnDestroy() {
await this.accountService.setShowHeader(true);
async ngOnDestroy(): Promise<void> {
await this.closeModal();
}
async closeModal() {
async closeModal(): Promise<void> {
await this.router.navigate(["/"]);
await this.desktopSettingsService.setModalMode(false);
this.session.notifyConfirmCreateCredential(false);
this.session.confirmChosenCipher(null);
if (this.session) {
this.session.notifyConfirmCreateCredential(false);
this.session.confirmChosenCipher(null);
}
}
}

View File

@@ -110,7 +110,6 @@ describe("Fido2VaultComponent", () => {
await component.ngOnInit();
expect(mockAccountService.setShowHeader).toHaveBeenCalledWith(false);
expect(mockFido2UserInterfaceService.getCurrentSession).toHaveBeenCalled();
expect(component.session).toBe(mockSession);
expect(component.cipherIds$).toBe(mockSession.availableCipherIds$);
@@ -139,15 +138,6 @@ describe("Fido2VaultComponent", () => {
});
});
describe("ngOnDestroy", () => {
it("should restore header visibility and clean up", async () => {
await component.ngOnInit();
await component.ngOnDestroy();
expect(mockAccountService.setShowHeader).toHaveBeenCalledWith(true);
});
});
describe("chooseCipher", () => {
const cipher = mockCiphers[0];
@@ -162,7 +152,6 @@ describe("Fido2VaultComponent", () => {
expect(mockSession.confirmChosenCipher).toHaveBeenCalledWith(cipher.id, true);
expect(mockRouter.navigate).toHaveBeenCalledWith(["/"]);
expect(mockDesktopSettingsService.setModalMode).toHaveBeenCalledWith(false);
});
it("should prompt for password when cipher requires reprompt", async () => {
@@ -193,7 +182,6 @@ describe("Fido2VaultComponent", () => {
await component.closeModal();
expect(mockRouter.navigate).toHaveBeenCalledWith(["/"]);
expect(mockDesktopSettingsService.setModalMode).toHaveBeenCalledWith(false);
expect(mockSession.notifyConfirmCreateCredential).toHaveBeenCalledWith(false);
expect(mockSession.confirmChosenCipher).toHaveBeenCalledWith(null);
});

View File

@@ -14,6 +14,7 @@ import {
BadgeModule,
ButtonModule,
DialogModule,
DialogService,
IconModule,
ItemModule,
SectionComponent,
@@ -52,8 +53,7 @@ export class Fido2VaultComponent implements OnInit, OnDestroy {
private destroy$ = new Subject<void>();
private ciphersSubject = new BehaviorSubject<CipherView[]>([]);
ciphers$: Observable<CipherView[]> = this.ciphersSubject.asObservable();
private cipherIdsSubject = new BehaviorSubject<string[]>([]);
cipherIds$: Observable<string[]>;
cipherIds$: Observable<string[]> | undefined;
readonly Icons = { BitwardenShield };
constructor(
@@ -61,19 +61,60 @@ export class Fido2VaultComponent implements OnInit, OnDestroy {
private readonly fido2UserInterfaceService: DesktopFido2UserInterfaceService,
private readonly cipherService: CipherService,
private readonly accountService: AccountService,
private readonly dialogService: DialogService,
private readonly logService: LogService,
private readonly passwordRepromptService: PasswordRepromptService,
private readonly router: Router,
) {}
async ngOnInit() {
await this.accountService.setShowHeader(false);
async ngOnInit(): Promise<void> {
this.session = this.fido2UserInterfaceService.getCurrentSession();
this.cipherIds$ = this.session.availableCipherIds$;
await this.loadCiphers();
}
async ngOnDestroy(): Promise<void> {
this.destroy$.next();
this.destroy$.complete();
}
async chooseCipher(cipher: CipherView): Promise<void> {
if (!this.session) {
await this.dialogService.openSimpleDialog({
title: { key: "unexpectedErrorShort" },
content: { key: "closeThisBitwardenWindow" },
type: "danger",
acceptButtonText: { key: "closeBitwarden" },
cancelButtonText: null,
});
await this.closeModal();
return;
}
const isConfirmed = await this.validateCipherAccess(cipher);
this.session.confirmChosenCipher(cipher.id, isConfirmed);
await this.closeModal();
}
async closeModal(): Promise<void> {
if (this.session) {
this.session.notifyConfirmCreateCredential(false);
this.session.confirmChosenCipher(null);
}
await this.router.navigate(["/"]);
}
private async loadCiphers(): Promise<void> {
const activeUserId = await firstValueFrom(
this.accountService.activeAccount$.pipe(map((a) => a?.id)),
);
this.session = this.fido2UserInterfaceService.getCurrentSession();
this.cipherIds$ = this.session?.availableCipherIds$;
if (!activeUserId) {
return;
}
this.cipherIds$.pipe(takeUntil(this.destroy$)).subscribe((cipherIds) => {
this.cipherService
@@ -81,34 +122,19 @@ export class Fido2VaultComponent implements OnInit, OnDestroy {
.then((ciphers) => {
this.ciphersSubject.next(ciphers.filter((cipher) => !cipher.deletedDate));
})
.catch((error) => this.logService.error(error));
.catch((error) => {
this.logService.error("Failed to load ciphers", error);
});
});
await this.closeModal();
}
async ngOnDestroy() {
await this.accountService.setShowHeader(true);
this.cipherIdsSubject.complete(); // Clean up the BehaviorSubject
}
async chooseCipher(cipher: CipherView) {
if (
cipher.reprompt !== CipherRepromptType.None &&
!(await this.passwordRepromptService.showPasswordPrompt())
) {
this.session?.confirmChosenCipher(cipher.id, false);
} else {
this.session?.confirmChosenCipher(cipher.id, true);
private async validateCipherAccess(cipher: CipherView): Promise<boolean> {
if (cipher.reprompt !== CipherRepromptType.None) {
return this.passwordRepromptService.showPasswordPrompt();
}
await this.router.navigate(["/"]);
await this.desktopSettingsService.setModalMode(false);
}
async closeModal() {
await this.router.navigate(["/"]);
await this.desktopSettingsService.setModalMode(false);
this.session.notifyConfirmCreateCredential(false);
this.session.confirmChosenCipher(null);
return true;
}
}

View File

@@ -125,6 +125,7 @@ export class DesktopFido2UserInterfaceSession implements Fido2UserInterfaceSessi
try {
// Check if we can return the credential without user interaction
await this.accountService.setShowHeader(false);
if (assumeUserPresence && cipherIds.length === 1 && !masterPasswordRepromptRequired) {
this.logService.debug(
"shortcut - Assuming user presence and returning cipherId",
@@ -151,6 +152,7 @@ export class DesktopFido2UserInterfaceSession implements Fido2UserInterfaceSessi
} finally {
// Make sure to clean up so the app is never stuck in modal mode?
await this.desktopSettingsService.setModalMode(false);
await this.accountService.setShowHeader(true);
}
}
@@ -249,6 +251,7 @@ export class DesktopFido2UserInterfaceSession implements Fido2UserInterfaceSessi
} finally {
// Make sure to clean up so the app is never stuck in modal mode?
await this.desktopSettingsService.setModalMode(false);
await this.accountService.setShowHeader(true);
}
}
@@ -266,6 +269,7 @@ export class DesktopFido2UserInterfaceSession implements Fido2UserInterfaceSessi
// Load the UI:
await this.desktopSettingsService.setModalMode(true, showTrafficButtons, position);
await this.centerOffscreenPopup();
await this.accountService.setShowHeader(false);
await this.router.navigate([
route,
{
@@ -335,6 +339,7 @@ export class DesktopFido2UserInterfaceSession implements Fido2UserInterfaceSessi
this.availableCipherIdsSubject.next(existingCipherIds);
await this.showUi("/fido2-excluded", this.windowObject.windowXy, false);
await this.accountService.setShowHeader(true);
}
async ensureUnlockedVault(): Promise<void> {