[PM-21041] Fix cipher view security tasks fetching (#14569)

* [PM-21041] Add taskEnabled$ dependency to tasks$ observable

* [PM-21041] Rework cipher view component to only check tasks for organization Login type ciphers

- Remove dependency on feature flag check (handled by tasks$ observable now)
- Add try/catch in case of request failures to avoid breaking component initialization

* [PM-21041] Remove now redundant taskEnabled$ chain

* [PM-21041] Fix tests

apps/browser/src/vault/popup/components/at-risk-callout/at-risk-password-callout.component.ts

@@ -1,7 +1,7 @@
 import { CommonModule } from "@angular/common";
 import { Component, inject } from "@angular/core";
 import { RouterModule } from "@angular/router";
-import { map, of, switchMap } from "rxjs";
+import { map, switchMap } from "rxjs";
 
 import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
 import { getUserId } from "@bitwarden/common/auth/services/account.service";
@@ -20,21 +20,7 @@ export class AtRiskPasswordCalloutComponent {
   private activeAccount$ = inject(AccountService).activeAccount$.pipe(getUserId);
 
   protected pendingTasks$ = this.activeAccount$.pipe(
-    switchMap((userId) =>
+    switchMap((userId) => this.taskService.pendingTasks$(userId)),
-      this.taskService.tasksEnabled$(userId).pipe(
+    map((tasks) => tasks.filter((t) => t.type === SecurityTaskType.UpdateAtRiskCredential)),
-        switchMap((enabled) => {
-          if (!enabled) {
-            return of([]);
-          }
-          return this.taskService
-            .pendingTasks$(userId)
-            .pipe(
-              map((tasks) =>
-                tasks.filter((t) => t.type === SecurityTaskType.UpdateAtRiskCredential),
-              ),
-            );
-        }),
-      ),
-    ),
   );
 }

apps/web/src/app/auth/settings/emergency-access/view/emergency-view-dialog.component.spec.ts

@@ -8,14 +8,16 @@ import { OrganizationService } from "@bitwarden/common/admin-console/abstraction
 import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
 import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
+import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
 import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
 import { Utils } from "@bitwarden/common/platform/misc/utils";
 import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/spec";
-import { UserId } from "@bitwarden/common/types/guid";
+import { UserId, EmergencyAccessId } from "@bitwarden/common/types/guid";
 import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
 import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction";
 import { CipherType } from "@bitwarden/common/vault/enums";
 import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
+import { LoginView } from "@bitwarden/common/vault/models/view/login.view";
 import { TaskService } from "@bitwarden/common/vault/tasks";
 import { DialogService, DialogRef, DIALOG_DATA } from "@bitwarden/components";
 import { ChangeLoginPasswordService } from "@bitwarden/vault";
@@ -28,14 +30,15 @@ describe("EmergencyViewDialogComponent", () => {
 
   const open = jest.fn();
   const close = jest.fn();
+  const emergencyAccessId = "emergency-access-id" as EmergencyAccessId;
 
   const mockCipher = {
     id: "cipher1",
     name: "Cipher",
     type: CipherType.Login,
-    login: { uris: [] },
+    login: { uris: [] } as Partial<LoginView>,
     card: {},
-  } as CipherView;
+  } as Partial<CipherView> as CipherView;
 
   const accountService: FakeAccountService = mockAccountServiceWith(Utils.newGuid() as UserId);
 
@@ -56,6 +59,7 @@ describe("EmergencyViewDialogComponent", () => {
         { provide: DIALOG_DATA, useValue: { cipher: mockCipher } },
         { provide: AccountService, useValue: accountService },
         { provide: TaskService, useValue: mock<TaskService>() },
+        { provide: LogService, useValue: mock<LogService>() },
       ],
     })
       .overrideComponent(EmergencyViewDialogComponent, {
@@ -94,18 +98,24 @@ describe("EmergencyViewDialogComponent", () => {
   });
 
   it("opens dialog", () => {
-    EmergencyViewDialogComponent.open({ open } as unknown as DialogService, { cipher: mockCipher });
+    EmergencyViewDialogComponent.open({ open } as unknown as DialogService, {
+      cipher: mockCipher,
+      emergencyAccessId,
+    });
 
     expect(open).toHaveBeenCalled();
   });
 
   it("closes the dialog", () => {
-    EmergencyViewDialogComponent.open({ open } as unknown as DialogService, { cipher: mockCipher });
+    EmergencyViewDialogComponent.open({ open } as unknown as DialogService, {
+      cipher: mockCipher,
+      emergencyAccessId,
+    });
     fixture.detectChanges();
 
     const cancelButton = fixture.debugElement.queryAll(By.css("button")).pop();
 
-    cancelButton.nativeElement.click();
+    cancelButton!.nativeElement.click();
 
     expect(close).toHaveBeenCalled();
   });

libs/common/src/vault/tasks/services/default-task.service.spec.ts

@@ -108,6 +108,34 @@ describe("Default task service", () => {
   });
 
   describe("tasks$", () => {
+    beforeEach(() => {
+      mockGetFeatureFlag$.mockReturnValue(new BehaviorSubject(true));
+      mockGetAllOrgs$.mockReturnValue(
+        new BehaviorSubject([
+          {
+            useRiskInsights: true,
+          },
+        ] as Organization[]),
+      );
+    });
+
+    it("should return an empty array if tasks are not enabled", async () => {
+      mockGetAllOrgs$.mockReturnValue(
+        new BehaviorSubject([
+          {
+            useRiskInsights: false,
+          },
+        ] as Organization[]),
+      );
+
+      const { tasks$ } = service;
+
+      const result = await firstValueFrom(tasks$("user-id" as UserId));
+
+      expect(result.length).toBe(0);
+      expect(mockApiSend).not.toHaveBeenCalled();
+    });
+
     it("should fetch tasks from the API when the state is null", async () => {
       mockApiSend.mockResolvedValue({
         data: [
@@ -153,6 +181,34 @@ describe("Default task service", () => {
   });
 
   describe("pendingTasks$", () => {
+    beforeEach(() => {
+      mockGetFeatureFlag$.mockReturnValue(new BehaviorSubject(true));
+      mockGetAllOrgs$.mockReturnValue(
+        new BehaviorSubject([
+          {
+            useRiskInsights: true,
+          },
+        ] as Organization[]),
+      );
+    });
+
+    it("should return an empty array if tasks are not enabled", async () => {
+      mockGetAllOrgs$.mockReturnValue(
+        new BehaviorSubject([
+          {
+            useRiskInsights: false,
+          },
+        ] as Organization[]),
+      );
+
+      const { pendingTasks$ } = service;
+
+      const result = await firstValueFrom(pendingTasks$("user-id" as UserId));
+
+      expect(result.length).toBe(0);
+      expect(mockApiSend).not.toHaveBeenCalled();
+    });
+
     it("should filter tasks to only pending tasks", async () => {
       fakeStateProvider.singleUser.mockFor("user-id" as UserId, SECURITY_TASKS, [
         {

libs/common/src/vault/tasks/services/default-task.service.ts

@@ -1,4 +1,14 @@
-import { combineLatest, filter, map, merge, Observable, of, Subscription, switchMap } from "rxjs";
+import {
+  combineLatest,
+  filter,
+  map,
+  merge,
+  Observable,
+  of,
+  Subscription,
+  switchMap,
+  distinctUntilChanged,
+} from "rxjs";
 
 import { ApiService } from "@bitwarden/common/abstractions/api.service";
 import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
@@ -45,10 +55,18 @@ export class DefaultTaskService implements TaskService {
         .organizations$(userId)
         .pipe(map((orgs) => orgs.some((o) => o.useRiskInsights))),
       this.configService.getFeatureFlag$(FeatureFlag.SecurityTasks),
-    ]).pipe(map(([atLeastOneOrgEnabled, flagEnabled]) => atLeastOneOrgEnabled && flagEnabled));
+    ]).pipe(
+      map(([atLeastOneOrgEnabled, flagEnabled]) => atLeastOneOrgEnabled && flagEnabled),
+      distinctUntilChanged(),
+    );
   });
 
   tasks$ = perUserCache$((userId) => {
+    return this.tasksEnabled$(userId).pipe(
+      switchMap((enabled) => {
+        if (!enabled) {
+          return of([]);
+        }
         return this.taskState(userId).state$.pipe(
           switchMap(async (tasks) => {
             if (tasks == null) {
@@ -60,6 +78,8 @@ export class DefaultTaskService implements TaskService {
           filterOutNullish(),
           map((tasks) => tasks.map((t) => new SecurityTask(t))),
         );
+      }),
+    );
   });
 
   pendingTasks$ = perUserCache$((userId) => {

libs/vault/src/cipher-view/cipher-view.component.html

@@ -3,7 +3,6 @@
     {{ "cardExpiredMessage" | i18n }}
   </bit-callout>
 
-  <ng-container *ngIf="isSecurityTasksEnabled$ | async">
   <bit-callout
     *ngIf="cipher?.login.uris.length > 0 && hadPendingChangePasswordTask"
     type="warning"
@@ -15,7 +14,7 @@
       <i class="bwi bwi-external-link tw-ml-1" aria-hidden="true"></i>
     </a>
   </bit-callout>
-  </ng-container>
+
   <!-- HELPER TEXT -->
   <p
     class="tw-text-sm tw-text-muted"
@@ -40,9 +39,7 @@
     *ngIf="hasLogin"
     [cipher]="cipher"
     [activeUserId]="activeUserId$ | async"
-    [hadPendingChangePasswordTask]="
+    [hadPendingChangePasswordTask]="hadPendingChangePasswordTask"
-      hadPendingChangePasswordTask && (isSecurityTasksEnabled$ | async)
-    "
     (handleChangePassword)="launchChangePassword()"
   ></app-login-credentials-view>
 

libs/vault/src/cipher-view/cipher-view.component.ts

@@ -12,12 +12,12 @@ import { Organization } from "@bitwarden/common/admin-console/models/domain/orga
 import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
 import { getUserId } from "@bitwarden/common/auth/services/account.service";
 import { isCardExpired } from "@bitwarden/common/autofill/utils";
-import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
+import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
-import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
 import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
 import { CipherId, CollectionId, EmergencyAccessId, UserId } from "@bitwarden/common/types/guid";
 import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
 import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction";
+import { CipherType } from "@bitwarden/common/vault/enums";
 import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
 import { FolderView } from "@bitwarden/common/vault/models/view/folder.view";
 import { SecurityTaskType, TaskService } from "@bitwarden/common/vault/tasks";
@@ -80,7 +80,6 @@ export class CipherViewComponent implements OnChanges, OnDestroy {
   private destroyed$: Subject<void> = new Subject();
   cardIsExpired: boolean = false;
   hadPendingChangePasswordTask: boolean = false;
-  isSecurityTasksEnabled$ = this.configService.getFeatureFlag$(FeatureFlag.SecurityTasks);
 
   constructor(
     private organizationService: OrganizationService,
@@ -90,8 +89,8 @@ export class CipherViewComponent implements OnChanges, OnDestroy {
     private defaultTaskService: TaskService,
     private platformUtilsService: PlatformUtilsService,
     private changeLoginPasswordService: ChangeLoginPasswordService,
-    private configService: ConfigService,
     private cipherService: CipherService,
+    private logService: LogService,
   ) {}
 
   async ngOnChanges() {
@@ -158,20 +157,15 @@ export class CipherViewComponent implements OnChanges, OnDestroy {
 
     const userId = await firstValueFrom(this.activeUserId$);
 
-    // Show Tasks for Manage and Edit permissions
+    if (this.cipher.organizationId) {
-    // Using cipherService to see if user has access to cipher in a non-AC context to address with Edit Except Password permissions
-    const allCiphers = await firstValueFrom(this.cipherService.ciphers$(userId));
-    const cipherServiceCipher = allCiphers[this.cipher?.id as CipherId];
-
-    if (cipherServiceCipher?.edit && cipherServiceCipher?.viewPassword) {
-      await this.checkPendingChangePasswordTasks(userId);
-    }
-
-    if (this.cipher.organizationId && userId) {
       this.organization$ = this.organizationService
         .organizations$(userId)
         .pipe(getOrganizationById(this.cipher.organizationId))
         .pipe(takeUntil(this.destroyed$));
+
+      if (this.cipher.type === CipherType.Login) {
+        await this.checkPendingChangePasswordTasks(userId);
+      }
     }
 
     if (this.cipher.folderId) {
@@ -182,7 +176,14 @@ export class CipherViewComponent implements OnChanges, OnDestroy {
   }
 
   async checkPendingChangePasswordTasks(userId: UserId): Promise<void> {
-    if (!(await firstValueFrom(this.isSecurityTasksEnabled$))) {
+    try {
+      // Show Tasks for Manage and Edit permissions
+      // Using cipherService to see if user has access to cipher in a non-AC context to address with Edit Except Password permissions
+      const allCiphers = await firstValueFrom(this.cipherService.ciphers$(userId));
+      const cipherServiceCipher = allCiphers[this.cipher?.id as CipherId];
+
+      if (!cipherServiceCipher?.edit || !cipherServiceCipher?.viewPassword) {
+        this.hadPendingChangePasswordTask = false;
         return;
       }
 
@@ -193,6 +194,10 @@ export class CipherViewComponent implements OnChanges, OnDestroy {
           task.cipherId === this.cipher?.id && task.type === SecurityTaskType.UpdateAtRiskCredential
         );
       });
+    } catch (error) {
+      this.hadPendingChangePasswordTask = false;
+      this.logService.error("Failed to retrieve change password tasks for cipher", error);
+    }
   }
 
   launchChangePassword = async () => {