From a205892dcb803b3e96b48e2099e1191ee2bf0f7d Mon Sep 17 00:00:00 2001 From: Alec Rippberger Date: Tue, 4 Mar 2025 22:18:25 -0600 Subject: [PATCH] Add and fix tests --- ...actor-auth-email-component.service.spec.ts | 20 ++ ...sion-two-factor-form-cache.service.spec.ts | 199 ++++++++++++++++++ ...extension-two-factor-form-cache.service.ts | 23 +- ...o-factor-form-cache.service.abstraction.ts | 30 +-- .../two-factor-auth.component.spec.ts | 13 +- 5 files changed, 253 insertions(+), 32 deletions(-) create mode 100644 apps/browser/src/auth/services/extension-two-factor-form-cache.service.spec.ts diff --git a/apps/browser/src/auth/services/extension-two-factor-auth-email-component.service.spec.ts b/apps/browser/src/auth/services/extension-two-factor-auth-email-component.service.spec.ts index 223375fd903..54b46afb92d 100644 --- a/apps/browser/src/auth/services/extension-two-factor-auth-email-component.service.spec.ts +++ b/apps/browser/src/auth/services/extension-two-factor-auth-email-component.service.spec.ts @@ -1,5 +1,6 @@ import { MockProxy, mock } from "jest-mock-extended"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { DialogService } from "@bitwarden/components"; // Must mock modules before importing @@ -26,22 +27,26 @@ describe("ExtensionTwoFactorAuthEmailComponentService", () => { let dialogService: MockProxy; let window: MockProxy; + let configService: MockProxy; beforeEach(() => { jest.clearAllMocks(); dialogService = mock(); window = mock(); + configService = mock(); extensionTwoFactorAuthEmailComponentService = new ExtensionTwoFactorAuthEmailComponentService( dialogService, window, + configService, ); }); describe("openPopoutIfApprovedForEmail2fa", () => { it("should open a popout if the user confirms the warning to popout the extension when in the popup", async () => { // Arrange + configService.getFeatureFlag.mockResolvedValue(false); dialogService.openSimpleDialog.mockResolvedValue(true); jest.spyOn(BrowserPopupUtils, "inPopup").mockReturnValue(true); @@ -61,6 +66,7 @@ describe("ExtensionTwoFactorAuthEmailComponentService", () => { it("should not open a popout if the user cancels the warning to popout the extension when in the popup", async () => { // Arrange + configService.getFeatureFlag.mockResolvedValue(false); dialogService.openSimpleDialog.mockResolvedValue(false); jest.spyOn(BrowserPopupUtils, "inPopup").mockReturnValue(true); @@ -80,6 +86,7 @@ describe("ExtensionTwoFactorAuthEmailComponentService", () => { it("should not open a popout if not in the popup", async () => { // Arrange + configService.getFeatureFlag.mockResolvedValue(false); jest.spyOn(BrowserPopupUtils, "inPopup").mockReturnValue(false); // Act @@ -89,5 +96,18 @@ describe("ExtensionTwoFactorAuthEmailComponentService", () => { expect(dialogService.openSimpleDialog).not.toHaveBeenCalled(); expect(openTwoFactorAuthEmailPopout).not.toHaveBeenCalled(); }); + + it("does not prompt or open a popout if the feature flag is enabled", async () => { + // Arrange + configService.getFeatureFlag.mockResolvedValue(true); + jest.spyOn(BrowserPopupUtils, "inPopup").mockReturnValue(true); + + // Act + await extensionTwoFactorAuthEmailComponentService.openPopoutIfApprovedForEmail2fa(); + + // Assert + expect(dialogService.openSimpleDialog).not.toHaveBeenCalled(); + expect(openTwoFactorAuthEmailPopout).not.toHaveBeenCalled(); + }); }); }); diff --git a/apps/browser/src/auth/services/extension-two-factor-form-cache.service.spec.ts b/apps/browser/src/auth/services/extension-two-factor-form-cache.service.spec.ts new file mode 100644 index 00000000000..3c78ea6aa3e --- /dev/null +++ b/apps/browser/src/auth/services/extension-two-factor-form-cache.service.spec.ts @@ -0,0 +1,199 @@ +import { signal } from "@angular/core"; +import { TestBed } from "@angular/core/testing"; +import { firstValueFrom } from "rxjs"; + +import { ViewCacheService } from "@bitwarden/angular/platform/abstractions/view-cache.service"; +import { TwoFactorFormData } from "@bitwarden/auth/angular"; +import { TwoFactorProviderType } from "@bitwarden/common/auth/enums/two-factor-provider-type"; +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; + +import { ExtensionTwoFactorFormCacheService } from "./extension-two-factor-form-cache.service"; + +describe("ExtensionTwoFactorFormCacheService", () => { + let service: ExtensionTwoFactorFormCacheService; + let testBed: TestBed; + const formDataSignal = signal(null); + const getFormDataSignal = jest.fn().mockReturnValue(formDataSignal); + const getFeatureFlag = jest.fn().mockResolvedValue(false); + const formDataSetMock = jest.spyOn(formDataSignal, "set"); + + const mockFormData: TwoFactorFormData = { + token: "123456", + remember: true, + selectedProviderType: TwoFactorProviderType.Authenticator, + emailSent: false, + }; + + beforeEach(() => { + getFormDataSignal.mockClear(); + getFeatureFlag.mockClear(); + formDataSetMock.mockClear(); + + testBed = TestBed.configureTestingModule({ + providers: [ + { provide: ViewCacheService, useValue: { signal: getFormDataSignal } }, + { provide: ConfigService, useValue: { getFeatureFlag } }, + ExtensionTwoFactorFormCacheService, + ], + }); + }); + + describe("feature enabled", () => { + beforeEach(async () => { + getFeatureFlag.mockImplementation((featureFlag: FeatureFlag) => { + if (featureFlag === FeatureFlag.PM9115_TwoFactorFormPersistence) { + return Promise.resolve(true); + } + return Promise.resolve(false); + }); + + service = testBed.inject(ExtensionTwoFactorFormCacheService); + }); + + describe("isEnabled$", () => { + it("emits true when feature flag is on", async () => { + const result = await firstValueFrom(service.isEnabled$()); + + expect(result).toBe(true); + expect(getFeatureFlag).toHaveBeenCalledWith(FeatureFlag.PM9115_TwoFactorFormPersistence); + }); + }); + + describe("isEnabled", () => { + it("returns true when feature flag is on", async () => { + const result = await service.isEnabled(); + + expect(result).toBe(true); + expect(getFeatureFlag).toHaveBeenCalledWith(FeatureFlag.PM9115_TwoFactorFormPersistence); + }); + }); + + describe("getFormData", () => { + it("returns cached form data", async () => { + formDataSignal.set(mockFormData); + + const result = await service.getFormData(); + + expect(result).toEqual(mockFormData); + }); + + it("returns null when cache is empty", async () => { + formDataSignal.set(null); + + const result = await service.getFormData(); + + expect(result).toBeNull(); + }); + }); + + describe("formData$", () => { + it("emits cached form data", async () => { + formDataSignal.set(mockFormData); + + const result = await firstValueFrom(service.formData$()); + + expect(result).toEqual(mockFormData); + }); + + it("emits null when cache is empty", async () => { + formDataSignal.set(null); + + const result = await firstValueFrom(service.formData$()); + + expect(result).toBeNull(); + }); + }); + + describe("saveFormData", () => { + it("updates the cached form data", async () => { + await service.saveFormData(mockFormData); + + expect(formDataSetMock).toHaveBeenCalledWith({ ...mockFormData }); + }); + + it("creates a shallow copy of the data", async () => { + const data = { ...mockFormData }; + + await service.saveFormData(data); + + expect(formDataSetMock).toHaveBeenCalledWith(data); + // Should be a new object, not the same reference + expect(formDataSetMock.mock.calls[0][0]).not.toBe(data); + }); + }); + + describe("clearFormData", () => { + it("sets the cache to null", async () => { + await service.clearFormData(); + + expect(formDataSetMock).toHaveBeenCalledWith(null); + }); + }); + }); + + describe("feature disabled", () => { + beforeEach(async () => { + formDataSignal.set(mockFormData); + getFeatureFlag.mockImplementation((featureFlag: FeatureFlag) => { + if (featureFlag === FeatureFlag.PM9115_TwoFactorFormPersistence) { + return Promise.resolve(false); + } + return Promise.resolve(false); + }); + + service = testBed.inject(ExtensionTwoFactorFormCacheService); + formDataSetMock.mockClear(); + }); + + describe("isEnabled$", () => { + it("emits false when feature flag is off", async () => { + const result = await firstValueFrom(service.isEnabled$()); + + expect(result).toBe(false); + expect(getFeatureFlag).toHaveBeenCalledWith(FeatureFlag.PM9115_TwoFactorFormPersistence); + }); + }); + + describe("isEnabled", () => { + it("returns false when feature flag is off", async () => { + const result = await service.isEnabled(); + + expect(result).toBe(false); + expect(getFeatureFlag).toHaveBeenCalledWith(FeatureFlag.PM9115_TwoFactorFormPersistence); + }); + }); + + describe("formData$", () => { + it("emits null when feature is disabled", async () => { + const result = await firstValueFrom(service.formData$()); + + expect(result).toBeNull(); + }); + }); + + describe("getFormData", () => { + it("returns null when feature is disabled", async () => { + const result = await service.getFormData(); + + expect(result).toBeNull(); + }); + }); + + describe("saveFormData", () => { + it("does not update cache when feature is disabled", async () => { + await service.saveFormData(mockFormData); + + expect(formDataSetMock).not.toHaveBeenCalled(); + }); + }); + + describe("clearFormData", () => { + it("still works when feature is disabled", async () => { + await service.clearFormData(); + + expect(formDataSetMock).toHaveBeenCalledWith(null); + }); + }); + }); +}); diff --git a/apps/browser/src/auth/services/extension-two-factor-form-cache.service.ts b/apps/browser/src/auth/services/extension-two-factor-form-cache.service.ts index 132191c5ad7..3e96290a6b3 100644 --- a/apps/browser/src/auth/services/extension-two-factor-form-cache.service.ts +++ b/apps/browser/src/auth/services/extension-two-factor-form-cache.service.ts @@ -1,5 +1,5 @@ import { Injectable, WritableSignal } from "@angular/core"; -import { Observable, from, of, switchMap } from "rxjs"; +import { Observable, of, switchMap, from } from "rxjs"; import { ViewCacheService } from "@bitwarden/angular/platform/abstractions/view-cache.service"; import { TwoFactorFormCacheService, TwoFactorFormData } from "@bitwarden/auth/angular"; @@ -49,14 +49,16 @@ export class ExtensionTwoFactorFormCacheService extends TwoFactorFormCacheServic }); } + /** + * Observable that emits the current enabled state + */ isEnabled$(): Observable { return from(this.configService.getFeatureFlag(FeatureFlag.PM9115_TwoFactorFormPersistence)); } - async isEnabled(): Promise { - return await this.configService.getFeatureFlag(FeatureFlag.PM9115_TwoFactorFormPersistence); - } - + /** + * Observable that emits the current form data + */ formData$(): Observable { return this.isEnabled$().pipe( switchMap((enabled) => { @@ -80,17 +82,6 @@ export class ExtensionTwoFactorFormCacheService extends TwoFactorFormCacheServic this.formDataCache.set({ ...data }); } - /** - * Retrieve form data from cache - */ - async getFormData(): Promise { - if (!(await this.isEnabled())) { - return null; - } - - return this.formDataCache(); - } - /** * Clear form data from cache */ diff --git a/libs/auth/src/angular/two-factor-auth/abstractions/two-factor-form-cache.service.abstraction.ts b/libs/auth/src/angular/two-factor-auth/abstractions/two-factor-form-cache.service.abstraction.ts index 4fa386dd944..a49dcd4fb43 100644 --- a/libs/auth/src/angular/two-factor-auth/abstractions/two-factor-form-cache.service.abstraction.ts +++ b/libs/auth/src/angular/two-factor-auth/abstractions/two-factor-form-cache.service.abstraction.ts @@ -1,4 +1,4 @@ -import { Observable } from "rxjs"; +import { Observable, firstValueFrom } from "rxjs"; import { TwoFactorProviderType } from "@bitwarden/common/auth/enums/two-factor-provider-type"; @@ -17,30 +17,36 @@ export interface TwoFactorFormData { */ export abstract class TwoFactorFormCacheService { /** - * Check if the form persistence feature is enabled - */ - abstract isEnabled(): Promise; - - /** - * Observable that emits the current enabled state + * Observable that emits the current enabled state of the feature flag */ abstract isEnabled$(): Observable; + /** + * Helper method that returns whether the feature is enabled + * @returns A promise that resolves to true if the feature is enabled + */ + async isEnabled(): Promise { + return firstValueFrom(this.isEnabled$()); + } + /** * Save form data to persistent storage */ abstract saveFormData(data: TwoFactorFormData): Promise; - /** - * Retrieve form data from persistent storage - */ - abstract getFormData(): Promise; - /** * Observable that emits the current form data */ abstract formData$(): Observable; + /** + * Helper method to retrieve form data + * @returns A promise that resolves to the form data + */ + async getFormData(): Promise { + return firstValueFrom(this.formData$()); + } + /** * Clear form data from persistent storage */ diff --git a/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.spec.ts b/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.spec.ts index 79856157aaa..d32ae18293b 100644 --- a/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.spec.ts +++ b/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.spec.ts @@ -1,10 +1,8 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { Component } from "@angular/core"; import { ComponentFixture, TestBed } from "@angular/core/testing"; -import { ActivatedRoute, convertToParamMap, Router } from "@angular/router"; +import { ActivatedRoute, Router, convertToParamMap } from "@angular/router"; import { mock, MockProxy } from "jest-mock-extended"; -import { BehaviorSubject } from "rxjs"; +import { BehaviorSubject, of } from "rxjs"; // eslint-disable-next-line no-restricted-imports import { WINDOW } from "@bitwarden/angular/services/injection-tokens"; @@ -39,6 +37,7 @@ import { DialogService, ToastService } from "@bitwarden/components"; import { AnonLayoutWrapperDataService } from "../anon-layout/anon-layout-wrapper-data.service"; +import { TwoFactorFormCacheService } from "./abstractions/two-factor-form-cache.service.abstraction"; import { TwoFactorAuthComponentService } from "./two-factor-auth-component.service"; import { TwoFactorAuthComponent } from "./two-factor-auth.component"; @@ -73,6 +72,7 @@ describe("TwoFactorAuthComponent", () => { let anonLayoutWrapperDataService: MockProxy; let mockEnvService: MockProxy; let mockLoginSuccessHandlerService: MockProxy; + let mockTwoFactorFormCacheService: MockProxy; let mockUserDecryptionOpts: { noMasterPassword: UserDecryptionOptions; @@ -113,6 +113,10 @@ describe("TwoFactorAuthComponent", () => { anonLayoutWrapperDataService = mock(); + mockTwoFactorFormCacheService = mock(); + mockTwoFactorFormCacheService.isEnabled$.mockReturnValue(of(false)); + mockTwoFactorFormCacheService.formData$.mockReturnValue(of(null)); + mockUserDecryptionOpts = { noMasterPassword: new UserDecryptionOptions({ hasMasterPassword: false, @@ -195,6 +199,7 @@ describe("TwoFactorAuthComponent", () => { { provide: EnvironmentService, useValue: mockEnvService }, { provide: AnonLayoutWrapperDataService, useValue: anonLayoutWrapperDataService }, { provide: LoginSuccessHandlerService, useValue: mockLoginSuccessHandlerService }, + { provide: TwoFactorFormCacheService, useValue: mockTwoFactorFormCacheService }, ], });