1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-22 19:23:52 +00:00

[PM-27762] Activity Tab - Password Change Progress - Assign tasks for new passwords (#17268)

* Update type guard for cipher ids on reports

* Update report model cipher id type and mock data

* Update security tasks api service to have copied getAllTasks function from the vault team

* Expose critical application at risk cipher ids

* Update cipher id type in report service. Update all activities service to move task function to task service

* Update module

* Update organization id sharing through components instead of multiple route fetchings

* Update view type of password change widget. Update variables to be signals. Refactor logic for calculations based on individual tasks

* Update usage of request password change function

* Update security tasks service to manage tasks

* Remove unused variable

* Alphabetized functions, added documentation. Removed injectable decorator

* Alphabetize constructor params for password health service

* Update providers

* Address NaN case on percentage. Address obsolete type casting to CipherID and any other claude comments

* Fix dependency array in test case
This commit is contained in:
Leslie Tilton
2025-11-07 12:04:05 -06:00
committed by GitHub
parent 4f9ae78598
commit ec07a5391a
23 changed files with 497 additions and 537 deletions

View File

@@ -1,3 +1,5 @@
import { CipherId } from "@bitwarden/common/types/guid";
import {
ApplicationHealthReportDetail,
MemberDetails,
@@ -10,7 +12,6 @@ import {
createValidator,
isBoolean,
isBoundedString,
isBoundedStringArray,
isBoundedStringOrNull,
isBoundedPositiveNumber,
BOUNDED_ARRAY_MAX_LENGTH,
@@ -33,6 +34,10 @@ export const isMemberDetails = createValidator<MemberDetails>({
});
export const isMemberDetailsArray = createBoundedArrayGuard(isMemberDetails);
export function isCipherId(value: unknown): value is CipherId {
return value == null || isBoundedString(value);
}
export const isCipherIdArray = createBoundedArrayGuard(isCipherId);
/**
* Type guard to validate ApplicationHealthReportDetail structure
* Exported for testability
@@ -40,11 +45,11 @@ export const isMemberDetailsArray = createBoundedArrayGuard(isMemberDetails);
*/
export const isApplicationHealthReportDetail = createValidator<ApplicationHealthReportDetail>({
applicationName: isBoundedString,
atRiskCipherIds: isBoundedStringArray,
atRiskCipherIds: isCipherIdArray,
atRiskMemberCount: isBoundedPositiveNumber,
atRiskMemberDetails: isMemberDetailsArray,
atRiskPasswordCount: isBoundedPositiveNumber,
cipherIds: isBoundedStringArray,
cipherIds: isCipherIdArray,
memberCount: isBoundedPositiveNumber,
memberDetails: isMemberDetailsArray,
passwordCount: isBoundedPositiveNumber,

View File

@@ -1,5 +1,6 @@
import { mock } from "jest-mock-extended";
import { CipherId } from "@bitwarden/common/types/guid";
import { CipherType } from "@bitwarden/common/vault/enums";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
@@ -13,11 +14,14 @@ import {
PasswordHealthData,
} from "../report-models";
const mockCipherId1 = "cipher-1" as CipherId;
const mockCipherId2 = "cipher-2" as CipherId;
const mockApplication1: ApplicationHealthReportDetail = {
applicationName: "application1.com",
passwordCount: 2,
atRiskPasswordCount: 1,
atRiskCipherIds: ["cipher-1"],
atRiskCipherIds: [mockCipherId1],
memberCount: 2,
atRiskMemberCount: 1,
memberDetails: [
@@ -33,10 +37,10 @@ const mockApplication1: ApplicationHealthReportDetail = {
userGuid: "user-id-2",
userName: "tom",
email: "tom2@application1.com",
cipherId: "cipher-2",
cipherId: mockCipherId2,
},
],
cipherIds: ["cipher-1", "cipher-2"],
cipherIds: [mockCipherId1, mockCipherId2],
};
const mockApplication2: ApplicationHealthReportDetail = {

View File

@@ -1,7 +1,7 @@
import { Opaque } from "type-fest";
import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string";
import { OrganizationReportId } from "@bitwarden/common/types/guid";
import { CipherId, OrganizationReportId } from "@bitwarden/common/types/guid";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
import { BadgeVariant } from "@bitwarden/components";
@@ -79,12 +79,12 @@ export type ApplicationHealthReportDetail = {
applicationName: string;
passwordCount: number;
atRiskPasswordCount: number;
atRiskCipherIds: string[];
atRiskCipherIds: CipherId[];
memberCount: number;
atRiskMemberCount: number;
memberDetails: MemberDetails[];
atRiskMemberDetails: MemberDetails[];
cipherIds: string[];
cipherIds: CipherId[];
};
// -------------------- Password Health Report Models --------------------

View File

@@ -1,7 +1,14 @@
import { from, Observable } from "rxjs";
import { ApiService } from "@bitwarden/common/abstractions/api.service";
import { ListResponse } from "@bitwarden/common/models/response/list.response";
import { OrganizationId } from "@bitwarden/common/types/guid";
import {
SecurityTask,
SecurityTaskData,
SecurityTaskResponse,
SecurityTaskStatus,
} from "@bitwarden/common/vault/tasks";
export type TaskMetrics = {
completedTasks: number;
@@ -22,4 +29,29 @@ export class SecurityTasksApiService {
return from(dbResponse as Promise<TaskMetrics>);
}
// Could not import from @bitwarden/bit-web
// Copying from /bitwarden_license/bit-web/src/app/vault/services/default-admin-task.service.ts
async getAllTasks(
organizationId: OrganizationId,
status?: SecurityTaskStatus | undefined,
): Promise<SecurityTask[]> {
const queryParams = new URLSearchParams();
queryParams.append("organizationId", organizationId);
if (status !== undefined) {
queryParams.append("status", status.toString());
}
const r = await this.apiService.send(
"GET",
`/tasks/organization?${queryParams.toString()}`,
null,
true,
true,
);
const response = new ListResponse(r, SecurityTaskResponse);
return response.data.map((d) => new SecurityTask(new SecurityTaskData(d)));
}
}

View File

@@ -28,7 +28,7 @@ describe("PasswordHealthService", () => {
auditService.passwordLeaked.mockImplementation((password: string) =>
Promise.resolve(password === "leaked" ? 2 : 0),
);
service = new PasswordHealthService(passwordStrengthService, auditService);
service = new PasswordHealthService(auditService, passwordStrengthService);
// Setup mock data
mockValidCipher = mock<CipherView>({

View File

@@ -14,8 +14,8 @@ import {
export class PasswordHealthService {
constructor(
private passwordStrengthService: PasswordStrengthServiceAbstraction,
private auditService: AuditService,
private passwordStrengthService: PasswordStrengthServiceAbstraction,
) {}
/**

View File

@@ -32,7 +32,7 @@ import {
} from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { getUserId } from "@bitwarden/common/auth/services/account.service";
import { OrganizationId, UserId } from "@bitwarden/common/types/guid";
import { CipherId, OrganizationId, 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 { LogService } from "@bitwarden/logging";
@@ -89,6 +89,10 @@ export class RiskInsightsOrchestratorService {
private _hasCiphersSubject$ = new BehaviorSubject<boolean | null>(null);
hasCiphers$ = this._hasCiphersSubject$.asObservable();
private _criticalApplicationAtRiskCipherIdsSubject$ = new BehaviorSubject<CipherId[]>([]);
readonly criticalApplicationAtRiskCipherIds$ =
this._criticalApplicationAtRiskCipherIdsSubject$.asObservable();
// ------------------------- Report Variables ----------------
private _rawReportDataSubject = new BehaviorSubject<ReportState>({
status: ReportStatus.Initializing,
@@ -1150,10 +1154,42 @@ export class RiskInsightsOrchestratorService {
this._reportStateSubscription = mergedReportState$
.pipe(takeUntil(this._destroy$))
.subscribe((state) => {
// Update the raw report data subject
this._rawReportDataSubject.next(state.reportState);
// Update the critical application at risk cipher ids for exposure
const reportState = state.reportState?.data;
if (reportState) {
const criticalApplicationAtRiskCipherIds = this._getCriticalApplicationCipherIds(
reportState.reportData || [],
reportState.applicationData || [],
);
this._criticalApplicationAtRiskCipherIdsSubject$.next(criticalApplicationAtRiskCipherIds);
}
});
}
// Gets the unique cipher IDs that are marked at risk in critical applications
private _getCriticalApplicationCipherIds(
applications: ApplicationHealthReportDetail[],
applicationData: OrganizationReportApplication[],
): CipherId[] {
const foundCipherIds = applications
.map((app) => {
const isCriticalApplication = this.reportService.isCriticalApplication(
app,
applicationData,
);
return isCriticalApplication ? app.atRiskCipherIds : [];
})
.flat();
// Use a set to ensure uniqueness
const uniqueCipherIds = new Set<CipherId>([...foundCipherIds]);
return [...uniqueCipherIds];
}
// Setup the user ID observable to track the current user
private _setupUserId() {
// Watch userId changes

View File

@@ -1,7 +1,12 @@
import { catchError, EMPTY, from, map, Observable, of, switchMap, throwError } from "rxjs";
import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string";
import { OrganizationId, OrganizationReportId, UserId } from "@bitwarden/common/types/guid";
import {
CipherId,
OrganizationId,
OrganizationReportId,
UserId,
} from "@bitwarden/common/types/guid";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
import { getUniqueMembers } from "../../helpers/risk-insights-data-mappers";
@@ -63,7 +68,7 @@ export class RiskInsightsReportService {
): Map<string, CipherView[]> {
const cipherMap = new Map<string, CipherView[]>();
applications.forEach((app) => {
const filteredCiphers = ciphers.filter((c) => app.cipherIds.includes(c.id));
const filteredCiphers = ciphers.filter((c) => app.cipherIds.includes(c.id as CipherId));
cipherMap.set(app.applicationName, filteredCiphers);
});
return cipherMap;
@@ -346,7 +351,7 @@ export class RiskInsightsReportService {
): ApplicationHealthReportDetail {
return {
applicationName: application,
cipherIds: [cipherReport.cipher.id],
cipherIds: [cipherReport.cipher.id as CipherId],
passwordCount: 1,
memberDetails: [...cipherReport.cipherMembers],
memberCount: cipherReport.cipherMembers.length,
@@ -367,7 +372,7 @@ export class RiskInsightsReportService {
memberDetails: getUniqueMembers(
existingReport.memberDetails.concat(newCipherReport.cipherMembers),
),
cipherIds: existingReport.cipherIds.concat(newCipherReport.cipher.id),
cipherIds: existingReport.cipherIds.concat(newCipherReport.cipher.id as CipherId),
};
}
@@ -377,7 +382,7 @@ export class RiskInsightsReportService {
);
return {
atRiskPasswordCount: report.atRiskPasswordCount + 1,
atRiskCipherIds: report.atRiskCipherIds.concat(cipherReport.cipher.id),
atRiskCipherIds: report.atRiskCipherIds.concat(cipherReport.cipher.id as CipherId),
atRiskMemberDetails,
atRiskMemberCount: atRiskMemberDetails.length,
};

View File

@@ -11,7 +11,6 @@ export class AllActivitiesService {
/// and critical applications.
/// Going forward, this class can be simplified by using the RiskInsightsDataService
/// as it contains the application summary data.
private reportSummarySubject$ = new BehaviorSubject<OrganizationReportSummary>({
totalMemberCount: 0,
totalCriticalMemberCount: 0,
@@ -31,12 +30,8 @@ export class AllActivitiesService {
private atRiskPasswordsCountSubject$ = new BehaviorSubject<number>(0);
atRiskPasswordsCount$ = this.atRiskPasswordsCountSubject$.asObservable();
private passwordChangeProgressMetricHasProgressBarSubject$ = new BehaviorSubject<boolean>(false);
passwordChangeProgressMetricHasProgressBar$ =
this.passwordChangeProgressMetricHasProgressBarSubject$.asObservable();
private taskCreatedCountSubject$ = new BehaviorSubject<number>(0);
taskCreatedCount$ = this.taskCreatedCountSubject$.asObservable();
private extendPasswordChangeWidgetSubject$ = new BehaviorSubject<boolean>(false);
extendPasswordChangeWidget$ = this.extendPasswordChangeWidgetSubject$.asObservable();
constructor(private dataService: RiskInsightsDataService) {
// All application summary changes
@@ -91,11 +86,7 @@ export class AllActivitiesService {
this.allApplicationsDetailsSubject$.next(applications);
}
setPasswordChangeProgressMetricHasProgressBar(hasProgressBar: boolean) {
this.passwordChangeProgressMetricHasProgressBarSubject$.next(hasProgressBar);
}
setTaskCreatedCount(count: number) {
this.taskCreatedCountSubject$.next(count);
setExtendPasswordWidget(hasProgressBar: boolean) {
this.extendPasswordChangeWidgetSubject$.next(hasProgressBar);
}
}

View File

@@ -1,7 +1,7 @@
import { BehaviorSubject, firstValueFrom, Observable, of, Subject } from "rxjs";
import { distinctUntilChanged, map } from "rxjs/operators";
import { OrganizationId } from "@bitwarden/common/types/guid";
import { CipherId, OrganizationId } from "@bitwarden/common/types/guid";
import { getAtRiskApplicationList, getAtRiskMemberList } from "../../helpers";
import {
@@ -39,6 +39,7 @@ export class RiskInsightsDataService {
readonly isGeneratingReport$: Observable<boolean> = of(false);
readonly criticalReportResults$: Observable<RiskInsightsEnrichedData | null> = of(null);
readonly hasCiphers$: Observable<boolean | null> = of(null);
readonly criticalApplicationAtRiskCipherIds$: Observable<CipherId[]> = of([]);
readonly reportProgress$: Observable<ReportProgress | null> = of(null);
// New applications that need review (reviewedDate === null)
@@ -64,6 +65,8 @@ export class RiskInsightsDataService {
this.enrichedReportData$ = this.orchestrator.enrichedReportData$;
this.criticalReportResults$ = this.orchestrator.criticalReportResults$;
this.newApplications$ = this.orchestrator.newApplications$;
this.criticalApplicationAtRiskCipherIds$ =
this.orchestrator.criticalApplicationAtRiskCipherIds$;
this.reportProgress$ = this.orchestrator.reportProgress$;
this.hasCiphers$ = this.orchestrator.hasCiphers$.pipe(distinctUntilChanged());