From 0ec3f661d58e0147341010d7c63b3a362a714f13 Mon Sep 17 00:00:00 2001 From: Nik Gilmore Date: Wed, 22 Oct 2025 16:19:57 -0700 Subject: [PATCH] [PM-22992] Send lastKnownRevisionDate with Attachment API calls (#16862) * Add lastKnownRevisionDate to Attachment methods. * Address issues raised by Claude PR * Fix string errors * Show error to user in event of attachment upload failure * Improve error handling for missing cipher * Add unit tests for attachment lastKnownRevisionDate * Remove generic title from toast errors * Move lastKnwonRevisionDate to function input --- .../models/request/attachment.request.ts | 1 + .../vault/models/request/cipher.request.ts | 1 + .../src/vault/services/cipher.service.spec.ts | 31 +++++++ .../src/vault/services/cipher.service.ts | 13 ++- .../cipher-file-upload.service.spec.ts | 82 +++++++++++++++++++ .../file-upload/cipher-file-upload.service.ts | 1 + .../cipher-attachments.component.spec.ts | 43 ++++++++++ .../cipher-attachments.component.ts | 13 +++ 8 files changed, 183 insertions(+), 2 deletions(-) create mode 100644 libs/common/src/vault/services/file-upload/cipher-file-upload.service.spec.ts diff --git a/libs/common/src/vault/models/request/attachment.request.ts b/libs/common/src/vault/models/request/attachment.request.ts index d058fa69d8b..80205835ab7 100644 --- a/libs/common/src/vault/models/request/attachment.request.ts +++ b/libs/common/src/vault/models/request/attachment.request.ts @@ -5,4 +5,5 @@ export class AttachmentRequest { key: string; fileSize: number; adminRequest: boolean; + lastKnownRevisionDate: Date; } diff --git a/libs/common/src/vault/models/request/cipher.request.ts b/libs/common/src/vault/models/request/cipher.request.ts index 63776c8aea6..b29d5865d2b 100644 --- a/libs/common/src/vault/models/request/cipher.request.ts +++ b/libs/common/src/vault/models/request/cipher.request.ts @@ -201,6 +201,7 @@ export class CipherRequest { this.attachments[attachment.id] = fileName; const attachmentRequest = new AttachmentRequest(); attachmentRequest.fileName = fileName; + attachmentRequest.lastKnownRevisionDate = cipher.revisionDate; if (attachment.key != null) { attachmentRequest.key = attachment.key.encryptedString; } diff --git a/libs/common/src/vault/services/cipher.service.spec.ts b/libs/common/src/vault/services/cipher.service.spec.ts index e6c22961673..85ce8bd0423 100644 --- a/libs/common/src/vault/services/cipher.service.spec.ts +++ b/libs/common/src/vault/services/cipher.service.spec.ts @@ -174,6 +174,37 @@ describe("Cipher Service", () => { expect(spy).toHaveBeenCalled(); }); + + it("should include lastKnownRevisionDate in the upload request", async () => { + const fileName = "filename"; + const fileData = new Uint8Array(10); + const testCipher = new Cipher(cipherData); + const expectedRevisionDate = "2022-01-31T12:00:00.000Z"; + + keyService.getOrgKey.mockReturnValue( + Promise.resolve(new SymmetricCryptoKey(new Uint8Array(32)) as OrgKey), + ); + keyService.makeDataEncKey.mockReturnValue( + Promise.resolve([ + new SymmetricCryptoKey(new Uint8Array(32)), + new EncString("encrypted-key"), + ] as any), + ); + + configService.checkServerMeetsVersionRequirement$.mockReturnValue(of(false)); + configService.getFeatureFlag + .calledWith(FeatureFlag.CipherKeyEncryption) + .mockResolvedValue(false); + + const uploadSpy = jest.spyOn(cipherFileUploadService, "upload").mockResolvedValue({} as any); + + await cipherService.saveAttachmentRawWithServer(testCipher, fileName, fileData, userId); + + // Verify upload was called with cipher that has revisionDate + expect(uploadSpy).toHaveBeenCalled(); + const cipherArg = uploadSpy.mock.calls[0][0]; + expect(cipherArg.revisionDate).toEqual(new Date(expectedRevisionDate)); + }); }); describe("createWithServer()", () => { diff --git a/libs/common/src/vault/services/cipher.service.ts b/libs/common/src/vault/services/cipher.service.ts index 41f94e02cdf..8032c69ed7c 100644 --- a/libs/common/src/vault/services/cipher.service.ts +++ b/libs/common/src/vault/services/cipher.service.ts @@ -937,7 +937,12 @@ export class CipherService implements CipherServiceAbstraction { cipher.attachments.forEach((attachment) => { if (attachment.key == null) { attachmentPromises.push( - this.shareAttachmentWithServer(attachment, cipher.id, organizationId), + this.shareAttachmentWithServer( + attachment, + cipher.id, + organizationId, + cipher.revisionDate, + ), ); } }); @@ -1722,7 +1727,10 @@ export class CipherService implements CipherServiceAbstraction { attachmentView: AttachmentView, cipherId: string, organizationId: string, + lastKnownRevisionDate: Date, ): Promise { + const activeUserId = await firstValueFrom(this.accountService.activeAccount$); + const attachmentResponse = await this.apiService.nativeFetch( new Request(attachmentView.url, { cache: "no-store" }), ); @@ -1731,7 +1739,6 @@ export class CipherService implements CipherServiceAbstraction { } const encBuf = await EncArrayBuffer.fromResponse(attachmentResponse); - const activeUserId = await firstValueFrom(this.accountService.activeAccount$); const userKey = await this.keyService.getUserKey(activeUserId.id); const decBuf = await this.encryptService.decryptFileData(encBuf, userKey); @@ -1752,9 +1759,11 @@ export class CipherService implements CipherServiceAbstraction { const blob = new Blob([encData.buffer], { type: "application/octet-stream" }); fd.append("key", dataEncKey[1].encryptedString); fd.append("data", blob, encFileName.encryptedString); + fd.append("lastKnownRevisionDate", lastKnownRevisionDate.toISOString()); } catch (e) { if (Utils.isNode && !Utils.isBrowser) { fd.append("key", dataEncKey[1].encryptedString); + fd.append("lastKnownRevisionDate", lastKnownRevisionDate.toISOString()); fd.append( "data", Buffer.from(encData.buffer) as any, diff --git a/libs/common/src/vault/services/file-upload/cipher-file-upload.service.spec.ts b/libs/common/src/vault/services/file-upload/cipher-file-upload.service.spec.ts new file mode 100644 index 00000000000..8837f00df6b --- /dev/null +++ b/libs/common/src/vault/services/file-upload/cipher-file-upload.service.spec.ts @@ -0,0 +1,82 @@ +import { mock } from "jest-mock-extended"; + +import { ApiService } from "../../../abstractions/api.service"; +import { EncString } from "../../../key-management/crypto/models/enc-string"; +import { FileUploadService } from "../../../platform/abstractions/file-upload/file-upload.service"; +import { Utils } from "../../../platform/misc/utils"; +import { EncArrayBuffer } from "../../../platform/models/domain/enc-array-buffer"; +import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-crypto-key"; +import { CipherType } from "../../enums/cipher-type"; +import { Cipher } from "../../models/domain/cipher"; +import { AttachmentUploadDataResponse } from "../../models/response/attachment-upload-data.response"; +import { CipherResponse } from "../../models/response/cipher.response"; + +import { CipherFileUploadService } from "./cipher-file-upload.service"; + +describe("CipherFileUploadService", () => { + const apiService = mock(); + const fileUploadService = mock(); + + let service: CipherFileUploadService; + + beforeEach(() => { + jest.clearAllMocks(); + + service = new CipherFileUploadService(apiService, fileUploadService); + }); + + describe("upload", () => { + it("should include lastKnownRevisionDate in the attachment request", async () => { + const cipherId = Utils.newGuid(); + const mockCipher = new Cipher({ + id: cipherId, + type: CipherType.Login, + name: "Test Cipher", + revisionDate: "2024-01-15T10:30:00.000Z", + } as any); + + const mockEncFileName = new EncString("encrypted-filename"); + const mockEncData = { + buffer: new ArrayBuffer(100), + } as unknown as EncArrayBuffer; + + const mockDataEncKey: [SymmetricCryptoKey, EncString] = [ + new SymmetricCryptoKey(new Uint8Array(32)), + new EncString("encrypted-key"), + ]; + + const mockUploadDataResponse = { + attachmentId: "attachment-id", + url: "https://upload.example.com", + fileUploadType: 0, + cipherResponse: { + id: cipherId, + type: CipherType.Login, + revisionDate: "2024-01-15T10:30:00.000Z", + } as CipherResponse, + cipherMiniResponse: null, + } as AttachmentUploadDataResponse; + + apiService.postCipherAttachment.mockResolvedValue(mockUploadDataResponse); + fileUploadService.upload.mockResolvedValue(undefined); + + await service.upload(mockCipher, mockEncFileName, mockEncData, false, mockDataEncKey); + + const callArgs = apiService.postCipherAttachment.mock.calls[0][1]; + + expect(apiService.postCipherAttachment).toHaveBeenCalledWith( + cipherId, + expect.objectContaining({ + key: "encrypted-key", + fileName: "encrypted-filename", + fileSize: 100, + adminRequest: false, + }), + ); + + // Verify lastKnownRevisionDate is set (it's converted to a Date object) + expect(callArgs.lastKnownRevisionDate).toBeDefined(); + expect(callArgs.lastKnownRevisionDate).toEqual(new Date("2024-01-15T10:30:00.000Z")); + }); + }); +}); diff --git 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 index 2fb2746366d..8d97a921748 100644 --- 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 @@ -33,6 +33,7 @@ export class CipherFileUploadService implements CipherFileUploadServiceAbstracti fileName: encFileName.encryptedString, fileSize: encData.buffer.byteLength, adminRequest: admin, + lastKnownRevisionDate: cipher.revisionDate, }; let response: CipherResponse; diff --git a/libs/vault/src/cipher-form/components/attachments/cipher-attachments.component.spec.ts b/libs/vault/src/cipher-form/components/attachments/cipher-attachments.component.spec.ts index 439c651e5ad..c88ce9f0301 100644 --- a/libs/vault/src/cipher-form/components/attachments/cipher-attachments.component.spec.ts +++ b/libs/vault/src/cipher-form/components/attachments/cipher-attachments.component.spec.ts @@ -240,6 +240,49 @@ describe("CipherAttachmentsComponent", () => { message: "maxFileSize", }); }); + + it("shows error toast with server message when saveAttachmentWithServer fails", async () => { + const file = { size: 100 } as File; + component.attachmentForm.controls.file.setValue(file); + + const serverError = new Error("Cipher has been modified by another client"); + saveAttachmentWithServer.mockRejectedValue(serverError); + + await component.submit(); + + expect(showToast).toHaveBeenCalledWith({ + variant: "error", + message: "Cipher has been modified by another client", + }); + }); + + it("shows error toast with fallback message when error has no message property", async () => { + const file = { size: 100 } as File; + component.attachmentForm.controls.file.setValue(file); + + saveAttachmentWithServer.mockRejectedValue({ code: "UNKNOWN_ERROR" }); + + await component.submit(); + + expect(showToast).toHaveBeenCalledWith({ + variant: "error", + message: "unexpectedError", + }); + }); + + it("shows error toast with string error message", async () => { + const file = { size: 100 } as File; + component.attachmentForm.controls.file.setValue(file); + + saveAttachmentWithServer.mockRejectedValue("Network connection failed"); + + await component.submit(); + + expect(showToast).toHaveBeenCalledWith({ + variant: "error", + message: "Network connection failed", + }); + }); }); describe("success", () => { diff --git a/libs/vault/src/cipher-form/components/attachments/cipher-attachments.component.ts b/libs/vault/src/cipher-form/components/attachments/cipher-attachments.component.ts index 0bcb31c7af9..9ae1c62bd3e 100644 --- a/libs/vault/src/cipher-form/components/attachments/cipher-attachments.component.ts +++ b/libs/vault/src/cipher-form/components/attachments/cipher-attachments.component.ts @@ -222,6 +222,19 @@ export class CipherAttachmentsComponent implements OnInit, AfterViewInit { this.onUploadSuccess.emit(); } catch (e) { this.logService.error(e); + + // Extract error message from server response, fallback to generic message + let errorMessage = this.i18nService.t("unexpectedError"); + if (typeof e === "string") { + errorMessage = e; + } else if (e?.message) { + errorMessage = e.message; + } + + this.toastService.showToast({ + variant: "error", + message: errorMessage, + }); } };