[PM-15882] Remove unlock with PIN policy (#13352)

* Remove policy with PIN in Web Vault * Remove policy with PIN in Browser Extension * Remove policy with PIN in Desktop * Remove policy with PIN in Desktop * unit tests coverage * unit tests coverage * unit tests coverage * private access method error * private access method error * private access method error * PM-18498: Unlock Options Padding Off When PIN Is Removed * PM-18498: Unlock Options Padding Off When PIN Is Removed
bitwarden · Feb 21, 2025 · 78202e1 · 78202e1
1 parent 3800610
commit 78202e1
Show file tree

Hide file tree

Showing 21 changed files with 746 additions and 25 deletions.
diff --git a/apps/browser/src/auth/popup/settings/account-security.component.html b/apps/browser/src/auth/popup/settings/account-security.component.html
@@ -11,7 +11,7 @@
         <h2 bitTypography="h6">{{ "unlockMethods" | i18n }}</h2>
       </bit-section-header>
       <bit-card>
-        <bit-form-control>
+        <bit-form-control [disableMargin]="!((pinEnabled$ | async) || this.form.value.pin)">
           <input bitCheckbox id="biometric" type="checkbox" formControlName="biometric" />
           <bit-label for="biometric" class="tw-whitespace-normal">{{
             "unlockWithBiometrics" | i18n
@@ -20,7 +20,11 @@ <h2 bitTypography="h6">{{ "unlockMethods" | i18n }}</h2>
             {{ biometricUnavailabilityReason }}
           </bit-hint>
         </bit-form-control>
-        <bit-form-control class="tw-pl-5" *ngIf="this.form.value.biometric && showAutoPrompt">
+        <bit-form-control
+          class="tw-pl-5"
+          [disableMargin]="!((pinEnabled$ | async) || this.form.value.pin)"
+          *ngIf="this.form.value.biometric && showAutoPrompt"
+        >
           <input
             bitCheckbox
             id="autoBiometricsPrompt"
@@ -33,6 +37,7 @@ <h2 bitTypography="h6">{{ "unlockMethods" | i18n }}</h2>
         </bit-form-control>
         <bit-form-control
           [disableMargin]="!(this.form.value.pin && showMasterPasswordOnClientRestartOption)"
+          *ngIf="(pinEnabled$ | async) || this.form.value.pin"
         >
           <input bitCheckbox id="pin" type="checkbox" formControlName="pin" />
           <bit-label for="pin" class="tw-whitespace-normal">{{ "unlockWithPin" | i18n }}</bit-label>

diff --git a/apps/browser/src/auth/popup/settings/account-security.component.spec.ts b/apps/browser/src/auth/popup/settings/account-security.component.spec.ts
@@ -0,0 +1,199 @@
+import { Component } from "@angular/core";
+import { ComponentFixture, TestBed } from "@angular/core/testing";
+import { By } from "@angular/platform-browser";
+import { mock } from "jest-mock-extended";
+import { firstValueFrom, of } from "rxjs";
+
+import { PinServiceAbstraction } from "@bitwarden/auth/common";
+import { VaultTimeoutSettingsService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout-settings.service";
+import { VaultTimeoutService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout.service";
+import { PolicyService } 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 { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction";
+import { VaultTimeoutAction } from "@bitwarden/common/enums/vault-timeout-action.enum";
+import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
+import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
+import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
+import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
+import { MessageSender } from "@bitwarden/common/platform/messaging";
+import { Utils } from "@bitwarden/common/platform/misc/utils";
+import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/spec";
+import { UserId } from "@bitwarden/common/types/guid";
+import { VaultTimeoutStringType } from "@bitwarden/common/types/vault-timeout.type";
+import { DialogService, ToastService } from "@bitwarden/components";
+import { BiometricStateService, BiometricsService, KeyService } from "@bitwarden/key-management";
+
+import { PopOutComponent } from "../../../platform/popup/components/pop-out.component";
+import { PopupRouterCacheService } from "../../../platform/popup/view-cache/popup-router-cache.service";
+
+import { AccountSecurityComponent } from "./account-security.component";
+
+@Component({
+  standalone: true,
+  selector: "app-pop-out",
+  template: ` <ng-content></ng-content>`,
+})
+class MockPopOutComponent {}
+
+describe("AccountSecurityComponent", () => {
+  let component: AccountSecurityComponent;
+  let fixture: ComponentFixture<AccountSecurityComponent>;
+
+  const mockUserId = Utils.newGuid() as UserId;
+  const accountService: FakeAccountService = mockAccountServiceWith(mockUserId);
+  const vaultTimeoutSettingsService = mock<VaultTimeoutSettingsService>();
+  const biometricStateService = mock<BiometricStateService>();
+  const policyService = mock<PolicyService>();
+  const pinServiceAbstraction = mock<PinServiceAbstraction>();
+
+  beforeEach(async () => {
+    await TestBed.configureTestingModule({
+      providers: [
+        { provide: AccountService, useValue: accountService },
+        { provide: AccountSecurityComponent, useValue: mock<AccountSecurityComponent>() },
+        { provide: BiometricsService, useValue: mock<BiometricsService>() },
+        { provide: BiometricStateService, useValue: biometricStateService },
+        { provide: DialogService, useValue: mock<DialogService>() },
+        { provide: EnvironmentService, useValue: mock<EnvironmentService>() },
+        { provide: I18nService, useValue: mock<I18nService>() },
+        { provide: MessageSender, useValue: mock<MessageSender>() },
+        { provide: KeyService, useValue: mock<KeyService>() },
+        { provide: PinServiceAbstraction, useValue: pinServiceAbstraction },
+        { provide: PlatformUtilsService, useValue: mock<PlatformUtilsService>() },
+        { provide: PolicyService, useValue: policyService },
+        { provide: PopupRouterCacheService, useValue: mock<PopupRouterCacheService>() },
+        { provide: StateService, useValue: mock<StateService>() },
+        { provide: ToastService, useValue: mock<ToastService>() },
+        { provide: UserVerificationService, useValue: mock<UserVerificationService>() },
+        { provide: VaultTimeoutService, useValue: mock<VaultTimeoutService>() },
+        { provide: VaultTimeoutSettingsService, useValue: vaultTimeoutSettingsService },
+      ],
+    })
+      .overrideComponent(AccountSecurityComponent, {
+        remove: {
+          imports: [PopOutComponent],
+        },
+        add: {
+          imports: [MockPopOutComponent],
+        },
+      })
+      .compileComponents();
+
+    fixture = TestBed.createComponent(AccountSecurityComponent);
+    component = fixture.componentInstance;
+    fixture.detectChanges();
+
+    vaultTimeoutSettingsService.getVaultTimeoutByUserId$.mockReturnValue(
+      of(VaultTimeoutStringType.OnLocked),
+    );
+    vaultTimeoutSettingsService.getVaultTimeoutActionByUserId$.mockReturnValue(
+      of(VaultTimeoutAction.Lock),
+    );
+    biometricStateService.promptAutomatically$ = of(false);
+    pinServiceAbstraction.isPinSet.mockResolvedValue(false);
+  });
+
+  it("pin enabled when RemoveUnlockWithPin policy is not set", async () => {
+    // @ts-strict-ignore
+    policyService.get$.mockReturnValue(of(null));
+
+    await component.ngOnInit();
+
+    await expect(firstValueFrom(component.pinEnabled$)).resolves.toBe(true);
+  });
+
+  it("pin enabled when RemoveUnlockWithPin policy is disabled", async () => {
+    const policy = new Policy();
+    policy.type = PolicyType.RemoveUnlockWithPin;
+    policy.enabled = false;
+
+    policyService.get$.mockReturnValue(of(policy));
+
+    await component.ngOnInit();
+
+    await expect(firstValueFrom(component.pinEnabled$)).resolves.toBe(true);
+
+    fixture.detectChanges();
+
+    const pinInputElement = fixture.debugElement.query(By.css("#pin"));
+    expect(pinInputElement).not.toBeNull();
+    expect(pinInputElement.name).toBe("input");
+  });
+
+  it("pin disabled when RemoveUnlockWithPin policy is enabled", async () => {
+    const policy = new Policy();
+    policy.type = PolicyType.RemoveUnlockWithPin;
+    policy.enabled = true;
+
+    policyService.get$.mockReturnValue(of(policy));
+
+    await component.ngOnInit();
+
+    await expect(firstValueFrom(component.pinEnabled$)).resolves.toBe(false);
+
+    fixture.detectChanges();
+
+    const pinInputElement = fixture.debugElement.query(By.css("#pin"));
+    expect(pinInputElement).toBeNull();
+  });
+
+  it("pin visible when RemoveUnlockWithPin policy is not set", async () => {
+    // @ts-strict-ignore
+    policyService.get$.mockReturnValue(of(null));
+
+    await component.ngOnInit();
+    fixture.detectChanges();
+
+    const pinInputElement = fixture.debugElement.query(By.css("#pin"));
+    expect(pinInputElement).not.toBeNull();
+    expect(pinInputElement.name).toBe("input");
+  });
+
+  it("pin visible when RemoveUnlockWithPin policy is disabled", async () => {
+    const policy = new Policy();
+    policy.type = PolicyType.RemoveUnlockWithPin;
+    policy.enabled = false;
+
+    policyService.get$.mockReturnValue(of(policy));
+
+    await component.ngOnInit();
+    fixture.detectChanges();
+
+    const pinInputElement = fixture.debugElement.query(By.css("#pin"));
+    expect(pinInputElement).not.toBeNull();
+    expect(pinInputElement.name).toBe("input");
+  });
+
+  it("pin visible when RemoveUnlockWithPin policy is enabled and pin set", async () => {
+    const policy = new Policy();
+    policy.type = PolicyType.RemoveUnlockWithPin;
+    policy.enabled = true;
+
+    policyService.get$.mockReturnValue(of(policy));
+
+    pinServiceAbstraction.isPinSet.mockResolvedValue(true);
+
+    await component.ngOnInit();
+    fixture.detectChanges();
+
+    const pinInputElement = fixture.debugElement.query(By.css("#pin"));
+    expect(pinInputElement).not.toBeNull();
+    expect(pinInputElement.name).toBe("input");
+  });
+
+  it("pin not visible when RemoveUnlockWithPin policy is enabled", async () => {
+    const policy = new Policy();
+    policy.type = PolicyType.RemoveUnlockWithPin;
+    policy.enabled = true;
+
+    policyService.get$.mockReturnValue(of(policy));
+
+    await component.ngOnInit();
+    fixture.detectChanges();
+
+    const pinInputElement = fixture.debugElement.query(By.css("#pin"));
+    expect(pinInputElement).toBeNull();
+  });
+});
diff --git a/apps/browser/src/auth/popup/settings/account-security.component.ts b/apps/browser/src/auth/popup/settings/account-security.component.ts
@@ -12,6 +12,8 @@ import {
   distinctUntilChanged,
   firstValueFrom,
   map,
+  Observable,
+  of,
   pairwise,
   startWith,
   Subject,
@@ -108,6 +110,7 @@ export class AccountSecurityComponent implements OnInit, OnDestroy {
   biometricUnavailabilityReason: string;
   showChangeMasterPass = true;
   showAutoPrompt = true;
+  pinEnabled$: Observable<boolean> = of(true);
 
   form = this.formBuilder.group({
     vaultTimeout: [null as VaultTimeout | null],
@@ -193,6 +196,12 @@ export class AccountSecurityComponent implements OnInit, OnDestroy {
       timeout = VaultTimeoutStringType.OnRestart;
     }
 
+    this.pinEnabled$ = this.policyService.get$(PolicyType.RemoveUnlockWithPin).pipe(
+      map((policy) => {
+        return policy == null || !policy.enabled;
+      }),
+    );
+
     const initialValues = {
       vaultTimeout: timeout,
       vaultTimeoutAction: await firstValueFrom(

diff --git a/apps/desktop/src/app/accounts/settings.component.html b/apps/desktop/src/app/accounts/settings.component.html
@@ -111,7 +111,7 @@ <h2>
                   }}</small>
                 </div>
               </ng-container>
-              <div class="form-group">
+              <div class="form-group" *ngIf="(pinEnabled$ | async) || this.form.value.pin">
                 <div class="checkbox">
                   <label for="pin">
                     <input id="pin" type="checkbox" formControlName="pin" />
@@ -163,7 +163,11 @@ <h2>
                       formControlName="requirePasswordOnStart"
                       (change)="updateRequirePasswordOnStart()"
                     />
-                    {{ "requirePasswordOnStart" | i18n }}
+                    @if (pinEnabled$ | async) {
+                      {{ "requirePasswordOnStart" | i18n }}
+                    } @else {
+                      {{ "requirePasswordWithoutPinOnStart" | i18n }}
+                    }
                   </label>
                 </div>
                 <small class="help-block form-group-child" *ngIf="isWindows">{{