[PM-15559] Fix hide passwords in AC for users that have view, except password (#12252)

* [PM-15559] Update admin dialog flows to first try the local state before using the API when not required

* [PM-15559] Clear initial values after creating a new cipher so that they do not override the newly created cipher for subsequent edits

apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.ts

@@ -281,17 +281,19 @@ export class VaultItemDialogComponent implements OnInit, OnDestroy {
     // If the cipher was newly created (via add/clone), switch the form to edit for subsequent edits.
     if (this._originalFormMode === "add" || this._originalFormMode === "clone") {
       this.formConfig.mode = "edit";
+      this.formConfig.initialValues = null;
     }
 
-    let cipher: Cipher;
+    let cipher = await this.cipherService.get(cipherView.id);
 
-    // When the form config is used within the Admin Console, retrieve the cipher from the admin endpoint
+    // When the form config is used within the Admin Console, retrieve the cipher from the admin endpoint (if not found in local state)
-    if (this.formConfig.isAdminConsole) {
+    if (this.formConfig.isAdminConsole && (cipher == null || this.formConfig.admin)) {
       const cipherResponse = await this.apiService.getCipherAdmin(cipherView.id);
+      cipherResponse.edit = true;
+      cipherResponse.viewPassword = true;
+
       const cipherData = new CipherData(cipherResponse);
       cipher = new Cipher(cipherData);
-    } else {
-      cipher = await this.cipherService.get(cipherView.id);
     }
 
     // Store the updated cipher so any following edits use the most up to date cipher

apps/web/src/app/vault/org-vault/services/admin-console-cipher-form-config.service.spec.ts

@@ -8,6 +8,7 @@ import { PolicyService } from "@bitwarden/common/admin-console/abstractions/poli
 import { OrganizationUserStatusType } from "@bitwarden/common/admin-console/enums";
 import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
 import { CipherId } from "@bitwarden/common/types/guid";
+import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
 
 import { RoutedVaultFilterService } from "../../individual-vault/vault-filter/services/routed-vault-filter.service";
 
@@ -52,12 +53,16 @@ describe("AdminConsoleCipherFormConfigService", () => {
   const organization$ = new BehaviorSubject<Organization>(testOrg as Organization);
   const organizations$ = new BehaviorSubject<Organization[]>([testOrg, testOrg2] as Organization[]);
   const getCipherAdmin = jest.fn().mockResolvedValue(null);
+  const getCipher = jest.fn().mockResolvedValue(null);
 
   beforeEach(async () => {
     getCipherAdmin.mockClear();
     getCipherAdmin.mockResolvedValue({ id: cipherId, name: "Test Cipher - (admin)" });
 
-    await TestBed.configureTestingModule({
+    getCipher.mockClear();
+    getCipher.mockResolvedValue({ id: cipherId, name: "Test Cipher" });
+
+    TestBed.configureTestingModule({
       providers: [
         AdminConsoleCipherFormConfigService,
         { provide: OrganizationService, useValue: { get$: () => organization$, organizations$ } },
@@ -74,14 +79,14 @@ describe("AdminConsoleCipherFormConfigService", () => {
           useValue: { filter$: new BehaviorSubject({ organizationId: testOrg.id }) },
         },
         { provide: ApiService, useValue: { getCipherAdmin } },
+        { provide: CipherService, useValue: { get: getCipher } },
       ],
     });
+    adminConsoleConfigService = TestBed.inject(AdminConsoleCipherFormConfigService);
   });
 
   describe("buildConfig", () => {
     it("sets individual attributes", async () => {
-      adminConsoleConfigService = TestBed.inject(AdminConsoleCipherFormConfigService);
-
       const { folders, hideIndividualVaultFields } = await adminConsoleConfigService.buildConfig(
         "add",
         cipherId,
@@ -92,8 +97,6 @@ describe("AdminConsoleCipherFormConfigService", () => {
     });
 
     it("sets mode based on passed mode", async () => {
-      adminConsoleConfigService = TestBed.inject(AdminConsoleCipherFormConfigService);
-
       const { mode } = await adminConsoleConfigService.buildConfig("edit", cipherId);
 
       expect(mode).toBe("edit");
@@ -122,8 +125,6 @@ describe("AdminConsoleCipherFormConfigService", () => {
     });
 
     it("sets `allowPersonalOwnership`", async () => {
-      adminConsoleConfigService = TestBed.inject(AdminConsoleCipherFormConfigService);
-
       policyAppliesToActiveUser$.next(true);
 
       let result = await adminConsoleConfigService.buildConfig("clone", cipherId);
@@ -138,8 +139,6 @@ describe("AdminConsoleCipherFormConfigService", () => {
     });
 
     it("disables personal ownership when not cloning", async () => {
-      adminConsoleConfigService = TestBed.inject(AdminConsoleCipherFormConfigService);
-
       policyAppliesToActiveUser$.next(false);
 
       let result = await adminConsoleConfigService.buildConfig("add", cipherId);
@@ -172,14 +171,32 @@ describe("AdminConsoleCipherFormConfigService", () => {
       expect(result.organizations).toEqual([testOrg, testOrg2]);
     });
 
-    it("retrieves the cipher from the admin service", async () => {
+    it("retrieves the cipher from the admin service when canEditAllCiphers is true", async () => {
       getCipherAdmin.mockResolvedValue({ id: cipherId, name: "Test Cipher - (admin)" });
+      testOrg.canEditAllCiphers = true;
 
-      adminConsoleConfigService = TestBed.inject(AdminConsoleCipherFormConfigService);
+      await adminConsoleConfigService.buildConfig("edit", cipherId);
-
-      await adminConsoleConfigService.buildConfig("add", cipherId);
 
       expect(getCipherAdmin).toHaveBeenCalledWith(cipherId);
     });
+
+    it("retrieves the cipher from the admin service when not found in local state", async () => {
+      getCipherAdmin.mockResolvedValue({ id: cipherId, name: "Test Cipher - (admin)" });
+      testOrg.canEditAllCiphers = false;
+      getCipher.mockResolvedValue(null);
+
+      await adminConsoleConfigService.buildConfig("edit", cipherId);
+
+      expect(getCipherAdmin).toHaveBeenCalledWith(cipherId);
+    });
+
+    it("retrieves the cipher from local state when admin is not required", async () => {
+      testOrg.canEditAllCiphers = false;
+
+      await adminConsoleConfigService.buildConfig("edit", cipherId);
+
+      expect(getCipherAdmin).not.toHaveBeenCalled();
+      expect(getCipher).toHaveBeenCalledWith(cipherId);
+    });
   });
 });

apps/web/src/app/vault/org-vault/services/admin-console-cipher-form-config.service.ts

@@ -5,8 +5,10 @@ import { CollectionAdminService } from "@bitwarden/admin-console/common";
 import { ApiService } from "@bitwarden/common/abstractions/api.service";
 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, OrganizationUserStatusType } from "@bitwarden/common/admin-console/enums";
+import { OrganizationUserStatusType, PolicyType } from "@bitwarden/common/admin-console/enums";
+import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
 import { CipherId } from "@bitwarden/common/types/guid";
+import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
 import { CipherType } from "@bitwarden/common/vault/enums";
 import { CipherData } from "@bitwarden/common/vault/models/data/cipher.data";
 import { Cipher } from "@bitwarden/common/vault/models/domain/cipher";
@@ -25,6 +27,7 @@ export class AdminConsoleCipherFormConfigService implements CipherFormConfigServ
   private organizationService: OrganizationService = inject(OrganizationService);
   private routedVaultFilterService: RoutedVaultFilterService = inject(RoutedVaultFilterService);
   private collectionAdminService: CollectionAdminService = inject(CollectionAdminService);
+  private cipherService: CipherService = inject(CipherService);
   private apiService: ApiService = inject(ApiService);
 
   private allowPersonalOwnership$ = this.policyService
@@ -57,7 +60,6 @@ export class AdminConsoleCipherFormConfigService implements CipherFormConfigServ
     cipherId?: CipherId,
     cipherType?: CipherType,
   ): Promise<CipherFormConfig> {
-    const cipher = await this.getCipher(cipherId);
     const [organization, allowPersonalOwnership, allOrganizations, allCollections] =
       await firstValueFrom(
         combineLatest([
@@ -74,7 +76,7 @@ export class AdminConsoleCipherFormConfigService implements CipherFormConfigServ
     // Only allow the user to assign to their personal vault when cloning and
     // the policies are enabled for it.
     const allowPersonalOwnershipOnlyForClone = mode === "clone" ? allowPersonalOwnership : false;
-
+    const cipher = await this.getCipher(cipherId, organization);
     return {
       mode,
       cipherType: cipher?.type ?? cipherType ?? CipherType.Login,
@@ -89,14 +91,26 @@ export class AdminConsoleCipherFormConfigService implements CipherFormConfigServ
     };
   }
 
-  private async getCipher(id?: CipherId): Promise<Cipher | null> {
+  private async getCipher(id: CipherId | null, organization: Organization): Promise<Cipher | null> {
     if (id == null) {
-      return Promise.resolve(null);
+      return null;
     }
 
-    // Retrieve the cipher through the means of an admin
+    const localCipher = await this.cipherService.get(id);
+
+    // Fetch from the API because we don't need the permissions in local state OR the cipher was not found (e.g. unassigned)
+    if (organization.canEditAllCiphers || localCipher == null) {
+      return await this.getCipherFromAdminApi(id);
+    }
+
+    return localCipher;
+  }
+
+  private async getCipherFromAdminApi(id: CipherId): Promise<Cipher> {
     const cipherResponse = await this.apiService.getCipherAdmin(id);
+    // Ensure admin response includes permissions that allow editing
     cipherResponse.edit = true;
+    cipherResponse.viewPassword = true;
 
     const cipherData = new CipherData(cipherResponse);
     return new Cipher(cipherData);