From a89e148804e20467c350e1f5b4ab91da9c58e65d Mon Sep 17 00:00:00 2001 From: Todd Martin <106564991+trmartin4@users.noreply.github.com> Date: Thu, 9 May 2024 13:24:11 -0400 Subject: [PATCH 1/3] [PM-7029] Remove key-rotation-feature-flag (#8816) * Removed key rotation feature flag. * Fixed tests * Removed unused dependency. * Remove KeyRotationImprovements from default const --- ...rganization-user-reset-password.service.ts | 19 ----------------- .../services/emergency-access.service.ts | 12 ----------- .../user-key-rotation.service.spec.ts | 11 ---------- .../key-rotation/user-key-rotation.service.ts | 21 +------------------ libs/common/src/enums/feature-flag.enum.ts | 2 -- 5 files changed, 1 insertion(+), 64 deletions(-) diff --git a/apps/web/src/app/admin-console/organizations/members/services/organization-user-reset-password/organization-user-reset-password.service.ts b/apps/web/src/app/admin-console/organizations/members/services/organization-user-reset-password/organization-user-reset-password.service.ts index fcdbe1e496..c029d2ecdb 100644 --- a/apps/web/src/app/admin-console/organizations/members/services/organization-user-reset-password/organization-user-reset-password.service.ts +++ b/apps/web/src/app/admin-console/organizations/members/services/organization-user-reset-password/organization-user-reset-password.service.ts @@ -165,23 +165,4 @@ export class OrganizationUserResetPasswordService { } return requests; } - - /** - * @deprecated Nov 6, 2023: Use new Key Rotation Service for posting rotated data. - */ - async postLegacyRotation( - userId: string, - requests: OrganizationUserResetPasswordWithIdRequest[], - ): Promise { - if (requests == null) { - return; - } - for (const request of requests) { - await this.organizationUserService.putOrganizationUserResetPasswordEnrollment( - request.organizationId, - userId, - request, - ); - } - } } diff --git a/apps/web/src/app/auth/emergency-access/services/emergency-access.service.ts b/apps/web/src/app/auth/emergency-access/services/emergency-access.service.ts index 819b80c1ad..a50a5adc6c 100644 --- a/apps/web/src/app/auth/emergency-access/services/emergency-access.service.ts +++ b/apps/web/src/app/auth/emergency-access/services/emergency-access.service.ts @@ -328,16 +328,4 @@ export class EmergencyAccessService { private async encryptKey(userKey: UserKey, publicKey: Uint8Array): Promise { return (await this.cryptoService.rsaEncrypt(userKey.key, publicKey)).encryptedString; } - - /** - * @deprecated Nov 6, 2023: Use new Key Rotation Service for posting rotated data. - */ - async postLegacyRotation(requests: EmergencyAccessWithIdRequest[]): Promise { - if (requests == null) { - return; - } - for (const request of requests) { - await this.emergencyAccessApiService.putEmergencyAccess(request.id, request); - } - } } diff --git a/apps/web/src/app/auth/key-rotation/user-key-rotation.service.spec.ts b/apps/web/src/app/auth/key-rotation/user-key-rotation.service.spec.ts index ec68556931..792ae15690 100644 --- a/apps/web/src/app/auth/key-rotation/user-key-rotation.service.spec.ts +++ b/apps/web/src/app/auth/key-rotation/user-key-rotation.service.spec.ts @@ -82,7 +82,6 @@ describe("KeyRotationService", () => { mockEncryptService, mockStateService, mockAccountService, - mockConfigService, mockKdfConfigService, ); }); @@ -191,16 +190,6 @@ describe("KeyRotationService", () => { ); }); - it("uses legacy rotation if feature flag is off", async () => { - mockConfigService.getFeatureFlag.mockResolvedValueOnce(false); - - await keyRotationService.rotateUserKeyAndEncryptedData("mockMasterPassword"); - - expect(mockApiService.postUserKeyUpdate).toHaveBeenCalled(); - expect(mockEmergencyAccessService.postLegacyRotation).toHaveBeenCalled(); - expect(mockResetPasswordService.postLegacyRotation).toHaveBeenCalled(); - }); - it("throws if server rotation fails", async () => { mockApiService.postUserKeyUpdate.mockRejectedValueOnce(new Error("mockError")); diff --git a/apps/web/src/app/auth/key-rotation/user-key-rotation.service.ts b/apps/web/src/app/auth/key-rotation/user-key-rotation.service.ts index dc5f933724..2763de71b3 100644 --- a/apps/web/src/app/auth/key-rotation/user-key-rotation.service.ts +++ b/apps/web/src/app/auth/key-rotation/user-key-rotation.service.ts @@ -5,8 +5,6 @@ import { AccountService } from "@bitwarden/common/auth/abstractions/account.serv import { DeviceTrustServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust.service.abstraction"; import { KdfConfigService } from "@bitwarden/common/auth/abstractions/kdf-config.service"; import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/auth/abstractions/master-password.service.abstraction"; -import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service"; import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; @@ -39,7 +37,6 @@ export class UserKeyRotationService { private encryptService: EncryptService, private stateService: StateService, private accountService: AccountService, - private configService: ConfigService, private kdfConfigService: KdfConfigService, ) {} @@ -90,11 +87,7 @@ export class UserKeyRotationService { request.emergencyAccessKeys = await this.emergencyAccessService.getRotatedKeys(newUserKey); request.resetPasswordKeys = await this.resetPasswordService.getRotatedKeys(newUserKey); - if (await this.configService.getFeatureFlag(FeatureFlag.KeyRotationImprovements)) { - await this.apiService.postUserKeyUpdate(request); - } else { - await this.rotateUserKeyAndEncryptedDataLegacy(request); - } + await this.apiService.postUserKeyUpdate(request); const activeAccount = await firstValueFrom(this.accountService.activeAccount$); await this.deviceTrustService.rotateDevicesTrust( @@ -139,16 +132,4 @@ export class UserKeyRotationService { }), ); } - - private async rotateUserKeyAndEncryptedDataLegacy(request: UpdateKeyRequest): Promise { - // Update keys, ciphers, folders, and sends - await this.apiService.postUserKeyUpdate(request); - - // Update emergency access keys - await this.emergencyAccessService.postLegacyRotation(request.emergencyAccessKeys); - - // Update account recovery keys - const userId = await this.stateService.getUserId(); - await this.resetPasswordService.postLegacyRotation(userId, request.resetPasswordKeys); - } } diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index 221b251f3c..ef8c4d61e4 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -9,7 +9,6 @@ export enum FeatureFlag { FlexibleCollectionsV1 = "flexible-collections-v-1", // v-1 is intentional VaultOnboarding = "vault-onboarding", GeneratorToolsModernization = "generator-tools-modernization", - KeyRotationImprovements = "key-rotation-improvements", FlexibleCollectionsMigration = "flexible-collections-migration", ShowPaymentMethodWarningBanners = "show-payment-method-warning-banners", EnableConsolidatedBilling = "enable-consolidated-billing", @@ -37,7 +36,6 @@ export const DefaultFeatureFlagValue = { [FeatureFlag.FlexibleCollectionsV1]: FALSE, [FeatureFlag.VaultOnboarding]: FALSE, [FeatureFlag.GeneratorToolsModernization]: FALSE, - [FeatureFlag.KeyRotationImprovements]: FALSE, [FeatureFlag.FlexibleCollectionsMigration]: FALSE, [FeatureFlag.ShowPaymentMethodWarningBanners]: FALSE, [FeatureFlag.EnableConsolidatedBilling]: FALSE, From 9eef1f0953c5d7a0f854d69306addc5c98ffd5ee Mon Sep 17 00:00:00 2001 From: Will Martin Date: Thu, 9 May 2024 13:47:05 -0400 Subject: [PATCH 2/3] fix merge error introduced in PM-5017 (#9102) --- .../app/billing/organizations/organization-plans.component.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/web/src/app/billing/organizations/organization-plans.component.ts b/apps/web/src/app/billing/organizations/organization-plans.component.ts index 992ee4f3a9..41af702ba0 100644 --- a/apps/web/src/app/billing/organizations/organization-plans.component.ts +++ b/apps/web/src/app/billing/organizations/organization-plans.component.ts @@ -595,7 +595,8 @@ export class OrganizationPlansComponent implements OnInit, OnDestroy { this.formPromise = doSubmit(); const organizationId = await this.formPromise; this.onSuccess.emit({ organizationId: organizationId }); - this.messagingService.send("organizationCreated", organizationId); + // TODO: No one actually listening to this message? + this.messagingService.send("organizationCreated", { organizationId }); }; private async updateOrganization(orgId: string) { From acc4251372c83ba02959df9ccfa556fede2cfee5 Mon Sep 17 00:00:00 2001 From: SmithThe4th Date: Thu, 9 May 2024 14:18:02 -0400 Subject: [PATCH 3/3] [PM-4577] Enhance passkey user verification to use configured unlock methods (#8746) * initial commit * fixed issue with clearing search index state * clear user index before account is totally cleaned up * added logout clear on option * removed redundant clear index from logout * Implemented user verification logic for the different use cases, added functions to pompt for user to set pin * added missing await and removed else if conditionals * fixed no return after user sets pin * added comment to further explain user verification when user is coming from lock screen * [PM-7836] UV not properly used when creating an item from [+] or Save passkey as new item (#8993) * added user verification using the save new login button and + button * removed commented out code * [PM-7808][PM-7848] UV Preferred/Required, Item has MP reprompt, user without MP incorrectly bypasses UV and When UV = discouraged, cannot save passkey to item using [+] button (#9015) --- .../src/auth/guards/fido2-auth.guard.ts | 4 +- apps/browser/src/popup/app.module.ts | 2 + .../src/popup/services/services.module.ts | 7 + .../popup/components/fido2/fido2.component.ts | 47 ++-- .../components/vault/add-edit.component.ts | 20 +- .../popup/utils/fido2-popout-session-data.ts | 1 + .../fido2-user-verification.service.spec.ts | 248 ++++++++++++++++++ .../fido2-user-verification.service.ts | 101 +++++++ 8 files changed, 398 insertions(+), 32 deletions(-) create mode 100644 apps/browser/src/vault/services/fido2-user-verification.service.spec.ts create mode 100644 apps/browser/src/vault/services/fido2-user-verification.service.ts diff --git a/apps/browser/src/auth/guards/fido2-auth.guard.ts b/apps/browser/src/auth/guards/fido2-auth.guard.ts index f6b560c71d..0c4e6268bf 100644 --- a/apps/browser/src/auth/guards/fido2-auth.guard.ts +++ b/apps/browser/src/auth/guards/fido2-auth.guard.ts @@ -26,7 +26,9 @@ export const fido2AuthGuard: CanActivateFn = async ( const authStatus = await authService.getAuthStatus(); if (authStatus === AuthenticationStatus.Locked) { - routerService.setPreviousUrl(state.url); + // Appending fromLock=true to the query params to indicate that the user is being redirected from the lock screen, this is used for user verification. + const previousUrl = `${state.url}&fromLock=true`; + routerService.setPreviousUrl(previousUrl); return router.createUrlTree(["/lock"], { queryParams: route.queryParams }); } diff --git a/apps/browser/src/popup/app.module.ts b/apps/browser/src/popup/app.module.ts index cbc711f107..e33a690aa8 100644 --- a/apps/browser/src/popup/app.module.ts +++ b/apps/browser/src/popup/app.module.ts @@ -14,6 +14,7 @@ import { EnvironmentSelectorComponent } from "@bitwarden/angular/auth/components import { JslibModule } from "@bitwarden/angular/jslib.module"; import { ColorPasswordCountPipe } from "@bitwarden/angular/pipes/color-password-count.pipe"; import { ColorPasswordPipe } from "@bitwarden/angular/pipes/color-password.pipe"; +import { UserVerificationDialogComponent } from "@bitwarden/auth/angular"; import { AvatarModule, ButtonModule, ToastModule } from "@bitwarden/components"; import { ExportScopeCalloutComponent } from "@bitwarden/vault-export-ui"; @@ -121,6 +122,7 @@ import "../platform/popup/locales"; PopupTabNavigationComponent, PopupFooterComponent, PopupHeaderComponent, + UserVerificationDialogComponent, ], declarations: [ ActionButtonsComponent, diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index dad8c39cdc..de2fc72747 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -87,6 +87,7 @@ import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.serv import { TotpService as TotpServiceAbstraction } from "@bitwarden/common/vault/abstractions/totp.service"; import { TotpService } from "@bitwarden/common/vault/services/totp.service"; import { DialogService, ToastService } from "@bitwarden/components"; +import { PasswordRepromptService } from "@bitwarden/vault"; import { UnauthGuardService } from "../../auth/popup/services"; import { AutofillService as AutofillServiceAbstraction } from "../../autofill/services/abstractions/autofill.service"; @@ -117,6 +118,7 @@ import { ForegroundMemoryStorageService } from "../../platform/storage/foregroun import { fromChromeRuntimeMessaging } from "../../platform/utils/from-chrome-runtime-messaging"; import { BrowserSendStateService } from "../../tools/popup/services/browser-send-state.service"; import { FilePopoutUtilsService } from "../../tools/popup/services/file-popout-utils.service"; +import { Fido2UserVerificationService } from "../../vault/services/fido2-user-verification.service"; import { VaultBrowserStateService } from "../../vault/services/vault-browser-state.service"; import { VaultFilterService } from "../../vault/services/vault-filter.service"; @@ -600,6 +602,11 @@ const safeProviders: SafeProvider[] = [ provide: CLIENT_TYPE, useValue: ClientType.Browser, }), + safeProvider({ + provide: Fido2UserVerificationService, + useClass: Fido2UserVerificationService, + deps: [PasswordRepromptService, UserVerificationService, DialogService], + }), ]; @NgModule({ diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts index 323d2ab4f2..8d46cc6033 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts @@ -27,13 +27,13 @@ import { LoginUriView } from "@bitwarden/common/vault/models/view/login-uri.view import { LoginView } from "@bitwarden/common/vault/models/view/login.view"; import { SecureNoteView } from "@bitwarden/common/vault/models/view/secure-note.view"; import { DialogService } from "@bitwarden/components"; -import { PasswordRepromptService } from "@bitwarden/vault"; import { ZonedMessageListenerService } from "../../../../platform/browser/zoned-message-listener.service"; import { BrowserFido2Message, BrowserFido2UserInterfaceSession, } from "../../../fido2/browser-fido2-user-interface.service"; +import { Fido2UserVerificationService } from "../../../services/fido2-user-verification.service"; import { VaultPopoutType } from "../../utils/vault-popout-window"; interface ViewData { @@ -59,6 +59,7 @@ export class Fido2Component implements OnInit, OnDestroy { protected data$: Observable; protected sessionId?: string; protected senderTabId?: string; + protected fromLock?: boolean; protected ciphers?: CipherView[] = []; protected displayedCiphers?: CipherView[] = []; protected loading = false; @@ -71,13 +72,13 @@ export class Fido2Component implements OnInit, OnDestroy { private router: Router, private activatedRoute: ActivatedRoute, private cipherService: CipherService, - private passwordRepromptService: PasswordRepromptService, private platformUtilsService: PlatformUtilsService, private domainSettingsService: DomainSettingsService, private searchService: SearchService, private logService: LogService, private dialogService: DialogService, private browserMessagingApi: ZonedMessageListenerService, + private fido2UserVerificationService: Fido2UserVerificationService, ) {} ngOnInit() { @@ -89,6 +90,7 @@ export class Fido2Component implements OnInit, OnDestroy { sessionId: queryParamMap.get("sessionId"), senderTabId: queryParamMap.get("senderTabId"), senderUrl: queryParamMap.get("senderUrl"), + fromLock: queryParamMap.get("fromLock"), })), ); @@ -101,6 +103,7 @@ export class Fido2Component implements OnInit, OnDestroy { this.sessionId = queryParams.sessionId; this.senderTabId = queryParams.senderTabId; this.url = queryParams.senderUrl; + this.fromLock = queryParams.fromLock === "true"; // For a 'NewSessionCreatedRequest', abort if it doesn't belong to the current session. if ( message.type === "NewSessionCreatedRequest" && @@ -210,7 +213,11 @@ export class Fido2Component implements OnInit, OnDestroy { protected async submit() { const data = this.message$.value; if (data?.type === "PickCredentialRequest") { - const userVerified = await this.handleUserVerification(data.userVerification, this.cipher); + const userVerified = await this.fido2UserVerificationService.handleUserVerification( + data.userVerification, + this.cipher, + this.fromLock, + ); this.send({ sessionId: this.sessionId, @@ -231,7 +238,11 @@ export class Fido2Component implements OnInit, OnDestroy { } } - const userVerified = await this.handleUserVerification(data.userVerification, this.cipher); + const userVerified = await this.fido2UserVerificationService.handleUserVerification( + data.userVerification, + this.cipher, + this.fromLock, + ); this.send({ sessionId: this.sessionId, @@ -248,14 +259,21 @@ export class Fido2Component implements OnInit, OnDestroy { const data = this.message$.value; if (data?.type === "ConfirmNewCredentialRequest") { const name = data.credentialName || data.rpId; - await this.createNewCipher(name); + const userVerified = await this.fido2UserVerificationService.handleUserVerification( + data.userVerification, + this.cipher, + this.fromLock, + ); + + if (!data.userVerification || userVerified) { + await this.createNewCipher(name); + } - // We are bypassing user verification pending implementation of PIN and biometric support. this.send({ sessionId: this.sessionId, cipherId: this.cipher?.id, type: "ConfirmNewCredentialResponse", - userVerified: data.userVerification, + userVerified, }); } @@ -304,6 +322,7 @@ export class Fido2Component implements OnInit, OnDestroy { uilocation: "popout", senderTabId: this.senderTabId, sessionId: this.sessionId, + fromLock: this.fromLock, userVerification: data.userVerification, singleActionPopout: `${VaultPopoutType.fido2Popout}_${this.sessionId}`, }, @@ -374,20 +393,6 @@ export class Fido2Component implements OnInit, OnDestroy { } } - private async handleUserVerification( - userVerificationRequested: boolean, - cipher: CipherView, - ): Promise { - const masterPasswordRepromptRequired = cipher && cipher.reprompt !== 0; - - if (masterPasswordRepromptRequired) { - return await this.passwordRepromptService.showPasswordPrompt(); - } - - // We are bypassing user verification pending implementation of PIN and biometric support. - return userVerificationRequested; - } - private send(msg: BrowserFido2Message) { BrowserFido2UserInterfaceSession.sendMessage({ sessionId: this.sessionId, diff --git a/apps/browser/src/vault/popup/components/vault/add-edit.component.ts b/apps/browser/src/vault/popup/components/vault/add-edit.component.ts index a566b054c0..05255a3c01 100644 --- a/apps/browser/src/vault/popup/components/vault/add-edit.component.ts +++ b/apps/browser/src/vault/popup/components/vault/add-edit.component.ts @@ -30,6 +30,7 @@ import { BrowserApi } from "../../../../platform/browser/browser-api"; import BrowserPopupUtils from "../../../../platform/popup/browser-popup-utils"; import { PopupCloseWarningService } from "../../../../popup/services/popup-close-warning.service"; import { BrowserFido2UserInterfaceSession } from "../../../fido2/browser-fido2-user-interface.service"; +import { Fido2UserVerificationService } from "../../../services/fido2-user-verification.service"; import { fido2PopoutSessionData$ } from "../../utils/fido2-popout-session-data"; import { closeAddEditVaultItemPopout, VaultPopoutType } from "../../utils/vault-popout-window"; @@ -69,6 +70,7 @@ export class AddEditComponent extends BaseAddEditComponent { dialogService: DialogService, datePipe: DatePipe, configService: ConfigService, + private fido2UserVerificationService: Fido2UserVerificationService, ) { super( cipherService, @@ -168,11 +170,17 @@ export class AddEditComponent extends BaseAddEditComponent { async submit(): Promise { const fido2SessionData = await firstValueFrom(this.fido2PopoutSessionData$); - const { isFido2Session, sessionId, userVerification } = fido2SessionData; + const { isFido2Session, sessionId, userVerification, fromLock } = fido2SessionData; const inFido2PopoutWindow = BrowserPopupUtils.inPopout(window) && isFido2Session; + if ( inFido2PopoutWindow && - !(await this.handleFido2UserVerification(sessionId, userVerification)) + userVerification && + !(await this.fido2UserVerificationService.handleUserVerification( + userVerification, + this.cipher, + fromLock, + )) ) { return false; } @@ -327,14 +335,6 @@ export class AddEditComponent extends BaseAddEditComponent { }, 200); } - private async handleFido2UserVerification( - sessionId: string, - userVerification: boolean, - ): Promise { - // We are bypassing user verification pending implementation of PIN and biometric support. - return true; - } - repromptChanged() { super.repromptChanged(); diff --git a/apps/browser/src/vault/popup/utils/fido2-popout-session-data.ts b/apps/browser/src/vault/popup/utils/fido2-popout-session-data.ts index 9917b7411d..a4d95ff48f 100644 --- a/apps/browser/src/vault/popup/utils/fido2-popout-session-data.ts +++ b/apps/browser/src/vault/popup/utils/fido2-popout-session-data.ts @@ -16,6 +16,7 @@ export function fido2PopoutSessionData$() { fallbackSupported: queryParams.fallbackSupported === "true", userVerification: queryParams.userVerification === "true", senderUrl: queryParams.senderUrl as string, + fromLock: queryParams.fromLock === "true", })), ); } diff --git a/apps/browser/src/vault/services/fido2-user-verification.service.spec.ts b/apps/browser/src/vault/services/fido2-user-verification.service.spec.ts new file mode 100644 index 0000000000..acee6ba20f --- /dev/null +++ b/apps/browser/src/vault/services/fido2-user-verification.service.spec.ts @@ -0,0 +1,248 @@ +import { MockProxy, mock } from "jest-mock-extended"; + +import { UserVerificationDialogComponent } from "@bitwarden/auth/angular"; +import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; +import { Utils } from "@bitwarden/common/platform/misc/utils"; +import { CipherRepromptType, CipherType } from "@bitwarden/common/vault/enums"; +import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; +import { DialogService } from "@bitwarden/components"; +import { PasswordRepromptService } from "@bitwarden/vault"; + +import { SetPinComponent } from "./../../auth/popup/components/set-pin.component"; +import { Fido2UserVerificationService } from "./fido2-user-verification.service"; + +jest.mock("@bitwarden/auth/angular", () => ({ + UserVerificationDialogComponent: { + open: jest.fn().mockResolvedValue({ userAction: "confirm", verificationSuccess: true }), + }, +})); + +jest.mock("../../auth/popup/components/set-pin.component", () => { + return { + SetPinComponent: { + open: jest.fn(), + }, + }; +}); + +describe("Fido2UserVerificationService", () => { + let fido2UserVerificationService: Fido2UserVerificationService; + + let passwordRepromptService: MockProxy; + let userVerificationService: MockProxy; + let dialogService: MockProxy; + let cipher: CipherView; + + beforeEach(() => { + passwordRepromptService = mock(); + userVerificationService = mock(); + dialogService = mock(); + + cipher = createCipherView(); + + fido2UserVerificationService = new Fido2UserVerificationService( + passwordRepromptService, + userVerificationService, + dialogService, + ); + + (UserVerificationDialogComponent.open as jest.Mock).mockResolvedValue({ + userAction: "confirm", + verificationSuccess: true, + }); + }); + + describe("handleUserVerification", () => { + describe("user verification requested is true", () => { + it("should return true if user is redirected from lock screen and master password reprompt is not required", async () => { + const result = await fido2UserVerificationService.handleUserVerification( + true, + cipher, + true, + ); + expect(result).toBe(true); + }); + + it("should call master password reprompt dialog if user is redirected from lock screen, has master password and master password reprompt is required", async () => { + cipher.reprompt = CipherRepromptType.Password; + userVerificationService.hasMasterPassword.mockResolvedValue(true); + passwordRepromptService.showPasswordPrompt.mockResolvedValue(true); + + const result = await fido2UserVerificationService.handleUserVerification( + true, + cipher, + true, + ); + + expect(passwordRepromptService.showPasswordPrompt).toHaveBeenCalled(); + expect(result).toBe(true); + }); + + it("should call user verification dialog if user is redirected from lock screen, does not have a master password and master password reprompt is required", async () => { + cipher.reprompt = CipherRepromptType.Password; + userVerificationService.hasMasterPassword.mockResolvedValue(false); + + const result = await fido2UserVerificationService.handleUserVerification( + true, + cipher, + true, + ); + + expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, { + clientSideOnlyVerification: true, + }); + expect(result).toBe(true); + }); + + it("should call user verification dialog if user is not redirected from lock screen, does not have a master password and master password reprompt is required", async () => { + cipher.reprompt = CipherRepromptType.Password; + userVerificationService.hasMasterPassword.mockResolvedValue(false); + + const result = await fido2UserVerificationService.handleUserVerification( + true, + cipher, + false, + ); + + expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, { + clientSideOnlyVerification: true, + }); + expect(result).toBe(true); + }); + + it("should call master password reprompt dialog if user is not redirected from lock screen, has a master password and master password reprompt is required", async () => { + cipher.reprompt = CipherRepromptType.Password; + userVerificationService.hasMasterPassword.mockResolvedValue(false); + passwordRepromptService.showPasswordPrompt.mockResolvedValue(true); + + const result = await fido2UserVerificationService.handleUserVerification( + true, + cipher, + false, + ); + + expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, { + clientSideOnlyVerification: true, + }); + expect(result).toBe(true); + }); + + it("should call user verification dialog if user is not redirected from lock screen and no master password reprompt is required", async () => { + const result = await fido2UserVerificationService.handleUserVerification( + true, + cipher, + false, + ); + + expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, { + clientSideOnlyVerification: true, + }); + expect(result).toBe(true); + }); + + it("should prompt user to set pin if user has no verification method", async () => { + (UserVerificationDialogComponent.open as jest.Mock).mockResolvedValue({ + userAction: "confirm", + verificationSuccess: false, + noAvailableClientVerificationMethods: true, + }); + + await fido2UserVerificationService.handleUserVerification(true, cipher, false); + + expect(SetPinComponent.open).toHaveBeenCalledWith(dialogService); + }); + }); + + describe("user verification requested is false", () => { + it("should return false if user is redirected from lock screen and master password reprompt is not required", async () => { + const result = await fido2UserVerificationService.handleUserVerification( + false, + cipher, + true, + ); + expect(result).toBe(false); + }); + + it("should return false if user is not redirected from lock screen and master password reprompt is not required", async () => { + const result = await fido2UserVerificationService.handleUserVerification( + false, + cipher, + false, + ); + expect(result).toBe(false); + }); + + it("should call master password reprompt dialog if user is redirected from lock screen, has master password and master password reprompt is required", async () => { + cipher.reprompt = CipherRepromptType.Password; + userVerificationService.hasMasterPassword.mockResolvedValue(true); + passwordRepromptService.showPasswordPrompt.mockResolvedValue(true); + + const result = await fido2UserVerificationService.handleUserVerification( + false, + cipher, + true, + ); + + expect(result).toBe(true); + expect(passwordRepromptService.showPasswordPrompt).toHaveBeenCalled(); + }); + + it("should call user verification dialog if user is redirected from lock screen, does not have a master password and master password reprompt is required", async () => { + cipher.reprompt = CipherRepromptType.Password; + userVerificationService.hasMasterPassword.mockResolvedValue(false); + + const result = await fido2UserVerificationService.handleUserVerification( + false, + cipher, + true, + ); + + expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, { + clientSideOnlyVerification: true, + }); + expect(result).toBe(true); + }); + + it("should call user verification dialog if user is not redirected from lock screen, does not have a master password and master password reprompt is required", async () => { + cipher.reprompt = CipherRepromptType.Password; + userVerificationService.hasMasterPassword.mockResolvedValue(false); + + const result = await fido2UserVerificationService.handleUserVerification( + false, + cipher, + false, + ); + + expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, { + clientSideOnlyVerification: true, + }); + expect(result).toBe(true); + }); + + it("should call master password reprompt dialog if user is not redirected from lock screen, has a master password and master password reprompt is required", async () => { + cipher.reprompt = CipherRepromptType.Password; + userVerificationService.hasMasterPassword.mockResolvedValue(false); + passwordRepromptService.showPasswordPrompt.mockResolvedValue(true); + + const result = await fido2UserVerificationService.handleUserVerification( + false, + cipher, + false, + ); + + expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, { + clientSideOnlyVerification: true, + }); + expect(result).toBe(true); + }); + }); + }); +}); + +function createCipherView() { + const cipher = new CipherView(); + cipher.id = Utils.newGuid(); + cipher.type = CipherType.Login; + cipher.reprompt = CipherRepromptType.None; + return cipher; +} diff --git a/apps/browser/src/vault/services/fido2-user-verification.service.ts b/apps/browser/src/vault/services/fido2-user-verification.service.ts new file mode 100644 index 0000000000..90c4d8ca61 --- /dev/null +++ b/apps/browser/src/vault/services/fido2-user-verification.service.ts @@ -0,0 +1,101 @@ +import { firstValueFrom } from "rxjs"; + +import { UserVerificationDialogComponent } from "@bitwarden/auth/angular"; +import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; +import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; +import { DialogService } from "@bitwarden/components"; +import { PasswordRepromptService } from "@bitwarden/vault"; + +import { SetPinComponent } from "../../auth/popup/components/set-pin.component"; + +export class Fido2UserVerificationService { + constructor( + private passwordRepromptService: PasswordRepromptService, + private userVerificationService: UserVerificationService, + private dialogService: DialogService, + ) {} + + /** + * Handles user verification for a user based on the cipher and user verification requested. + * @param userVerificationRequested Indicates if user verification is required or not. + * @param cipher Contains details about the cipher including master password reprompt. + * @param fromLock Indicates if the request is from the lock screen. + * @returns + */ + async handleUserVerification( + userVerificationRequested: boolean, + cipher: CipherView, + fromLock: boolean, + ): Promise { + const masterPasswordRepromptRequired = cipher && cipher.reprompt !== 0; + + // If the request is from the lock screen, treat unlocking the vault as user verification, + // unless a master password reprompt is required. + if (fromLock) { + return masterPasswordRepromptRequired + ? await this.handleMasterPasswordReprompt() + : userVerificationRequested; + } + + if (masterPasswordRepromptRequired) { + return await this.handleMasterPasswordReprompt(); + } + + if (userVerificationRequested) { + return await this.showUserVerificationDialog(); + } + + return userVerificationRequested; + } + + private async showMasterPasswordReprompt(): Promise { + return await this.passwordRepromptService.showPasswordPrompt(); + } + + private async showUserVerificationDialog(): Promise { + const result = await UserVerificationDialogComponent.open(this.dialogService, { + clientSideOnlyVerification: true, + }); + + if (result.userAction === "cancel") { + return; + } + + // Handle unsuccessful verification attempts. + if (!result.verificationSuccess) { + // Check if no client-side verification methods are available. + if (result.noAvailableClientVerificationMethods) { + return await this.promptUserToSetPin(); + } + return; + } + + return result.verificationSuccess; + } + + private async handleMasterPasswordReprompt(): Promise { + const hasMasterPassword = await this.userVerificationService.hasMasterPassword(); + + // TDE users have no master password, so we need to use the UserVerification prompt + return hasMasterPassword + ? await this.showMasterPasswordReprompt() + : await this.showUserVerificationDialog(); + } + + private async promptUserToSetPin() { + const dialogRef = SetPinComponent.open(this.dialogService); + + if (!dialogRef) { + return; + } + + const userHasPinSet = await firstValueFrom(dialogRef.closed); + + if (!userHasPinSet) { + return; + } + + // If the user has set a PIN, re-invoke the user verification dialog to complete the verification process. + return await this.showUserVerificationDialog(); + } +}