1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-06 00:13:28 +00:00

[PM-20040] all tasks complete banner (#16033)

* saved WIP

* created at risk password callout service to hold state for callout data. wip

* update at-risk-password-callout to use states for tracking showing and dismissing success banner

* adding spec file for new serive

* update styles to match figma

* minor wording changes

* fix undefined lint error in at risk password callout

* moved service to libs

* added another route guard so when user clears all at risk items they are directed back to the vault page

* small cleanup in at risk callout component and at risk pw guard

* clean up code in at risk password callout component

* update state to memory

* refactor for readability at risk password callout component

* move state update logic from component to at risk password callout service

* fix: bypass router cache on back() in popout

* Revert "fix: bypass router cache on back() in popout"

This reverts commit 23f9312434.

* refactor updatePendingTasksState call

* refactor at risk password callout component and service. remove signals, implement logic through observables. Completed value for tasks utilized.

* clean up completedTasks in at risk password callout service

* add updated state value to prevent banner among diff clients

* move hasInteracted call to page component to avoid looping

* remove excess call in service

* update icon null logic in banner component

* update the callout to use a new banner

* fix classes

* updating banners in at risk password callout component

* anchor tag

* move at-risk callout to above nudges

* update `showCompletedTasksBanner$` variable naming

---------

Co-authored-by: Andreas Coroiu <andreas.coroiu@gmail.com>
Co-authored-by: Nick Krantz <nick@livefront.com>
This commit is contained in:
Jason Ng
2025-10-22 12:37:58 -04:00
committed by GitHub
parent 3812e5d81b
commit 0340a881ae
15 changed files with 405 additions and 41 deletions

View File

@@ -5715,6 +5715,9 @@
"confirmKeyConnectorDomain": {
"message": "Confirm Key Connector domain"
},
"atRiskLoginsSecured": {
"message": "Great job securing your at-risk logins!"
},
"settingDisabledByPolicy": {
"message": "This setting is disabled by your organization's policy.",
"description": "This hint text is displayed when a user setting is disabled due to an organization policy."

View File

@@ -77,7 +77,10 @@ import { IntroCarouselComponent } from "../vault/popup/components/vault-v2/intro
import { PasswordHistoryV2Component } from "../vault/popup/components/vault-v2/vault-password-history-v2/vault-password-history-v2.component";
import { VaultV2Component } from "../vault/popup/components/vault-v2/vault-v2.component";
import { ViewV2Component } from "../vault/popup/components/vault-v2/view-v2/view-v2.component";
import { canAccessAtRiskPasswords } from "../vault/popup/guards/at-risk-passwords.guard";
import {
canAccessAtRiskPasswords,
hasAtRiskPasswords,
} from "../vault/popup/guards/at-risk-passwords.guard";
import { clearVaultStateGuard } from "../vault/popup/guards/clear-vault-state.guard";
import { IntroCarouselGuard } from "../vault/popup/guards/intro-carousel.guard";
import { AppearanceV2Component } from "../vault/popup/settings/appearance-v2.component";
@@ -692,7 +695,7 @@ const routes: Routes = [
{
path: "at-risk-passwords",
component: AtRiskPasswordsComponent,
canActivate: [authGuard, canAccessAtRiskPasswords],
canActivate: [authGuard, canAccessAtRiskPasswords, hasAtRiskPasswords],
},
{
path: "account-switcher",

View File

@@ -1,8 +1,28 @@
<bit-callout *ngIf="(pendingTasks$ | async)?.length as taskCount" type="warning" [title]="''">
<a bitLink [routerLink]="'/at-risk-passwords'">
{{
(taskCount === 1 ? "reviewAndChangeAtRiskPassword" : "reviewAndChangeAtRiskPasswordsPlural")
| i18n: taskCount.toString()
}}
</a>
</bit-callout>
@if ((currentPendingTasks$ | async)?.length > 0) {
<bit-banner
class="-tw-m-5 tw-flex tw-flex-col tw-pt-2 tw-px-2 tw-mb-3"
bannerType="warning"
[showClose]="false"
>
<a bitLink linkType="secondary" [routerLink]="'/at-risk-passwords'">
{{
((currentPendingTasks$ | async)?.length === 1
? "reviewAndChangeAtRiskPassword"
: "reviewAndChangeAtRiskPasswordsPlural"
) | i18n: (currentPendingTasks$ | async)?.length.toString()
}}
</a>
</bit-banner>
}
@if (showCompletedTasksBanner$ | async) {
<bit-banner
class="-tw-m-5 tw-flex tw-flex-col tw-pt-2 tw-px-2 tw-mb-3"
bannerType="info"
[icon]="null"
[showClose]="true"
(onClose)="successBannerDismissed()"
>
{{ "atRiskLoginsSecured" | i18n }}
</bit-banner>
}

View File

@@ -1,42 +1,47 @@
import { CommonModule } from "@angular/common";
import { Component, inject } from "@angular/core";
import { RouterModule } from "@angular/router";
import { combineLatest, map, switchMap } from "rxjs";
import { firstValueFrom, switchMap } from "rxjs";
import { JslibModule } from "@bitwarden/angular/jslib.module";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { getUserId } from "@bitwarden/common/auth/services/account.service";
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import { SecurityTaskType, TaskService } from "@bitwarden/common/vault/tasks";
import { AnchorLinkDirective, CalloutModule } from "@bitwarden/components";
import { AnchorLinkDirective, CalloutModule, BannerModule } from "@bitwarden/components";
import { I18nPipe } from "@bitwarden/ui-common";
import { AtRiskPasswordCalloutData, AtRiskPasswordCalloutService } from "@bitwarden/vault";
@Component({
selector: "vault-at-risk-password-callout",
imports: [CommonModule, AnchorLinkDirective, RouterModule, CalloutModule, I18nPipe],
imports: [
AnchorLinkDirective,
CommonModule,
RouterModule,
CalloutModule,
I18nPipe,
BannerModule,
JslibModule,
],
providers: [AtRiskPasswordCalloutService],
templateUrl: "./at-risk-password-callout.component.html",
})
export class AtRiskPasswordCalloutComponent {
private taskService = inject(TaskService);
private cipherService = inject(CipherService);
private activeAccount$ = inject(AccountService).activeAccount$.pipe(getUserId);
private atRiskPasswordCalloutService = inject(AtRiskPasswordCalloutService);
protected pendingTasks$ = this.activeAccount$.pipe(
switchMap((userId) =>
combineLatest([
this.taskService.pendingTasks$(userId),
this.cipherService.cipherViews$(userId),
]),
),
map(([tasks, ciphers]) =>
tasks.filter((t) => {
const associatedCipher = ciphers.find((c) => c.id === t.cipherId);
return (
t.type === SecurityTaskType.UpdateAtRiskCredential &&
associatedCipher &&
!associatedCipher.isDeleted
);
}),
),
showCompletedTasksBanner$ = this.activeAccount$.pipe(
switchMap((userId) => this.atRiskPasswordCalloutService.showCompletedTasksBanner$(userId)),
);
currentPendingTasks$ = this.activeAccount$.pipe(
switchMap((userId) => this.atRiskPasswordCalloutService.pendingTasks$(userId)),
);
async successBannerDismissed() {
const updateObject: AtRiskPasswordCalloutData = {
hasInteractedWithTasks: true,
tasksBannerDismissed: true,
};
const userId = await firstValueFrom(this.activeAccount$);
this.atRiskPasswordCalloutService.updateAtRiskPasswordState(userId, updateObject);
}
}

View File

@@ -14,6 +14,9 @@ import { AutofillSettingsServiceAbstraction } from "@bitwarden/common/autofill/s
import { InlineMenuVisibilitySetting } from "@bitwarden/common/autofill/types";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
import { StateProvider } from "@bitwarden/common/platform/state";
import { FakeAccountService, FakeStateProvider } from "@bitwarden/common/spec";
import { UserId } from "@bitwarden/common/types/guid";
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
import { EndUserNotificationService } from "@bitwarden/common/vault/notifications";
@@ -24,6 +27,7 @@ import {
ChangeLoginPasswordService,
DefaultChangeLoginPasswordService,
PasswordRepromptService,
AtRiskPasswordCalloutService,
} from "@bitwarden/vault";
import { PopupHeaderComponent } from "../../../../platform/popup/layout/popup-header.component";
@@ -68,6 +72,9 @@ describe("AtRiskPasswordsComponent", () => {
let mockNotifications$: BehaviorSubject<NotificationView[]>;
let mockInlineMenuVisibility$: BehaviorSubject<InlineMenuVisibilitySetting>;
let calloutDismissed$: BehaviorSubject<boolean>;
let mockAtRiskPasswordCalloutService: any;
let stateProvider: FakeStateProvider;
let mockAccountService: FakeAccountService;
const setInlineMenuVisibility = jest.fn();
const mockToastService = mock<ToastService>();
const mockAtRiskPasswordPageService = mock<AtRiskPasswordPageService>();
@@ -112,6 +119,11 @@ describe("AtRiskPasswordsComponent", () => {
mockToastService.showToast.mockClear();
mockDialogService.open.mockClear();
mockAtRiskPasswordPageService.isCalloutDismissed.mockReturnValue(calloutDismissed$);
mockAccountService = {
activeAccount$: of({ id: "user" as UserId }),
activeUserId: "user" as UserId,
} as unknown as FakeAccountService;
stateProvider = new FakeStateProvider(mockAccountService);
await TestBed.configureTestingModule({
imports: [AtRiskPasswordsComponent],
@@ -141,7 +153,7 @@ describe("AtRiskPasswordsComponent", () => {
},
},
{ provide: I18nService, useValue: { t: (key: string) => key } },
{ provide: AccountService, useValue: { activeAccount$: of({ id: "user" }) } },
{ provide: AccountService, useValue: mockAccountService },
{ provide: PlatformUtilsService, useValue: mock<PlatformUtilsService>() },
{ provide: PasswordRepromptService, useValue: mock<PasswordRepromptService>() },
{
@@ -152,6 +164,8 @@ describe("AtRiskPasswordsComponent", () => {
},
},
{ provide: ToastService, useValue: mockToastService },
{ provide: StateProvider, useValue: stateProvider },
{ provide: AtRiskPasswordCalloutService, useValue: mockAtRiskPasswordCalloutService },
],
})
.overrideModule(JslibModule, {

View File

@@ -41,6 +41,7 @@ import {
TypographyModule,
} from "@bitwarden/components";
import {
AtRiskPasswordCalloutService,
ChangeLoginPasswordService,
DefaultChangeLoginPasswordService,
PasswordRepromptService,
@@ -75,6 +76,7 @@ import { AtRiskPasswordPageService } from "./at-risk-password-page.service";
providers: [
AtRiskPasswordPageService,
{ provide: ChangeLoginPasswordService, useClass: DefaultChangeLoginPasswordService },
AtRiskPasswordCalloutService,
],
selector: "vault-at-risk-passwords",
templateUrl: "./at-risk-passwords.component.html",
@@ -95,6 +97,7 @@ export class AtRiskPasswordsComponent implements OnInit {
private dialogService = inject(DialogService);
private endUserNotificationService = inject(EndUserNotificationService);
private destroyRef = inject(DestroyRef);
private atRiskPasswordCalloutService = inject(AtRiskPasswordCalloutService);
/**
* The cipher that is currently being launched. Used to show a loading spinner on the badge button.
@@ -199,6 +202,11 @@ export class AtRiskPasswordsComponent implements OnInit {
}
this.markTaskNotificationsAsRead();
this.atRiskPasswordCalloutService.updateAtRiskPasswordState(userId, {
hasInteractedWithTasks: true,
tasksBannerDismissed: false,
});
}
private markTaskNotificationsAsRead() {

View File

@@ -41,6 +41,9 @@
</bit-spotlight>
</ng-container>
<ng-container *ngIf="vaultState !== VaultStateEnum.Empty">
<!-- At-Risk callout displays as a banner and should always remain visually above all other callouts -->
<vault-at-risk-password-callout></vault-at-risk-password-callout>
<div class="tw-mb-4" *ngIf="showHasItemsVaultSpotlight$ | async">
<bit-spotlight
[title]="'hasItemsVaultNudgeTitle' | i18n"
@@ -53,7 +56,6 @@
</ul>
</bit-spotlight>
</div>
<vault-at-risk-password-callout></vault-at-risk-password-callout>
<app-vault-header-v2></app-vault-header-v2>
</ng-container>
</ng-container>

View File

@@ -1,10 +1,11 @@
import { inject } from "@angular/core";
import { CanActivateFn, Router } from "@angular/router";
import { map, switchMap } from "rxjs";
import { combineLatest, map, switchMap } from "rxjs";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { TaskService } from "@bitwarden/common/vault/tasks";
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import { SecurityTaskType, TaskService } from "@bitwarden/common/vault/tasks";
import { filterOutNullish } from "@bitwarden/common/vault/utils/observable-utilities";
import { ToastService } from "@bitwarden/components";
@@ -32,3 +33,38 @@ export const canAccessAtRiskPasswords: CanActivateFn = () => {
}),
);
};
export const hasAtRiskPasswords: CanActivateFn = () => {
const accountService = inject(AccountService);
const taskService = inject(TaskService);
const cipherService = inject(CipherService);
const router = inject(Router);
return accountService.activeAccount$.pipe(
filterOutNullish(),
switchMap((user) =>
combineLatest([
taskService.pendingTasks$(user.id),
cipherService.cipherViews$(user.id).pipe(
filterOutNullish(),
map((ciphers) => Object.fromEntries(ciphers.map((c) => [c.id, c]))),
),
]).pipe(
map(([tasks, ciphers]) => {
const hasAtRiskCiphers = tasks.some(
(t) =>
t.type === SecurityTaskType.UpdateAtRiskCredential &&
t.cipherId != null &&
ciphers[t.cipherId] != null &&
!ciphers[t.cipherId].isDeleted,
);
if (!hasAtRiskCiphers) {
return router.createUrlTree(["/tabs/vault"]);
}
return true;
}),
),
),
);
};

View File

@@ -25,6 +25,12 @@ export abstract class TaskService {
*/
abstract pendingTasks$(userId: UserId): Observable<SecurityTask[]>;
/**
* Observable of completed tasks for a given user.
* @param userId
*/
abstract completedTasks$(userId: UserId): Observable<SecurityTask[]>;
/**
* Retrieves tasks from the API for a given user and updates the local state.
* @param userId

View File

@@ -80,6 +80,12 @@ export class DefaultTaskService implements TaskService {
);
});
completedTasks$ = perUserCache$((userId) => {
return this.tasks$(userId).pipe(
map((tasks) => tasks.filter((t) => t.status === SecurityTaskStatus.Completed)),
);
});
async refreshTasks(userId: UserId): Promise<void> {
await this.fetchTasksFromApi(userId);
}

View File

@@ -38,7 +38,8 @@ const defaultIcon: Record<BannerType, string> = {
export class BannerComponent implements OnInit {
readonly bannerType = input<BannerType>("info");
readonly icon = model<string>();
// passing `null` will remove the icon from element from the banner
readonly icon = model<string | null>();
readonly useAlertRole = input(true);
readonly showClose = input(true);
@@ -47,7 +48,7 @@ export class BannerComponent implements OnInit {
@Output() onClose = new EventEmitter<void>();
ngOnInit(): void {
if (!this.icon()) {
if (!this.icon() && this.icon() !== null) {
this.icon.set(defaultIcon[this.bannerType()]);
}
}

View File

@@ -218,3 +218,4 @@ export const VAULT_BROWSER_INTRO_CAROUSEL = new StateDefinition(
"vaultBrowserIntroCarousel",
"disk",
);
export const VAULT_AT_RISK_PASSWORDS_MEMORY = new StateDefinition("vaultAtRiskPasswords", "memory");

View File

@@ -1,3 +1,7 @@
export {
AtRiskPasswordCalloutService,
AtRiskPasswordCalloutData,
} from "./services/at-risk-password-callout.service";
export { PasswordRepromptService } from "./services/password-reprompt.service";
export { CopyCipherFieldService, CopyAction } from "./services/copy-cipher-field.service";
export { CopyCipherFieldDirective } from "./components/copy-cipher-field.directive";

View File

@@ -0,0 +1,162 @@
import { TestBed } from "@angular/core/testing";
import { firstValueFrom, of } from "rxjs";
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import {
SecurityTask,
SecurityTaskStatus,
SecurityTaskType,
TaskService,
} from "@bitwarden/common/vault/tasks";
import { StateProvider } from "@bitwarden/state";
import { UserId } from "@bitwarden/user-core";
import { FakeSingleUserState } from "../../../common/spec/fake-state";
import {
AtRiskPasswordCalloutData,
AtRiskPasswordCalloutService,
} from "./at-risk-password-callout.service";
const fakeUserState = () =>
({
update: jest.fn().mockResolvedValue(undefined),
state$: of(null),
}) as unknown as FakeSingleUserState<AtRiskPasswordCalloutData>;
class MockCipherView {
constructor(
public id: string,
private deleted: boolean,
) {}
get isDeleted() {
return this.deleted;
}
}
describe("AtRiskPasswordCalloutService", () => {
let service: AtRiskPasswordCalloutService;
const mockTaskService = {
pendingTasks$: jest.fn(),
completedTasks$: jest.fn(),
};
const mockCipherService = { cipherViews$: jest.fn() };
const mockStateProvider = { getUser: jest.fn().mockReturnValue(fakeUserState()) };
const userId: UserId = "user1" as UserId;
beforeEach(() => {
TestBed.configureTestingModule({
providers: [
AtRiskPasswordCalloutService,
{
provide: TaskService,
useValue: mockTaskService,
},
{
provide: CipherService,
useValue: mockCipherService,
},
{
provide: StateProvider,
useValue: mockStateProvider,
},
],
});
service = TestBed.inject(AtRiskPasswordCalloutService);
});
describe("completedTasks$", () => {
it(" should return true if completed tasks exist", async () => {
const tasks: SecurityTask[] = [
{
id: "t1",
cipherId: "c1",
type: SecurityTaskType.UpdateAtRiskCredential,
status: SecurityTaskStatus.Completed,
} as any,
{
id: "t2",
cipherId: "c2",
type: SecurityTaskType.UpdateAtRiskCredential,
status: SecurityTaskStatus.Pending,
} as any,
{
id: "t3",
cipherId: "nope",
type: SecurityTaskType.UpdateAtRiskCredential,
status: SecurityTaskStatus.Completed,
} as any,
{
id: "t4",
cipherId: "c3",
type: SecurityTaskType.UpdateAtRiskCredential,
status: SecurityTaskStatus.Completed,
} as any,
];
jest.spyOn(mockTaskService, "completedTasks$").mockReturnValue(of(tasks));
const result = await firstValueFrom(service.completedTasks$(userId));
expect(result).toEqual(tasks[0]);
expect(result?.id).toBe("t1");
});
});
describe("showCompletedTasksBanner$", () => {
beforeEach(() => {
jest.spyOn(mockTaskService, "pendingTasks$").mockReturnValue(of([]));
jest.spyOn(mockTaskService, "completedTasks$").mockReturnValue(of([]));
jest.spyOn(mockCipherService, "cipherViews$").mockReturnValue(of([]));
});
it("should return false if banner has been dismissed", async () => {
const state: AtRiskPasswordCalloutData = {
hasInteractedWithTasks: true,
tasksBannerDismissed: true,
};
const mockState = { ...fakeUserState(), state$: of(state) };
mockStateProvider.getUser.mockReturnValue(mockState);
const result = await firstValueFrom(service.showCompletedTasksBanner$(userId));
expect(result).toBe(false);
});
it("should return true when has completed tasks, no pending tasks, and banner not dismissed", async () => {
const completedTasks = [
{
id: "t1",
cipherId: "c1",
type: SecurityTaskType.UpdateAtRiskCredential,
status: SecurityTaskStatus.Completed,
},
];
const ciphers = [new MockCipherView("c1", false)];
const state: AtRiskPasswordCalloutData = {
hasInteractedWithTasks: true,
tasksBannerDismissed: false,
};
jest.spyOn(mockTaskService, "completedTasks$").mockReturnValue(of(completedTasks));
jest.spyOn(mockCipherService, "cipherViews$").mockReturnValue(of(ciphers));
mockStateProvider.getUser.mockReturnValue({ state$: of(state) });
const result = await firstValueFrom(service.showCompletedTasksBanner$(userId));
expect(result).toBe(true);
});
it("returns false when no completed tasks", async () => {
const state: AtRiskPasswordCalloutData = {
hasInteractedWithTasks: true,
tasksBannerDismissed: false,
};
mockStateProvider.getUser.mockReturnValue({ state$: of(state) });
const result = await firstValueFrom(service.showCompletedTasksBanner$(userId));
expect(result).toBe(false);
});
});
});

View File

@@ -0,0 +1,93 @@
import { Injectable } from "@angular/core";
import { combineLatest, map, Observable } from "rxjs";
import {
SingleUserState,
StateProvider,
UserKeyDefinition,
VAULT_AT_RISK_PASSWORDS_MEMORY,
} from "@bitwarden/common/platform/state";
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import { SecurityTask, SecurityTaskType, TaskService } from "@bitwarden/common/vault/tasks";
import { UserId } from "@bitwarden/user-core";
export type AtRiskPasswordCalloutData = {
hasInteractedWithTasks: boolean;
tasksBannerDismissed: boolean;
};
export const AT_RISK_PASSWORD_CALLOUT_KEY = new UserKeyDefinition<AtRiskPasswordCalloutData>(
VAULT_AT_RISK_PASSWORDS_MEMORY,
"atRiskPasswords",
{
deserializer: (jsonData) => jsonData,
clearOn: ["lock", "logout"],
},
);
@Injectable()
export class AtRiskPasswordCalloutService {
constructor(
private taskService: TaskService,
private cipherService: CipherService,
private stateProvider: StateProvider,
) {}
pendingTasks$(userId: UserId): Observable<SecurityTask[]> {
return combineLatest([
this.taskService.pendingTasks$(userId),
this.cipherService.cipherViews$(userId),
]).pipe(
map(([tasks, ciphers]) => {
return tasks.filter((t: SecurityTask) => {
const associatedCipher = ciphers.find((c) => c.id === t.cipherId);
return (
t.type === SecurityTaskType.UpdateAtRiskCredential &&
associatedCipher &&
!associatedCipher.isDeleted
);
});
}),
);
}
completedTasks$(userId: UserId): Observable<SecurityTask | undefined> {
return this.taskService.completedTasks$(userId).pipe(
map((tasks) => {
return tasks.find((t: SecurityTask) => t.type === SecurityTaskType.UpdateAtRiskCredential);
}),
);
}
showCompletedTasksBanner$(userId: UserId): Observable<boolean> {
return combineLatest([
this.pendingTasks$(userId),
this.completedTasks$(userId),
this.atRiskPasswordState(userId).state$,
]).pipe(
map(([pendingTasks, completedTasks, state]) => {
const hasPendingTasks = pendingTasks.length > 0;
const bannerDismissed = state?.tasksBannerDismissed ?? false;
const hasInteracted = state?.hasInteractedWithTasks ?? false;
// This will ensure the banner remains visible only in the client the user resolved their tasks in
// e.g. if the user did not see tasks in the browser, and resolves them in the web, the browser will not show the banner
if (!hasPendingTasks && (!hasInteracted || bannerDismissed)) {
return false;
}
// Show banner if there are completed tasks and no pending tasks, and banner hasn't been dismissed
return !!completedTasks && !hasPendingTasks && !(state?.tasksBannerDismissed ?? false);
}),
);
}
atRiskPasswordState(userId: UserId): SingleUserState<AtRiskPasswordCalloutData> {
return this.stateProvider.getUser(userId, AT_RISK_PASSWORD_CALLOUT_KEY);
}
updateAtRiskPasswordState(userId: UserId, updatedState: AtRiskPasswordCalloutData): void {
void this.atRiskPasswordState(userId).update(() => updatedState);
}
}