bitwarden/browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser synced 2026-02-03 10:13:31 +00:00
Code Issues Releases Wiki Activity

Remove newApplications from OrganizationReportSummary - use derived observable instead

Browse Source 
The newApplications field was redundant in the summary type because it can be
calculated from applicationData (where reviewedDate === null). This commit:

1. **Removes newApplications from OrganizationReportSummary type**
   - No longer stored in encrypted summary
   - Reduces encryption overhead

2. **Removes all validation logic for newApplications**
   - Removed from type guards (isOrganizationReportSummary)
   - Removed from validation function (validateOrganizationReportSummary)
   - Removed from allowedKeys list

3. **Removes newApplications from summary calculation**
   - risk-insights-report.service.ts: Removed dummy data generation
   - all-activities.service.ts: Removed from initial state and setter
   - helpers/risk-insights-data-mappers.ts: Removed from createNewSummaryData()

4. **Creates newApplications$ observable in orchestrator**
   - Derives new applications from applicationData
   - Filters where reviewedDate === null
   - Uses distinctUntilChanged and shareReplay for efficiency

5. **Exposes newApplications$ through data service**
   - Added to RiskInsightsDataService
   - Wired up in constructor

6. **Updates component to use the observable**
   - all-activity.component.ts: Subscribe to dataService.newApplications$
   - Removes dependency on summary.newApplications

7. **Removes all newApplications tests**
   - Removed backward compatibility tests (no longer needed)
   - Removed validation tests for newApplications
   - Updated mock data to exclude newApplications
   - Removed from risk-insights-report.service.spec.ts

Benefits:
- Single source of truth (derived from applicationData)
- Eliminates redundant data storage
- Reduces encrypted payload size
- Cleaner architecture (computed vs stored data)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Claude
2025-10-30 13:08:02 +00:00
parent e12ede57bc
commit 4521262f2b
11 changed files with 30 additions and 238 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
bitwarden_license/bit-common/src/dirt/reports/risk-insights/helpers/risk-insights-data-mappers.ts
View File

