From 405c30008a821410955defeab82ecdee99812972 Mon Sep 17 00:00:00 2001 From: Jared Date: Wed, 8 Apr 2026 11:17:14 -0400 Subject: [PATCH] [PM-31778] Multi-step policy edit dialog (#19406) * implement multi-step policy ui framework * policy wip * Refactor organization data ownership policy component to use multi-step dialog and signals. Remove deprecated dialog component and update related tests. Add organizationId input to base policy edit component and implement save logic in the new dialog structure. * Update multi-step policy edit dialog to handle loading state and improve template initialization. Adjust button disable logic to account for loading status and modify ViewChild static property for proper change detection. * Refactor policy confirmation logic in base policy edit component. Remove deprecated confirm method and update policy edit dialog to handle confirmation checks dynamically. Adjust organization data ownership policy component to align with new confirmation structure. * Update apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/auto-confirm-policy.component.html Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> * Refactor multi-step policy dialog component and remove legacy confirm method - Deleted the multi-step policy plan documentation. - Introduced a new `MultiStepPolicyEditDialogComponent` to handle multi-step workflows. - Updated `BasePolicyEditComponent` to support multi-step configurations and removed the deprecated `confirm()` method. - Enhanced `PolicyStep` type to include optional `disableSave` and `bodyContent` properties. - Adjusted HTML templates to utilize signals for dynamic content rendering. - Refactored existing policy components to integrate with the new multi-step dialog structure. * Refactor AutoConfirmPolicy component constructor to use readonly modifier for firstTimeDialog parameter * Update multi-step policy edit dialog template to ensure proper rendering of policy form before lifecycle hooks. Added comment for clarity on rendering behavior. * Update apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/auto-confirm-policy.component.html Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> * Add confirmation step before policy submission in dialog component - Introduced an optional `confirm` method in `BasePolicyEditComponent` to allow components to implement a confirmation step before saving. - Updated `PolicyEditDialogComponent` to call the `confirm` method and handle user confirmation. - Added new test files for `AutoConfirmPolicyEditComponent` and `MultiStepPolicyEditDialogComponent` to ensure proper functionality and behavior of the new confirmation logic. * Update apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/vnext-organization-data-ownership.component.ts Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> * Refactor policy edit components to streamline side effects and improve save logic - Moved side effect logic from inline definitions to dedicated methods in AutoConfirmPolicyEditComponent. - Updated vNextOrganizationDataOwnershipPolicyComponent to remove unused template references. - Adjusted PolicyStep type to allow for side effects returning void. - Enhanced save button logic in MultiStepPolicyEditDialogComponent to utilize a computed signal for save state management. * Enhance MultiStepPolicyEditDialogComponent tests and update references - Refactored tests in multi-step policy edit dialog to improve saveDisabled logic and ensure accurate state management. - Updated references from AutoConfirmPolicyDialogComponent to MultiStepPolicyEditDialogComponent in web-vault-prompt service tests for consistency. * Refactor auto-confirm policy component template and update imports - Improved readability of the auto-confirm policy component template by formatting the button element. - Cleaned up import statements in the auto-confirm policy component test file for consistency. * Update WebVaultPromptService tests to validate firstTimeDialog property - Added assertion to ensure the firstTimeDialog property is correctly passed in the policy data during dialog closure in WebVaultPromptService tests. * Refactor policy edit components to support migration to new policy steps - Added TODO comments in BasePolicyEditComponent and PolicyEditDialogComponent to indicate the removal of the confirm method when the `MigrateMyVaultToMyItems` feature flag is deprecated. - Updated vNextOrganizationDataOwnershipPolicyComponent to ensure proper handling of organizationId. - Enhanced MultiStepPolicyEditDialogComponent template for improved subtitle handling and readability. * Refactor policy edit components to utilize Angular's input function - Replaced traditional @Input() properties with the new input() function in BasePolicyEditComponent for better reactivity. - Updated policy edit dialog component to set inputs using the new method for improved clarity and consistency. - Adjusted various policy components to ensure compatibility with the new input handling, enhancing overall code maintainability. * fix(policy-edit): update policy access method to use function call syntax Changed the way policy properties are accessed in the auto-confirm policy component template, ensuring compatibility with the updated policy structure. * refactor(session-timeout): update policyResponse handling in component and tests Modified the SessionTimeoutPolicyComponent to use function call syntax for accessing policyResponse. Updated corresponding unit tests to reflect this change, ensuring consistent handling of policy data across the component and its tests. * refactor(policy-edit-dialog): simplify test structure and improve saveDisabled logic Updated the unit tests for MultiStepPolicyEditDialogComponent to eliminate the use of fakeAsync and detectChanges, replacing them with direct state manipulation and TestBed.flushEffects(). This change enhances test clarity and reliability. Additionally, refactored the account age calculation in WebVaultExtensionPromptService to use UTC midnight for accurate day counting, addressing potential off-by-one errors. * Fix change to prompt service which was unneeded * refactor(policy-edit): enhance savePolicy method and inject dependencies Updated the BasePolicyEditComponent to inject AccountService, KeyService, and PolicyApiServiceAbstraction. Refactored the savePolicy method to streamline the policy saving process, ensuring proper handling of organization keys and error management. Adjusted AutoConfirmPolicyEditComponent and vNextOrganizationDataOwnershipPolicyComponent to utilize the new savePolicy implementation. * refactor(policy-edit): remove unused AccountService dependency Removed the AccountService dependency from MasterPasswordPolicyComponent and ResetPasswordPolicyComponent as it was not utilized. Added a getter for autoConfirmPolicy in AutoConfirmPolicyEditComponent to enhance code clarity. * refactor(policy-edit): update sideEffect callbacks in AutoConfirmPolicyEditComponent Modified the sideEffect properties in the AutoConfirmPolicyEditComponent to use arrow functions, ensuring proper context binding for the savePolicy and navigateToExtensionPromptStep methods. * refactor(policy-edit): update sideEffect to use arrow function in vNextOrganizationDataOwnershipPolicyComponent Changed the sideEffect property in the vNextOrganizationDataOwnershipPolicyComponent to use an arrow function for the savePolicy method, ensuring proper context binding. * Fixes tests for PR * refactor(vault): improve account age calculation to use UTC boundaries Updated the account age calculation in WebVaultExtensionPromptService to utilize UTC midnight boundaries, preventing issues with Daylight Saving Time skewing the day count. * Refactor PolicyEditDialogComponent to improve signal usage for save button state management - Changed saveDisabled property to use a private signal for better encapsulation. - Updated template bindings to call loading() and saveDisabled() as functions for correct state handling. - Enhanced observables management for save button disable logic to ensure proper state updates. * Fix optional chaining in policy confirmation and add policy steps to vNext organization data ownership component * Update tests for vNext organization data ownership component to use fixture inputs and add missing service mocks --------- Co-authored-by: Brandon Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> --- apps/desktop/src/app/app.component.ts | 2 +- apps/desktop/src/app/app.module.ts | 3 +- .../policies/base-policy-edit.component.ts | 88 ++++-- .../policies/policies.component.ts | 1 + .../auto-confirm-policy.component.html | 62 +++- .../auto-confirm-policy.component.spec.ts | 234 ++++++++++++++ .../auto-confirm-policy.component.ts | 171 ++++++++-- .../master-password.component.spec.ts | 4 + .../master-password.component.ts | 4 +- .../organization-data-ownership.component.ts | 6 +- .../remove-unlock-with-pin.component.spec.ts | 60 ++-- .../reset-password.component.ts | 6 +- .../single-org.component.ts | 4 +- .../uri-match-default.component.ts | 2 +- ...anization-data-ownership.component.spec.ts | 31 +- ...t-organization-data-ownership.component.ts | 21 +- .../policies/policy-edit-dialog.component.ts | 15 +- ...-confirm-edit-policy-dialog.component.html | 93 ------ ...nfirm-edit-policy-dialog.component.spec.ts | 276 ---------------- ...to-confirm-edit-policy-dialog.component.ts | 298 ------------------ .../policies/policy-edit-dialogs/index.ts | 3 +- .../policies/policy-edit-dialogs/models.ts | 25 +- ...lti-step-policy-edit-dialog.component.html | 47 +++ ...-step-policy-edit-dialog.component.spec.ts | 189 +++++++++++ ...multi-step-policy-edit-dialog.component.ts | 166 ++++++++++ ...wnership-edit-policy-dialog.component.html | 55 ---- ...-ownership-edit-policy-dialog.component.ts | 212 ------------- .../web-vault-extension-prompt.service.ts | 10 +- .../services/web-vault-prompt.service.spec.ts | 18 +- .../services/web-vault-prompt.service.ts | 11 +- .../session-timeout.component.spec.ts | 62 ++-- .../policies/session-timeout.component.ts | 2 +- 32 files changed, 1093 insertions(+), 1088 deletions(-) create mode 100644 apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/auto-confirm-policy.component.spec.ts delete mode 100644 apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/auto-confirm-edit-policy-dialog.component.html delete mode 100644 apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/auto-confirm-edit-policy-dialog.component.spec.ts delete mode 100644 apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/auto-confirm-edit-policy-dialog.component.ts create mode 100644 apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/multi-step-policy-edit-dialog.component.html create mode 100644 apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/multi-step-policy-edit-dialog.component.spec.ts create mode 100644 apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/multi-step-policy-edit-dialog.component.ts delete mode 100644 apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/organization-data-ownership-edit-policy-dialog.component.html delete mode 100644 apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/organization-data-ownership-edit-policy-dialog.component.ts diff --git a/apps/desktop/src/app/app.component.ts b/apps/desktop/src/app/app.component.ts index c3bd54b7cf4..a90f0266380 100644 --- a/apps/desktop/src/app/app.component.ts +++ b/apps/desktop/src/app/app.component.ts @@ -105,7 +105,7 @@ const SyncInterval = 6 * 60 * 60 * 1000; // 6 hours
- +
diff --git a/apps/desktop/src/app/app.module.ts b/apps/desktop/src/app/app.module.ts index 41ee28d27c6..a64d8588b07 100644 --- a/apps/desktop/src/app/app.module.ts +++ b/apps/desktop/src/app/app.module.ts @@ -10,7 +10,7 @@ import { BrowserAnimationsModule } from "@angular/platform-browser/animations"; import { JslibModule } from "@bitwarden/angular/jslib.module"; import { PremiumUpgradePromptService } from "@bitwarden/common/vault/abstractions/premium-upgrade-prompt.service"; -import { IconModule } from "@bitwarden/components"; +import { IconModule, SpinnerComponent } from "@bitwarden/components"; import { SshAgentService } from "../autofill/services/ssh-agent.service"; import { PremiumComponent } from "../billing/app/accounts/premium.component"; @@ -36,6 +36,7 @@ import { ServicesModule } from "./services/services.module"; AppRoutingModule, JslibModule, IconModule, + SpinnerComponent, ReactiveFormsModule, OverlayModule, ServicesModule, diff --git a/apps/web/src/app/admin-console/organizations/policies/base-policy-edit.component.ts b/apps/web/src/app/admin-console/organizations/policies/base-policy-edit.component.ts index 8bce87111e0..5bd628607d0 100644 --- a/apps/web/src/app/admin-console/organizations/policies/base-policy-edit.component.ts +++ b/apps/web/src/app/admin-console/organizations/policies/base-policy-edit.component.ts @@ -1,19 +1,26 @@ -import { Directive, Input, OnInit } from "@angular/core"; +import { Directive, OnInit, Signal, inject, input, signal } from "@angular/core"; import { FormControl, UntypedFormGroup } from "@angular/forms"; -import { Observable, of } from "rxjs"; +import { Observable, firstValueFrom, of, switchMap } from "rxjs"; import { Constructor } from "type-fest"; +import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { PolicyRequest } from "@bitwarden/common/admin-console/models/request/policy.request"; import { VNextSavePolicyRequest } from "@bitwarden/common/admin-console/models/request/v-next-save-policy.request"; import { PolicyStatusResponse } from "@bitwarden/common/admin-console/models/response/policy-status.response"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; +import { assertNonNullish } from "@bitwarden/common/auth/utils"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; +import { OrganizationId } from "@bitwarden/common/types/guid"; import { OrgKey } from "@bitwarden/common/types/key"; import { DialogConfig, DialogRef, DialogService } from "@bitwarden/components"; +import { KeyService } from "@bitwarden/key-management"; import { PolicyCategory } from "./pipes/policy-category"; import type { PolicyEditDialogData, PolicyEditDialogResult } from "./policy-edit-dialog.component"; +import type { PolicyStep, PolicyStepResult } from "./policy-edit-dialogs/models"; /** * Interface for policy dialog components. @@ -91,12 +98,14 @@ export abstract class BasePolicyEditDefinition { */ @Directive() export abstract class BasePolicyEditComponent implements OnInit { - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @Input() policyResponse: PolicyStatusResponse | undefined; - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @Input() policy: BasePolicyEditDefinition | undefined; + readonly policyResponse = input(undefined); + readonly policy = input(undefined); + readonly currentStep = input>(signal(0)); + readonly organizationId = input(undefined); + + protected readonly accountService = inject(AccountService); + protected readonly keyService = inject(KeyService); + protected readonly policyApiService = inject(PolicyApiServiceAbstraction); /** * Whether the policy is enabled. @@ -108,16 +117,31 @@ export abstract class BasePolicyEditComponent implements OnInit { */ data: UntypedFormGroup | undefined; - ngOnInit(): void { - this.enabled.setValue(this.policyResponse?.enabled ?? false); + /** + * Optional multi-step configuration for policies that require multiple steps to complete. + */ + policySteps?: PolicyStep[]; - if (this.policyResponse?.data != null) { + ngOnInit(): void { + this.enabled.setValue(this.policyResponse()?.enabled ?? false); + + if (this.policyResponse()?.data != null) { this.loadData(); } } + /** + * An optional guard called before submission in {@link PolicyEditDialogComponent}. + * Return `false` to abort the save (e.g. when the user cancels a warning dialog). + * Components that need a confirmation step before saving should override this method. + * + * TODO: Remove this method when the `MigrateMyVaultToMyItems` feature flag is removed. + * New policy components should use {@link policySteps} with a `sideEffect` instead. + */ + confirm?(): Promise; + async buildVNextRequest(orgKey: OrgKey): Promise { - if (!this.policy) { + if (!this.policy()) { throw new Error("Policy was not found"); } @@ -130,7 +154,7 @@ export abstract class BasePolicyEditComponent implements OnInit { } buildRequest() { - if (!this.policy) { + if (!this.policy()) { throw new Error("Policy was not found"); } @@ -143,16 +167,40 @@ export abstract class BasePolicyEditComponent implements OnInit { } /** - * This is called before the policy is saved. If it returns false, it will not be saved - * and the user will remain on the policy edit dialog. - * This can be used to trigger an additional confirmation modal before saving. - * */ - confirm(): Promise | boolean { - return true; + * Saves the policy via the vNext API. Subclasses that require additional steps or side effects + * (e.g. enabling a prerequisite policy) should override this method. + */ + protected async savePolicy(): Promise { + if (!this.policy()) { + throw new Error("Policy was not found"); + } + + const orgKeys = await firstValueFrom( + this.accountService.activeAccount$.pipe( + getUserId, + switchMap((userId) => this.keyService.orgKeys$(userId)), + ), + ); + + assertNonNullish(orgKeys, "Org keys not provided"); + + const orgKey = orgKeys[this.organizationId() as OrganizationId]; + + if (orgKey == null) { + throw new Error("No encryption key for this organization."); + } + + const request = await this.buildVNextRequest(orgKey); + + await this.policyApiService.putPolicyVNext( + this.organizationId() ?? "", + this.policy()!.type, + request, + ); } protected loadData() { - this.data?.patchValue(this.policyResponse?.data ?? {}); + this.data?.patchValue(this.policyResponse()?.data ?? {}); } /** diff --git a/apps/web/src/app/admin-console/organizations/policies/policies.component.ts b/apps/web/src/app/admin-console/organizations/policies/policies.component.ts index 3e812a69caa..648e35f3e49 100644 --- a/apps/web/src/app/admin-console/organizations/policies/policies.component.ts +++ b/apps/web/src/app/admin-console/organizations/policies/policies.component.ts @@ -159,6 +159,7 @@ export class PoliciesComponent { edit(policy: BasePolicyEditDefinition, organization: Organization) { const dialogComponent: PolicyDialogComponent = policy.editDialogComponent ?? PolicyEditDialogComponent; + dialogComponent.open(this.dialogService, { data: { policy: policy, diff --git a/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/auto-confirm-policy.component.html b/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/auto-confirm-policy.component.html index 394f5194b7b..03f10857fd1 100644 --- a/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/auto-confirm-policy.component.html +++ b/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/auto-confirm-policy.component.html @@ -1,6 +1,21 @@ - + +
+ @let showBadge = autoConfirmPolicy?.firstTimeDialog ?? false; + @if (showBadge) { + {{ "availableNow" | i18n }} + } + + {{ (showBadge ? "autoConfirm" : "editPolicy") | i18n }} + @if (!showBadge) { + + {{ policy()!.name | i18n }} + + } + +
+ - +

{{ "autoConfirmPolicyEditDescription" | i18n }}

@@ -46,7 +61,32 @@ {{ "autoConfirmCheckBoxLabel" | i18n }} - + + + + + + + {{ "howToTurnOnAutoConfirm" | i18n }} + + +
@@ -61,3 +101,19 @@ + + + + + diff --git a/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/auto-confirm-policy.component.spec.ts b/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/auto-confirm-policy.component.spec.ts new file mode 100644 index 00000000000..31e383e1f1e --- /dev/null +++ b/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/auto-confirm-policy.component.spec.ts @@ -0,0 +1,234 @@ +import { NO_ERRORS_SCHEMA } from "@angular/core"; +import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { Router } from "@angular/router"; +import { MockProxy, mock } from "jest-mock-extended"; +import { of } from "rxjs"; + +import { AutomaticUserConfirmationService, AutoConfirmState } from "@bitwarden/auto-confirm"; +import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; +import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; +import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; +import { PolicyType } from "@bitwarden/common/admin-console/enums"; +import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { Policy } from "@bitwarden/common/admin-console/models/domain/policy"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/spec"; +import { OrganizationId, UserId } from "@bitwarden/common/types/guid"; +import { newGuid } from "@bitwarden/guid"; +import { KeyService } from "@bitwarden/key-management"; + +import { AutoConfirmPolicy, AutoConfirmPolicyEditComponent } from "./auto-confirm-policy.component"; + +describe("AutoConfirmPolicy", () => { + it("has correct attributes", () => { + const policy = new AutoConfirmPolicy(); + + expect(policy.name).toBe("automaticUserConfirmation"); + expect(policy.description).toBe("autoConfirmDescription"); + expect(policy.type).toBe(PolicyType.AutoConfirm); + expect(policy.component).toBe(AutoConfirmPolicyEditComponent); + expect(policy.showDescription).toBe(false); + }); +}); + +describe("AutoConfirmPolicyEditComponent — policySteps[0].sideEffect", () => { + let component: AutoConfirmPolicyEditComponent; + let fixture: ComponentFixture; + let accountService: FakeAccountService; + let organizationService: MockProxy; + let policyService: MockProxy; + let policyApiService: MockProxy; + let autoConfirmService: MockProxy; + let router: MockProxy; + + const userId = newGuid() as UserId; + const orgId = newGuid() as OrganizationId; + + /** Convenience factory for Organization-shaped test objects. */ + function makeOrg(isAdmin: boolean, canManagePolicies: boolean): Organization { + return { id: orgId, isAdmin, canManagePolicies } as unknown as Organization; + } + + /** Convenience factory for Policy-shaped test objects. */ + function makePolicy(type: PolicyType, enabled: boolean): Policy { + return { type, enabled } as Policy; + } + + beforeEach(async () => { + accountService = mockAccountServiceWith(userId); + organizationService = mock(); + policyService = mock(); + policyApiService = mock(); + autoConfirmService = mock(); + router = mock(); + + // Defaults: admin org, no policies, API calls succeed, setup dialog visible + organizationService.organizations$.mockReturnValue(of([makeOrg(true, false)])); + policyService.policies$.mockReturnValue(of([])); + policyApiService.putPolicyVNext.mockResolvedValue(undefined); + autoConfirmService.configuration$.mockReturnValue( + of(Object.assign(new AutoConfirmState(), { showSetupDialog: true })), + ); + autoConfirmService.upsert.mockResolvedValue(undefined); + + await TestBed.configureTestingModule({ + providers: [ + { provide: AccountService, useValue: accountService }, + { provide: KeyService, useValue: mock() }, + { provide: OrganizationService, useValue: organizationService }, + { provide: PolicyService, useValue: policyService }, + { provide: PolicyApiServiceAbstraction, useValue: policyApiService }, + { provide: AutomaticUserConfirmationService, useValue: autoConfirmService }, + { provide: Router, useValue: router }, + ], + schemas: [NO_ERRORS_SCHEMA], + }).compileComponents(); + + fixture = TestBed.createComponent(AutoConfirmPolicyEditComponent); + component = fixture.componentInstance; + + // Simulate the inputs that MultiStepPolicyEditDialogComponent provides + fixture.componentRef.setInput("organizationId", orgId); + fixture.componentRef.setInput("policy", new AutoConfirmPolicy()); + component.enabled.setValue(true); + // Intentionally skip detectChanges() — viewChild signals are not needed for sideEffect tests + }); + + async function runSideEffect() { + return component.policySteps[0].sideEffect!(); + } + + describe("SingleOrg prerequisite enablement", () => { + it("enables SingleOrg before saving AutoConfirm when SingleOrg is not already enabled", async () => { + policyService.policies$.mockReturnValue(of([])); + + await runSideEffect(); + + expect(policyApiService.putPolicyVNext).toHaveBeenCalledWith(orgId, PolicyType.SingleOrg, { + policy: { enabled: true, data: null }, + metadata: null, + }); + expect(policyApiService.putPolicyVNext).toHaveBeenCalledWith( + orgId, + PolicyType.AutoConfirm, + expect.objectContaining({ policy: expect.objectContaining({ enabled: true }) }), + ); + }); + + it("does not enable SingleOrg when it is already enabled", async () => { + policyService.policies$.mockReturnValue(of([makePolicy(PolicyType.SingleOrg, true)])); + + await runSideEffect(); + + const singleOrgEnableCalls = policyApiService.putPolicyVNext.mock.calls.filter( + ([, type, req]) => type === PolicyType.SingleOrg && req?.policy?.enabled === true, + ); + expect(singleOrgEnableCalls).toHaveLength(0); + }); + + it("saves AutoConfirm with the enabled value from the form control", async () => { + component.enabled.setValue(false); + + await runSideEffect(); + + expect(policyApiService.putPolicyVNext).toHaveBeenCalledWith( + orgId, + PolicyType.AutoConfirm, + expect.objectContaining({ policy: expect.objectContaining({ enabled: false }) }), + ); + }); + }); + + describe("rollback on AutoConfirm save failure", () => { + it("rolls back SingleOrg when AutoConfirm save fails and SingleOrg was enabled during this action", async () => { + policyService.policies$.mockReturnValue(of([])); // SingleOrg not previously enabled + + policyApiService.putPolicyVNext + .mockResolvedValueOnce(undefined) // SingleOrg enable → success + .mockRejectedValueOnce(new Error("network error")) // AutoConfirm save → fail + .mockResolvedValueOnce(undefined); // SingleOrg rollback → success + + await expect(runSideEffect()).rejects.toThrow("network error"); + + expect(policyApiService.putPolicyVNext).toHaveBeenCalledWith(orgId, PolicyType.SingleOrg, { + policy: { enabled: false, data: null }, + metadata: null, + }); + }); + + it("does not roll back SingleOrg when AutoConfirm save fails but SingleOrg was already enabled", async () => { + policyService.policies$.mockReturnValue( + of([makePolicy(PolicyType.SingleOrg, true)]), // SingleOrg already on + ); + policyApiService.putPolicyVNext.mockRejectedValueOnce(new Error("network error")); + + await expect(runSideEffect()).rejects.toThrow("network error"); + + const singleOrgDisableCalls = policyApiService.putPolicyVNext.mock.calls.filter( + ([, type, req]) => type === PolicyType.SingleOrg && req?.policy?.enabled === false, + ); + expect(singleOrgDisableCalls).toHaveLength(0); + }); + }); + + describe("dialog close behavior", () => { + it("returns { closeDialog: true } when disabling AutoConfirm (no extension step needed)", async () => { + component.enabled.setValue(false); + + const result = await runSideEffect(); + + expect(result).toEqual({ closeDialog: true }); + }); + + it("returns { closeDialog: true } when user has manage-policies permission only (no admin role)", async () => { + organizationService.organizations$.mockReturnValue( + of([makeOrg(false, true)]), // not admin, but canManagePolicies + ); + component.enabled.setValue(true); + + const result = await runSideEffect(); + + expect(result).toEqual({ closeDialog: true }); + }); + + it("returns undefined to proceed to the extension step when enabling as a full admin", async () => { + organizationService.organizations$.mockReturnValue(of([makeOrg(true, false)])); + component.enabled.setValue(true); + + const result = await runSideEffect(); + + expect(result).toBeUndefined(); + }); + }); + + describe("setup dialog dismissal", () => { + it("dismisses the first-time setup dialog prompt after a successful save", async () => { + const currentState = Object.assign(new AutoConfirmState(), { showSetupDialog: true }); + autoConfirmService.configuration$.mockReturnValue(of(currentState)); + + await runSideEffect(); + + expect(autoConfirmService.upsert).toHaveBeenCalledWith(userId, { + ...currentState, + showSetupDialog: false, + }); + }); + + it("preserves the rest of the AutoConfirmState when dismissing the setup dialog", async () => { + const currentState = Object.assign(new AutoConfirmState(), { + enabled: true, + showSetupDialog: true, + showBrowserNotification: false, + }); + autoConfirmService.configuration$.mockReturnValue(of(currentState)); + + await runSideEffect(); + + expect(autoConfirmService.upsert).toHaveBeenCalledWith(userId, { + enabled: true, + showSetupDialog: false, + showBrowserNotification: false, + }); + }); + }); +}); diff --git a/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/auto-confirm-policy.component.ts b/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/auto-confirm-policy.component.ts index b6c3dc0d0de..728d5f159b8 100644 --- a/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/auto-confirm-policy.component.ts +++ b/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/auto-confirm-policy.component.ts @@ -1,25 +1,26 @@ -// FIXME(https://bitwarden.atlassian.net/browse/CL-1062): `OnPush` components should not use mutable properties -/* eslint-disable @bitwarden/components/enforce-readonly-angular-properties */ -import { - ChangeDetectionStrategy, - Component, - OnInit, - Signal, - TemplateRef, - viewChild, -} from "@angular/core"; -import { BehaviorSubject, map, Observable } from "rxjs"; +import { ChangeDetectionStrategy, Component, Signal, TemplateRef, viewChild } from "@angular/core"; +import { Router } from "@angular/router"; +import { combineLatest, defer, firstValueFrom, map, Observable, startWith, switchMap } from "rxjs"; import { AutoConfirmSvg } from "@bitwarden/assets/svg"; +import { AutomaticUserConfirmationService } from "@bitwarden/auto-confirm"; +import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; +import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; +import { getById } from "@bitwarden/common/platform/misc"; import { SharedModule } from "../../../../shared"; import { BasePolicyEditDefinition, BasePolicyEditComponent } from "../base-policy-edit.component"; import { PolicyCategory } from "../pipes/policy-category"; -import { AutoConfirmPolicyDialogComponent } from "../policy-edit-dialogs/auto-confirm-edit-policy-dialog.component"; +import { + MultiStepPolicyEditDialogComponent, + PolicyStep, + PolicyStepResult, +} from "../policy-edit-dialogs"; export class AutoConfirmPolicy extends BasePolicyEditDefinition { name = "automaticUserConfirmation"; @@ -29,7 +30,11 @@ export class AutoConfirmPolicy extends BasePolicyEditDefinition { priority = 90; component = AutoConfirmPolicyEditComponent; showDescription = false; - editDialogComponent = AutoConfirmPolicyDialogComponent; + editDialogComponent = MultiStepPolicyEditDialogComponent; + + constructor(readonly firstTimeDialog: boolean = false) { + super(); + } override display$(organization: Organization, configService: ConfigService): Observable { return configService @@ -44,21 +49,133 @@ export class AutoConfirmPolicy extends BasePolicyEditDefinition { imports: [SharedModule], changeDetection: ChangeDetectionStrategy.OnPush, }) -export class AutoConfirmPolicyEditComponent extends BasePolicyEditComponent implements OnInit { - protected readonly autoConfirmSvg = AutoConfirmSvg; - private readonly policyForm: Signal | undefined> = viewChild("step0"); - private readonly extensionButton: Signal | undefined> = viewChild("step1"); - - protected step: number = 0; - protected steps = [this.policyForm, this.extensionButton]; - - protected singleOrgEnabled$: BehaviorSubject = new BehaviorSubject(false); - - setSingleOrgEnabled(enabled: boolean) { - this.singleOrgEnabled$.next(enabled); +export class AutoConfirmPolicyEditComponent extends BasePolicyEditComponent { + constructor( + private readonly organizationService: OrganizationService, + private readonly policyService: PolicyService, + private readonly autoConfirmService: AutomaticUserConfirmationService, + private readonly router: Router, + ) { + super(); } - setStep(step: number) { - this.step = step; + protected readonly autoConfirmSvg = AutoConfirmSvg; + + protected get autoConfirmPolicy(): AutoConfirmPolicy | undefined { + return this.policy() as AutoConfirmPolicy | undefined; + } + + private readonly step0Title: Signal> = viewChild.required("step0Title"); + private readonly step0Content: Signal> = viewChild.required("step0Content"); + private readonly step0Footer: Signal> = viewChild.required("step0Footer"); + + private readonly step1Title: Signal> = viewChild.required("step1Title"); + private readonly step1Content: Signal> = viewChild.required("step1Content"); + private readonly step1Footer: Signal> = viewChild.required("step1Footer"); + + protected readonly autoConfirmEnabled$: Observable = + this.accountService.activeAccount$.pipe( + getUserId, + switchMap((userId) => this.policyService.policies$(userId)), + map((policies) => policies.find((p) => p.type === PolicyType.AutoConfirm)?.enabled ?? false), + ); + + protected readonly singleOrgEnabled$: Observable = + this.accountService.activeAccount$.pipe( + getUserId, + switchMap((userId) => this.policyService.policies$(userId)), + map((policies) => policies.find((p) => p.type === PolicyType.SingleOrg)?.enabled ?? false), + ); + + // defer() ensures this.organizationId() is read at subscription time rather than at + // class-field initialization time, where it would still be undefined. + protected readonly managePoliciesOnly$: Observable = defer(() => + this.accountService.activeAccount$.pipe( + getUserId, + switchMap((userId) => this.organizationService.organizations$(userId)), + getById(this.organizationId()), + map((organization) => (!organization?.isAdmin && organization?.canManagePolicies) ?? false), + ), + ); + + protected readonly saveDisabled$ = combineLatest([ + this.autoConfirmEnabled$, + this.enabled.valueChanges.pipe(startWith(this.enabled.value)), + ]).pipe(map(([policyEnabled, value]) => !policyEnabled && !value)); + + readonly policySteps: PolicyStep[] = [ + { + titleContent: this.step0Title, + bodyContent: this.step0Content, + footerContent: this.step0Footer, + disableSave: this.saveDisabled$, + sideEffect: () => this.savePolicy(), + }, + { + titleContent: this.step1Title, + bodyContent: this.step1Content, + footerContent: this.step1Footer, + sideEffect: () => this.navigateToExtensionPromptStep(), + }, + ]; + + protected override async savePolicy(): Promise { + const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); + + const organizations = await firstValueFrom(this.organizationService.organizations$(userId)); + const organization = organizations.find((o) => o.id === this.organizationId()) ?? null; + const managePoliciesOnly = (!organization?.isAdmin && organization?.canManagePolicies) ?? false; + + const policies = await firstValueFrom(this.policyService.policies$(userId)); + const singleOrgAlreadyEnabled = + policies.find((p) => p.type === PolicyType.SingleOrg)?.enabled ?? false; + const enabledSingleOrgDuringAction = !singleOrgAlreadyEnabled; + + // AutoConfirm requires SingleOrg; enable it as a prerequisite if not already on. + if (enabledSingleOrgDuringAction) { + await this.policyApiService.putPolicyVNext( + this.organizationId() ?? "", + PolicyType.SingleOrg, + { + policy: { enabled: true, data: null }, + metadata: null, + }, + ); + } + + try { + const request = await this.buildRequest(); + await this.policyApiService.putPolicyVNext( + this.organizationId() ?? "", + PolicyType.AutoConfirm, + { policy: request, metadata: null }, + ); + } catch (error) { + // Roll back the SingleOrg enablement if AutoConfirm save fails. + if (enabledSingleOrgDuringAction) { + await this.policyApiService.putPolicyVNext( + this.organizationId() ?? "", + PolicyType.SingleOrg, + { policy: { enabled: false, data: null }, metadata: null }, + ); + } + throw error; + } + + // Dismiss the first-time setup dialog prompt now that the admin has configured the policy. + const currentState = await firstValueFrom(this.autoConfirmService.configuration$(userId)); + await this.autoConfirmService.upsert(userId, { ...currentState, showSetupDialog: false }); + + // Close immediately when disabling (no extension step needed) or when the user only has + // manage-policies permission and cannot configure the client-side extension setting. + if (!this.enabled.value || managePoliciesOnly) { + return { closeDialog: true }; + } + } + + private async navigateToExtensionPromptStep(): Promise { + await this.router.navigate(["/browser-extension-prompt"], { + queryParams: { url: "AutoConfirm" }, + }); } } diff --git a/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/master-password.component.spec.ts b/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/master-password.component.spec.ts index b22f5687dd2..c11d87682e7 100644 --- a/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/master-password.component.spec.ts +++ b/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/master-password.component.spec.ts @@ -3,9 +3,11 @@ import { ComponentFixture, TestBed } from "@angular/core/testing"; import { mock } from "jest-mock-extended"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; +import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; +import { KeyService } from "@bitwarden/key-management"; import { MasterPasswordPolicyComponent } from "./master-password.component"; @@ -19,6 +21,8 @@ describe("MasterPasswordPolicyComponent", () => { { provide: I18nService, useValue: mock() }, { provide: OrganizationService, useValue: mock() }, { provide: AccountService, useValue: mock() }, + { provide: KeyService, useValue: mock() }, + { provide: PolicyApiServiceAbstraction, useValue: mock() }, ], schemas: [NO_ERRORS_SCHEMA], }).compileComponents(); diff --git a/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/master-password.component.ts b/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/master-password.component.ts index 38206b979a7..e7123e7c01c 100644 --- a/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/master-password.component.ts +++ b/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/master-password.component.ts @@ -13,7 +13,6 @@ import { } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { MasterPasswordPolicyOptions } from "@bitwarden/common/admin-console/models/domain/master-password-policy-options"; -import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; @@ -61,7 +60,6 @@ export class MasterPasswordPolicyComponent extends BasePolicyEditComponent imple private formBuilder: FormBuilder, i18nService: I18nService, private organizationService: OrganizationService, - private accountService: AccountService, ) { super(); @@ -81,7 +79,7 @@ export class MasterPasswordPolicyComponent extends BasePolicyEditComponent imple const organization = await firstValueFrom( this.organizationService .organizations$(userId) - .pipe(getOrganizationById(this.policyResponse.organizationId)), + .pipe(getOrganizationById(this.policyResponse()!.organizationId)), ); this.showKeyConnectorInfo = organization.keyConnectorEnabled; } diff --git a/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/organization-data-ownership.component.ts b/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/organization-data-ownership.component.ts index 403ec7b807c..97f65546c9f 100644 --- a/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/organization-data-ownership.component.ts +++ b/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/organization-data-ownership.component.ts @@ -58,8 +58,8 @@ export class OrganizationDataOwnershipPolicyComponent // eslint-disable-next-line @angular-eslint/prefer-signals @ViewChild("dialog", { static: true }) warningContent!: TemplateRef; - override async confirm(): Promise { - if (this.policyResponse?.enabled && !this.enabled.value) { + async confirm(): Promise { + if (this.policyResponse()?.enabled && !this.enabled.value) { const dialogRef = this.dialogService.open(this.warningContent, { positionStrategy: new CenterPositionStrategy(), }); @@ -72,7 +72,7 @@ export class OrganizationDataOwnershipPolicyComponent async buildVNextRequest( orgKey: OrgKey, ): Promise { - if (!this.policy) { + if (!this.policy()) { throw new Error("Policy was not found"); } diff --git a/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/remove-unlock-with-pin.component.spec.ts b/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/remove-unlock-with-pin.component.spec.ts index da0e9ede217..a8f9571110c 100644 --- a/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/remove-unlock-with-pin.component.spec.ts +++ b/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/remove-unlock-with-pin.component.spec.ts @@ -3,10 +3,13 @@ import { ComponentFixture, TestBed } from "@angular/core/testing"; import { By } from "@angular/platform-browser"; import { mock } from "jest-mock-extended"; +import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { PolicyStatusResponse } from "@bitwarden/common/admin-console/models/response/policy-status.response"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { OrgKey } from "@bitwarden/common/types/key"; +import { KeyService } from "@bitwarden/key-management"; import { RemoveUnlockWithPinPolicy, @@ -34,6 +37,9 @@ describe("RemoveUnlockWithPinPolicyComponent", () => { providers: [ { provide: I18nService, useValue: mock() }, { provide: I18nService, useValue: i18nService }, + { provide: AccountService, useValue: mock() }, + { provide: KeyService, useValue: mock() }, + { provide: PolicyApiServiceAbstraction, useValue: mock() }, ], schemas: [NO_ERRORS_SCHEMA], }).compileComponents(); @@ -43,11 +49,14 @@ describe("RemoveUnlockWithPinPolicyComponent", () => { }); it("input selected on load when policy enabled", async () => { - component.policyResponse = new PolicyStatusResponse({ - organizationId: "org1", - type: PolicyType.RemoveUnlockWithPin, - enabled: true, - }); + fixture.componentRef.setInput( + "policyResponse", + new PolicyStatusResponse({ + organizationId: "org1", + type: PolicyType.RemoveUnlockWithPin, + enabled: true, + }), + ); component.ngOnInit(); fixture.detectChanges(); @@ -63,11 +72,14 @@ describe("RemoveUnlockWithPinPolicyComponent", () => { }); it("input not selected on load when policy disabled", async () => { - component.policyResponse = new PolicyStatusResponse({ - organizationId: "org1", - type: PolicyType.RemoveUnlockWithPin, - enabled: false, - }); + fixture.componentRef.setInput( + "policyResponse", + new PolicyStatusResponse({ + organizationId: "org1", + type: PolicyType.RemoveUnlockWithPin, + enabled: false, + }), + ); component.ngOnInit(); fixture.detectChanges(); @@ -83,11 +95,14 @@ describe("RemoveUnlockWithPinPolicyComponent", () => { }); it("turn on message label", async () => { - component.policyResponse = new PolicyStatusResponse({ - organizationId: "org1", - type: PolicyType.RemoveUnlockWithPin, - enabled: false, - }); + fixture.componentRef.setInput( + "policyResponse", + new PolicyStatusResponse({ + organizationId: "org1", + type: PolicyType.RemoveUnlockWithPin, + enabled: false, + }), + ); i18nService.t.mockReturnValue("Turn on"); component.ngOnInit(); @@ -99,12 +114,15 @@ describe("RemoveUnlockWithPinPolicyComponent", () => { }); it("buildVNextRequest should delegate to buildRequest and wrap with null metadata", async () => { - component.policy = new RemoveUnlockWithPinPolicy(); - component.policyResponse = new PolicyStatusResponse({ - organizationId: "org1", - type: PolicyType.RemoveUnlockWithPin, - enabled: true, - }); + fixture.componentRef.setInput("policy", new RemoveUnlockWithPinPolicy()); + fixture.componentRef.setInput( + "policyResponse", + new PolicyStatusResponse({ + organizationId: "org1", + type: PolicyType.RemoveUnlockWithPin, + enabled: true, + }), + ); component.ngOnInit(); const buildRequestSpy = jest.spyOn(component, "buildRequest"); diff --git a/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/reset-password.component.ts b/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/reset-password.component.ts index 30e50cc258a..5e08718ccd3 100644 --- a/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/reset-password.component.ts +++ b/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/reset-password.component.ts @@ -11,7 +11,6 @@ import { } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; -import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; @@ -59,7 +58,6 @@ export class ResetPasswordPolicyComponent extends BasePolicyEditComponent implem constructor( private formBuilder: FormBuilder, private organizationService: OrganizationService, - private accountService: AccountService, ) { super(); } @@ -73,14 +71,14 @@ export class ResetPasswordPolicyComponent extends BasePolicyEditComponent implem throw new Error("No user found."); } - if (!this.policyResponse) { + if (!this.policyResponse()) { throw new Error("Policies not found"); } const organization = await firstValueFrom( this.organizationService .organizations$(userId) - .pipe(getOrganizationById(this.policyResponse.organizationId)), + .pipe(getOrganizationById(this.policyResponse()!.organizationId)), ); if (!organization) { diff --git a/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/single-org.component.ts b/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/single-org.component.ts index f471a0ba1c4..78216b84662 100644 --- a/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/single-org.component.ts +++ b/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/single-org.component.ts @@ -25,10 +25,10 @@ export class SingleOrgPolicyComponent extends BasePolicyEditComponent implements async ngOnInit() { super.ngOnInit(); - if (!this.policyResponse) { + if (!this.policyResponse()) { throw new Error("Policies not found"); } - if (!this.policyResponse.canToggleState) { + if (!this.policyResponse()!.canToggleState) { this.enabled.disable(); } } diff --git a/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/uri-match-default.component.ts b/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/uri-match-default.component.ts index 48d5dfc9c4a..869c05c9d57 100644 --- a/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/uri-match-default.component.ts +++ b/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/uri-match-default.component.ts @@ -56,7 +56,7 @@ export class UriMatchDefaultPolicyComponent extends BasePolicyEditComponent { } protected loadData() { - const uriMatchDetection = this.policyResponse?.data?.uriMatchDetection; + const uriMatchDetection = this.policyResponse()?.data?.uriMatchDetection; this.data?.patchValue({ uriMatchDetection: uriMatchDetection, diff --git a/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/vnext-organization-data-ownership.component.spec.ts b/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/vnext-organization-data-ownership.component.spec.ts index 970350a6ab1..ebe72de27ab 100644 --- a/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/vnext-organization-data-ownership.component.spec.ts +++ b/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/vnext-organization-data-ownership.component.spec.ts @@ -5,6 +5,7 @@ import { mock, MockProxy } from "jest-mock-extended"; import { of } from "rxjs"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; +import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { PolicyStatusResponse } from "@bitwarden/common/admin-console/models/response/policy-status.response"; @@ -12,6 +13,7 @@ import { AccountService } from "@bitwarden/common/auth/abstractions/account.serv import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { OrganizationId, UserId } from "@bitwarden/common/types/guid"; +import { KeyService } from "@bitwarden/key-management"; import { vNextOrganizationDataOwnershipPolicy, @@ -66,6 +68,8 @@ describe("vNextOrganizationDataOwnershipPolicyComponent", () => { { provide: EncryptService, useValue: mock() }, { provide: OrganizationService, useValue: mockOrganizationService }, { provide: AccountService, useValue: mockAccountService }, + { provide: KeyService, useValue: mock() }, + { provide: PolicyApiServiceAbstraction, useValue: mock() }, ], schemas: [NO_ERRORS_SCHEMA], }).compileComponents(); @@ -76,7 +80,7 @@ describe("vNextOrganizationDataOwnershipPolicyComponent", () => { describe("loadData with null server response", () => { it("should default enableIndividualItemsTransfer to false when data is null", async () => { - component.policyResponse = makePolicyResponse(true, null); + fixture.componentRef.setInput("policyResponse", makePolicyResponse(true, null)); await component.ngOnInit(); @@ -84,7 +88,10 @@ describe("vNextOrganizationDataOwnershipPolicyComponent", () => { }); it("should default enableIndividualItemsTransfer to false when the attribute is null", async () => { - component.policyResponse = makePolicyResponse(true, { enableIndividualItemsTransfer: null }); + fixture.componentRef.setInput( + "policyResponse", + makePolicyResponse(true, { enableIndividualItemsTransfer: null }), + ); await component.ngOnInit(); @@ -95,7 +102,7 @@ describe("vNextOrganizationDataOwnershipPolicyComponent", () => { describe("useMyItems conditional behavior", () => { it("should enable enableIndividualItemsTransfer control when policy is enabled and organization.useMyItems is true", async () => { setupOrg(true); - component.policyResponse = makePolicyResponse(true); + fixture.componentRef.setInput("policyResponse", makePolicyResponse(true)); await component.ngOnInit(); @@ -104,7 +111,7 @@ describe("vNextOrganizationDataOwnershipPolicyComponent", () => { it("should keep enableIndividualItemsTransfer control disabled when policy is enabled but organization.useMyItems is false", async () => { setupOrg(false); - component.policyResponse = makePolicyResponse(true); + fixture.componentRef.setInput("policyResponse", makePolicyResponse(true)); await component.ngOnInit(); @@ -113,7 +120,7 @@ describe("vNextOrganizationDataOwnershipPolicyComponent", () => { it("should keep enableIndividualItemsTransfer control disabled when organization is not found", async () => { // mockOrganizationService returns [] by default (no org found) - component.policyResponse = makePolicyResponse(true); + fixture.componentRef.setInput("policyResponse", makePolicyResponse(true)); await component.ngOnInit(); @@ -122,7 +129,7 @@ describe("vNextOrganizationDataOwnershipPolicyComponent", () => { it("should enable enableIndividualItemsTransfer control when enabled changes to true and useMyItems is true", async () => { setupOrg(true); - component.policyResponse = makePolicyResponse(false); + fixture.componentRef.setInput("policyResponse", makePolicyResponse(false)); await component.ngOnInit(); component.enabled.setValue(true); @@ -132,7 +139,7 @@ describe("vNextOrganizationDataOwnershipPolicyComponent", () => { it("should keep enableIndividualItemsTransfer control disabled when enabled changes to true but useMyItems is false", async () => { setupOrg(false); - component.policyResponse = makePolicyResponse(false); + fixture.componentRef.setInput("policyResponse", makePolicyResponse(false)); await component.ngOnInit(); component.enabled.setValue(true); @@ -144,7 +151,10 @@ describe("vNextOrganizationDataOwnershipPolicyComponent", () => { describe("buildRequestData", () => { it("should return enableIndividualItemsTransfer: false when useMyItems is false, even if control value is true", async () => { setupOrg(false); - component.policyResponse = makePolicyResponse(true, { enableIndividualItemsTransfer: true }); + fixture.componentRef.setInput( + "policyResponse", + makePolicyResponse(true, { enableIndividualItemsTransfer: true }), + ); await component.ngOnInit(); @@ -155,7 +165,10 @@ describe("vNextOrganizationDataOwnershipPolicyComponent", () => { it("should return enableIndividualItemsTransfer: true when useMyItems is true and control value is true", async () => { setupOrg(true); - component.policyResponse = makePolicyResponse(true, { enableIndividualItemsTransfer: true }); + fixture.componentRef.setInput( + "policyResponse", + makePolicyResponse(true, { enableIndividualItemsTransfer: true }), + ); await component.ngOnInit(); diff --git a/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/vnext-organization-data-ownership.component.ts b/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/vnext-organization-data-ownership.component.ts index 5a79c1c67af..968350ab294 100644 --- a/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/vnext-organization-data-ownership.component.ts +++ b/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/vnext-organization-data-ownership.component.ts @@ -7,7 +7,6 @@ import { OrganizationService } from "@bitwarden/common/admin-console/abstraction import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { VNextSavePolicyRequest } from "@bitwarden/common/admin-console/models/request/v-next-save-policy.request"; -import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; @@ -21,7 +20,8 @@ import { EncString } from "@bitwarden/sdk-internal"; import { SharedModule } from "../../../../shared"; import { BasePolicyEditDefinition, BasePolicyEditComponent } from "../base-policy-edit.component"; import { PolicyCategory } from "../pipes/policy-category"; -import { OrganizationDataOwnershipPolicyDialogComponent } from "../policy-edit-dialogs"; +import { MultiStepPolicyEditDialogComponent } from "../policy-edit-dialogs"; +import { PolicyStep } from "../policy-edit-dialogs/models"; type VNextSaveOrganizationDataOwnershipPolicyRequest = VNextSavePolicyRequest<{ defaultUserCollectionName: string; @@ -40,7 +40,7 @@ export class vNextOrganizationDataOwnershipPolicy extends BasePolicyEditDefiniti component = vNextOrganizationDataOwnershipPolicyComponent; showDescription = false; - editDialogComponent = OrganizationDataOwnershipPolicyDialogComponent; + editDialogComponent = MultiStepPolicyEditDialogComponent; override display$(organization: Organization, configService: ConfigService): Observable { return configService.getFeatureFlag$(FeatureFlag.MigrateMyVaultToMyItems); @@ -64,7 +64,6 @@ export class vNextOrganizationDataOwnershipPolicyComponent private readonly encryptService: EncryptService, private readonly formBuilder: FormBuilder, private readonly organizationService: OrganizationService, - private readonly accountService: AccountService, ) { super(); @@ -78,6 +77,12 @@ export class vNextOrganizationDataOwnershipPolicyComponent }); } + override readonly policySteps: PolicyStep[] = [ + { + sideEffect: () => this.savePolicy(), + }, + ]; + readonly data = this.formBuilder.group({ enableIndividualItemsTransfer: [{ value: false, disabled: true }], }); @@ -90,7 +95,7 @@ export class vNextOrganizationDataOwnershipPolicyComponent override async ngOnInit(): Promise { super.ngOnInit(); - const orgId = this.policyResponse?.organizationId as OrganizationId | undefined; + const orgId = this.policyResponse()?.organizationId as OrganizationId | undefined; if (orgId) { const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); const org = await firstValueFrom( @@ -105,11 +110,11 @@ export class vNextOrganizationDataOwnershipPolicyComponent } protected override loadData() { - if (!this.policyResponse?.data) { + if (!this.policyResponse()?.data) { return; } - const data = this.policyResponse.data as OrganizationDataOwnershipPolicyData; + const data = this.policyResponse()!.data as OrganizationDataOwnershipPolicyData; this.data.patchValue({ enableIndividualItemsTransfer: data.enableIndividualItemsTransfer ?? false, }); @@ -126,7 +131,7 @@ export class vNextOrganizationDataOwnershipPolicyComponent async buildVNextRequest( orgKey: OrgKey, ): Promise { - if (!this.policy) { + if (!this.policy()) { throw new Error("Policy was not found"); } diff --git a/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialog.component.ts b/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialog.component.ts index de8c36d0368..c0666772f9f 100644 --- a/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialog.component.ts +++ b/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialog.component.ts @@ -5,6 +5,7 @@ import { Component, DestroyRef, Inject, + Signal, ViewContainerRef, inject, signal, @@ -61,7 +62,8 @@ export class PolicyEditDialogComponent implements AfterViewInit { protected readonly policyType = PolicyType; protected readonly loading = signal(true); protected readonly enabled = false; - protected readonly saveDisabled = signal(false); + private readonly _saveDisabled = signal(false); + protected readonly saveDisabled: Signal = this._saveDisabled; protected readonly policyComponent = signal(undefined); readonly formGroup = this.formBuilder.group({ @@ -106,9 +108,10 @@ export class PolicyEditDialogComponent implements AfterViewInit { throw new Error("Template not initialized."); } - const component = policyFormRef.createComponent(this.data.policy.component).instance; - component.policy = this.data.policy; - component.policyResponse = policyResponse; + const componentRef = policyFormRef.createComponent(this.data.policy.component); + componentRef.setInput("policy", this.data.policy); + componentRef.setInput("policyResponse", policyResponse); + const component = componentRef.instance; this.policyComponent.set(component); if (component.data) { @@ -117,7 +120,7 @@ export class PolicyEditDialogComponent implements AfterViewInit { map((status) => status !== "VALID" || !policyResponse.canToggleState), takeUntilDestroyed(this.destroyRef), ) - .subscribe((disabled) => this.saveDisabled.set(disabled)); + .subscribe((disabled) => this._saveDisabled.set(disabled)); } this.cdr.detectChanges(); @@ -145,7 +148,7 @@ export class PolicyEditDialogComponent implements AfterViewInit { throw new Error("PolicyComponent not initialized."); } - if ((await policyComponent.confirm()) == false) { + if ((await policyComponent.confirm?.()) == false) { this.dialogRef.close(); return; } diff --git a/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/auto-confirm-edit-policy-dialog.component.html b/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/auto-confirm-edit-policy-dialog.component.html deleted file mode 100644 index 16d7e136d70..00000000000 --- a/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/auto-confirm-edit-policy-dialog.component.html +++ /dev/null @@ -1,93 +0,0 @@ -
- - - @let title = (multiStepSubmit | async)[currentStep()]?.titleContent(); - @if (title) { - - } - - - - @if (loading()) { -
- - {{ "loading" | i18n }} -
- } -
- @if (policy.showDescription) { -

{{ policy.description | i18n }}

- } -
- - - - @let footer = (multiStepSubmit | async)[currentStep()]?.footerContent(); - @if (footer) { - - } - - -
- - -
- @let showBadge = firstTimeDialog(); - @if (showBadge) { - {{ "availableNow" | i18n }} - } - - {{ (showBadge ? "automaticUserConfirmation" : "editPolicy") | i18n }} - @if (!showBadge) { - - {{ policy.name | i18n }} - - } - -
- - - - {{ "howToTurnOnAutoConfirm" | i18n }} - - - - - - - - - - - diff --git a/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/auto-confirm-edit-policy-dialog.component.spec.ts b/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/auto-confirm-edit-policy-dialog.component.spec.ts deleted file mode 100644 index b81e26f827b..00000000000 --- a/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/auto-confirm-edit-policy-dialog.component.spec.ts +++ /dev/null @@ -1,276 +0,0 @@ -import { NO_ERRORS_SCHEMA } from "@angular/core"; -import { ComponentFixture, TestBed } from "@angular/core/testing"; -import { FormBuilder } from "@angular/forms"; -import { Router } from "@angular/router"; -import { mock, MockProxy } from "jest-mock-extended"; -import { of } from "rxjs"; - -import { AutomaticUserConfirmationService } from "@bitwarden/auto-confirm"; -import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; -import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; -import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; -import { PolicyType } from "@bitwarden/common/admin-console/enums"; -import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; -import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; -import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; -import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/spec"; -import { OrganizationId, UserId } from "@bitwarden/common/types/guid"; -import { DIALOG_DATA, DialogRef, ToastService } from "@bitwarden/components"; -import { newGuid } from "@bitwarden/guid"; -import { KeyService } from "@bitwarden/key-management"; - -import { AutoConfirmPolicyEditComponent } from "../policy-edit-definitions/auto-confirm-policy.component"; - -import { - AutoConfirmPolicyDialogComponent, - AutoConfirmPolicyDialogData, -} from "./auto-confirm-edit-policy-dialog.component"; - -describe("AutoConfirmPolicyDialogComponent", () => { - let component: AutoConfirmPolicyDialogComponent; - let fixture: ComponentFixture; - - let mockPolicyApiService: MockProxy; - let mockAccountService: FakeAccountService; - let mockOrganizationService: MockProxy; - let mockPolicyService: MockProxy; - let mockRouter: MockProxy; - let mockAutoConfirmService: MockProxy; - let mockDialogRef: MockProxy; - let mockToastService: MockProxy; - let mockI18nService: MockProxy; - let mockKeyService: MockProxy; - - const mockUserId = newGuid() as UserId; - const mockOrgId = newGuid() as OrganizationId; - - const mockOrg = { - id: mockOrgId, - name: "Test Organization", - enabled: true, - isAdmin: true, - canManagePolicies: true, - } as Organization; - - const mockDialogData: AutoConfirmPolicyDialogData = { - organization: mockOrg, - policy: { - name: "automaticUserConfirmation", - description: "Auto Confirm Policy", - type: PolicyType.AutoConfirm, - component: {} as any, - showDescription: true, - display$: () => of(true), - }, - firstTimeDialog: false, - }; - - beforeEach(async () => { - mockPolicyApiService = mock(); - mockAccountService = mockAccountServiceWith(mockUserId); - mockOrganizationService = mock(); - mockPolicyService = mock(); - mockRouter = mock(); - mockAutoConfirmService = mock(); - mockDialogRef = mock(); - mockToastService = mock(); - mockI18nService = mock(); - mockKeyService = mock(); - - mockPolicyService.policies$.mockReturnValue(of([])); - mockOrganizationService.organizations$.mockReturnValue(of([mockOrg])); - - await TestBed.configureTestingModule({ - imports: [AutoConfirmPolicyDialogComponent], - providers: [ - FormBuilder, - { provide: DIALOG_DATA, useValue: mockDialogData }, - { provide: AccountService, useValue: mockAccountService }, - { provide: PolicyApiServiceAbstraction, useValue: mockPolicyApiService }, - { provide: I18nService, useValue: mockI18nService }, - { provide: DialogRef, useValue: mockDialogRef }, - { provide: ToastService, useValue: mockToastService }, - { provide: KeyService, useValue: mockKeyService }, - { provide: OrganizationService, useValue: mockOrganizationService }, - { provide: PolicyService, useValue: mockPolicyService }, - { provide: Router, useValue: mockRouter }, - { provide: AutomaticUserConfirmationService, useValue: mockAutoConfirmService }, - ], - schemas: [NO_ERRORS_SCHEMA], - }) - .overrideComponent(AutoConfirmPolicyDialogComponent, { - set: { template: "
" }, - }) - .compileComponents(); - - fixture = TestBed.createComponent(AutoConfirmPolicyDialogComponent); - component = fixture.componentInstance; - }); - - afterEach(() => { - jest.resetAllMocks(); - }); - - it("should create", () => { - expect(component).toBeTruthy(); - }); - - describe("handleSubmit", () => { - beforeEach(() => { - // Mock the policyComponent as an AutoConfirmPolicyEditComponent instance so instanceof check passes - const mockPolicyEditComponent = Object.assign( - Object.create(AutoConfirmPolicyEditComponent.prototype), - { - buildRequest: jest.fn().mockResolvedValue({ enabled: true, data: null }), - enabled: { value: true }, - setSingleOrgEnabled: jest.fn(), - }, - ) as AutoConfirmPolicyEditComponent; - component["policyComponent"].set(mockPolicyEditComponent); - - mockAutoConfirmService.configuration$.mockReturnValue( - of({ enabled: false, showSetupDialog: true, showBrowserNotification: undefined }), - ); - mockAutoConfirmService.upsert.mockResolvedValue(undefined); - mockI18nService.t.mockReturnValue("Policy updated"); - }); - - it("should enable SingleOrg policy when it was not already enabled", async () => { - mockPolicyApiService.putPolicyVNext.mockResolvedValue({} as any); - - // Call handleSubmit with singleOrgEnabled = false (meaning it needs to be enabled) - await component["handleSubmit"](false); - - // First call should be SingleOrg enable - expect(mockPolicyApiService.putPolicyVNext).toHaveBeenNthCalledWith( - 1, - mockOrgId, - PolicyType.SingleOrg, - { policy: { enabled: true, data: null } }, - ); - }); - - it("should not enable SingleOrg policy when it was already enabled", async () => { - mockPolicyApiService.putPolicyVNext.mockResolvedValue({} as any); - - // Call handleSubmit with singleOrgEnabled = true (meaning it's already enabled) - await component["handleSubmit"](true); - - // Should only call putPolicyVNext once (for AutoConfirm, not SingleOrg) - expect(mockPolicyApiService.putPolicyVNext).toHaveBeenCalledTimes(1); - expect(mockPolicyApiService.putPolicyVNext).toHaveBeenCalledWith( - mockOrgId, - PolicyType.AutoConfirm, - { policy: { enabled: true, data: null } }, - ); - }); - - it("should rollback SingleOrg policy when AutoConfirm fails and SingleOrg was enabled during action", async () => { - const autoConfirmError = new Error("AutoConfirm failed"); - - // First call (SingleOrg enable) succeeds, second call (AutoConfirm) fails, third call (SingleOrg rollback) succeeds - mockPolicyApiService.putPolicyVNext - .mockResolvedValueOnce({} as any) // SingleOrg enable - .mockRejectedValueOnce(autoConfirmError) // AutoConfirm fails - .mockResolvedValueOnce({} as any); // SingleOrg rollback - - await expect(component["handleSubmit"](false)).rejects.toThrow("AutoConfirm failed"); - - // Verify: SingleOrg enabled, AutoConfirm attempted, SingleOrg rolled back - expect(mockPolicyApiService.putPolicyVNext).toHaveBeenCalledTimes(3); - expect(mockPolicyApiService.putPolicyVNext).toHaveBeenNthCalledWith( - 1, - mockOrgId, - PolicyType.SingleOrg, - { policy: { enabled: true, data: null } }, - ); - expect(mockPolicyApiService.putPolicyVNext).toHaveBeenNthCalledWith( - 2, - mockOrgId, - PolicyType.AutoConfirm, - { policy: { enabled: true, data: null } }, - ); - expect(mockPolicyApiService.putPolicyVNext).toHaveBeenNthCalledWith( - 3, - mockOrgId, - PolicyType.SingleOrg, - { policy: { enabled: false, data: null } }, - ); - }); - - it("should not rollback SingleOrg policy when AutoConfirm fails but SingleOrg was already enabled", async () => { - const autoConfirmError = new Error("AutoConfirm failed"); - - // AutoConfirm call fails (SingleOrg was already enabled, so no SingleOrg calls) - mockPolicyApiService.putPolicyVNext.mockRejectedValue(autoConfirmError); - - await expect(component["handleSubmit"](true)).rejects.toThrow("AutoConfirm failed"); - - // Verify only AutoConfirm was called (no SingleOrg enable/rollback) - expect(mockPolicyApiService.putPolicyVNext).toHaveBeenCalledTimes(1); - expect(mockPolicyApiService.putPolicyVNext).toHaveBeenCalledWith( - mockOrgId, - PolicyType.AutoConfirm, - { policy: { enabled: true, data: null } }, - ); - }); - - it("should keep both policies enabled when both submissions succeed", async () => { - mockPolicyApiService.putPolicyVNext.mockResolvedValue({} as any); - - await component["handleSubmit"](false); - - // Verify two calls: SingleOrg enable and AutoConfirm enable - expect(mockPolicyApiService.putPolicyVNext).toHaveBeenCalledTimes(2); - expect(mockPolicyApiService.putPolicyVNext).toHaveBeenNthCalledWith( - 1, - mockOrgId, - PolicyType.SingleOrg, - { policy: { enabled: true, data: null } }, - ); - expect(mockPolicyApiService.putPolicyVNext).toHaveBeenNthCalledWith( - 2, - mockOrgId, - PolicyType.AutoConfirm, - { policy: { enabled: true, data: null } }, - ); - }); - - it("should re-throw the error after rollback", async () => { - const autoConfirmError = new Error("Network error"); - - mockPolicyApiService.putPolicyVNext - .mockResolvedValueOnce({} as any) // SingleOrg enable - .mockRejectedValueOnce(autoConfirmError) // AutoConfirm fails - .mockResolvedValueOnce({} as any); // SingleOrg rollback - - await expect(component["handleSubmit"](false)).rejects.toThrow("Network error"); - }); - }); - - describe("setSingleOrgPolicy", () => { - it("should call putPolicyVNext with enabled: true when enabling", async () => { - mockPolicyApiService.putPolicyVNext.mockResolvedValue({} as any); - - await component["setSingleOrgPolicy"](true); - - expect(mockPolicyApiService.putPolicyVNext).toHaveBeenCalledWith( - mockOrgId, - PolicyType.SingleOrg, - { policy: { enabled: true, data: null } }, - ); - }); - - it("should call putPolicyVNext with enabled: false when disabling", async () => { - mockPolicyApiService.putPolicyVNext.mockResolvedValue({} as any); - - await component["setSingleOrgPolicy"](false); - - expect(mockPolicyApiService.putPolicyVNext).toHaveBeenCalledWith( - mockOrgId, - PolicyType.SingleOrg, - { policy: { enabled: false, data: null } }, - ); - }); - }); -}); diff --git a/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/auto-confirm-edit-policy-dialog.component.ts b/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/auto-confirm-edit-policy-dialog.component.ts deleted file mode 100644 index ee3fb335276..00000000000 --- a/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/auto-confirm-edit-policy-dialog.component.ts +++ /dev/null @@ -1,298 +0,0 @@ -import { - AfterViewInit, - ChangeDetectorRef, - Component, - Inject, - signal, - Signal, - TemplateRef, - viewChild, -} from "@angular/core"; -import { FormBuilder } from "@angular/forms"; -import { Router } from "@angular/router"; -import { - combineLatest, - firstValueFrom, - map, - Observable, - of, - shareReplay, - startWith, - switchMap, - tap, -} from "rxjs"; - -import { AutomaticUserConfirmationService } from "@bitwarden/auto-confirm"; -import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; -import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; -import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; -import { PolicyType } from "@bitwarden/common/admin-console/enums"; -import { PolicyRequest } from "@bitwarden/common/admin-console/models/request/policy.request"; -import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; -import { getUserId } from "@bitwarden/common/auth/services/account.service"; -import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; -import { getById } from "@bitwarden/common/platform/misc"; -import { - DIALOG_DATA, - DialogConfig, - DialogRef, - DialogService, - ToastService, -} from "@bitwarden/components"; -import { KeyService } from "@bitwarden/key-management"; - -import { SharedModule } from "../../../../shared"; -import { AutoConfirmPolicyEditComponent } from "../policy-edit-definitions/auto-confirm-policy.component"; -import { - PolicyEditDialogComponent, - PolicyEditDialogData, - PolicyEditDialogResult, -} from "../policy-edit-dialog.component"; - -import { MultiStepSubmit } from "./models"; - -export type AutoConfirmPolicyDialogData = PolicyEditDialogData & { - firstTimeDialog?: boolean; -}; - -/** - * Custom policy dialog component for Auto-Confirm policy. - * Satisfies the PolicyDialogComponent interface structurally - * via its static open() function. - */ -// FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush -// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection -@Component({ - templateUrl: "auto-confirm-edit-policy-dialog.component.html", - imports: [SharedModule], -}) -export class AutoConfirmPolicyDialogComponent - extends PolicyEditDialogComponent - implements AfterViewInit -{ - policyType = PolicyType; - - protected readonly firstTimeDialog = signal(false); - protected readonly currentStep = signal(0); - protected multiStepSubmit: Observable = of([]); - protected autoConfirmEnabled$: Observable = this.accountService.activeAccount$.pipe( - getUserId, - switchMap((userId) => this.policyService.policies$(userId)), - map((policies) => policies.find((p) => p.type === PolicyType.AutoConfirm)?.enabled ?? false), - ); - // Users with manage policies custom permission should not see the dialog's second step since - // they do not have permission to configure the setting. This will only allow them to configure - // the policy. - protected managePoliciesOnly$: Observable = this.accountService.activeAccount$.pipe( - getUserId, - switchMap((userId) => this.organizationService.organizations$(userId)), - getById(this.data.organization.id), - map((organization) => (!organization?.isAdmin && organization?.canManagePolicies) ?? false), - ); - - private readonly submitPolicy: Signal | undefined> = viewChild("step0"); - private readonly openExtension: Signal | undefined> = viewChild("step1"); - - private readonly submitPolicyTitle: Signal | undefined> = - viewChild("step0Title"); - private readonly openExtensionTitle: Signal | undefined> = - viewChild("step1Title"); - - protected saveDisabled$: Observable | undefined; - - private get typedPolicyComponent(): AutoConfirmPolicyEditComponent | undefined { - const component = this.policyComponent(); - return component instanceof AutoConfirmPolicyEditComponent ? component : undefined; - } - - constructor( - @Inject(DIALOG_DATA) protected data: AutoConfirmPolicyDialogData, - accountService: AccountService, - policyApiService: PolicyApiServiceAbstraction, - i18nService: I18nService, - cdr: ChangeDetectorRef, - formBuilder: FormBuilder, - dialogRef: DialogRef, - toastService: ToastService, - keyService: KeyService, - private organizationService: OrganizationService, - private policyService: PolicyService, - private router: Router, - private autoConfirmService: AutomaticUserConfirmationService, - ) { - super( - data, - accountService, - policyApiService, - i18nService, - cdr, - formBuilder, - dialogRef, - toastService, - keyService, - ); - - this.firstTimeDialog.set(data.firstTimeDialog ?? false); - } - - /** - * Instantiates the child policy component and inserts it into the view. - */ - async ngAfterViewInit() { - await super.ngAfterViewInit(); - - const typedComponent = this.typedPolicyComponent; - if (typedComponent) { - this.saveDisabled$ = combineLatest([ - this.autoConfirmEnabled$, - typedComponent.enabled.valueChanges.pipe(startWith(typedComponent.enabled.value)), - ]).pipe(map(([policyEnabled, value]) => !policyEnabled && !value)); - } - - this.multiStepSubmit = this.accountService.activeAccount$.pipe( - getUserId, - switchMap((userId) => this.policyService.policies$(userId)), - map((policies) => policies.find((p) => p.type === PolicyType.SingleOrg)?.enabled ?? false), - tap((singleOrgPolicyEnabled) => - this.typedPolicyComponent?.setSingleOrgEnabled(singleOrgPolicyEnabled), - ), - switchMap((singleOrgPolicyEnabled) => this.buildMultiStepSubmit(singleOrgPolicyEnabled)), - shareReplay({ bufferSize: 1, refCount: true }), - ); - } - - private buildMultiStepSubmit(singleOrgPolicyEnabled: boolean): Observable { - return this.managePoliciesOnly$.pipe( - map((managePoliciesOnly) => { - const submitSteps = [ - { - sideEffect: () => this.handleSubmit(singleOrgPolicyEnabled ?? false), - footerContent: this.submitPolicy, - titleContent: this.submitPolicyTitle, - }, - ]; - - if (!managePoliciesOnly) { - submitSteps.push({ - sideEffect: () => this.openBrowserExtension(), - footerContent: this.openExtension, - titleContent: this.openExtensionTitle, - }); - } - return submitSteps; - }), - ); - } - - private async handleSubmit(singleOrgEnabled: boolean) { - const enabledSingleOrgDuringAction = !singleOrgEnabled; - - if (enabledSingleOrgDuringAction) { - await this.setSingleOrgPolicy(true); - } - - try { - await this.submitAutoConfirm(); - } catch (error) { - // Roll back SingleOrg if we enabled it during this action - if (enabledSingleOrgDuringAction) { - await this.setSingleOrgPolicy(false); - } - throw error; - } - } - - /** - * Triggers policy submission for auto confirm. - * @returns boolean: true if multi-submit workflow should continue, false otherwise. - */ - private async submitAutoConfirm() { - const typedComponent = this.typedPolicyComponent; - if (!typedComponent) { - throw new Error("PolicyComponent not initialized."); - } - - const autoConfirmRequest = await typedComponent.buildRequest(); - - await this.policyApiService.putPolicyVNext(this.data.organization.id, this.data.policy.type, { - policy: autoConfirmRequest, - }); - - const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); - - const currentAutoConfirmState = await firstValueFrom( - this.autoConfirmService.configuration$(userId), - ); - - await this.autoConfirmService.upsert(userId, { - ...currentAutoConfirmState, - showSetupDialog: false, - }); - - this.toastService.showToast({ - variant: "success", - message: this.i18nService.t("editedPolicyId", this.i18nService.t(this.data.policy.name)), - }); - - if (!typedComponent.enabled.value) { - this.dialogRef.close("saved"); - } - } - - private async setSingleOrgPolicy(enabled: boolean): Promise { - const singleOrgRequest: PolicyRequest = { - enabled, - data: null, - }; - - await this.policyApiService.putPolicyVNext(this.data.organization.id, PolicyType.SingleOrg, { - policy: singleOrgRequest, - }); - } - - private async openBrowserExtension() { - await this.router.navigate(["/browser-extension-prompt"], { - queryParams: { url: "AutoConfirm" }, - }); - } - - submit = async () => { - const typedComponent = this.typedPolicyComponent; - if (!typedComponent) { - throw new Error("PolicyComponent not initialized."); - } - - if ((await typedComponent.confirm()) == false) { - this.dialogRef.close(); - return; - } - - try { - const multiStepSubmit = await firstValueFrom(this.multiStepSubmit); - const sideEffect = multiStepSubmit[this.currentStep()].sideEffect; - if (sideEffect) { - await sideEffect(); - } - - if (this.currentStep() === multiStepSubmit.length - 1) { - this.dialogRef.close("saved"); - return; - } - - this.currentStep.update((value) => value + 1); - typedComponent.setStep(this.currentStep()); - } catch (error: any) { - this.toastService.showToast({ - variant: "error", - message: error.message, - }); - } - }; - - static open = ( - dialogService: DialogService, - config: DialogConfig, - ) => { - return dialogService.open(AutoConfirmPolicyDialogComponent, config); - }; -} diff --git a/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/index.ts b/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/index.ts index 307d0da04b0..8670b2e7a37 100644 --- a/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/index.ts +++ b/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/index.ts @@ -1,3 +1,2 @@ -export * from "./auto-confirm-edit-policy-dialog.component"; -export * from "./organization-data-ownership-edit-policy-dialog.component"; +export * from "./multi-step-policy-edit-dialog.component"; export * from "./models"; diff --git a/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/models.ts b/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/models.ts index 86120623701..7c15536b4ae 100644 --- a/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/models.ts +++ b/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/models.ts @@ -1,7 +1,24 @@ import { Signal, TemplateRef } from "@angular/core"; +import { Observable } from "rxjs"; -export type MultiStepSubmit = { - sideEffect?: () => Promise; - footerContent: Signal | undefined>; - titleContent: Signal | undefined>; +export type PolicyStepResult = { closeDialog?: boolean } | undefined; + +export type PolicyStep = { + // Side effect to execute when submitting this step. + // Return { closeDialog: true } to show the success toast and close the dialog immediately, + // bypassing any remaining steps. Useful when a step conditionally ends the workflow early + // (e.g. when disabling a policy or when the user lacks permission to see subsequent steps). + sideEffect?: () => Promise; + + // Optional: Custom title template. If undefined, uses default: "Edit policy" with policy name subtitle + titleContent?: Signal | undefined>; + + // Optional: Custom body template. If undefined, renders the policy component's template + bodyContent?: Signal | undefined>; + + // Optional: Custom footer template. If undefined, uses default: "Save" (primary) + "Cancel" (secondary) + footerContent?: Signal | undefined>; + + // Optional: Observable to disable save button. If undefined, defaults to form validation state + disableSave?: Observable; }; diff --git a/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/multi-step-policy-edit-dialog.component.html b/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/multi-step-policy-edit-dialog.component.html new file mode 100644 index 00000000000..fb93a828972 --- /dev/null +++ b/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/multi-step-policy-edit-dialog.component.html @@ -0,0 +1,47 @@ +
+ @let titleTemplate = policySteps()[currentStep()]?.titleContent?.(); + + + @if (titleTemplate) { + + } + + + + @if (policy.showDescription) { +

{{ policy.description | i18n }}

+ } + + @let bodyTemplate = policySteps()[currentStep()]?.bodyContent?.(); + @if (bodyTemplate) { + + } + + + + + + @let footerTemplate = policySteps()[currentStep()]?.footerContent?.(); + @if (footerTemplate) { + + } @else { + + + } + + +
diff --git a/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/multi-step-policy-edit-dialog.component.spec.ts b/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/multi-step-policy-edit-dialog.component.spec.ts new file mode 100644 index 00000000000..b74f2f6b6ca --- /dev/null +++ b/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/multi-step-policy-edit-dialog.component.spec.ts @@ -0,0 +1,189 @@ +import { NO_ERRORS_SCHEMA } from "@angular/core"; +import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { ReactiveFormsModule, UntypedFormGroup } from "@angular/forms"; +import { MockProxy, mock } from "jest-mock-extended"; +import { of } from "rxjs"; + +import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { DIALOG_DATA, DialogRef, ToastService } from "@bitwarden/components"; +import { KeyService } from "@bitwarden/key-management"; + +import { BasePolicyEditComponent, BasePolicyEditDefinition } from "../base-policy-edit.component"; +import { PolicyEditDialogData, PolicyEditDialogResult } from "../policy-edit-dialog.component"; + +import { PolicyStep } from "./models"; +import { MultiStepPolicyEditDialogComponent } from "./multi-step-policy-edit-dialog.component"; + +describe("MultiStepPolicyEditDialogComponent", () => { + let component: MultiStepPolicyEditDialogComponent; + let fixture: ComponentFixture; + let toastService: MockProxy; + let i18nService: MockProxy; + let dialogRef: MockProxy>; + let policyComponent: MockProxy; + + const dialogData: PolicyEditDialogData = { + policy: { + name: "testPolicy", + description: "testDesc", + type: 0, + component: class {} as any, + showDescription: true, + display$: () => of(true), + } as BasePolicyEditDefinition, + organizationId: "org-1", + }; + + beforeEach(async () => { + toastService = mock(); + i18nService = mock(); + i18nService.t.mockReturnValue("translated"); + dialogRef = mock>(); + policyComponent = mock(); + + await TestBed.configureTestingModule({ + imports: [ReactiveFormsModule], + providers: [ + { provide: DIALOG_DATA, useValue: dialogData }, + { provide: AccountService, useValue: mock() }, + { provide: PolicyApiServiceAbstraction, useValue: mock() }, + { provide: I18nService, useValue: i18nService }, + { provide: DialogRef, useValue: dialogRef }, + { provide: ToastService, useValue: toastService }, + { provide: KeyService, useValue: mock() }, + ], + schemas: [NO_ERRORS_SCHEMA], + }).compileComponents(); + + fixture = TestBed.createComponent(MultiStepPolicyEditDialogComponent); + component = fixture.componentInstance; + // Intentionally skip detectChanges() to avoid triggering ngAfterViewInit, + // which calls load() and policyFormViewRef() in the real component. + }); + + /** Sets up the component state as if ngAfterViewInit had run with the given steps. */ + function setupSteps(steps: PolicyStep[]) { + (component as any).policySteps.set(steps); + (component as any).policyComponent.set(policyComponent); + } + + describe("submit()", () => { + it("throws when policyComponent is not initialized", async () => { + await expect(component.submit()).rejects.toThrow("PolicyComponent not initialized."); + }); + + it("advances to next step when side effect returns undefined on a non-last step", async () => { + const sideEffect0 = jest.fn().mockResolvedValue(undefined); + setupSteps([{ sideEffect: sideEffect0 }, {}]); + + await component.submit(); + + expect(component.currentStep()).toBe(1); + expect(dialogRef.close).not.toHaveBeenCalled(); + }); + + it("closes dialog with success toast when side effect resolves on the last step", async () => { + const sideEffect = jest.fn().mockResolvedValue(undefined); + setupSteps([{ sideEffect }]); + + await component.submit(); + + expect(toastService.showToast).toHaveBeenCalledWith( + expect.objectContaining({ variant: "success" }), + ); + expect(dialogRef.close).toHaveBeenCalledWith("saved"); + }); + + it("closes dialog immediately when side effect returns { closeDialog: true } on a non-last step", async () => { + const sideEffect0 = jest.fn().mockResolvedValue({ closeDialog: true }); + const sideEffect1 = jest.fn().mockResolvedValue(undefined); + setupSteps([{ sideEffect: sideEffect0 }, { sideEffect: sideEffect1 }]); + + await component.submit(); + + expect(dialogRef.close).toHaveBeenCalledWith("saved"); + expect(toastService.showToast).toHaveBeenCalledWith( + expect.objectContaining({ variant: "success" }), + ); + // Step was not advanced since we closed early + expect(component.currentStep()).toBe(0); + // Subsequent side effect was never invoked + expect(sideEffect1).not.toHaveBeenCalled(); + }); + + it("shows error toast and does not advance step when side effect throws", async () => { + const error = new Error("Save failed"); + setupSteps([{ sideEffect: jest.fn().mockRejectedValue(error) }, {}]); + + await component.submit(); + + expect(toastService.showToast).toHaveBeenCalledWith( + expect.objectContaining({ variant: "error", message: "Save failed" }), + ); + expect(component.currentStep()).toBe(0); + expect(dialogRef.close).not.toHaveBeenCalled(); + }); + + it("advances step on a non-last step when no side effect is defined", async () => { + setupSteps([{}, {}]); + + await component.submit(); + + expect(component.currentStep()).toBe(1); + expect(dialogRef.close).not.toHaveBeenCalled(); + }); + + it("closes dialog with success toast on the last step when no side effect is defined", async () => { + setupSteps([{}]); + + await component.submit(); + + expect(toastService.showToast).toHaveBeenCalledWith( + expect.objectContaining({ variant: "success" }), + ); + expect(dialogRef.close).toHaveBeenCalledWith("saved"); + }); + }); + + describe("saveDisabled signal", () => { + // These tests set state directly via setupSteps() and use TestBed.flushEffects() to + // propagate signal changes. This avoids detectChanges(), which would trigger the async + // ngAfterViewInit and its createComponent() call against the bare (undecorated) test class. + + it("is true when the current step's disableSave observable emits true", () => { + setupSteps([{ disableSave: of(true) }]); + TestBed.flushEffects(); + + expect((component as any).saveDisabled()).toBe(true); + }); + + it("is false when step has no disableSave and policyComponent has no data", () => { + policyComponent.data = undefined; + setupSteps([{}]); + TestBed.flushEffects(); + + expect((component as any).saveDisabled()).toBe(false); + }); + + it("is false when step has no disableSave and the data form is valid", () => { + policyComponent.data = new UntypedFormGroup({}); + setupSteps([{}]); + TestBed.flushEffects(); + + expect((component as any).saveDisabled()).toBe(false); + }); + + it("reflects the new step's disableSave after advancing to the next step", () => { + policyComponent.data = undefined; + setupSteps([{}, { disableSave: of(true) }]); + TestBed.flushEffects(); + + component.currentStep.set(1); + TestBed.flushEffects(); + + expect((component as any).saveDisabled()).toBe(true); + }); + }); +}); diff --git a/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/multi-step-policy-edit-dialog.component.ts b/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/multi-step-policy-edit-dialog.component.ts new file mode 100644 index 00000000000..0818db27139 --- /dev/null +++ b/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/multi-step-policy-edit-dialog.component.ts @@ -0,0 +1,166 @@ +import { + AfterViewInit, + ChangeDetectionStrategy, + ChangeDetectorRef, + Component, + Inject, + Signal, + ViewContainerRef, + WritableSignal, + computed, + signal, + viewChild, +} from "@angular/core"; +import { toObservable, toSignal } from "@angular/core/rxjs-interop"; +import { FormBuilder } from "@angular/forms"; +import { map, of, startWith, switchMap } from "rxjs"; + +import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { + DIALOG_DATA, + DialogConfig, + DialogRef, + DialogService, + ToastService, +} from "@bitwarden/components"; +import { KeyService } from "@bitwarden/key-management"; + +import { SharedModule } from "../../../../shared"; +import { + PolicyEditDialogComponent, + PolicyEditDialogData, + PolicyEditDialogResult, +} from "../policy-edit-dialog.component"; + +import { PolicyStep } from "./models"; + +@Component({ + selector: "app-multi-step-policy-edit-dialog", + templateUrl: "multi-step-policy-edit-dialog.component.html", + imports: [SharedModule], + changeDetection: ChangeDetectionStrategy.OnPush, +}) +export class MultiStepPolicyEditDialogComponent + extends PolicyEditDialogComponent + implements AfterViewInit +{ + private readonly policyFormViewRef: Signal = viewChild( + "policyForm", + { read: ViewContainerRef }, + ); + + protected readonly policySteps: WritableSignal = signal([]); + readonly currentStep: WritableSignal = signal(0); + + private readonly currentStepConfig = computed(() => this.policySteps()[this.currentStep()]); + + protected readonly saveDisabled = toSignal( + toObservable(this.currentStepConfig).pipe( + switchMap((stepConfig) => { + if (stepConfig?.disableSave) { + return stepConfig.disableSave; + } + const policyComponent = this.policyComponent(); + if (policyComponent?.data) { + return policyComponent.data.statusChanges.pipe( + startWith(policyComponent.data.status), + map((status) => status !== "VALID"), + ); + } + return of(false); + }), + ), + { initialValue: false }, + ); + + constructor( + @Inject(DIALOG_DATA) data: PolicyEditDialogData, + accountService: AccountService, + policyApiService: PolicyApiServiceAbstraction, + i18nService: I18nService, + changeDetectorRef: ChangeDetectorRef, + formBuilder: FormBuilder, + dialogRef: DialogRef, + toastService: ToastService, + keyService: KeyService, + ) { + super( + data, + accountService, + policyApiService, + i18nService, + changeDetectorRef, + formBuilder, + dialogRef, + toastService, + keyService, + ); + } + + override async ngAfterViewInit() { + const policyResponse = await this.load(); + this.loading.set(false); + + const policyFormRef = this.policyFormViewRef(); + if (!policyFormRef) { + throw new Error("Template not initialized."); + } + + // Create the policy component instance + const componentRef = policyFormRef.createComponent(this.data.policy.component); + componentRef.setInput("policyResponse", policyResponse); + componentRef.setInput("policy", this.data.policy); + componentRef.setInput("currentStep", this.currentStep); + componentRef.setInput("organizationId", this.data.organization.id); + const component = componentRef.instance; + this.policyComponent.set(component); + + // Read step configuration from child component. + // Setting policySteps triggers currentStepConfig to recompute, which re-evaluates saveDisabled. + this.policySteps.set(component.policySteps ?? []); + } + + override readonly submit = async () => { + if (!this.policyComponent()) { + throw new Error("PolicyComponent not initialized."); + } + + try { + // Execute side effect for current step (if defined) + const sideEffect = this.policySteps()[this.currentStep()]?.sideEffect; + const result = sideEffect ? await sideEffect() : undefined; + + // A sideEffect can return { closeDialog: true } to end the workflow early + // (e.g. when disabling a policy or for users without permission to see later steps). + const isLastStep = this.currentStep() === this.policySteps().length - 1; + if (isLastStep || (typeof result === "object" && result.closeDialog)) { + this.toastService.showToast({ + variant: "success", + message: this.i18nService.t("editedPolicyId", this.i18nService.t(this.data.policy.name)), + }); + this.dialogRef.close("saved"); + return; + } + + // Not the last step - advance to next step + this.currentStep.update((value) => value + 1); + } catch (error: any) { + this.toastService.showToast({ + variant: "error", + message: error.message, + }); + } + }; + + static override readonly open = ( + dialogService: DialogService, + config: DialogConfig, + ) => { + return dialogService.open( + MultiStepPolicyEditDialogComponent, + config, + ); + }; +} diff --git a/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/organization-data-ownership-edit-policy-dialog.component.html b/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/organization-data-ownership-edit-policy-dialog.component.html deleted file mode 100644 index 9a74db6554d..00000000000 --- a/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/organization-data-ownership-edit-policy-dialog.component.html +++ /dev/null @@ -1,55 +0,0 @@ -
- - - @let title = multiStepSubmit()[currentStep()]?.titleContent(); - @if (title) { - - } - - - - @if (loading()) { -
- - {{ "loading" | i18n }} -
- } -
- @if (policy.showDescription) { -

{{ policy.description | i18n }}

- } -
- - - - @let footer = multiStepSubmit()[currentStep()]?.footerContent(); - @if (footer) { - - } - - -
- - - {{ policy.name | i18n }} - - - - - - - diff --git a/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/organization-data-ownership-edit-policy-dialog.component.ts b/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/organization-data-ownership-edit-policy-dialog.component.ts deleted file mode 100644 index a911d510cb1..00000000000 --- a/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/organization-data-ownership-edit-policy-dialog.component.ts +++ /dev/null @@ -1,212 +0,0 @@ -import { - AfterViewInit, - ChangeDetectorRef, - Component, - Inject, - signal, - TemplateRef, - viewChild, - WritableSignal, -} from "@angular/core"; -import { FormBuilder } from "@angular/forms"; -import { - catchError, - combineLatest, - defer, - firstValueFrom, - from, - map, - Observable, - of, - startWith, - switchMap, -} from "rxjs"; - -import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; -import { PolicyType } from "@bitwarden/common/admin-console/enums"; -import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; -import { getUserId } from "@bitwarden/common/auth/services/account.service"; -import { assertNonNullish } from "@bitwarden/common/auth/utils"; -import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; -import { - DIALOG_DATA, - DialogConfig, - DialogRef, - DialogService, - ToastService, -} from "@bitwarden/components"; -import { KeyService } from "@bitwarden/key-management"; - -import { SharedModule } from "../../../../shared"; -import { vNextOrganizationDataOwnershipPolicyComponent } from "../policy-edit-definitions"; -import { - PolicyEditDialogComponent, - PolicyEditDialogData, - PolicyEditDialogResult, -} from "../policy-edit-dialog.component"; - -import { MultiStepSubmit } from "./models"; - -/** - * Custom policy dialog component for Centralize Organization Data - * Ownership policy. Satisfies the PolicyDialogComponent interface - * structurally via its static open() function. - */ -// FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush -// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection -@Component({ - templateUrl: "organization-data-ownership-edit-policy-dialog.component.html", - imports: [SharedModule], -}) -export class OrganizationDataOwnershipPolicyDialogComponent - extends PolicyEditDialogComponent - implements AfterViewInit -{ - policyType = PolicyType; - - protected centralizeDataOwnershipEnabled$: Observable = defer(() => - from( - this.policyApiService.getPolicy( - this.data.organization.id, - PolicyType.OrganizationDataOwnership, - ), - ).pipe( - map((policy) => policy.enabled), - catchError(() => of(false)), - ), - ); - - protected readonly currentStep: WritableSignal = signal(0); - protected readonly multiStepSubmit: WritableSignal = signal([]); - - private readonly policyForm = viewChild.required>("step0"); - private readonly policyFormTitle = viewChild.required>("step0Title"); - - protected saveDisabled$: Observable | undefined; - - private get typedPolicyComponent(): vNextOrganizationDataOwnershipPolicyComponent | undefined { - const component = this.policyComponent(); - return component instanceof vNextOrganizationDataOwnershipPolicyComponent - ? component - : undefined; - } - - constructor( - @Inject(DIALOG_DATA) protected data: PolicyEditDialogData, - accountService: AccountService, - policyApiService: PolicyApiServiceAbstraction, - i18nService: I18nService, - cdr: ChangeDetectorRef, - formBuilder: FormBuilder, - dialogRef: DialogRef, - toastService: ToastService, - protected keyService: KeyService, - ) { - super( - data, - accountService, - policyApiService, - i18nService, - cdr, - formBuilder, - dialogRef, - toastService, - keyService, - ); - } - - async ngAfterViewInit() { - await super.ngAfterViewInit(); - - const typedComponent = this.typedPolicyComponent; - if (typedComponent) { - this.saveDisabled$ = combineLatest([ - this.centralizeDataOwnershipEnabled$, - typedComponent.enabled.valueChanges.pipe(startWith(typedComponent.enabled.value)), - ]).pipe(map(([policyEnabled, value]) => !policyEnabled && !value)); - } - - this.multiStepSubmit.set(this.buildMultiStepSubmit()); - } - - private buildMultiStepSubmit(): MultiStepSubmit[] { - return [ - { - sideEffect: () => this.handleSubmit(), - footerContent: this.policyForm, - titleContent: this.policyFormTitle, - }, - ]; - } - - private async handleSubmit() { - const typedComponent = this.typedPolicyComponent; - if (!typedComponent) { - throw new Error("PolicyComponent not initialized."); - } - - const orgKey = await firstValueFrom( - this.accountService.activeAccount$.pipe( - getUserId, - switchMap((userId) => this.keyService.orgKeys$(userId)), - ), - ); - - assertNonNullish(orgKey, "Org key not provided"); - - const request = await typedComponent.buildVNextRequest(orgKey[this.data.organization.id]); - - await this.policyApiService.putPolicyVNext( - this.data.organization.id, - this.data.policy.type, - request, - ); - - this.toastService.showToast({ - variant: "success", - message: this.i18nService.t("editedPolicyId", this.i18nService.t(this.data.policy.name)), - }); - - if (!typedComponent.enabled.value) { - this.dialogRef.close("saved"); - } - } - - submit = async () => { - const typedComponent = this.typedPolicyComponent; - if (!typedComponent) { - throw new Error("PolicyComponent not initialized."); - } - - if ((await typedComponent.confirm()) == false) { - this.dialogRef.close(); - return; - } - - try { - const sideEffect = this.multiStepSubmit()[this.currentStep()].sideEffect; - if (sideEffect) { - await sideEffect(); - } - - if (this.currentStep() === this.multiStepSubmit().length - 1) { - this.dialogRef.close("saved"); - return; - } - - this.currentStep.update((value) => value + 1); - } catch (error: any) { - this.toastService.showToast({ - variant: "error", - message: error.message, - }); - } - }; - - static open = (dialogService: DialogService, config: DialogConfig) => { - return dialogService.open( - OrganizationDataOwnershipPolicyDialogComponent, - config, - ); - }; -} diff --git a/apps/web/src/app/vault/services/web-vault-extension-prompt.service.ts b/apps/web/src/app/vault/services/web-vault-extension-prompt.service.ts index 3e13935f94c..7f570a88f9d 100644 --- a/apps/web/src/app/vault/services/web-vault-extension-prompt.service.ts +++ b/apps/web/src/app/vault/services/web-vault-extension-prompt.service.ts @@ -93,8 +93,14 @@ export class WebVaultExtensionPromptService { ); const now = new Date(); - const accountAgeMs = now.getTime() - creationDate.getTime(); - const accountAgeDays = accountAgeMs / (1000 * 60 * 60 * 24); + // Use UTC midnight boundaries to avoid DST skewing the day count. + const nowUtcDay = Date.UTC(now.getFullYear(), now.getMonth(), now.getDate()); + const creationUtcDay = Date.UTC( + creationDate.getFullYear(), + creationDate.getMonth(), + creationDate.getDate(), + ); + const accountAgeDays = (nowUtcDay - creationUtcDay) / (1000 * 60 * 60 * 24); const minAgeDays = minAccountAgeDays ?? 0; const maxAgeDays = 30; diff --git a/apps/web/src/app/vault/services/web-vault-prompt.service.spec.ts b/apps/web/src/app/vault/services/web-vault-prompt.service.spec.ts index fac78788a21..11931392bbc 100644 --- a/apps/web/src/app/vault/services/web-vault-prompt.service.spec.ts +++ b/apps/web/src/app/vault/services/web-vault-prompt.service.spec.ts @@ -15,7 +15,7 @@ import { LogService } from "@bitwarden/logging"; import { VaultItemsTransferService } from "@bitwarden/vault"; import { - AutoConfirmPolicyDialogComponent, + MultiStepPolicyEditDialogComponent, PolicyEditDialogResult, } from "../../admin-console/organizations/policies"; import { UnifiedUpgradePromptService } from "../../billing/individual/upgrade/services"; @@ -137,7 +137,7 @@ describe("WebVaultPromptService", () => { } as unknown as DialogRef; const openSpy = jest - .spyOn(AutoConfirmPolicyDialogComponent, "open") + .spyOn(MultiStepPolicyEditDialogComponent, "open") .mockReturnValue(dialogRefMock); void service.conditionallyPromptUser(); @@ -148,10 +148,12 @@ describe("WebVaultPromptService", () => { data: { policy: expect.any(Object), organization: expect.objectContaining({ id: mockOrganizationId }), - firstTimeDialog: true, }, }); + const passedPolicy = openSpy.mock.calls[0][1].data.policy; + expect(passedPolicy.firstTimeDialog).toBe(true); + dialogClosedSubject.next(null); })); @@ -167,7 +169,7 @@ describe("WebVaultPromptService", () => { } as Organization; organizations$.mockReturnValueOnce(of([mockOrg])); - const openSpy = jest.spyOn(AutoConfirmPolicyDialogComponent, "open"); + const openSpy = jest.spyOn(MultiStepPolicyEditDialogComponent, "open"); void service.conditionallyPromptUser(); @@ -193,7 +195,7 @@ describe("WebVaultPromptService", () => { } as Organization; organizations$.mockReturnValueOnce(of([mockOrg])); - const openSpy = jest.spyOn(AutoConfirmPolicyDialogComponent, "open"); + const openSpy = jest.spyOn(MultiStepPolicyEditDialogComponent, "open"); void service.conditionallyPromptUser(); @@ -214,7 +216,7 @@ describe("WebVaultPromptService", () => { } as Organization; organizations$.mockReturnValueOnce(of([mockOrg])); - const openSpy = jest.spyOn(AutoConfirmPolicyDialogComponent, "open"); + const openSpy = jest.spyOn(MultiStepPolicyEditDialogComponent, "open"); void service.conditionallyPromptUser(); @@ -231,7 +233,7 @@ describe("WebVaultPromptService", () => { policies$.mockReturnValueOnce(of([])); organizations$.mockReturnValueOnce(of([])); - const openSpy = jest.spyOn(AutoConfirmPolicyDialogComponent, "open"); + const openSpy = jest.spyOn(MultiStepPolicyEditDialogComponent, "open"); void service.conditionallyPromptUser(); @@ -254,7 +256,7 @@ describe("WebVaultPromptService", () => { organizations$.mockReturnValueOnce(of([mockOrg])); - const openSpy = jest.spyOn(AutoConfirmPolicyDialogComponent, "open"); + const openSpy = jest.spyOn(MultiStepPolicyEditDialogComponent, "open"); void service.conditionallyPromptUser(); diff --git a/apps/web/src/app/vault/services/web-vault-prompt.service.ts b/apps/web/src/app/vault/services/web-vault-prompt.service.ts index fb7726faf23..fd42d116d14 100644 --- a/apps/web/src/app/vault/services/web-vault-prompt.service.ts +++ b/apps/web/src/app/vault/services/web-vault-prompt.service.ts @@ -15,8 +15,8 @@ import { LogService } from "@bitwarden/logging"; import { VaultItemsTransferService } from "@bitwarden/vault"; import { - AutoConfirmPolicyDialogComponent, AutoConfirmPolicy, + MultiStepPolicyEditDialogComponent, } from "../../admin-console/organizations/policies"; import { UnifiedUpgradePromptService } from "../../billing/individual/upgrade/services"; @@ -64,12 +64,11 @@ export class WebVaultPromptService { this.checkForAutoConfirm(); } - private async openAutoConfirmFeatureDialog(organization: Organization) { - AutoConfirmPolicyDialogComponent.open(this.dialogService, { + private openAutoConfirmFeatureDialog(organization: Organization) { + MultiStepPolicyEditDialogComponent.open(this.dialogService, { data: { - policy: new AutoConfirmPolicy(), + policy: new AutoConfirmPolicy(true), organization: organization, - firstTimeDialog: true, }, }); } @@ -108,7 +107,7 @@ export class WebVaultPromptService { organization.canEnableAutoConfirmPolicy; if (showDialog) { - await this.openAutoConfirmFeatureDialog(organization); + this.openAutoConfirmFeatureDialog(organization); await this.autoConfirmService.upsert(userId, { ...autoConfirmState, diff --git a/bitwarden_license/bit-web/src/app/key-management/policies/session-timeout.component.spec.ts b/bitwarden_license/bit-web/src/app/key-management/policies/session-timeout.component.spec.ts index 322d383434a..0339492cfa3 100644 --- a/bitwarden_license/bit-web/src/app/key-management/policies/session-timeout.component.spec.ts +++ b/bitwarden_license/bit-web/src/app/key-management/policies/session-timeout.component.spec.ts @@ -6,7 +6,9 @@ import { By } from "@angular/platform-browser"; import { mock } from "jest-mock-extended"; import { Observable, of } from "rxjs"; +import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; import { PolicyResponse } from "@bitwarden/common/admin-console/models/response/policy.response"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { SessionTimeoutAction, SessionTimeoutType, @@ -14,6 +16,7 @@ import { import { VaultTimeoutAction } from "@bitwarden/common/key-management/vault-timeout"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { DialogRef, DialogService } from "@bitwarden/components"; +import { KeyService } from "@bitwarden/key-management"; import { SessionTimeoutConfirmationNeverComponent } from "./session-timeout-confirmation-never.component"; import { SessionTimeoutPolicyComponent } from "./session-timeout.component"; @@ -47,7 +50,13 @@ describe("SessionTimeoutPolicyComponent", () => { const testBed = TestBed.configureTestingModule({ imports: [SessionTimeoutPolicyComponent, ReactiveFormsModule], - providers: [FormBuilder, { provide: I18nService, useValue: mockI18nService }], + providers: [ + FormBuilder, + { provide: I18nService, useValue: mockI18nService }, + { provide: AccountService, useValue: mock() }, + { provide: KeyService, useValue: mock() }, + { provide: PolicyApiServiceAbstraction, useValue: mock() }, + ], }); // Override DialogService provided from SharedModule (which includes DialogModule) @@ -80,13 +89,16 @@ describe("SessionTimeoutPolicyComponent", () => { } function setPolicyResponseType(type: SessionTimeoutType) { - component.policyResponse = new PolicyResponse({ - Data: { - type, - minutes: 480, - action: null, - }, - }); + fixture.componentRef.setInput( + "policyResponse", + new PolicyResponse({ + Data: { + type, + minutes: 480, + action: null, + }, + }), + ); } describe("initialization and data loading", () => { @@ -104,7 +116,7 @@ describe("SessionTimeoutPolicyComponent", () => { } it("should initialize with default state when policy have no value", () => { - component.policyResponse = undefined; + fixture.componentRef.setInput("policyResponse", undefined); fixture.detectChanges(); @@ -122,12 +134,15 @@ describe("SessionTimeoutPolicyComponent", () => { // This is for backward compatibility when type field did not exist it("should load as custom type when type field does not exist but minutes does", () => { - component.policyResponse = new PolicyResponse({ - Data: { - minutes: 500, - action: VaultTimeoutAction.Lock, - }, - }); + fixture.componentRef.setInput( + "policyResponse", + new PolicyResponse({ + Data: { + minutes: 500, + action: VaultTimeoutAction.Lock, + }, + }), + ); fixture.detectChanges(); @@ -159,13 +174,16 @@ describe("SessionTimeoutPolicyComponent", () => { ["custom", VaultTimeoutAction.Lock], ["custom", VaultTimeoutAction.LogOut], ])("should load correctly when policy type is %s and action is %s", (type, action) => { - component.policyResponse = new PolicyResponse({ - Data: { - type, - minutes: 510, - action, - }, - }); + fixture.componentRef.setInput( + "policyResponse", + new PolicyResponse({ + Data: { + type, + minutes: 510, + action, + }, + }), + ); fixture.detectChanges(); diff --git a/bitwarden_license/bit-web/src/app/key-management/policies/session-timeout.component.ts b/bitwarden_license/bit-web/src/app/key-management/policies/session-timeout.component.ts index 39be5f48c1d..c279ed63ea6 100644 --- a/bitwarden_license/bit-web/src/app/key-management/policies/session-timeout.component.ts +++ b/bitwarden_license/bit-web/src/app/key-management/policies/session-timeout.component.ts @@ -166,7 +166,7 @@ export class SessionTimeoutPolicyComponent } private get policyData(): MaximumSessionTimeoutPolicyData | null { - return this.policyResponse?.data ?? null; + return this.policyResponse()?.data ?? null; } private async confirmTypeChange(newType: SessionTimeoutType): Promise {