Introduce a stricter use of the OrganizationId type on org-vault exports (#15836)

Co-authored-by: Daniel James Smith <djsmith85@users.noreply.github.com>

apps/web/src/app/tools/vault-export/org-vault-export.component.ts

@@ -1,8 +1,7 @@
-// FIXME: Update this file to be type safe and remove this and next line
-// @ts-strict-ignore
 import { Component, OnInit } from "@angular/core";
 import { ActivatedRoute } from "@angular/router";
 
+import { isId, OrganizationId } from "@bitwarden/common/types/guid";
 import { ExportComponent } from "@bitwarden/vault-export-ui";
 
 import { HeaderModule } from "../../layouts/header/header.module";
@@ -13,18 +12,27 @@ import { SharedModule } from "../../shared";
   imports: [SharedModule, ExportComponent, HeaderModule],
 })
 export class OrganizationVaultExportComponent implements OnInit {
-  protected routeOrgId: string = null;
+  protected routeOrgId: OrganizationId | undefined = undefined;
   protected loading = false;
   protected disabled = false;
 
   constructor(private route: ActivatedRoute) {}
 
   async ngOnInit() {
-    this.routeOrgId = this.route.snapshot.paramMap.get("organizationId");
+    const orgIdParam = this.route.snapshot.paramMap.get("organizationId");
+    if (orgIdParam === undefined) {
+      throw new Error("`organizationId` is a required route parameter");
+    }
+
+    if (!isId<OrganizationId>(orgIdParam)) {
+      throw new Error("Invalid OrganizationId provided in route parameter `organizationId`");
+    }
+
+    this.routeOrgId = orgIdParam;
   }
 
   /**
    * Callback that is called after a successful export.
    */
-  protected async onSuccessfulExport(organizationId: string): Promise<void> {}
+  protected async onSuccessfulExport(organizationId: OrganizationId): Promise<void> {}
 }

libs/common/src/types/guid.spec.ts

@@ -0,0 +1,35 @@
+import { isId, emptyGuid, OrganizationId } from "./guid";
+
+describe("isId tests", () => {
+  it("should return true for a valid guid string", () => {
+    // Example valid GUID
+    const validGuid = "12345678-1234-1234-1234-123456789abc";
+    expect(isId(validGuid)).toBe(true);
+  });
+
+  it("should return false for an invalid guid string", () => {
+    // Example invalid GUID
+    const invalidGuid = "not-a-guid";
+    expect(isId(invalidGuid)).toBe(false);
+  });
+
+  it("should return false for non-string values", () => {
+    expect(isId(undefined)).toBe(false);
+    expect(isId(null)).toBe(false);
+    expect(isId(123)).toBe(false);
+    expect(isId({})).toBe(false);
+    expect(isId([])).toBe(false);
+  });
+
+  it("should return true for the emptyGuid constant if it is a valid guid", () => {
+    expect(isId(emptyGuid)).toBe(true);
+  });
+
+  it("should infer type OrganizationId when using isId<OrganizationId>", () => {
+    const orgId: string = "12345678-1234-1234-1234-123456789abc";
+    if (isId<OrganizationId>(orgId)) {
+      return;
+    }
+    throw new Error("Type guard failed, orgId is not a valid OrganizationId");
+  });
+});

libs/common/src/types/guid.ts

@@ -1,5 +1,7 @@
 import { Opaque } from "type-fest";
 
+import { isGuid } from "@bitwarden/guid";
+
 export type Guid = Opaque<string, "Guid">;
 
 // Convenience re-export of UserId from it's original location, any library that
@@ -26,3 +28,23 @@ export type OrganizationReportId = Opaque<string, "OrganizationReportId">;
  * A string representation of an empty guid.
  */
 export const emptyGuid = "00000000-0000-0000-0000-000000000000";
