[PM-24227] Enable TS-strict for Collection Domain models (#15765)

* wip ts-strict

* wip ts-strict

* wip

* cleanup

* cleanup

* fix story

* fix story

* fix story

* wip

* clean up CollectionAdminView construction

* fix deprecated function call

* fix cli

* clean up

* fix story

* wip

* fix cli

* requested changes

* clean up, fixing minor bugs, more type saftey

* assign props in static ctor, clean up

libs/admin-console/src/common/collections/models/collection-admin.view.ts

@@ -1,7 +1,10 @@
 import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
+import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service";
+import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string";
+import { OrgKey } from "@bitwarden/common/types/key";
 
 import { CollectionAccessSelectionView } from "./collection-access-selection.view";
-import { CollectionAccessDetailsResponse } from "./collection.response";
+import { CollectionAccessDetailsResponse, CollectionResponse } from "./collection.response";
 import { CollectionView } from "./collection.view";
 
 // TODO: this is used to represent the pseudo "Unassigned" collection as well as
@@ -24,24 +27,6 @@ export class CollectionAdminView extends CollectionView {
    */
   assigned: boolean = false;
 
-  constructor(response?: CollectionAccessDetailsResponse) {
-    super(response);
-
-    if (!response) {
-      return;
-    }
-
-    this.groups = response.groups
-      ? response.groups.map((g) => new CollectionAccessSelectionView(g))
-      : [];
-
-    this.users = response.users
-      ? response.users.map((g) => new CollectionAccessSelectionView(g))
-      : [];
-
-    this.assigned = response.assigned;
-  }
-
   /**
    * Returns true if the user can edit a collection (including user and group access) from the Admin Console.
    */
@@ -115,4 +100,46 @@ export class CollectionAdminView extends CollectionView {
   get isUnassignedCollection() {
     return this.id === Unassigned;
   }
+
+  static async fromCollectionAccessDetails(
+    collection: CollectionAccessDetailsResponse,
+    encryptService: EncryptService,
+    orgKey: OrgKey,
+  ): Promise<CollectionAdminView> {
+    const view = new CollectionAdminView({ ...collection });
+    view.name = await encryptService.decryptString(new EncString(view.name), orgKey);
+    view.assigned = collection.assigned;
+    view.readOnly = collection.readOnly;
+    view.hidePasswords = collection.hidePasswords;
+    view.manage = collection.manage;
+    view.unmanaged = collection.unmanaged;
+    view.type = collection.type;
+    view.externalId = collection.externalId;
+
+    view.groups = collection.groups
+      ? collection.groups.map((g) => new CollectionAccessSelectionView(g))
+      : [];
+
+    view.users = collection.users
+      ? collection.users.map((g) => new CollectionAccessSelectionView(g))
+      : [];
+
+    return view;
+  }
+
+  static async fromCollectionResponse(
+    collection: CollectionResponse,
+    encryptService: EncryptService,
+    orgKey: OrgKey,
+  ): Promise<CollectionAdminView> {
+    const collectionAdminView = new CollectionAdminView({
+      id: collection.id,
+      name: await encryptService.decryptString(new EncString(collection.name), orgKey),
+      organizationId: collection.organizationId,
+    });
+
+    collectionAdminView.externalId = collection.externalId;
+
+    return collectionAdminView;
+  }
 }

libs/admin-console/src/common/collections/models/collection-with-id.request.ts

@@ -10,7 +10,10 @@ export class CollectionWithIdRequest extends CollectionRequest {
     if (collection == null) {
       return;
     }
-    super(collection);
+    super({
+      name: collection.name,
+      externalId: collection.externalId,
+    });
     this.id = collection.id;
   }
 }

libs/admin-console/src/common/collections/models/collection.data.ts

@@ -2,18 +2,18 @@ import { Jsonify } from "type-fest";
 
 import { CollectionId, OrganizationId } from "@bitwarden/common/types/guid";
 
-import { CollectionType } from "./collection";
+import { CollectionType, CollectionTypes } from "./collection";
 import { CollectionDetailsResponse } from "./collection.response";
 
 export class CollectionData {
   id: CollectionId;
   organizationId: OrganizationId;
   name: string;
-  externalId: string;
-  readOnly: boolean;
-  manage: boolean;
-  hidePasswords: boolean;
-  type: CollectionType;
+  externalId: string | undefined;
+  readOnly: boolean = false;
+  manage: boolean = false;
+  hidePasswords: boolean = false;
+  type: CollectionType = CollectionTypes.SharedCollection;
 
   constructor(response: CollectionDetailsResponse) {
     this.id = response.id;

libs/admin-console/src/common/collections/models/collection.request.ts

@@ -1,20 +1,30 @@
-// FIXME: Update this file to be type safe and remove this and next line
-// @ts-strict-ignore
 import { SelectionReadOnlyRequest } from "@bitwarden/common/admin-console/models/request/selection-read-only.request";
-
-import { Collection } from "./collection";
+import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string";
 
 export class CollectionRequest {
   name: string;
-  externalId: string;
+  externalId: string | undefined;
   groups: SelectionReadOnlyRequest[] = [];
   users: SelectionReadOnlyRequest[] = [];
 
-  constructor(collection?: Collection) {
-    if (collection == null) {
-      return;
+  constructor(c: {
+    name: EncString;
+    users?: SelectionReadOnlyRequest[];
+    groups?: SelectionReadOnlyRequest[];
+    externalId?: string;
+  }) {
+    if (!c.name || !c.name.encryptedString) {
+      throw new Error("Name not provided for CollectionRequest.");
+    }
+
+    this.name = c.name.encryptedString;
+    this.externalId = c.externalId;
+
+    if (c.groups) {
+      this.groups = c.groups;
+    }
+    if (c.users) {
+      this.users = c.users;
     }
-    this.name = collection.name ? collection.name.encryptedString : null;
-    this.externalId = collection.externalId;
   }
 }

libs/admin-console/src/common/collections/models/collection.response.ts

@@ -2,14 +2,14 @@ import { SelectionReadOnlyResponse } from "@bitwarden/common/admin-console/model
 import { BaseResponse } from "@bitwarden/common/models/response/base.response";
 import { CollectionId, OrganizationId } from "@bitwarden/common/types/guid";
 
-import { CollectionType } from "./collection";
+import { CollectionType, CollectionTypes } from "./collection";
 
 export class CollectionResponse extends BaseResponse {
   id: CollectionId;
   organizationId: OrganizationId;
   name: string;
-  externalId: string;
-  type: CollectionType;
+  externalId: string | undefined;
+  type: CollectionType = CollectionTypes.SharedCollection;
 
   constructor(response: any) {
     super(response);
@@ -17,7 +17,7 @@ export class CollectionResponse extends BaseResponse {
     this.organizationId = this.getResponseProperty("OrganizationId");
     this.name = this.getResponseProperty("Name");
     this.externalId = this.getResponseProperty("ExternalId");
-    this.type = this.getResponseProperty("Type");
+    this.type = this.getResponseProperty("Type") ?? CollectionTypes.SharedCollection;
   }
 }
 

libs/admin-console/src/common/collections/models/collection.spec.ts

@@ -1,50 +1,62 @@
-import { makeSymmetricCryptoKey, mockEnc } from "@bitwarden/common/spec";
+import { MockProxy, mock } from "jest-mock-extended";
+
+import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service";
+import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string";
+import { makeSymmetricCryptoKey } from "@bitwarden/common/spec";
 import { CollectionId, OrganizationId } from "@bitwarden/common/types/guid";
 import { OrgKey } from "@bitwarden/common/types/key";
 
 import { Collection, CollectionTypes } from "./collection";
 import { CollectionData } from "./collection.data";
+import { CollectionDetailsResponse } from "./collection.response";
 
 describe("Collection", () => {
   let data: CollectionData;
+  let encService: MockProxy<EncryptService>;
 
   beforeEach(() => {
-    data = {
-      id: "id" as CollectionId,
-      organizationId: "orgId" as OrganizationId,
-      name: "encName",
-      externalId: "extId",
-      readOnly: true,
-      manage: true,
-      hidePasswords: true,
-      type: CollectionTypes.DefaultUserCollection,
-    };
+    data = new CollectionData(
+      new CollectionDetailsResponse({
+        id: "id" as CollectionId,
+        organizationId: "orgId" as OrganizationId,
+        name: "encName",
+        externalId: "extId",
+        readOnly: true,
+        manage: true,
+        hidePasswords: true,
+        type: CollectionTypes.DefaultUserCollection,
+      }),
+    );
+    encService = mock<EncryptService>();
+    encService.decryptString.mockResolvedValue("encName");
   });
 
-  it("Convert from empty", () => {
-    const data = new CollectionData({} as any);
-    const card = new Collection(data);
-
-    expect(card).toEqual({
-      externalId: null,
-      hidePasswords: null,
-      id: null,
-      name: null,
-      organizationId: null,
-      readOnly: null,
-      manage: null,
-      type: null,
+  it("Convert from partial", () => {
+    const card = new Collection({
+      name: new EncString("name"),
+      organizationId: "orgId" as OrganizationId,
+      id: "id" as CollectionId,
     });
+    expect(() => card).not.toThrow();
+
+    expect(card.name).not.toBe(null);
+    expect(card.organizationId).not.toBe(null);
+    expect(card.id).not.toBe(null);
+    expect(card.externalId).toBe(undefined);
+    expect(card.readOnly).toBe(false);
+    expect(card.manage).toBe(false);
+    expect(card.hidePasswords).toBe(false);
+    expect(card.type).toEqual(CollectionTypes.SharedCollection);
   });
 
   it("Convert", () => {
-    const collection = new Collection(data);
+    const collection = Collection.fromCollectionData(data);
 
     expect(collection).toEqual({
       id: "id",
       organizationId: "orgId",
       name: { encryptedString: "encName", encryptionType: 0 },
-      externalId: { encryptedString: "extId", encryptionType: 0 },
+      externalId: "extId",
       readOnly: true,
       manage: true,
       hidePasswords: true,
@@ -53,10 +65,11 @@ describe("Collection", () => {
   });
 
   it("Decrypt", async () => {
-    const collection = new Collection();
-    collection.id = "id" as CollectionId;
-    collection.organizationId = "orgId" as OrganizationId;
-    collection.name = mockEnc("encName");
+    const collection = new Collection({
+      name: new EncString("encName"),
+      organizationId: "orgId" as OrganizationId,
+      id: "id" as CollectionId,
+    });
     collection.externalId = "extId";
     collection.readOnly = false;
     collection.hidePasswords = false;
@@ -65,7 +78,7 @@ describe("Collection", () => {
 
     const key = makeSymmetricCryptoKey<OrgKey>();
 
-    const view = await collection.decrypt(key);
+    const view = await collection.decrypt(key, encService);
 
     expect(view).toEqual({
       externalId: "extId",

libs/admin-console/src/common/collections/models/collection.ts

@@ -1,5 +1,6 @@
+import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service";
 import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string";
-import Domain, { EncryptableKeys } from "@bitwarden/common/platform/models/domain/domain-base";
+import Domain from "@bitwarden/common/platform/models/domain/domain-base";
 import { CollectionId, OrganizationId } from "@bitwarden/common/types/guid";
 import { OrgKey } from "@bitwarden/common/types/key";
 
@@ -14,45 +15,63 @@ export const CollectionTypes = {
 export type CollectionType = (typeof CollectionTypes)[keyof typeof CollectionTypes];
 
 export class Collection extends Domain {
-  id: CollectionId | undefined;
-  organizationId: OrganizationId | undefined;
-  name: EncString | undefined;
+  id: CollectionId;
+  organizationId: OrganizationId;
+  name: EncString;
   externalId: string | undefined;
   readOnly: boolean = false;
   hidePasswords: boolean = false;
   manage: boolean = false;
   type: CollectionType = CollectionTypes.SharedCollection;
 
-  constructor(obj?: CollectionData | null) {
+  constructor(c: { id: CollectionId; name: EncString; organizationId: OrganizationId }) {
     super();
-    if (obj == null) {
-      return;
+    this.id = c.id;
+    this.name = c.name;
+    this.organizationId = c.organizationId;
+  }
+
+  static fromCollectionData(obj: CollectionData): Collection {
+    if (obj == null || obj.name == null || obj.organizationId == null) {
+      throw new Error("CollectionData must contain name and organizationId.");
     }
 
-    this.buildDomainModel(
-      this,
-      obj,
-      {
-        id: null,
-        organizationId: null,
-        name: null,
-        externalId: null,
-        readOnly: null,
-        hidePasswords: null,
-        manage: null,
-        type: null,
-      },
-      ["id", "organizationId", "readOnly", "hidePasswords", "manage", "type"],
+    const collection = new Collection({
+      ...obj,
+      name: new EncString(obj.name),
+    });
+
+    collection.externalId = obj.externalId;
+    collection.readOnly = obj.readOnly;
+    collection.hidePasswords = obj.hidePasswords;
+    collection.manage = obj.manage;
+    collection.type = obj.type;
+
+    return collection;
+  }
+
+  static async fromCollectionView(
+    view: CollectionView,
+    encryptService: EncryptService,
+    orgKey: OrgKey,
+  ): Promise<Collection> {
+    return Object.assign(
+      new Collection({
+        name: await encryptService.encryptString(view.name, orgKey),
+        id: view.id,
+        organizationId: view.organizationId,
+      }),
+      view,
     );
   }
 
-  decrypt(orgKey: OrgKey): Promise<CollectionView> {
-    return this.decryptObj<Collection, CollectionView>(
-      this,
-      new CollectionView(this),
-      ["name"] as EncryptableKeys<Collection, CollectionView>[],
-      this.organizationId ?? null,
-      orgKey,
-    );
+  decrypt(orgKey: OrgKey, encryptService: EncryptService): Promise<CollectionView> {
+    return CollectionView.fromCollection(this, encryptService, orgKey);
+  }
+
+  // @TODO: This would be better off in Collection.Utils. Move this there when
+  // refactoring to a shared lib.
+  static isCollectionId(id: any): id is CollectionId {
+    return typeof id === "string" && id != null;
   }
 }

libs/admin-console/src/common/collections/models/collection.view.ts

@@ -1,8 +1,11 @@
 import { Jsonify } from "type-fest";
 
 import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
+import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service";
+import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string";
 import { View } from "@bitwarden/common/models/view/view";
 import { CollectionId, OrganizationId } from "@bitwarden/common/types/guid";
+import { OrgKey } from "@bitwarden/common/types/key";
 import { ITreeNodeObject } from "@bitwarden/common/vault/models/domain/tree-node";
 
 import { Collection, CollectionType, CollectionTypes } from "./collection";
@@ -11,9 +14,9 @@ import { CollectionAccessDetailsResponse } from "./collection.response";
 export const NestingDelimiter = "/";
 
 export class CollectionView implements View, ITreeNodeObject {
-  id: CollectionId | undefined;
-  organizationId: OrganizationId | undefined;
-  name: string = "";
+  id: CollectionId;
+  organizationId: OrganizationId;
+  name: string;
   externalId: string | undefined;
   // readOnly applies to the items within a collection
   readOnly: boolean = false;
@@ -22,24 +25,10 @@ export class CollectionView implements View, ITreeNodeObject {
   assigned: boolean = false;
   type: CollectionType = CollectionTypes.SharedCollection;
 
-  constructor(c?: Collection | CollectionAccessDetailsResponse) {
-    if (!c) {
-      return;
-    }
-
+  constructor(c: { id: CollectionId; organizationId: OrganizationId; name: string }) {
     this.id = c.id;
     this.organizationId = c.organizationId;
-    this.externalId = c.externalId;
-    if (c instanceof Collection) {
-      this.readOnly = c.readOnly;
-      this.hidePasswords = c.hidePasswords;
-      this.manage = c.manage;
-      this.assigned = true;
-    }
-    if (c instanceof CollectionAccessDetailsResponse) {
-      this.assigned = c.assigned;
-    }
-    this.type = c.type;
+    this.name = c.name;
   }
 
   canEditItems(org: Organization): boolean {
@@ -94,11 +83,53 @@ export class CollectionView implements View, ITreeNodeObject {
     return false;
   }
 
-  static fromJSON(obj: Jsonify<CollectionView>) {
-    return Object.assign(new CollectionView(new Collection()), obj);
-  }
-
   get isDefaultCollection() {
     return this.type == CollectionTypes.DefaultUserCollection;
   }
+
+  // FIXME: we should not use a CollectionView object for the vault filter header because it is not a real
+  // CollectionView and this violates ts-strict rules.
+  static vaultFilterHead(): CollectionView {
+    return new CollectionView({
+      id: "" as CollectionId,
+      organizationId: "" as OrganizationId,
+      name: "",
+    });
+  }
+
+  static async fromCollection(
+    collection: Collection,
+    encryptService: EncryptService,
+    key: OrgKey,
+  ): Promise<CollectionView> {
+    const view: CollectionView = Object.assign(
+      new CollectionView({ ...collection, name: "" }),
+      collection,
+    );
+    view.name = await encryptService.decryptString(collection.name, key);
+    view.assigned = true;
+    return view;
+  }
+
+  static async fromCollectionAccessDetails(
+    collection: CollectionAccessDetailsResponse,
+    encryptService: EncryptService,
+    orgKey: OrgKey,
+  ): Promise<CollectionView> {
+    const view = new CollectionView({ ...collection });
+
+    view.name = await encryptService.decryptString(new EncString(collection.name), orgKey);
+    view.externalId = collection.externalId;
+    view.type = collection.type;
+    view.assigned = collection.assigned;
+    return view;
+  }
+
+  static fromJSON(obj: Jsonify<CollectionView>) {
+    return Object.assign(new CollectionView({ ...obj }), obj);
+  }
+
+  encrypt(orgKey: OrgKey, encryptService: EncryptService): Promise<Collection> {
+    return Collection.fromCollectionView(this, encryptService, orgKey);
+  }
 }