[PM-11249] Sync attachment updates across platforms (#11758)

* update extension refresh form when an attachment is added or removed

- This is needed because the revision date was updated on the server and the locally stored cipher needs to match.

* receive updated cipher from delete attachment endpoint

- deleting an attachment will now alter the revision timestamp on a cipher.

* patch the cipher when an attachment is added or deleted

* migrate vault component to use the `cipherViews$` observable

* reference `cipherViews$` on desktop for vault-items

- This avoid race conditions where ciphers are cleared out in the background. `cipherViews` should always emit the latest views

* return CipherData from cipher service so that consumers have the updated cipher right away

* use the updated cipher from attachment endpoints to refresh the details within the add/edit components on desktop

apps/desktop/src/vault/app/vault/add-edit.component.ts

@@ -22,6 +22,7 @@ import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/pl
 import { SdkService } from "@bitwarden/common/platform/abstractions/sdk/sdk.service";
 import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
 import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction";
+import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
 import { CipherAuthorizationService } from "@bitwarden/common/vault/services/cipher-authorization.service";
 import { DialogService, ToastService } from "@bitwarden/components";
 import { SshKeyPasswordPromptComponent } from "@bitwarden/importer/ui";
@@ -148,6 +149,16 @@ export class AddEditComponent extends BaseAddEditComponent implements OnInit, On
     );
   }
 
