diff --git a/apps/desktop/src/autofill/services/desktop-autotype-policy.service.spec.ts b/apps/desktop/src/autofill/services/desktop-autotype-policy.service.spec.ts index 7fb30333e28..555e6ceef5b 100644 --- a/apps/desktop/src/autofill/services/desktop-autotype-policy.service.spec.ts +++ b/apps/desktop/src/autofill/services/desktop-autotype-policy.service.spec.ts @@ -1,8 +1,10 @@ import { TestBed } from "@angular/core/testing"; import { mock, MockProxy } from "jest-mock-extended"; -import { BehaviorSubject, firstValueFrom, take, timeout, TimeoutError } from "rxjs"; +import { BehaviorSubject, firstValueFrom, take } from "rxjs"; import { InternalPolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; +import { PolicyType } from "@bitwarden/common/admin-console/enums"; +import { Policy } from "@bitwarden/common/admin-console/models/domain/policy"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; @@ -18,10 +20,10 @@ describe("DesktopAutotypeDefaultSettingPolicy", () => { let policyService: MockProxy; let configService: MockProxy; - let mockAccountSubject: BehaviorSubject<{ id: UserId } | null>; + let mockAccountSubject: BehaviorSubject; let mockFeatureFlagSubject: BehaviorSubject; let mockAuthStatusSubject: BehaviorSubject; - let mockPolicyAppliesSubject: BehaviorSubject; + let mockPoliciesSubject: BehaviorSubject; const mockUserId = "user-123" as UserId; @@ -36,7 +38,7 @@ describe("DesktopAutotypeDefaultSettingPolicy", () => { mockAuthStatusSubject = new BehaviorSubject( AuthenticationStatus.Unlocked, ); - mockPolicyAppliesSubject = new BehaviorSubject(false); + mockPoliciesSubject = new BehaviorSubject([]); accountService = mock(); authService = mock(); @@ -50,9 +52,7 @@ describe("DesktopAutotypeDefaultSettingPolicy", () => { authService.authStatusFor$ = jest .fn() .mockImplementation((_: UserId) => mockAuthStatusSubject.asObservable()); - policyService.policyAppliesToUser$ = jest - .fn() - .mockReturnValue(mockPolicyAppliesSubject.asObservable()); + policyService.policies$ = jest.fn().mockReturnValue(mockPoliciesSubject.asObservable()); TestBed.configureTestingModule({ providers: [ @@ -72,7 +72,7 @@ describe("DesktopAutotypeDefaultSettingPolicy", () => { mockAccountSubject.complete(); mockFeatureFlagSubject.complete(); mockAuthStatusSubject.complete(); - mockPolicyAppliesSubject.complete(); + mockPoliciesSubject.complete(); }); describe("autotypeDefaultSetting$", () => { @@ -82,11 +82,20 @@ describe("DesktopAutotypeDefaultSettingPolicy", () => { expect(result).toBeNull(); }); - it("should not emit when no active account", async () => { + it("does not emit until an account appears", async () => { mockAccountSubject.next(null); - await expect( - firstValueFrom(service.autotypeDefaultSetting$.pipe(timeout({ first: 30 }))), - ).rejects.toBeInstanceOf(TimeoutError); + + mockAccountSubject.next({ id: mockUserId } as Account); + mockAuthStatusSubject.next(AuthenticationStatus.Unlocked); + mockPoliciesSubject.next([ + { + type: PolicyType.AutotypeDefaultSetting, + enabled: true, + } as Policy, + ]); + + const result = await firstValueFrom(service.autotypeDefaultSetting$.pipe(take(1))); + expect(result).toBe(true); }); it("should emit null when user is not unlocked", async () => { @@ -96,34 +105,56 @@ describe("DesktopAutotypeDefaultSettingPolicy", () => { }); it("should emit null when no autotype policy exists", async () => { - mockPolicyAppliesSubject.next(false); + mockPoliciesSubject.next([]); const policy = await firstValueFrom(service.autotypeDefaultSetting$.pipe(take(1))); expect(policy).toBeNull(); }); it("should emit true when autotype policy is enabled", async () => { - mockPolicyAppliesSubject.next(true); + mockPoliciesSubject.next([ + { + type: PolicyType.AutotypeDefaultSetting, + enabled: true, + } as Policy, + ]); const policyStatus = await firstValueFrom(service.autotypeDefaultSetting$.pipe(take(1))); expect(policyStatus).toBe(true); }); - it("should emit false when autotype policy is disabled", async () => { - mockPolicyAppliesSubject.next(false); + it("should emit null when autotype policy is disabled", async () => { + mockPoliciesSubject.next([ + { + type: PolicyType.AutotypeDefaultSetting, + enabled: false, + } as Policy, + ]); const policyStatus = await firstValueFrom(service.autotypeDefaultSetting$.pipe(take(1))); expect(policyStatus).toBeNull(); }); it("should emit null when autotype policy does not apply", async () => { - mockPolicyAppliesSubject.next(false); + mockPoliciesSubject.next([ + { + type: PolicyType.RequireSso, + enabled: true, + } as Policy, + ]); const policy = await firstValueFrom(service.autotypeDefaultSetting$.pipe(take(1))); expect(policy).toBeNull(); }); it("should react to authentication status changes", async () => { + mockPoliciesSubject.next([ + { + type: PolicyType.AutotypeDefaultSetting, + enabled: true, + } as Policy, + ]); + // Expect one emission when unlocked mockAuthStatusSubject.next(AuthenticationStatus.Unlocked); const first = await firstValueFrom(service.autotypeDefaultSetting$.pipe(take(1))); - expect(first).toBeNull(); + expect(first).toBe(true); // Expect null emission when locked mockAuthStatusSubject.next(AuthenticationStatus.Locked); @@ -134,33 +165,131 @@ describe("DesktopAutotypeDefaultSettingPolicy", () => { it("should react to account changes", async () => { const newUserId = "user-456" as UserId; + mockPoliciesSubject.next([ + { + type: PolicyType.AutotypeDefaultSetting, + enabled: true, + } as Policy, + ]); + // First value for original user const firstValue = await firstValueFrom(service.autotypeDefaultSetting$.pipe(take(1))); - expect(firstValue).toBeNull(); + expect(firstValue).toBe(true); // Change account and expect a new emission mockAccountSubject.next({ id: newUserId, - }); + } as Account); const secondValue = await firstValueFrom(service.autotypeDefaultSetting$.pipe(take(1))); - expect(secondValue).toBeNull(); + expect(secondValue).toBe(true); // Verify the auth lookup was switched to the new user expect(authService.authStatusFor$).toHaveBeenCalledWith(newUserId); + expect(policyService.policies$).toHaveBeenCalledWith(newUserId); }); it("should react to policy changes", async () => { - mockPolicyAppliesSubject.next(false); + mockPoliciesSubject.next([]); const nullValue = await firstValueFrom(service.autotypeDefaultSetting$.pipe(take(1))); expect(nullValue).toBeNull(); - mockPolicyAppliesSubject.next(true); + mockPoliciesSubject.next([ + { + type: PolicyType.AutotypeDefaultSetting, + enabled: true, + } as Policy, + ]); const trueValue = await firstValueFrom(service.autotypeDefaultSetting$.pipe(take(1))); expect(trueValue).toBe(true); - mockPolicyAppliesSubject.next(false); + mockPoliciesSubject.next([]); const nullValueAgain = await firstValueFrom(service.autotypeDefaultSetting$.pipe(take(1))); expect(nullValueAgain).toBeNull(); }); + + it("emits null again if the feature flag turns off after emitting", async () => { + mockPoliciesSubject.next([ + { type: PolicyType.AutotypeDefaultSetting, enabled: true } as Policy, + ]); + expect(await firstValueFrom(service.autotypeDefaultSetting$.pipe(take(1)))).toBe(true); + + mockFeatureFlagSubject.next(false); + expect(await firstValueFrom(service.autotypeDefaultSetting$.pipe(take(1)))).toBeNull(); + }); + + it("replays the latest value to late subscribers", async () => { + mockPoliciesSubject.next([ + { type: PolicyType.AutotypeDefaultSetting, enabled: true } as Policy, + ]); + + await firstValueFrom(service.autotypeDefaultSetting$.pipe(take(1))); + + const late = await firstValueFrom(service.autotypeDefaultSetting$.pipe(take(1))); + expect(late).toBe(true); + }); + + it("does not re-emit when effective value is unchanged", async () => { + mockAccountSubject.next({ id: mockUserId } as Account); + mockAuthStatusSubject.next(AuthenticationStatus.Unlocked); + + const policies = [ + { + type: PolicyType.AutotypeDefaultSetting, + enabled: true, + } as Policy, + ]; + + mockPoliciesSubject.next(policies); + const first = await firstValueFrom(service.autotypeDefaultSetting$.pipe(take(1))); + expect(first).toBe(true); + + let emissionCount = 0; + const subscription = service.autotypeDefaultSetting$.subscribe(() => { + emissionCount++; + }); + + mockPoliciesSubject.next(policies); + + await new Promise((resolve) => setTimeout(resolve, 50)); + subscription.unsubscribe(); + + expect(emissionCount).toBe(1); + }); + + it("does not emit policy values while locked; emits after unlocking", async () => { + mockAuthStatusSubject.next(AuthenticationStatus.Locked); + mockPoliciesSubject.next([ + { type: PolicyType.AutotypeDefaultSetting, enabled: true } as Policy, + ]); + + expect(await firstValueFrom(service.autotypeDefaultSetting$.pipe(take(1)))).toBeNull(); + + mockAuthStatusSubject.next(AuthenticationStatus.Unlocked); + expect(await firstValueFrom(service.autotypeDefaultSetting$.pipe(take(1)))).toBe(true); + }); + + it("emits correctly if auth unlocks before policies arrive", async () => { + mockAccountSubject.next({ id: mockUserId } as Account); + mockAuthStatusSubject.next(AuthenticationStatus.Unlocked); + mockPoliciesSubject.next([ + { + type: PolicyType.AutotypeDefaultSetting, + enabled: true, + } as Policy, + ]); + + const result = await firstValueFrom(service.autotypeDefaultSetting$.pipe(take(1))); + expect(result).toBe(true); + }); + + it("wires dependencies with initial user id", async () => { + mockPoliciesSubject.next([ + { type: PolicyType.AutotypeDefaultSetting, enabled: true } as Policy, + ]); + await firstValueFrom(service.autotypeDefaultSetting$.pipe(take(1))); + + expect(authService.authStatusFor$).toHaveBeenCalledWith(mockUserId); + expect(policyService.policies$).toHaveBeenCalledWith(mockUserId); + }); }); }); diff --git a/apps/desktop/src/autofill/services/desktop-autotype-policy.service.ts b/apps/desktop/src/autofill/services/desktop-autotype-policy.service.ts index 887a30ef6f6..d3ae67d2c8d 100644 --- a/apps/desktop/src/autofill/services/desktop-autotype-policy.service.ts +++ b/apps/desktop/src/autofill/services/desktop-autotype-policy.service.ts @@ -34,7 +34,7 @@ export class DesktopAutotypeDefaultSettingPolicy { } return this.accountService.activeAccount$.pipe( - filter((account) => account != null), + filter((account) => account != null && account.id != null), getUserId, distinctUntilChanged(), switchMap((userId) => { @@ -43,13 +43,16 @@ export class DesktopAutotypeDefaultSettingPolicy { distinctUntilChanged(), ); - const policy$ = this.policyService - .policyAppliesToUser$(PolicyType.AutotypeDefaultSetting, userId) - .pipe( - map((appliesToUser) => (appliesToUser ? true : null)), - distinctUntilChanged(), - shareReplay({ bufferSize: 1, refCount: true }), - ); + const policy$ = this.policyService.policies$(userId).pipe( + map((policies) => { + const autotypePolicy = policies.find( + (policy) => policy.type === PolicyType.AutotypeDefaultSetting && policy.enabled, + ); + return autotypePolicy ? true : null; + }), + distinctUntilChanged(), + shareReplay({ bufferSize: 1, refCount: true }), + ); return isUnlocked$.pipe(switchMap((unlocked) => (unlocked ? policy$ : of(null)))); }),