mirror of
https://github.com/bitwarden/browser
synced 2025-12-11 13:53:34 +00:00
[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
This commit is contained in:
@@ -5,4 +5,5 @@ export class AttachmentRequest {
|
|||||||
key: string;
|
key: string;
|
||||||
fileSize: number;
|
fileSize: number;
|
||||||
adminRequest: boolean;
|
adminRequest: boolean;
|
||||||
|
lastKnownRevisionDate: Date;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -201,6 +201,7 @@ export class CipherRequest {
|
|||||||
this.attachments[attachment.id] = fileName;
|
this.attachments[attachment.id] = fileName;
|
||||||
const attachmentRequest = new AttachmentRequest();
|
const attachmentRequest = new AttachmentRequest();
|
||||||
attachmentRequest.fileName = fileName;
|
attachmentRequest.fileName = fileName;
|
||||||
|
attachmentRequest.lastKnownRevisionDate = cipher.revisionDate;
|
||||||
if (attachment.key != null) {
|
if (attachment.key != null) {
|
||||||
attachmentRequest.key = attachment.key.encryptedString;
|
attachmentRequest.key = attachment.key.encryptedString;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -174,6 +174,37 @@ describe("Cipher Service", () => {
|
|||||||
|
|
||||||
expect(spy).toHaveBeenCalled();
|
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<any>(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()", () => {
|
describe("createWithServer()", () => {
|
||||||
|
|||||||
@@ -937,7 +937,12 @@ export class CipherService implements CipherServiceAbstraction {
|
|||||||
cipher.attachments.forEach((attachment) => {
|
cipher.attachments.forEach((attachment) => {
|
||||||
if (attachment.key == null) {
|
if (attachment.key == null) {
|
||||||
attachmentPromises.push(
|
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,
|
attachmentView: AttachmentView,
|
||||||
cipherId: string,
|
cipherId: string,
|
||||||
organizationId: string,
|
organizationId: string,
|
||||||
|
lastKnownRevisionDate: Date,
|
||||||
): Promise<any> {
|
): Promise<any> {
|
||||||
|
const activeUserId = await firstValueFrom(this.accountService.activeAccount$);
|
||||||
|
|
||||||
const attachmentResponse = await this.apiService.nativeFetch(
|
const attachmentResponse = await this.apiService.nativeFetch(
|
||||||
new Request(attachmentView.url, { cache: "no-store" }),
|
new Request(attachmentView.url, { cache: "no-store" }),
|
||||||
);
|
);
|
||||||
@@ -1731,7 +1739,6 @@ export class CipherService implements CipherServiceAbstraction {
|
|||||||
}
|
}
|
||||||
|
|
||||||
const encBuf = await EncArrayBuffer.fromResponse(attachmentResponse);
|
const encBuf = await EncArrayBuffer.fromResponse(attachmentResponse);
|
||||||
const activeUserId = await firstValueFrom(this.accountService.activeAccount$);
|
|
||||||
const userKey = await this.keyService.getUserKey(activeUserId.id);
|
const userKey = await this.keyService.getUserKey(activeUserId.id);
|
||||||
const decBuf = await this.encryptService.decryptFileData(encBuf, userKey);
|
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" });
|
const blob = new Blob([encData.buffer], { type: "application/octet-stream" });
|
||||||
fd.append("key", dataEncKey[1].encryptedString);
|
fd.append("key", dataEncKey[1].encryptedString);
|
||||||
fd.append("data", blob, encFileName.encryptedString);
|
fd.append("data", blob, encFileName.encryptedString);
|
||||||
|
fd.append("lastKnownRevisionDate", lastKnownRevisionDate.toISOString());
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
if (Utils.isNode && !Utils.isBrowser) {
|
if (Utils.isNode && !Utils.isBrowser) {
|
||||||
fd.append("key", dataEncKey[1].encryptedString);
|
fd.append("key", dataEncKey[1].encryptedString);
|
||||||
|
fd.append("lastKnownRevisionDate", lastKnownRevisionDate.toISOString());
|
||||||
fd.append(
|
fd.append(
|
||||||
"data",
|
"data",
|
||||||
Buffer.from(encData.buffer) as any,
|
Buffer.from(encData.buffer) as any,
|
||||||
|
|||||||
@@ -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<ApiService>();
|
||||||
|
const fileUploadService = mock<FileUploadService>();
|
||||||
|
|
||||||
|
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"));
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -33,6 +33,7 @@ export class CipherFileUploadService implements CipherFileUploadServiceAbstracti
|
|||||||
fileName: encFileName.encryptedString,
|
fileName: encFileName.encryptedString,
|
||||||
fileSize: encData.buffer.byteLength,
|
fileSize: encData.buffer.byteLength,
|
||||||
adminRequest: admin,
|
adminRequest: admin,
|
||||||
|
lastKnownRevisionDate: cipher.revisionDate,
|
||||||
};
|
};
|
||||||
|
|
||||||
let response: CipherResponse;
|
let response: CipherResponse;
|
||||||
|
|||||||
@@ -240,6 +240,49 @@ describe("CipherAttachmentsComponent", () => {
|
|||||||
message: "maxFileSize",
|
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", () => {
|
describe("success", () => {
|
||||||
|
|||||||
@@ -222,6 +222,19 @@ export class CipherAttachmentsComponent implements OnInit, AfterViewInit {
|
|||||||
this.onUploadSuccess.emit();
|
this.onUploadSuccess.emit();
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
this.logService.error(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,
|
||||||
|
});
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user