[SG 623] Send Service Refactor (#4327)

* Split out api methods into sendApiService * Move SendService and abstraction * Libs updates * Web updates * CLI updates * Desktop updates * libs send service fixes * browser factory additions * Browser updates * Fix service injection for CLI SendReceiveCommand * Deprecate directly calling send state service methods * SendService observables updates * Update components to use new observables * Modify CLI to use state service instead of observables * Remove unnecessary await on get() * Move delete() to InternalSendService * SendService unit tests * Split fileUploadService by send and cipher * send and cipher service factory updates * Add file upload methods to get around circular dependency issues * Move api methods from sendService to sendApiService * Update cipherService to use fileApi methods * libs service injection and component changes * browser service injection and component changes * Desktop component changes * Web component changes * cipher service test fix * Fix file capitalization * CLI service import and command updates * Remove extra abstract fileUploadService * WIP: Condense callbacks for file upload Co-authored-by: Robyn MacCallum <robyntmaccallum@gmail.com> * Send callbacks for file upload * Fix circular service dependencies * Fix response return on upload * Fix function definitions * Service injection fixes and bug fixes * Fix folder casing * Service injection cleanup * Remove deleted file from capital letters whitelist * Create new SendApiService for popup * Move cipherFileUploadService to vault * Move SendFileUploadService methods into SendApiService * Rename methods to remove 'WithServer' * Properly subscribe to sendViews * Fix Send serialization * Implement fromJSON on sendFile and sendText * [PM-1347] Fix send key serialization (#4989) * Properly serialize key on send fromJSON * Remove call that nulled out decrypted sends * Fix null checks in fromJSON methods for models * lint fixes --------- Co-authored-by: Matt Gibson <mgibson@bitwarden.com>
2025-12-16 08:13:42 +00:00 · 2023-03-28 12:37:40 -04:00
parent c2bfb2497b
commit d799529428
58 changed files with 1333 additions and 663 deletions
--- a/libs/common/src/vault/abstractions/file-upload/cipher-file-upload.service.ts
+++ b/libs/common/src/vault/abstractions/file-upload/cipher-file-upload.service.ts
@@ -0,0 +1,15 @@
+import { EncArrayBuffer } from "../../../models/domain/enc-array-buffer";
+import { EncString } from "../../../models/domain/enc-string";
+import { SymmetricCryptoKey } from "../../../models/domain/symmetric-crypto-key";
+import { Cipher } from "../../models/domain/cipher";
+import { CipherResponse } from "../../models/response/cipher.response";
+
+export abstract class CipherFileUploadService {
+  upload: (
+    cipher: Cipher,
+    encFileName: EncString,
+    encData: EncArrayBuffer,
+    admin: boolean,
+    dataEncKey: [SymmetricCryptoKey, EncString]
+  ) => Promise<CipherResponse>;
+}
--- a/libs/common/src/vault/services/cipher.service.spec.ts
+++ b/libs/common/src/vault/services/cipher.service.spec.ts
@@ -4,7 +4,6 @@ import { Arg, Substitute, SubstituteOf } from "@fluffy-spoon/substitute";
 import { ApiService } from "../../abstractions/api.service";
 import { CryptoService } from "../../abstractions/crypto.service";
 import { EncryptService } from "../../abstractions/encrypt.service";
-import { FileUploadService } from "../../abstractions/fileUpload.service";
 import { I18nService } from "../../abstractions/i18n.service";
 import { LogService } from "../../abstractions/log.service";
 import { SearchService } from "../../abstractions/search.service";
@@ -13,6 +12,7 @@ import { StateService } from "../../abstractions/state.service";
 import { EncArrayBuffer } from "../../models/domain/enc-array-buffer";
 import { EncString } from "../../models/domain/enc-string";
 import { SymmetricCryptoKey } from "../../models/domain/symmetric-crypto-key";
+import { CipherFileUploadService } from "../abstractions/file-upload/cipher-file-upload.service";
 import { Cipher } from "../models/domain/cipher";

 import { CipherService } from "./cipher.service";
@@ -25,7 +25,7 @@ describe("Cipher Service", () => {
  let stateService: SubstituteOf<StateService>;
  let settingsService: SubstituteOf<SettingsService>;
  let apiService: SubstituteOf<ApiService>;
-  let fileUploadService: SubstituteOf<FileUploadService>;
+  let cipherFileUploadService: SubstituteOf<CipherFileUploadService>;
  let i18nService: SubstituteOf<I18nService>;
  let searchService: SubstituteOf<SearchService>;
  let logService: SubstituteOf<LogService>;
@@ -38,7 +38,7 @@ describe("Cipher Service", () => {
    stateService = Substitute.for<StateService>();
    settingsService = Substitute.for<SettingsService>();
    apiService = Substitute.for<ApiService>();
-    fileUploadService = Substitute.for<FileUploadService>();
+    cipherFileUploadService = Substitute.for<CipherFileUploadService>();
    i18nService = Substitute.for<I18nService>();
    searchService = Substitute.for<SearchService>();
    logService = Substitute.for<LogService>();
@@ -51,12 +51,12 @@ describe("Cipher Service", () => {
      cryptoService,
      settingsService,
      apiService,
-      fileUploadService,
      i18nService,
      () => searchService,
      logService,
      stateService,
-      encryptService
+      encryptService,
+      cipherFileUploadService
    );
  });

@@ -67,8 +67,8 @@ describe("Cipher Service", () => {

    await cipherService.saveAttachmentRawWithServer(new Cipher(), fileName, fileData);

-    fileUploadService
+    cipherFileUploadService
      .received(1)
-      .uploadCipherAttachment(Arg.any(), Arg.any(), new EncString(ENCRYPTED_TEXT), ENCRYPTED_BYTES);
+      .upload(Arg.any(), Arg.any(), ENCRYPTED_BYTES, Arg.any(), Arg.any());
  });
 });
--- a/libs/common/src/vault/services/cipher.service.ts
+++ b/libs/common/src/vault/services/cipher.service.ts
@@ -3,7 +3,6 @@ import { firstValueFrom } from "rxjs";
 import { ApiService } from "../../abstractions/api.service";
 import { CryptoService } from "../../abstractions/crypto.service";
 import { EncryptService } from "../../abstractions/encrypt.service";
-import { FileUploadService } from "../../abstractions/fileUpload.service";
 import { I18nService } from "../../abstractions/i18n.service";
 import { LogService } from "../../abstractions/log.service";
 import { SearchService } from "../../abstractions/search.service";
@@ -21,6 +20,7 @@ import { SymmetricCryptoKey } from "../../models/domain/symmetric-crypto-key";
 import { ErrorResponse } from "../../models/response/error.response";
 import { View } from "../../models/view/view";
 import { CipherService as CipherServiceAbstraction } from "../abstractions/cipher.service";
+import { CipherFileUploadService } from "../abstractions/file-upload/cipher-file-upload.service";
 import { CipherType } from "../enums/cipher-type";
 import { CipherData } from "../models/data/cipher.data";
 import { Attachment } from "../models/domain/attachment";
@@ -33,7 +33,6 @@ import { LoginUri } from "../models/domain/login-uri";
 import { Password } from "../models/domain/password";
 import { SecureNote } from "../models/domain/secure-note";
 import { SortedCiphersCache } from "../models/domain/sorted-ciphers-cache";
-import { AttachmentRequest } from "../models/request/attachment.request";
 import { CipherBulkDeleteRequest } from "../models/request/cipher-bulk-delete.request";
 import { CipherBulkMoveRequest } from "../models/request/cipher-bulk-move.request";
 import { CipherBulkRestoreRequest } from "../models/request/cipher-bulk-restore.request";
@@ -58,12 +57,12 @@ export class CipherService implements CipherServiceAbstraction {
    private cryptoService: CryptoService,
    private settingsService: SettingsService,
    private apiService: ApiService,
-    private fileUploadService: FileUploadService,
    private i18nService: I18nService,
    private searchService: () => SearchService,
    private logService: LogService,
    private stateService: StateService,
-    private encryptService: EncryptService
+    private encryptService: EncryptService,
+    private cipherFileUploadService: CipherFileUploadService
  ) {}

  async getDecryptedCipherCache(): Promise<CipherView[]> {
@@ -725,41 +724,13 @@ export class CipherService implements CipherServiceAbstraction {
    const dataEncKey = await this.cryptoService.makeEncKey(key);
    const encData = await this.cryptoService.encryptToBytes(data, dataEncKey[0]);

-    const request: AttachmentRequest = {
-      key: dataEncKey[1].encryptedString,
-      fileName: encFileName.encryptedString,
-      fileSize: encData.buffer.byteLength,
-      adminRequest: admin,
-    };
-
-    let response: CipherResponse;
-    try {
-      const uploadDataResponse = await this.apiService.postCipherAttachment(cipher.id, request);
-      response = admin ? uploadDataResponse.cipherMiniResponse : uploadDataResponse.cipherResponse;
-      await this.fileUploadService.uploadCipherAttachment(
-        admin,
-        uploadDataResponse,
-        encFileName,
-        encData
-      );
-    } catch (e) {
-      if (
-        (e instanceof ErrorResponse && (e as ErrorResponse).statusCode === 404) ||
-        (e as ErrorResponse).statusCode === 405
-      ) {
-        response = await this.legacyServerAttachmentFileUpload(
-          admin,
-          cipher.id,
-          encFileName,
-          encData,
-          dataEncKey[1]
-        );
-      } else if (e instanceof ErrorResponse) {
-        throw new Error((e as ErrorResponse).getSingleMessage());
-      } else {
-        throw e;
-      }
-    }
+    const response = await this.cipherFileUploadService.upload(
+      cipher,
+      encFileName,
+      encData,
+      admin,
+      dataEncKey
+    );

    const cData = new CipherData(response, cipher.collectionIds);
    if (!admin) {
@@ -768,52 +739,6 @@ export class CipherService implements CipherServiceAbstraction {
    return new Cipher(cData);
  }

-  /**
-   * @deprecated Mar 25 2021: This method has been deprecated in favor of direct uploads.
-   * This method still exists for backward compatibility with old server versions.
-   */
-  async legacyServerAttachmentFileUpload(
-    admin: boolean,
-    cipherId: string,
-    encFileName: EncString,
-    encData: EncArrayBuffer,
-    key: EncString
-  ) {
-    const fd = new FormData();
-    try {
-      const blob = new Blob([encData.buffer], { type: "application/octet-stream" });
-      fd.append("key", key.encryptedString);
-      fd.append("data", blob, encFileName.encryptedString);
-    } catch (e) {
-      if (Utils.isNode && !Utils.isBrowser) {
-        fd.append("key", key.encryptedString);
-        fd.append(
-          "data",
-          Buffer.from(encData.buffer) as any,
-          {
-            filepath: encFileName.encryptedString,
-            contentType: "application/octet-stream",
-          } as any
-        );
-      } else {
-        throw e;
-      }
-    }
-
-    let response: CipherResponse;
-    try {
-      if (admin) {
-        response = await this.apiService.postCipherAttachmentAdminLegacy(cipherId, fd);
-      } else {
-        response = await this.apiService.postCipherAttachmentLegacy(cipherId, fd);
-      }
-    } catch (e) {
-      throw new Error((e as ErrorResponse).getSingleMessage());
-    }
-
-    return response;
-  }
-
  async saveCollectionsWithServer(cipher: Cipher): Promise<any> {
    const request = new CipherCollectionsRequest(cipher.collectionIds);
    await this.apiService.putCipherCollections(cipher.id, request);
--- a/libs/common/src/vault/services/file-upload/cipher-file-upload.service.ts
+++ b/libs/common/src/vault/services/file-upload/cipher-file-upload.service.ts
@@ -0,0 +1,157 @@
+import { ApiService } from "../../../abstractions/api.service";
+import {
+  FileUploadApiMethods,
+  FileUploadService,
+} from "../../../abstractions/file-upload/file-upload.service";
+import { Utils } from "../../../misc/utils";
+import { EncArrayBuffer } from "../../../models/domain/enc-array-buffer";
+import { EncString } from "../../../models/domain/enc-string";
+import { SymmetricCryptoKey } from "../../../models/domain/symmetric-crypto-key";
+import { ErrorResponse } from "../../../models/response/error.response";
+import { CipherFileUploadService as CipherFileUploadServiceAbstraction } from "../../abstractions/file-upload/cipher-file-upload.service";
+import { Cipher } from "../../models/domain/cipher";
+import { AttachmentRequest } from "../../models/request/attachment.request";
+import { AttachmentUploadDataResponse } from "../../models/response/attachment-upload-data.response";
+import { CipherResponse } from "../../models/response/cipher.response";
+
+export class CipherFileUploadService implements CipherFileUploadServiceAbstraction {
+  constructor(private apiService: ApiService, private fileUploadService: FileUploadService) {}
+
+  async upload(
+    cipher: Cipher,
+    encFileName: EncString,
+    encData: EncArrayBuffer,
+    admin: boolean,
+    dataEncKey: [SymmetricCryptoKey, EncString]
+  ): Promise<CipherResponse> {
+    const request: AttachmentRequest = {
+      key: dataEncKey[1].encryptedString,
+      fileName: encFileName.encryptedString,
+      fileSize: encData.buffer.byteLength,
+      adminRequest: admin,
+    };
+
+    let response: CipherResponse;
+    try {
+      const uploadDataResponse = await this.apiService.postCipherAttachment(cipher.id, request);
+      response = admin ? uploadDataResponse.cipherMiniResponse : uploadDataResponse.cipherResponse;
+      await this.fileUploadService.upload(
+        uploadDataResponse,
+        encFileName,
+        encData,
+        this.generateMethods(uploadDataResponse, response, request.adminRequest)
+      );
+    } catch (e) {
+      if (
+        (e instanceof ErrorResponse && (e as ErrorResponse).statusCode === 404) ||
+        (e as ErrorResponse).statusCode === 405
+      ) {
+        response = await this.legacyServerAttachmentFileUpload(
+          request.adminRequest,
+          cipher.id,
+          encFileName,
+          encData,
+          dataEncKey[1]
+        );
+      } else if (e instanceof ErrorResponse) {
+        throw new Error((e as ErrorResponse).getSingleMessage());
+      } else {
+        throw e;
+      }
+    }
+    return response;
+  }
+
+  private generateMethods(
+    uploadData: AttachmentUploadDataResponse,
+    response: CipherResponse,
+    isAdmin: boolean
+  ): FileUploadApiMethods {
+    return {
+      postDirect: this.generatePostDirectCallback(uploadData, isAdmin),
+      renewFileUploadUrl: this.generateRenewFileUploadUrlCallback(uploadData, response, isAdmin),
+      rollback: this.generateRollbackCallback(response, uploadData, isAdmin),
+    };
+  }
+
+  private generatePostDirectCallback(uploadData: AttachmentUploadDataResponse, isAdmin: boolean) {
+    return (data: FormData) => {
+      const response = isAdmin ? uploadData.cipherMiniResponse : uploadData.cipherResponse;
+      return this.apiService.postAttachmentFile(response.id, uploadData.attachmentId, data);
+    };
+  }
+
+  private generateRenewFileUploadUrlCallback(
+    uploadData: AttachmentUploadDataResponse,
+    response: CipherResponse,
+    isAdmin: boolean
+  ) {
+    return async () => {
+      const renewResponse = await this.apiService.renewAttachmentUploadUrl(
+        response.id,
+        uploadData.attachmentId
+      );
+      return renewResponse?.url;
+    };
+  }
+
+  private generateRollbackCallback(
+    response: CipherResponse,
+    uploadData: AttachmentUploadDataResponse,
+    isAdmin: boolean
+  ) {
+    return () => {
+      if (isAdmin) {
+        return this.apiService.deleteCipherAttachmentAdmin(response.id, uploadData.attachmentId);
+      } else {
+        return this.apiService.deleteCipherAttachment(response.id, uploadData.attachmentId);
+      }
+    };
+  }
+
+  /**
+   * @deprecated Mar 25 2021: This method has been deprecated in favor of direct uploads.
+   * This method still exists for backward compatibility with old server versions.
+   */
+  async legacyServerAttachmentFileUpload(
+    admin: boolean,
+    cipherId: string,
+    encFileName: EncString,
+    encData: EncArrayBuffer,
+    key: EncString
+  ) {
+    const fd = new FormData();
+    try {
+      const blob = new Blob([encData.buffer], { type: "application/octet-stream" });
+      fd.append("key", key.encryptedString);
+      fd.append("data", blob, encFileName.encryptedString);
+    } catch (e) {
+      if (Utils.isNode && !Utils.isBrowser) {
+        fd.append("key", key.encryptedString);
+        fd.append(
+          "data",
+          Buffer.from(encData.buffer) as any,
+          {
+            filepath: encFileName.encryptedString,
+            contentType: "application/octet-stream",
+          } as any
+        );
+      } else {
+        throw e;
+      }
+    }
+
+    let response: CipherResponse;
+    try {
+      if (admin) {
+        response = await this.apiService.postCipherAttachmentAdminLegacy(cipherId, fd);
+      } else {
+        response = await this.apiService.postCipherAttachmentLegacy(cipherId, fd);
+      }
+    } catch (e) {
+      throw new Error((e as ErrorResponse).getSingleMessage());
+    }
+
+    return response;
+  }
+}
--- a/libs/common/src/vault/services/sync/sync.service.ts
+++ b/libs/common/src/vault/services/sync/sync.service.ts
@@ -2,7 +2,8 @@ import { ApiService } from "../../../abstractions/api.service";
 import { CryptoService } from "../../../abstractions/crypto.service";
 import { LogService } from "../../../abstractions/log.service";
 import { MessagingService } from "../../../abstractions/messaging.service";
-import { SendService } from "../../../abstractions/send.service";
+import { SendApiService } from "../../../abstractions/send/send-api.service.abstraction";
+import { InternalSendService } from "../../../abstractions/send/send.service.abstraction";
 import { SettingsService } from "../../../abstractions/settings.service";
 import { StateService } from "../../../abstractions/state.service";
 import { CollectionService } from "../../../admin-console/abstractions/collection.service";
@@ -47,13 +48,14 @@ export class SyncService implements SyncServiceAbstraction {
    private collectionService: CollectionService,
    private messagingService: MessagingService,
    private policyService: InternalPolicyService,
-    private sendService: SendService,
+    private sendService: InternalSendService,
    private logService: LogService,
    private keyConnectorService: KeyConnectorService,
    private stateService: StateService,
    private providerService: ProviderService,
    private folderApiService: FolderApiServiceAbstraction,
    private organizationService: InternalOrganizationService,
+    private sendApiService: SendApiService,
    private logoutCallback: (expired: boolean) => Promise<void>
  ) {}

@@ -230,12 +232,12 @@ export class SyncService implements SyncServiceAbstraction {
    this.syncStarted();
    if (await this.stateService.getIsAuthenticated()) {
      try {
-        const localSend = await this.sendService.get(notification.id);
+        const localSend = this.sendService.get(notification.id);
        if (
          (!isEdit && localSend == null) ||
          (isEdit && localSend != null && localSend.revisionDate < notification.revisionDate)
        ) {
-          const remoteSend = await this.apiService.getSend(notification.id);
+          const remoteSend = await this.sendApiService.getSend(notification.id);
          if (remoteSend != null) {
            await this.sendService.upsert(new SendData(remoteSend));
            this.messagingService.send("syncedUpsertedSend", { sendId: notification.id });