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(); + } +}