@@ -73,7 +73,6 @@ export function createNewSummaryData(): OrganizationReportSummary {
totalCriticalAtRiskMemberCount: 0,
totalCriticalApplicationCount: 0,
totalCriticalAtRiskApplicationCount: 0,
newApplications: [],
};
}
export function getAtRiskApplicationList(

1
bitwarden_license/bit-common/src/dirt/reports/risk-insights/models/mocks/mock-data.ts
View File

@@ -77,7 +77,6 @@ export const mockSummaryData: OrganizationReportSummary = {
totalCriticalAtRiskMemberCount: 1,
totalCriticalApplicationCount: 1,
totalCriticalAtRiskApplicationCount: 1,
newApplications: [],
};
export const mockApplicationData: OrganizationReportApplication[] = [
{

2
bitwarden_license/bit-common/src/dirt/reports/risk-insights/models/report-models.ts
View File

@@ -55,8 +55,6 @@ export type OrganizationReportSummary = {
totalCriticalMemberCount: number;
totalCriticalAtRiskMemberCount: number;
totalCriticalAtRiskApplicationCount: number;
/** Optional for backward compatibility - legacy encrypted data predates this field */
newApplications?: string[];
};
/**

20
bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-orchestrator.service.ts
View File

@@ -98,6 +98,26 @@ export class RiskInsightsOrchestratorService {
// --------------------------- Critical Application data ---------------------
criticalReportResults$: Observable<RiskInsightsEnrichedData | null> = of(null);
// --------------------------- New Applications Observable ---------------------
/**
* Observable that emits the list of new application names (applications not yet reviewed).
* Derived from applicationData where reviewedDate === null.
*/
newApplications$: Observable<string[]> = this.rawReportData$.pipe(
map((reportState) => {
if (!reportState.data?.applicationData) {
return [];
}
return reportState.data.applicationData
.filter((app) => app.reviewedDate === null)
.map((app) => app.applicationName);
}),
distinctUntilChanged(
(prev, curr) => prev.length === curr.length && prev.every((app, i) => app === curr[i]),
),
shareReplay({ bufferSize: 1, refCount: true }),
);
// --------------------------- Trigger subjects ---------------------
private _initializeOrganizationTriggerSubject = new Subject<OrganizationId>();
private _fetchReportTriggerSubject = new Subject<void>();

1
bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-report.service.spec.ts
View File

@@ -175,7 +175,6 @@ describe("RiskInsightsReportService", () => {
totalCriticalAtRiskMemberCount: 1,
totalCriticalApplicationCount: 1,
totalCriticalAtRiskApplicationCount: 1,
newApplications: [],
},
applicationData: [],
};

17
bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-report.service.ts
View File

@@ -57,22 +57,6 @@ export class RiskInsightsReportService {
const uniqueAtRiskMembers = getUniqueMembers(atRiskMembers);
// TODO: Replace with actual new applications detection logic (PM-26185)
const dummyNewApplications = [
"github.com",
"google.com",
"stackoverflow.com",
"gitlab.com",
"bitbucket.org",
"npmjs.com",
"docker.com",
"aws.amazon.com",
"azure.microsoft.com",
"jenkins.io",
"terraform.io",
"kubernetes.io",
"atlassian.net",
];
return {
totalMemberCount: uniqueMembers.length,
totalAtRiskMemberCount: uniqueAtRiskMembers.length,
@@ -82,7 +66,6 @@ export class RiskInsightsReportService {
totalCriticalAtRiskMemberCount: 0,
totalCriticalApplicationCount: 0,
totalCriticalAtRiskApplicationCount: 0,
newApplications: dummyNewApplications,
};
}

189
bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-type-guards.spec.ts
View File

@@ -158,7 +158,6 @@ describe("Risk Insights Type Guards", () => {
totalCriticalMemberCount: 4,
totalCriticalAtRiskMemberCount: 1,
totalCriticalAtRiskApplicationCount: 1,
newApplications: ["app-1", "app-2"],
};
expect(() => validateOrganizationReportSummary(validData)).not.toThrow();
@@ -174,7 +173,6 @@ describe("Risk Insights Type Guards", () => {
totalCriticalMemberCount: 4,
totalCriticalAtRiskMemberCount: 1,
totalCriticalAtRiskApplicationCount: 1,
newApplications: ["app-1"],
};
expect(() => validateOrganizationReportSummary(invalidData)).toThrow(
@@ -186,7 +184,6 @@ describe("Risk Insights Type Guards", () => {
const invalidData = {
totalMemberCount: 10,
// missing multiple fields
newApplications: ["app-1"],
};
expect(() => validateOrganizationReportSummary(invalidData)).toThrow(
@@ -204,7 +201,6 @@ describe("Risk Insights Type Guards", () => {
totalCriticalMemberCount: 4,
totalCriticalAtRiskMemberCount: 1,
totalCriticalAtRiskApplicationCount: 1,
newApplications: ["app-1"],
};
expect(() => validateOrganizationReportSummary(invalidData)).toThrow(
@@ -212,111 +208,6 @@ describe("Risk Insights Type Guards", () => {
);
});
it("should throw error for non-array newApplications", () => {
const invalidData = {
totalMemberCount: 10,
totalApplicationCount: 5,
totalAtRiskMemberCount: 2,
totalAtRiskApplicationCount: 1,
totalCriticalApplicationCount: 3,
totalCriticalMemberCount: 4,
totalCriticalAtRiskMemberCount: 1,
totalCriticalAtRiskApplicationCount: 1,
newApplications: "not-an-array",
};
expect(() => validateOrganizationReportSummary(invalidData)).toThrow(
/Invalid OrganizationReportSummary.*newApplications/,
);
});
it("should throw error for empty string in newApplications", () => {
const invalidData = {
totalMemberCount: 10,
totalApplicationCount: 5,
totalAtRiskMemberCount: 2,
totalAtRiskApplicationCount: 1,
totalCriticalApplicationCount: 3,
totalCriticalMemberCount: 4,
totalCriticalAtRiskMemberCount: 1,
totalCriticalAtRiskApplicationCount: 1,
newApplications: ["app-1", "", "app-3"], // empty string
};
expect(() => validateOrganizationReportSummary(invalidData)).toThrow(
/Invalid OrganizationReportSummary/,
);
});
// Backward compatibility tests - legacy encrypted data predates newApplications field
it("should accept OrganizationReportSummary without newApplications field (backward compatibility)", () => {
const validLegacyData = {
totalMemberCount: 10,
totalApplicationCount: 5,
totalAtRiskMemberCount: 2,
totalAtRiskApplicationCount: 1,
totalCriticalApplicationCount: 3,
totalCriticalMemberCount: 4,
totalCriticalAtRiskMemberCount: 1,
totalCriticalAtRiskApplicationCount: 1,
// newApplications field intentionally omitted - legacy data
};
expect(() => validateOrganizationReportSummary(validLegacyData)).not.toThrow();
expect(validateOrganizationReportSummary(validLegacyData)).toEqual(validLegacyData);
});
it("should accept OrganizationReportSummary with undefined newApplications (backward compatibility)", () => {
const validData = {
totalMemberCount: 10,
totalApplicationCount: 5,
totalAtRiskMemberCount: 2,
totalAtRiskApplicationCount: 1,
totalCriticalApplicationCount: 3,
totalCriticalMemberCount: 4,
totalCriticalAtRiskMemberCount: 1,
totalCriticalAtRiskApplicationCount: 1,
newApplications: undefined as string[] | undefined,
};
expect(() => validateOrganizationReportSummary(validData)).not.toThrow();
});
it("should enforce array length limits on newApplications when present", () => {
const invalidData = {
totalMemberCount: 10,
totalApplicationCount: 5,
totalAtRiskMemberCount: 2,
totalAtRiskApplicationCount: 1,
totalCriticalApplicationCount: 3,
totalCriticalMemberCount: 4,
totalCriticalAtRiskMemberCount: 1,
totalCriticalAtRiskApplicationCount: 1,
newApplications: new Array(50001).fill("app"), // exceeds MAX_ARRAY_LENGTH
};
expect(() => validateOrganizationReportSummary(invalidData)).toThrow(
/Invalid OrganizationReportSummary.*newApplications/,
);
});
it("should enforce string length limits on newApplications when present", () => {
const invalidData = {
totalMemberCount: 10,
totalApplicationCount: 5,
totalAtRiskMemberCount: 2,
totalAtRiskApplicationCount: 1,
totalCriticalApplicationCount: 3,
totalCriticalMemberCount: 4,
totalCriticalAtRiskMemberCount: 1,
totalCriticalAtRiskApplicationCount: 1,
newApplications: ["app-1", "a".repeat(1001)], // exceeds MAX_STRING_LENGTH
};
expect(() => validateOrganizationReportSummary(invalidData)).toThrow(
/Invalid OrganizationReportSummary.*newApplications/,
);
});
});
describe("validateOrganizationReportApplicationArray", () => {
@@ -621,7 +512,6 @@ describe("Risk Insights Type Guards", () => {
totalCriticalMemberCount: 4,
totalCriticalAtRiskMemberCount: 1,
totalCriticalAtRiskApplicationCount: 1,
newApplications: ["app-1"],
};
expect(isOrganizationReportSummary(validData)).toBe(true);
});
@@ -636,7 +526,6 @@ describe("Risk Insights Type Guards", () => {
totalCriticalMemberCount: 4,
totalCriticalAtRiskMemberCount: 1,
totalCriticalAtRiskApplicationCount: 1,
newApplications: ["app-1"],
};
expect(isOrganizationReportSummary(invalidData)).toBe(false);
});
@@ -651,7 +540,6 @@ describe("Risk Insights Type Guards", () => {
totalCriticalMemberCount: 4,
totalCriticalAtRiskMemberCount: 1,
totalCriticalAtRiskApplicationCount: 1,
newApplications: ["app-1"],
};
expect(isOrganizationReportSummary(invalidData)).toBe(false);
});
@@ -666,7 +554,6 @@ describe("Risk Insights Type Guards", () => {
totalCriticalMemberCount: 4,
totalCriticalAtRiskMemberCount: 1,
totalCriticalAtRiskApplicationCount: 1,
newApplications: ["app-1"],
};
expect(isOrganizationReportSummary(invalidData)).toBe(false);
});
@@ -681,87 +568,11 @@ describe("Risk Insights Type Guards", () => {
totalCriticalMemberCount: 4,
totalCriticalAtRiskMemberCount: 1,
totalCriticalAtRiskApplicationCount: 1,
newApplications: ["app-1"],
extraField: "should be rejected",
};
expect(isOrganizationReportSummary(invalidData)).toBe(false);
});
// Backward compatibility tests - legacy encrypted data predates newApplications field
it("should return true for OrganizationReportSummary without newApplications field (backward compatibility)", () => {
const validLegacyData = {
totalMemberCount: 10,
totalApplicationCount: 5,
totalAtRiskMemberCount: 2,
totalAtRiskApplicationCount: 1,
totalCriticalApplicationCount: 3,
totalCriticalMemberCount: 4,
totalCriticalAtRiskMemberCount: 1,
totalCriticalAtRiskApplicationCount: 1,
// newApplications field intentionally omitted - legacy data
};
expect(isOrganizationReportSummary(validLegacyData)).toBe(true);
});
it("should return true for OrganizationReportSummary with undefined newApplications (backward compatibility)", () => {
const validData = {
totalMemberCount: 10,
totalApplicationCount: 5,
totalAtRiskMemberCount: 2,
totalAtRiskApplicationCount: 1,
totalCriticalApplicationCount: 3,
totalCriticalMemberCount: 4,
totalCriticalAtRiskMemberCount: 1,
totalCriticalAtRiskApplicationCount: 1,
newApplications: undefined as string[] | undefined,
};
expect(isOrganizationReportSummary(validData)).toBe(true);
});
it("should return false for empty string in newApplications when present", () => {
const invalidData = {
totalMemberCount: 10,
totalApplicationCount: 5,
totalAtRiskMemberCount: 2,
totalAtRiskApplicationCount: 1,
totalCriticalApplicationCount: 3,
totalCriticalMemberCount: 4,
totalCriticalAtRiskMemberCount: 1,
totalCriticalAtRiskApplicationCount: 1,
newApplications: ["app-1", "", "app-3"], // empty string should be rejected
};
expect(isOrganizationReportSummary(invalidData)).toBe(false);
});
it("should return false when newApplications exceeds array length limit", () => {
const invalidData = {
totalMemberCount: 10,
totalApplicationCount: 5,
totalAtRiskMemberCount: 2,
totalAtRiskApplicationCount: 1,
totalCriticalApplicationCount: 3,
totalCriticalMemberCount: 4,
totalCriticalAtRiskMemberCount: 1,
totalCriticalAtRiskApplicationCount: 1,
newApplications: new Array(50001).fill("app"), // exceeds MAX_ARRAY_LENGTH
};
expect(isOrganizationReportSummary(invalidData)).toBe(false);
});
it("should return false when newApplications contains strings exceeding length limit", () => {
const invalidData = {
totalMemberCount: 10,
totalApplicationCount: 5,
totalAtRiskMemberCount: 2,
totalAtRiskApplicationCount: 1,
totalCriticalApplicationCount: 3,
totalCriticalMemberCount: 4,
totalCriticalAtRiskMemberCount: 1,
totalCriticalAtRiskApplicationCount: 1,
newApplications: ["app-1", "a".repeat(1001)], // exceeds MAX_STRING_LENGTH
};
expect(isOrganizationReportSummary(invalidData)).toBe(false);
});
});
describe("isOrganizationReportApplication", () => {

24
bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-type-guards.ts
View File

@@ -181,7 +181,6 @@ export function isOrganizationReportSummary(obj: any): obj is OrganizationReport
"totalCriticalMemberCount",
"totalCriticalAtRiskMemberCount",
"totalCriticalAtRiskApplicationCount",
"newApplications",
];
const actualKeys = Object.keys(obj);
const hasUnexpectedProps = actualKeys.some((key) => !allowedKeys.includes(key));
@@ -229,15 +228,7 @@ export function isOrganizationReportSummary(obj: any): obj is OrganizationReport
Number.isFinite(obj.totalCriticalAtRiskApplicationCount) &&
Number.isSafeInteger(obj.totalCriticalAtRiskApplicationCount) &&
obj.totalCriticalAtRiskApplicationCount >= 0 &&
obj.totalCriticalAtRiskApplicationCount <= MAX_COUNT &&
// newApplications is optional (backward compatibility - legacy encrypted data predates this field)
(obj.newApplications === undefined ||
(Array.isArray(obj.newApplications) &&
obj.newApplications.length <= MAX_ARRAY_LENGTH &&
obj.newApplications.every(
(app: any) =>
typeof app === "string" && app.length > 0 && app.length <= MAX_STRING_LENGTH,
)))
obj.totalCriticalAtRiskApplicationCount <= MAX_COUNT
);
}
@@ -349,19 +340,6 @@ export function validateOrganizationReportSummary(data: any): OrganizationReport
if (typeof data?.totalCriticalAtRiskApplicationCount !== "number") {
missingFields.push("totalCriticalAtRiskApplicationCount (number)");
}
// newApplications is optional (backward compatibility - legacy encrypted data predates this field)
// Only validate if present, but enforce all constraints to prevent DoS attacks
if (
data?.newApplications !== undefined &&
(!Array.isArray(data?.newApplications) ||
data.newApplications.length > MAX_ARRAY_LENGTH ||
!data.newApplications.every(
(app: any) =>
typeof app === "string" && app.length > 0 && app.length <= MAX_STRING_LENGTH,
))
) {
missingFields.push("newApplications (optional string[])");
}
throw new Error(
`Invalid OrganizationReportSummary: ${missingFields.length > 0 ? `missing or invalid fields: ${missingFields.join(", ")}` : "structure validation failed"}`,

2
bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/view/all-activities.service.ts
View File

@@ -20,7 +20,6 @@ export class AllActivitiesService {
totalCriticalApplicationCount: 0,
totalAtRiskApplicationCount: 0,
totalCriticalAtRiskApplicationCount: 0,
newApplications: [],
});
reportSummary$ = this.reportSummarySubject$.asObservable();
@@ -78,7 +77,6 @@ export class AllActivitiesService {
totalAtRiskMemberCount: summary.totalAtRiskMemberCount,
totalApplicationCount: summary.totalApplicationCount,
totalAtRiskApplicationCount: summary.totalAtRiskApplicationCount,
newApplications: summary.newApplications,
});
}

2
bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/view/risk-insights-data.service.ts
View File

@@ -28,6 +28,7 @@ export class RiskInsightsDataService {
readonly enrichedReportData$: Observable<RiskInsightsEnrichedData | null> = of(null);
readonly isGeneratingReport$: Observable<boolean> = of(false);
readonly criticalReportResults$: Observable<RiskInsightsEnrichedData | null> = of(null);
readonly newApplications$: Observable<string[]> = of([]);
// ------------------------- Drawer Variables ---------------------
// Drawer variables unified into a single BehaviorSubject
@@ -48,6 +49,7 @@ export class RiskInsightsDataService {
this.organizationDetails$ = this.orchestrator.organizationDetails$;
this.enrichedReportData$ = this.orchestrator.enrichedReportData$;
this.criticalReportResults$ = this.orchestrator.criticalReportResults$;
this.newApplications$ = this.orchestrator.newApplications$;
// Expose the loading state
this.isLoading$ = this.reportState$.pipe(

9
bitwarden_license/bit-web/src/app/dirt/access-intelligence/activity/all-activity.component.ts
View File

@@ -69,8 +69,13 @@ export class AllActivityComponent implements OnInit {
this.totalCriticalAppsAtRiskMemberCount = summary.totalCriticalAtRiskMemberCount;
this.totalCriticalAppsCount = summary.totalCriticalApplicationCount;
this.totalCriticalAppsAtRiskCount = summary.totalCriticalAtRiskApplicationCount;
this.newApplications = summary.newApplications ?? [];
this.newApplicationsCount = summary.newApplications?.length ?? 0;
});
this.dataService.newApplications$
.pipe(takeUntilDestroyed(this.destroyRef))
.subscribe((newApps) => {
this.newApplications = newApps;
this.newApplicationsCount = newApps.length;
});
this.allActivitiesService.passwordChangeProgressMetricHasProgressBar$