+
+/**
+ * Determines if the provided value is a valid GUID string.
+ *
+ * @typeParam SomeGuid - The input type, defaults to `Guid`.
+ * @typeParam Output - The output type, resolves to `SomeGuid` if it is an opaque string, otherwise to `Guid` if `SomeGuid` is a string, or `never`.
+ * @param id - The value to check.
+ * @returns `true` if `id` is a string and a valid GUID, otherwise `false`.
+ */
+export function isId<
+  SomeGuid extends string = Guid,
+  // eslint-disable-next-line @typescript-eslint/no-unused-vars
+  Output = SomeGuid extends Opaque<string, infer T>
+    ? SomeGuid
+    : SomeGuid extends string
+      ? Guid
+      : never,
+>(id: unknown): id is Output {
+  return typeof id === "string" && isGuid(id);
+}

libs/tools/export/vault-export/vault-export-core/src/services/org-vault-export.service.abstraction.ts

@@ -1,15 +1,17 @@
+import { OrganizationId } from "@bitwarden/common/types/guid";
+
 import { ExportedVaultAsString } from "../types";
 
 import { ExportFormat } from "./vault-export.service.abstraction";
 
 export abstract class OrganizationVaultExportServiceAbstraction {
   abstract getPasswordProtectedExport: (
-    organizationId: string,
+    organizationId: OrganizationId,
     password: string,
     onlyManagedCollections: boolean,
   ) => Promise<ExportedVaultAsString>;
   abstract getOrganizationExport: (
-    organizationId: string,
+    organizationId: OrganizationId,
     format: ExportFormat,
     onlyManagedCollections: boolean,
   ) => Promise<ExportedVaultAsString>;

libs/tools/export/vault-export/vault-export-core/src/services/org-vault-export.service.ts

@@ -65,7 +65,7 @@ export class OrganizationVaultExportService
    * @returns The exported vault
    */
   async getPasswordProtectedExport(
-    organizationId: string,
+    organizationId: OrganizationId,
     password: string,
     onlyManagedCollections: boolean,
   ): Promise<ExportedVaultAsString> {
@@ -94,7 +94,7 @@ export class OrganizationVaultExportService
    * @throws Error if the organization policies prevent the export
    */
   async getOrganizationExport(
-    organizationId: string,
+    organizationId: OrganizationId,
     format: ExportFormat = "csv",
     onlyManagedCollections: boolean,
   ): Promise<ExportedVaultAsString> {
@@ -128,7 +128,7 @@ export class OrganizationVaultExportService
 
   private async getOrganizationDecryptedExport(
     activeUserId: UserId,
-    organizationId: string,
+    organizationId: OrganizationId,
     format: "json" | "csv",
   ): Promise<string> {
     const decCollections: CollectionView[] = [];
@@ -138,49 +138,42 @@ export class OrganizationVaultExportService
     const restrictions = await firstValueFrom(this.restrictedItemTypesService.restricted$);
 
     promises.push(
-      this.vaultExportApiService
-        .getOrganizationExport(organizationId as OrganizationId)
-        .then((exportData) => {
-          const exportPromises: Promise<void>[] = [];
-          if (exportData != null) {
-            if (exportData.collections != null && exportData.collections.length > 0) {
-              exportData.collections.forEach((c) => {
-                const collection = Collection.fromCollectionData(
-                  new CollectionData(c as CollectionDetailsResponse),
-                );
+      this.vaultExportApiService.getOrganizationExport(organizationId).then((exportData) => {
+        const exportPromises: Promise<void>[] = [];
+        if (exportData != null) {
+          if (exportData.collections != null && exportData.collections.length > 0) {
+            exportData.collections.forEach((c) => {
+              const collection = Collection.fromCollectionData(
+                new CollectionData(c as CollectionDetailsResponse),
+              );
+              exportPromises.push(
+                firstValueFrom(this.keyService.activeUserOrgKeys$)
+                  .then((keys) => collection.decrypt(keys[organizationId], this.encryptService))
+                  .then((decCol) => {
+                    decCollections.push(decCol);
+                  }),
+              );
+            });
+          }
+          if (exportData.ciphers != null && exportData.ciphers.length > 0) {
+            exportData.ciphers
+              .filter((c) => c.deletedDate === null)
+              .forEach(async (c) => {
+                const cipher = new Cipher(new CipherData(c));
                 exportPromises.push(
-                  firstValueFrom(this.keyService.activeUserOrgKeys$)
-                    .then((keys) =>
-                      collection.decrypt(
-                        keys[organizationId as OrganizationId],
-                        this.encryptService,
-                      ),
-                    )
-                    .then((decCol) => {
-                      decCollections.push(decCol);
-                    }),
+                  this.cipherService.decrypt(cipher, activeUserId).then((decCipher) => {
+                    if (
+                      !this.restrictedItemTypesService.isCipherRestricted(decCipher, restrictions)
+                    ) {
+                      decCiphers.push(decCipher);
+                    }
+                  }),
                 );
               });
-            }
-            if (exportData.ciphers != null && exportData.ciphers.length > 0) {
-              exportData.ciphers
-                .filter((c) => c.deletedDate === null)
-                .forEach(async (c) => {
-                  const cipher = new Cipher(new CipherData(c));
-                  exportPromises.push(
-                    this.cipherService.decrypt(cipher, activeUserId).then((decCipher) => {
-                      if (
-                        !this.restrictedItemTypesService.isCipherRestricted(decCipher, restrictions)
-                      ) {
-                        decCiphers.push(decCipher);
-                      }
-                    }),
-                  );
-                });
-            }
           }
-          return Promise.all(exportPromises);
-        }),
+        }
+        return Promise.all(exportPromises);
+      }),
     );
 
     await Promise.all(promises);
@@ -191,15 +184,13 @@ export class OrganizationVaultExportService
     return this.buildJsonExport(decCollections, decCiphers);
   }
 
-  private async getOrganizationEncryptedExport(organizationId: string): Promise<string> {
+  private async getOrganizationEncryptedExport(organizationId: OrganizationId): Promise<string> {
     const collections: Collection[] = [];
     const ciphers: Cipher[] = [];
 
     const restrictions = await firstValueFrom(this.restrictedItemTypesService.restricted$);
 
-    const exportData = await this.vaultExportApiService.getOrganizationExport(
-      organizationId as OrganizationId,
-    );
+    const exportData = await this.vaultExportApiService.getOrganizationExport(organizationId);
 
     if (exportData == null) {
       return;
@@ -229,7 +220,7 @@ export class OrganizationVaultExportService
 
   private async getDecryptedManagedExport(
     activeUserId: UserId,
-    organizationId: string,
+    organizationId: OrganizationId,
     format: "json" | "csv",
   ): Promise<string> {
     let decCiphers: CipherView[] = [];
@@ -271,7 +262,7 @@ export class OrganizationVaultExportService
 
   private async getEncryptedManagedExport(
     activeUserId: UserId,
-    organizationId: string,
+    organizationId: OrganizationId,
   ): Promise<string> {
     let encCiphers: Cipher[] = [];
     let allCiphers: Cipher[] = [];
@@ -308,7 +299,7 @@ export class OrganizationVaultExportService
   }
 
   private async BuildEncryptedExport(
-    organizationId: string,
+    organizationId: OrganizationId,
     collections: Collection[],
     ciphers: Cipher[],
   ): Promise<string> {

libs/tools/export/vault-export/vault-export-core/src/services/vault-export.service.abstraction.ts

@@ -1,3 +1,5 @@
+import { OrganizationId } from "@bitwarden/common/types/guid";
+
 import { ExportedVault } from "../types";
 
 export const EXPORT_FORMATS = ["csv", "json", "encrypted_json", "zip"] as const;
@@ -6,7 +8,7 @@ export type ExportFormat = (typeof EXPORT_FORMATS)[number];
 export abstract class VaultExportServiceAbstraction {
   abstract getExport: (format: ExportFormat, password: string) => Promise<ExportedVault>;
   abstract getOrganizationExport: (
-    organizationId: string,
+    organizationId: OrganizationId,
     format: ExportFormat,
     password: string,
     onlyManagedCollections?: boolean,

libs/tools/export/vault-export/vault-export-core/src/services/vault-export.service.ts

@@ -1,4 +1,5 @@
 import { Utils } from "@bitwarden/common/platform/misc/utils";
+import { OrganizationId } from "@bitwarden/common/types/guid";
 
 import { ExportedVault } from "../types";
 
@@ -42,7 +43,7 @@ export class VaultExportService implements VaultExportServiceAbstraction {
    * @throws Error if the organization policies prevent the export
    */
   async getOrganizationExport(
-    organizationId: string,
+    organizationId: OrganizationId,
     format: ExportFormat,
     password: string,
     onlyManagedCollections = false,

libs/tools/export/vault-export/vault-export-ui/src/components/export.component.ts

@@ -29,10 +29,7 @@ import { JslibModule } from "@bitwarden/angular/jslib.module";
 import { PasswordStrengthV2Component } from "@bitwarden/angular/tools/password-strength/password-strength-v2.component";
 import { UserVerificationDialogComponent } from "@bitwarden/auth/angular";
 import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service";
-import {
-  getOrganizationById,
-  OrganizationService,
-} from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
+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";
@@ -42,8 +39,10 @@ import { EventType } from "@bitwarden/common/enums";
 import { FileDownloadService } from "@bitwarden/common/platform/abstractions/file-download/file-download.service";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
+import { getById } from "@bitwarden/common/platform/misc";
 import { Utils } from "@bitwarden/common/platform/misc/utils";
 import { pin } from "@bitwarden/common/tools/rx";
+import { isId, OrganizationId } from "@bitwarden/common/types/guid";
 import {
   AsyncActionsModule,
   BitSubmitDirective,
@@ -84,9 +83,9 @@ import { ExportScopeCalloutComponent } from "./export-scope-callout.component";
   ],
 })
 export class ExportComponent implements OnInit, OnDestroy, AfterViewInit {
-  private _organizationId: string;
+  private _organizationId: OrganizationId | undefined;
 
-  get organizationId(): string {
+  get organizationId(): OrganizationId | undefined {
     return this._organizationId;
   }
 
@@ -94,14 +93,23 @@ export class ExportComponent implements OnInit, OnDestroy, AfterViewInit {
    * Enables the hosting control to pass in an organizationId
    * If a organizationId is provided, the organization selection is disabled.
    */
-  @Input() set organizationId(value: string) {
+  @Input() set organizationId(value: OrganizationId | string | undefined) {
+    if (Utils.isNullOrEmpty(value)) {
+      this._organizationId = undefined;
+      return;
+    }
+
+    if (!isId<OrganizationId>(value)) {
+      this._organizationId = undefined;
+      return;
+    }
+
     this._organizationId = value;
+
     getUserId(this.accountService.activeAccount$)
       .pipe(
         switchMap((userId) =>
-          this.organizationService
-            .organizations$(userId)
-            .pipe(getOrganizationById(this._organizationId)),
+          this.organizationService.organizations$(userId).pipe(getById(this._organizationId)),
         ),
       )
       .pipe(takeUntil(this.destroy$))
@@ -133,11 +141,11 @@ export class ExportComponent implements OnInit, OnDestroy, AfterViewInit {
 
   /**
    * Emits when the creation and download of the export-file have succeeded
-   * - Emits an null/empty string when exporting from an individual vault
+   * - Emits an undefined when exporting from an individual vault
    * - Emits the organizationId when exporting from an organizationl vault
    * */
   @Output()
-  onSuccessfulExport = new EventEmitter<string>();
+  onSuccessfulExport = new EventEmitter<OrganizationId | undefined>();
 
   @ViewChild(PasswordStrengthV2Component) passwordStrengthComponent: PasswordStrengthV2Component;