+  /**
+   * Updates the cipher when an attachment is altered.
+   * Note: This only updates the `attachments` and `revisionDate`
+   * properties to ensure any in-progress edits are not lost.
+   */
+  patchCipherAttachments(cipher: CipherView) {
+    this.cipher.attachments = cipher.attachments;
+    this.cipher.revisionDate = cipher.revisionDate;
+  }
+
   async importSshKeyFromClipboard(password: string = "") {
     const key = await this.platformUtilsService.readFromClipboard();
     const parsedKey = await ipc.platform.sshAgent.importKey(key, password);

apps/desktop/src/vault/app/vault/vault.component.ts

@@ -159,11 +159,6 @@ export class VaultComponent implements OnInit, OnDestroy {
             await this.vaultFilterComponent.reloadCollectionsAndFolders(this.activeFilter);
             await this.vaultFilterComponent.reloadOrganizations();
             break;
-          case "refreshCiphers":
-            // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
-            // eslint-disable-next-line @typescript-eslint/no-floating-promises
-            this.vaultItemsComponent.refresh();
-            break;
           case "modalShown":
             this.showingModal = true;
             break;
@@ -535,9 +530,19 @@ export class VaultComponent implements OnInit, OnDestroy {
 
     let madeAttachmentChanges = false;
     // eslint-disable-next-line rxjs-angular/prefer-takeuntil
-    childComponent.onUploadedAttachment.subscribe(() => (madeAttachmentChanges = true));
+    childComponent.onUploadedAttachment.subscribe((cipher) => {
+      madeAttachmentChanges = true;
+      // Update the edit component cipher with the updated cipher,
+      // which is needed because the revision date is updated when an attachment is altered
+      this.addEditComponent.patchCipherAttachments(cipher);
+    });
     // eslint-disable-next-line rxjs-angular/prefer-takeuntil
-    childComponent.onDeletedAttachment.subscribe(() => (madeAttachmentChanges = true));
+    childComponent.onDeletedAttachment.subscribe((cipher) => {
+      madeAttachmentChanges = true;
+      // Update the edit component cipher with the updated cipher,
+      // which is needed because the revision date is updated when an attachment is altered
+      this.addEditComponent.patchCipherAttachments(cipher);
+    });
 
     // eslint-disable-next-line rxjs-angular/prefer-takeuntil, rxjs/no-async-subscribe
     this.modal.onClosed.subscribe(async () => {

apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.ts

@@ -43,6 +43,7 @@ import {
   DecryptionFailureDialogComponent,
 } from "@bitwarden/vault";
 
+import { CipherFormComponent } from "../../../../../../../libs/vault/src/cipher-form/components/cipher-form.component";
 import { SharedModule } from "../../../shared/shared.module";
 import {
   AttachmentDialogCloseResult,
@@ -144,6 +145,8 @@ export class VaultItemDialogComponent implements OnInit, OnDestroy {
   @ViewChild("dialogContent")
   protected dialogContent: ElementRef<HTMLElement>;
 
+  @ViewChild(CipherFormComponent) cipherFormComponent!: CipherFormComponent;
+
   /**
    * Tracks if the cipher was ever modified while the dialog was open. Used to ensure the dialog emits the correct result
    * in case of closing with the X button or ESC key.
@@ -432,6 +435,22 @@ export class VaultItemDialogComponent implements OnInit, OnDestroy {
       result.action === AttachmentDialogResult.Removed ||
       result.action === AttachmentDialogResult.Uploaded
     ) {
+      const updatedCipher = await this.cipherService.get(this.formConfig.originalCipher?.id);
+      const activeUserId = await firstValueFrom(
+        this.accountService.activeAccount$.pipe(map((a) => a?.id)),
+      );
+
+      const updatedCipherView = await updatedCipher.decrypt(
+        await this.cipherService.getKeyForCipherKeyDecryption(updatedCipher, activeUserId),
+      );
+
+      this.cipherFormComponent.patchCipher((currentCipher) => {
+        currentCipher.attachments = updatedCipherView.attachments;
+        currentCipher.revisionDate = updatedCipherView.revisionDate;
+
+        return currentCipher;
+      });
+
       this._cipherModified = true;
     }
   };

libs/angular/src/vault/components/attachments.component.ts

@@ -16,6 +16,7 @@ import { StateService } from "@bitwarden/common/platform/abstractions/state.serv
 import { EncArrayBuffer } from "@bitwarden/common/platform/models/domain/enc-array-buffer";
 import { UserId } from "@bitwarden/common/types/guid";
 import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
+import { CipherData } from "@bitwarden/common/vault/models/data/cipher.data";
 import { Cipher } from "@bitwarden/common/vault/models/domain/cipher";
 import { AttachmentView } from "@bitwarden/common/vault/models/view/attachment.view";
 import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
@@ -26,15 +27,15 @@ import { KeyService } from "@bitwarden/key-management";
 export class AttachmentsComponent implements OnInit {
   @Input() cipherId: string;
   @Input() viewOnly: boolean;
-  @Output() onUploadedAttachment = new EventEmitter();
+  @Output() onUploadedAttachment = new EventEmitter<CipherView>();
   @Output() onDeletedAttachment = new EventEmitter();
   @Output() onReuploadedAttachment = new EventEmitter();
 
   cipher: CipherView;
   cipherDomain: Cipher;
   canAccessAttachments: boolean;
   formPromise: Promise<any>;
-  deletePromises: { [id: string]: Promise<any> } = {};
+  deletePromises: { [id: string]: Promise<CipherData> } = {};
   reuploadPromises: { [id: string]: Promise<any> } = {};
   emergencyAccessId?: string = null;
   protected componentName = "";
@@ -96,7 +97,7 @@ export class AttachmentsComponent implements OnInit {
         title: null,
         message: this.i18nService.t("attachmentSaved"),
       });
-      this.onUploadedAttachment.emit();
+      this.onUploadedAttachment.emit(this.cipher);
     } catch (e) {
       this.logService.error(e);
     }
@@ -125,7 +126,16 @@ export class AttachmentsComponent implements OnInit {
 
     try {
       this.deletePromises[attachment.id] = this.deleteCipherAttachment(attachment.id);
-      await this.deletePromises[attachment.id];
+      const updatedCipher = await this.deletePromises[attachment.id];
+
+      const activeUserId = await firstValueFrom(
+        this.accountService.activeAccount$.pipe(map((a) => a?.id)),
+      );
+      const cipher = new Cipher(updatedCipher);
+      this.cipher = await cipher.decrypt(
+        await this.cipherService.getKeyForCipherKeyDecryption(cipher, activeUserId),
+      );
+
       this.toastService.showToast({
         variant: "success",
         title: null,
@@ -140,7 +150,7 @@ export class AttachmentsComponent implements OnInit {
     }
 
     this.deletePromises[attachment.id] = null;
-    this.onDeletedAttachment.emit();
+    this.onDeletedAttachment.emit(this.cipher);
   }
 
   async download(attachment: AttachmentView) {

libs/angular/src/vault/components/vault-items.component.ts

@@ -1,7 +1,8 @@
 // FIXME: Update this file to be type safe and remove this and next line
 // @ts-strict-ignore
 import { Directive, EventEmitter, Input, OnDestroy, OnInit, Output } from "@angular/core";
-import { BehaviorSubject, firstValueFrom, from, Subject, switchMap, takeUntil } from "rxjs";
+import { takeUntilDestroyed } from "@angular/core/rxjs-interop";
+import { BehaviorSubject, Subject, firstValueFrom, from, switchMap, takeUntil } from "rxjs";
 
 import { SearchService } from "@bitwarden/common/abstractions/search.service";
 import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
@@ -40,7 +41,12 @@ export class VaultItemsComponent implements OnInit, OnDestroy {
   constructor(
     protected searchService: SearchService,
     protected cipherService: CipherService,
-  ) {}
+  ) {
+    this.cipherService.cipherViews$.pipe(takeUntilDestroyed()).subscribe((ciphers) => {
+      void this.doSearch(ciphers);
+      this.loaded = true;
+    });
+  }
 
   ngOnInit(): void {
     this._searchText$
@@ -117,7 +123,7 @@ export class VaultItemsComponent implements OnInit, OnDestroy {
   protected deletedFilter: (cipher: CipherView) => boolean = (c) => c.isDeleted === this.deleted;
 
   protected async doSearch(indexedCiphers?: CipherView[]) {
-    indexedCiphers = indexedCiphers ?? (await this.cipherService.getAllDecrypted());
+    indexedCiphers = indexedCiphers ?? (await firstValueFrom(this.cipherService.cipherViews$));
 
     const failedCiphers = await firstValueFrom(this.cipherService.failedToDecryptCiphers$);
     if (failedCiphers != null && failedCiphers.length > 0) {

libs/angular/src/vault/components/view.component.ts

@@ -11,7 +11,7 @@ import {
   OnInit,
   Output,
 } from "@angular/core";
-import { firstValueFrom, map, Observable } from "rxjs";
+import { filter, firstValueFrom, map, Observable } from "rxjs";
 
 import { ApiService } from "@bitwarden/common/abstractions/api.service";
 import { AuditService } from "@bitwarden/common/abstractions/audit.service";
@@ -144,11 +144,15 @@ export class ViewComponent implements OnDestroy, OnInit {
   async load() {
     this.cleanUp();
 
-    const cipher = await this.cipherService.get(this.cipherId);
     const activeUserId = await firstValueFrom(this.activeUserId$);
-    this.cipher = await cipher.decrypt(
-      await this.cipherService.getKeyForCipherKeyDecryption(cipher, activeUserId),
+    // Grab individual cipher from `cipherViews$` for the most up-to-date information
+    this.cipher = await firstValueFrom(
+      this.cipherService.cipherViews$.pipe(
+        map((ciphers) => ciphers.find((c) => c.id === this.cipherId)),
+        filter((cipher) => !!cipher),
+      ),
     );
+
     this.canAccessPremium = await firstValueFrom(
       this.billingAccountProfileStateService.hasPremiumFromAnySource$(activeUserId),
     );

libs/common/src/services/api.service.ts

@@ -702,7 +702,7 @@ export class ApiService implements ApiServiceAbstraction {
   }
 
   deleteCipherAttachment(id: string, attachmentId: string): Promise<any> {
-    return this.send("DELETE", "/ciphers/" + id + "/attachment/" + attachmentId, null, true, false);
+    return this.send("DELETE", "/ciphers/" + id + "/attachment/" + attachmentId, null, true, true);
   }
 
   deleteCipherAttachmentAdmin(id: string, attachmentId: string): Promise<any> {

libs/common/src/vault/abstractions/cipher.service.ts

@@ -154,8 +154,8 @@ export abstract class CipherService implements UserKeyRotationDataProvider<Ciphe
   delete: (id: string | string[]) => Promise<any>;
   deleteWithServer: (id: string, asAdmin?: boolean) => Promise<any>;
   deleteManyWithServer: (ids: string[], asAdmin?: boolean) => Promise<any>;
-  deleteAttachment: (id: string, attachmentId: string) => Promise<void>;
-  deleteAttachmentWithServer: (id: string, attachmentId: string) => Promise<void>;
+  deleteAttachment: (id: string, revisionDate: string, attachmentId: string) => Promise<CipherData>;
+  deleteAttachmentWithServer: (id: string, attachmentId: string) => Promise<CipherData>;
   sortCiphersByLastUsed: (a: CipherView, b: CipherView) => number;
   sortCiphersByLastUsedThenName: (a: CipherView, b: CipherView) => number;
   getLocaleSortingFunction: () => (a: CipherView, b: CipherView) => number;

libs/common/src/vault/services/cipher.service.ts

@@ -1078,7 +1078,11 @@ export class CipherService implements CipherServiceAbstraction {
     await this.delete(ids);
   }
 
-  async deleteAttachment(id: string, attachmentId: string): Promise<void> {
+  async deleteAttachment(
+    id: string,
+    revisionDate: string,
+    attachmentId: string,
+  ): Promise<CipherData> {
     let ciphers = await firstValueFrom(this.ciphers$);
     const cipherId = id as CipherId;
     // eslint-disable-next-line
@@ -1092,22 +1096,31 @@ export class CipherService implements CipherServiceAbstraction {
       }
     }
 
+    // Deleting the cipher updates the revision date on the server,
+    // Update the stored `revisionDate` to match
+    ciphers[cipherId].revisionDate = revisionDate;
+
     await this.clearCache();
     await this.encryptedCiphersState.update(() => {
       if (ciphers == null) {
         ciphers = {};
       }
       return ciphers;
     });
+
+    return ciphers[cipherId];
   }
 
-  async deleteAttachmentWithServer(id: string, attachmentId: string): Promise<void> {
+  async deleteAttachmentWithServer(id: string, attachmentId: string): Promise<CipherData> {
+    let cipherResponse = null;
     try {
-      await this.apiService.deleteCipherAttachment(id, attachmentId);
+      cipherResponse = await this.apiService.deleteCipherAttachment(id, attachmentId);
     } catch (e) {
       return Promise.reject((e as ErrorResponse).getSingleMessage());
     }
-    await this.deleteAttachment(id, attachmentId);
+    const cipherData = CipherData.fromJSON(cipherResponse?.cipher);
+
+    return await this.deleteAttachment(id, cipherData.revisionDate, attachmentId);
   }
 
   sortCiphersByLastUsed(a: CipherView, b: CipherView): number {

libs/vault/src/cipher-form/index.ts

@@ -9,3 +9,4 @@ export { TotpCaptureService } from "./abstractions/totp-capture.service";
 export { CipherFormGenerationService } from "./abstractions/cipher-form-generation.service";
 export { DefaultCipherFormConfigService } from "./services/default-cipher-form-config.service";
 export { CipherFormGeneratorComponent } from "./components/cipher-generator/cipher-form-generator.component";
+export { CipherFormContainer } from "../cipher-form/cipher-form-container";