diff --git a/apps/browser/src/vault/popup/components/at-risk-callout/at-risk-password-callout.component.ts b/apps/browser/src/vault/popup/components/at-risk-callout/at-risk-password-callout.component.ts index fa4137d984..ed78d9433f 100644 --- a/apps/browser/src/vault/popup/components/at-risk-callout/at-risk-password-callout.component.ts +++ b/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) => - this.taskService.tasksEnabled$(userId).pipe( - switchMap((enabled) => { - if (!enabled) { - return of([]); - } - return this.taskService - .pendingTasks$(userId) - .pipe( - map((tasks) => - tasks.filter((t) => t.type === SecurityTaskType.UpdateAtRiskCredential), - ), - ); - }), - ), - ), + switchMap((userId) => this.taskService.pendingTasks$(userId)), + map((tasks) => tasks.filter((t) => t.type === SecurityTaskType.UpdateAtRiskCredential)), ); } diff --git a/apps/web/src/app/auth/settings/emergency-access/view/emergency-view-dialog.component.spec.ts b/apps/web/src/app/auth/settings/emergency-access/view/emergency-view-dialog.component.spec.ts index 3ffb54f5b1..b341fc4f8e 100644 --- a/apps/web/src/app/auth/settings/emergency-access/view/emergency-view-dialog.component.spec.ts +++ b/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, card: {}, - } as CipherView; + } as Partial 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() }, + { provide: LogService, useValue: mock() }, ], }) .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(); }); diff --git a/libs/common/src/vault/tasks/services/default-task.service.spec.ts b/libs/common/src/vault/tasks/services/default-task.service.spec.ts index 4d468d0976..cb22d1296b 100644 --- a/libs/common/src/vault/tasks/services/default-task.service.spec.ts +++ b/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, [ { diff --git a/libs/common/src/vault/tasks/services/default-task.service.ts b/libs/common/src/vault/tasks/services/default-task.service.ts index 7386102263..a50f736f7f 100644 --- a/libs/common/src/vault/tasks/services/default-task.service.ts +++ b/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,20 +55,30 @@ 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.taskState(userId).state$.pipe( - switchMap(async (tasks) => { - if (tasks == null) { - await this.fetchTasksFromApi(userId); - return null; + return this.tasksEnabled$(userId).pipe( + switchMap((enabled) => { + if (!enabled) { + return of([]); } - return tasks; + return this.taskState(userId).state$.pipe( + switchMap(async (tasks) => { + if (tasks == null) { + await this.fetchTasksFromApi(userId); + return null; + } + return tasks; + }), + filterOutNullish(), + map((tasks) => tasks.map((t) => new SecurityTask(t))), + ); }), - filterOutNullish(), - map((tasks) => tasks.map((t) => new SecurityTask(t))), ); }); diff --git a/libs/vault/src/cipher-view/cipher-view.component.html b/libs/vault/src/cipher-view/cipher-view.component.html index 99f1b1e47f..68edd265d0 100644 --- a/libs/vault/src/cipher-view/cipher-view.component.html +++ b/libs/vault/src/cipher-view/cipher-view.component.html @@ -3,19 +3,18 @@ {{ "cardExpiredMessage" | i18n }} - - - - - {{ "changeAtRiskPassword" | i18n }} - - - - + + + + {{ "changeAtRiskPassword" | i18n }} + + + +

diff --git a/libs/vault/src/cipher-view/cipher-view.component.ts b/libs/vault/src/cipher-view/cipher-view.component.ts index e748fa4665..cf78e78a65 100644 --- a/libs/vault/src/cipher-view/cipher-view.component.ts +++ b/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 { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; +import { LogService } from "@bitwarden/common/platform/abstractions/log.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 = 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 - // 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) { + if (this.cipher.organizationId) { 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,17 +176,28 @@ export class CipherViewComponent implements OnChanges, OnDestroy { } async checkPendingChangePasswordTasks(userId: UserId): Promise { - if (!(await firstValueFrom(this.isSecurityTasksEnabled$))) { - return; + 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; + } + + const tasks = await firstValueFrom(this.defaultTaskService.pendingTasks$(userId)); + + this.hadPendingChangePasswordTask = tasks?.some((task) => { + return ( + 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); } - - const tasks = await firstValueFrom(this.defaultTaskService.pendingTasks$(userId)); - - this.hadPendingChangePasswordTask = tasks?.some((task) => { - return ( - task.cipherId === this.cipher?.id && task.type === SecurityTaskType.UpdateAtRiskCredential - ); - }); } launchChangePassword = async () => {