[PM-22717] Expose DefaultUserCollectionEmail to clients (#15643)

* enforce restrictions based on collection type, set default collection type

* fix ts strict errors

* fix default collection enforcement in vault header

* enforce default collection restrictions in vault collection row

* enforce default collection restrictions in AC vault header

* enforce default collection restriction for select all

* fix ts strict error

* switch to signal, fix feature flag

* fix story

* clean up

* remove feature flag, move check for defaultCollecion to CollecitonView

* fix test

* remove unused configService

* fix test: coerce null to undefined for collection Id

* clean up leaky abstraction for default collection

* fix ts-strict error

* fix parens

* add new property to models, update logic, refactor for ts-strict

* fix type

* rename defaultCollection getter

* clean up

* clean up

* clean up, add comment, fix submit

* add comment

* add feature flag

* check model for name

* cleanup readonly logic, remove featureflag logic

* wip

* refactor CollectionRequest into Create and Update models

* fix readonly logic

* cleanup

* set defaultUserCollectionEmail in decryption from Collection

* split save into update/create methods

* fix readonly logic

* fix collections post and put requests

* add defaultUserCollection email to model when submitting collection dialog

libs/admin-console/src/common/collections/abstractions/collection-admin.service.ts

@@ -10,7 +10,11 @@ export abstract class CollectionAdminService {
     organizationId: string,
     userId: UserId,
   ): Observable<CollectionAdminView[]>;
-  abstract save(
+  abstract update(
+    collection: CollectionAdminView,
+    userId: UserId,
+  ): Promise<CollectionDetailsResponse>;
+  abstract create(
     collection: CollectionAdminView,
     userId: UserId,
   ): Promise<CollectionDetailsResponse>;

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

@@ -101,6 +101,17 @@ export class CollectionAdminView extends CollectionView {
     return this.id === Unassigned;
   }
 
+  /**
+   * Returns true if the collection name can be edited. Editing the collection name is restricted for collections
+   * that were DefaultUserCollections but where the relevant user has been offboarded.
+   * When this occurs, the offboarded user's email is treated as the collection name, and cannot be edited.
+   * This is important for security so that the server cannot ask the client to encrypt arbitrary data.
+   * WARNING! This is an IMPORTANT restriction that MUST be maintained for security purposes.
+   * Do not edit or remove this unless you understand why.
+   */
+  override canEditName(org: Organization): boolean {
+    return (this.canEdit(org) && !this.defaultUserCollectionEmail) || super.canEditName(org);
+  }
   static async fromCollectionAccessDetails(
     collection: CollectionAccessDetailsResponse,
     encryptService: EncryptService,
@@ -115,6 +126,7 @@ export class CollectionAdminView extends CollectionView {
     view.unmanaged = collection.unmanaged;
     view.type = collection.type;
     view.externalId = collection.externalId;
+    view.defaultUserCollectionEmail = collection.defaultUserCollectionEmail;
 
     view.groups = collection.groups
       ? collection.groups.map((g) => new CollectionAccessSelectionView(g))

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

@@ -1,19 +1,18 @@
-// FIXME: Update this file to be type safe and remove this and next line
-// @ts-strict-ignore
 import { Collection } from "./collection";
-import { CollectionRequest } from "./collection.request";
+import { BaseCollectionRequest } from "./collection.request";
 
-export class CollectionWithIdRequest extends CollectionRequest {
+export class CollectionWithIdRequest extends BaseCollectionRequest {
   id: string;
+  name: string;
 
-  constructor(collection?: Collection) {
-    if (collection == null) {
-      return;
+  constructor(collection: Collection) {
+    if (collection == null || collection.name == null || collection.name.encryptedString == null) {
+      throw new Error("CollectionWithIdRequest must contain name.");
     }
     super({
-      name: collection.name,
       externalId: collection.externalId,
     });
+    this.name = collection.name.encryptedString;
     this.id = collection.id;
   }
 }

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

@@ -9,6 +9,7 @@ export class CollectionData {
   id: CollectionId;
   organizationId: OrganizationId;
   name: string;
+  defaultUserCollectionEmail: string | undefined;
   externalId: string | undefined;
   readOnly: boolean = false;
   manage: boolean = false;
@@ -24,6 +25,7 @@ export class CollectionData {
     this.manage = response.manage;
     this.hidePasswords = response.hidePasswords;
     this.type = response.type;
+    this.defaultUserCollectionEmail = response.defaultUserCollectionEmail;
   }
 
   static fromJSON(obj: Jsonify<CollectionData | null>): CollectionData | null {

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

@@ -1,23 +1,20 @@
 import { SelectionReadOnlyRequest } from "@bitwarden/common/admin-console/models/request/selection-read-only.request";
 import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string";
 
-export class CollectionRequest {
-  name: string;
+export abstract class BaseCollectionRequest {
   externalId: string | undefined;
   groups: SelectionReadOnlyRequest[] = [];
   users: SelectionReadOnlyRequest[] = [];
 
-  constructor(c: {
-    name: EncString;
+  static isUpdate = (request: BaseCollectionRequest): request is UpdateCollectionRequest => {
+    return request instanceof UpdateCollectionRequest;
+  };
+
+  protected constructor(c: {
     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) {
@@ -28,3 +25,36 @@ export class CollectionRequest {
     }
   }
 }
+
+export class CreateCollectionRequest extends BaseCollectionRequest {
+  name: string;
+
+  constructor(c: {
+    name: EncString;
+    users?: SelectionReadOnlyRequest[];
+    groups?: SelectionReadOnlyRequest[];
+    externalId?: string;
+  }) {
+    super(c);
+
+    if (!c.name || !c.name.encryptedString) {
+      throw new Error("Name not provided for CollectionRequest.");
+    }
+
+    this.name = c.name.encryptedString;
+  }
+}
+
+export class UpdateCollectionRequest extends BaseCollectionRequest {
+  name: string | null;
+
+  constructor(c: {
+    name: EncString | null;
+    users?: SelectionReadOnlyRequest[];
+    groups?: SelectionReadOnlyRequest[];
+    externalId?: string;
+  }) {
+    super(c);
+    this.name = c.name?.encryptedString ?? null;
+  }
+}

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

@@ -8,6 +8,7 @@ export class CollectionResponse extends BaseResponse {
   id: CollectionId;
   organizationId: OrganizationId;
   name: string;
+  defaultUserCollectionEmail: string | undefined;
   externalId: string | undefined;
   type: CollectionType = CollectionTypes.SharedCollection;
 
@@ -17,6 +18,7 @@ export class CollectionResponse extends BaseResponse {
     this.organizationId = this.getResponseProperty("OrganizationId");
     this.name = this.getResponseProperty("Name");
     this.externalId = this.getResponseProperty("ExternalId");
+    this.defaultUserCollectionEmail = this.getResponseProperty("DefaultUserCollectionEmail");
     this.type = this.getResponseProperty("Type") ?? CollectionTypes.SharedCollection;
   }
 }

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

@@ -25,6 +25,7 @@ describe("Collection", () => {
         manage: true,
         hidePasswords: true,
         type: CollectionTypes.DefaultUserCollection,
+        defaultUserCollectionEmail: "defaultCollectionEmail",
       }),
     );
     encService = mock<EncryptService>();
@@ -61,6 +62,7 @@ describe("Collection", () => {
       manage: true,
       hidePasswords: true,
       type: CollectionTypes.DefaultUserCollection,
+      defaultUserCollectionEmail: "defaultCollectionEmail",
     });
   });
 
@@ -75,6 +77,7 @@ describe("Collection", () => {
     collection.hidePasswords = false;
     collection.manage = true;
     collection.type = CollectionTypes.DefaultUserCollection;
+    collection.defaultUserCollectionEmail = "defaultCollectionEmail";
 
     const key = makeSymmetricCryptoKey<OrgKey>();
 
@@ -84,12 +87,13 @@ describe("Collection", () => {
       externalId: "extId",
       hidePasswords: false,
       id: "id",
-      name: "encName",
+      _name: "encName",
       organizationId: "orgId",
       readOnly: false,
       manage: true,
       assigned: true,
       type: CollectionTypes.DefaultUserCollection,
+      defaultUserCollectionEmail: "defaultCollectionEmail",
     });
   });
 });

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

@@ -23,6 +23,7 @@ export class Collection extends Domain {
   hidePasswords: boolean = false;
   manage: boolean = false;
   type: CollectionType = CollectionTypes.SharedCollection;
+  defaultUserCollectionEmail: string | undefined;
 
   constructor(c: { id: CollectionId; name: EncString; organizationId: OrganizationId }) {
     super();
@@ -46,6 +47,7 @@ export class Collection extends Domain {
     collection.hidePasswords = obj.hidePasswords;
     collection.manage = obj.manage;
     collection.type = obj.type;
+    collection.defaultUserCollectionEmail = obj.defaultUserCollectionEmail;
 
     return collection;
   }

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

@@ -16,7 +16,6 @@ export const NestingDelimiter = "/";
 export class CollectionView implements View, ITreeNodeObject {
   id: CollectionId;
   organizationId: OrganizationId;
-  name: string;
   externalId: string | undefined;
   // readOnly applies to the items within a collection
   readOnly: boolean = false;
@@ -24,11 +23,22 @@ export class CollectionView implements View, ITreeNodeObject {
   manage: boolean = false;
   assigned: boolean = false;
   type: CollectionType = CollectionTypes.SharedCollection;
+  defaultUserCollectionEmail: string | undefined;
+
+  private _name: string;
 
   constructor(c: { id: CollectionId; organizationId: OrganizationId; name: string }) {
     this.id = c.id;
     this.organizationId = c.organizationId;
-    this.name = c.name;
+    this._name = c.name;
+  }
+
+  set name(name: string) {
+    this._name = name;
+  }
+
+  get name(): string {
+    return this.defaultUserCollectionEmail ?? this._name;
   }
 
   canEditItems(org: Organization): boolean {
@@ -83,6 +93,18 @@ export class CollectionView implements View, ITreeNodeObject {
     return false;
   }
 
+  /**
+   * Returns true if the collection name can be edited. Editing the collection name is restricted for collections
+   * that were DefaultUserCollections but where the relevant user has been offboarded.
+   * When this occurs, the offboarded user's email is treated as the collection name, and cannot be edited.
+   * This is important for security so that the server cannot ask the client to encrypt arbitrary data.
+   * WARNING! This is an IMPORTANT restriction that MUST be maintained for security purposes.
+   * Do not edit or remove this unless you understand why.
+   */
+  canEditName(org: Organization): boolean {
+    return this.canEdit(org) && !this.defaultUserCollectionEmail;
+  }
+
   get isDefaultCollection() {
     return this.type == CollectionTypes.DefaultUserCollection;
   }
@@ -111,6 +133,7 @@ export class CollectionView implements View, ITreeNodeObject {
     view.hidePasswords = collection.hidePasswords;
     view.manage = collection.manage;
     view.type = collection.type;
+    view.defaultUserCollectionEmail = collection.defaultUserCollectionEmail;
     return view;
   }
 
@@ -125,6 +148,7 @@ export class CollectionView implements View, ITreeNodeObject {
     view.externalId = collection.externalId;
     view.type = collection.type;
     view.assigned = collection.assigned;
+    view.defaultUserCollectionEmail = collection.defaultUserCollectionEmail;
     return view;
   }
 

libs/admin-console/src/common/collections/services/default-collection-admin.service.ts

@@ -1,6 +1,10 @@
 import { combineLatest, firstValueFrom, from, map, Observable, of, switchMap } from "rxjs";
 
 import { ApiService } from "@bitwarden/common/abstractions/api.service";
+import {
+  getOrganizationById,
+  OrganizationService,
+} from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
 import { SelectionReadOnlyRequest } from "@bitwarden/common/admin-console/models/request/selection-read-only.request";
 import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service";
 import { CollectionId, OrganizationId, UserId } from "@bitwarden/common/types/guid";
@@ -10,13 +14,15 @@ import { KeyService } from "@bitwarden/key-management";
 import { CollectionAdminService, CollectionService } from "../abstractions";
 import {
   CollectionData,
-  CollectionRequest,
   CollectionAccessDetailsResponse,
   CollectionDetailsResponse,
   CollectionResponse,
   BulkCollectionAccessRequest,
   CollectionAccessSelectionView,
   CollectionAdminView,
+  BaseCollectionRequest,
+  UpdateCollectionRequest,
+  CreateCollectionRequest,
 } from "../models";
 
 export class DefaultCollectionAdminService implements CollectionAdminService {
@@ -25,6 +31,7 @@ export class DefaultCollectionAdminService implements CollectionAdminService {
     private keyService: KeyService,
     private encryptService: EncryptService,
     private collectionService: CollectionService,
+    private organizationService: OrganizationService,
   ) {}
 
   collectionAdminViews$(organizationId: string, userId: UserId): Observable<CollectionAdminView[]> {
@@ -45,27 +52,40 @@ export class DefaultCollectionAdminService implements CollectionAdminService {
     );
   }
 
-  async save(collection: CollectionAdminView, userId: UserId): Promise<CollectionDetailsResponse> {
-    const request = await this.encrypt(collection, userId);
-
-    let response: CollectionDetailsResponse;
-    if (collection.id == null) {
-      response = await this.apiService.postCollection(collection.organizationId, request);
-      collection.id = response.id;
-    } else {
-      response = await this.apiService.putCollection(
-        collection.organizationId,
-        collection.id,
-        request,
-      );
+  async update(
+    collection: CollectionAdminView,
+    userId: UserId,
+  ): Promise<CollectionDetailsResponse> {
+    const request = await this.encrypt(collection, userId, true);
+    if (!BaseCollectionRequest.isUpdate(request)) {
+      throw new Error("Cannot update collection with CreateCollectionRequest.");
     }
 
-    if (response.assigned) {
-      await this.collectionService.upsert(new CollectionData(response), userId);
-    } else {
-      await this.collectionService.delete([collection.id as CollectionId], userId);
+    const response = await this.apiService.putCollection(
+      collection.organizationId,
+      collection.id,
+      request,
+    );
+
+    await this.updateLocalCollections(response, collection, userId);
+
+    return response;
+  }
+
+  async create(
+    collection: CollectionAdminView,
+    userId: UserId,
+  ): Promise<CollectionDetailsResponse> {
+    const request = await this.encrypt(collection, userId, false);
+    if (BaseCollectionRequest.isUpdate(request)) {
+      throw new Error("Cannot create collection with UpdateCollectionRequest.");
     }
 
+    const response = await this.apiService.postCollection(collection.organizationId, request);
+    collection.id = response.id;
+
+    await this.updateLocalCollections(response, collection, userId);
+
     return response;
   }
 
@@ -73,6 +93,16 @@ export class DefaultCollectionAdminService implements CollectionAdminService {
     await this.apiService.deleteCollection(organizationId, collectionId);
   }
 
+  private async updateLocalCollections(
+    response: CollectionDetailsResponse,
+    collection: CollectionAdminView,
+    userId: UserId,
+  ) {
+    response.assigned
+      ? await this.collectionService.upsert(new CollectionData(response), userId)
+      : await this.collectionService.delete([collection.id as CollectionId], userId);
+  }
+
   async bulkAssignAccess(
     organizationId: string,
     collectionIds: string[],
@@ -118,10 +148,15 @@ export class DefaultCollectionAdminService implements CollectionAdminService {
       );
     });
 
-    return await Promise.all(promises);
+    const r = await Promise.all(promises);
+    return r;
   }
 
-  private async encrypt(model: CollectionAdminView, userId: UserId): Promise<CollectionRequest> {
+  private async encrypt(
+    model: CollectionAdminView,
+    userId: UserId,
+    editMode: boolean,
+  ): Promise<UpdateCollectionRequest | CreateCollectionRequest> {
     if (!model.organizationId) {
       throw new Error("Collection has no organization id.");
     }
@@ -154,14 +189,31 @@ export class DefaultCollectionAdminService implements CollectionAdminService {
         new SelectionReadOnlyRequest(user.id, user.readOnly, user.hidePasswords, user.manage),
     );
 
-    const collectionRequest = new CollectionRequest({
+    if (editMode) {
+      const org = await firstValueFrom(
+        this.organizationService
+          .organizations$(userId)
+          .pipe(getOrganizationById(model.organizationId)),
+      );
+      if (org == null) {
+        throw new Error("No Organization found.");
+      }
+      return new UpdateCollectionRequest({
+        name: model.canEditName(org)
+          ? await this.encryptService.encryptString(model.name, key)
+          : null,
+        externalId: model.externalId,
+        users,
+        groups,
+      });
+    }
+
+    return new CreateCollectionRequest({
       name: await this.encryptService.encryptString(model.name, key),
       externalId: model.externalId,
       users,
       groups,
     });
-
-    return collectionRequest;
   }
 }
 

libs/admin-console/src/common/collections/services/default-collection.service.ts

@@ -216,7 +216,7 @@ export class DefaultCollectionService implements CollectionService {
   getAllNested(collections: CollectionView[]): TreeNode<CollectionView>[] {
     const nodes: TreeNode<CollectionView>[] = [];
     collections.forEach((c) => {
-      const collectionCopy = Object.assign(new CollectionView({ ...c }), c);
+      const collectionCopy = Object.assign(new CollectionView({ ...c, name: c.name }), c);
 
       const parts = c.name != null ? c.name.replace(/^\/+|\/+$/g, "").split(NestingDelimiter) : [];
       ServiceUtils.nestedTraverse(nodes, 0, parts, collectionCopy, undefined, NestingDelimiter);