From b723536a5762bf750de059efc737f1fe88da6068 Mon Sep 17 00:00:00 2001 From: Graham Walker Date: Mon, 26 Jan 2026 20:08:49 -0600 Subject: [PATCH] PM-20112 adding making changes to include a new bug for the Member Access Report --- apps/web/src/locales/en/messages.json | 3 + .../member-access-report.component.html | 11 +- .../member-access-report.component.spec.ts | 148 +++++ .../model/member-access-progress.spec.ts | 156 +++++ .../model/member-access-progress.ts | 2 +- .../member-access-report.service.spec.ts | 606 ++++++++++++++---- .../services/member-access-report.service.ts | 35 +- 7 files changed, 820 insertions(+), 141 deletions(-) create mode 100644 bitwarden_license/bit-web/src/app/dirt/reports/member-access-report/member-access-report.component.spec.ts create mode 100644 bitwarden_license/bit-web/src/app/dirt/reports/member-access-report/model/member-access-progress.spec.ts diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index aa145c230fd..a956dd238cb 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -4558,6 +4558,9 @@ } } }, + "reportGenerationComplete": { + "message": "Report generation complete" + }, "riskInsightsRunReport": { "message": "Run report" }, diff --git a/bitwarden_license/bit-web/src/app/dirt/reports/member-access-report/member-access-report.component.html b/bitwarden_license/bit-web/src/app/dirt/reports/member-access-report/member-access-report.component.html index 301e7d864a9..c2091d1a452 100644 --- a/bitwarden_license/bit-web/src/app/dirt/reports/member-access-report/member-access-report.component.html +++ b/bitwarden_license/bit-web/src/app/dirt/reports/member-access-report/member-access-report.component.html @@ -55,12 +55,13 @@ >
- -
- {{ row.email }} -
+ @if (row.name) { +
+ {{ row.email }} +
+ }
diff --git a/bitwarden_license/bit-web/src/app/dirt/reports/member-access-report/member-access-report.component.spec.ts b/bitwarden_license/bit-web/src/app/dirt/reports/member-access-report/member-access-report.component.spec.ts new file mode 100644 index 00000000000..5e2293ebb9d --- /dev/null +++ b/bitwarden_license/bit-web/src/app/dirt/reports/member-access-report/member-access-report.component.spec.ts @@ -0,0 +1,148 @@ +/** + * Unit tests for MemberAccessReportComponent + * + * These tests focus on the member column display logic to ensure: + * - Users with names show name as primary text and email as secondary + * - Users without names show email as primary text with no secondary text + */ + +describe("MemberAccessReportComponent - Member Column Display Logic", () => { + describe("Template logic for member name display", () => { + it("should use name when available (name || email pattern)", () => { + const user = { name: "John Doe", email: "john@example.com" }; + const displayText = user.name || user.email; + expect(displayText).toBe("John Doe"); + }); + + it("should fallback to email when name is empty string", () => { + const user = { name: "", email: "noname@example.com" }; + const displayText = user.name || user.email; + expect(displayText).toBe("noname@example.com"); + }); + + it("should fallback to email when name is null", () => { + const user = { name: null as any, email: "null@example.com" }; + const displayText = user.name || user.email; + expect(displayText).toBe("null@example.com"); + }); + + it("should fallback to email when name is undefined", () => { + const user = { name: undefined as any, email: "undefined@example.com" }; + const displayText = user.name || user.email; + expect(displayText).toBe("undefined@example.com"); + }); + }); + + describe("Template logic for showing secondary email", () => { + it("should show secondary email when user has a name", () => { + const user = { name: "John Doe", email: "john@example.com" }; + const showSecondary = !!user.name; + expect(showSecondary).toBe(true); + }); + + it("should not show secondary email when name is empty string", () => { + const user = { name: "", email: "noname@example.com" }; + const showSecondary = !!user.name; + expect(showSecondary).toBe(false); + }); + + it("should not show secondary email when name is null", () => { + const user = { name: null as any, email: "null@example.com" }; + const showSecondary = !!user.name; + expect(showSecondary).toBe(false); + }); + + it("should not show secondary email when name is undefined", () => { + const user = { name: undefined as any, email: "undefined@example.com" }; + const showSecondary = !!user.name; + expect(showSecondary).toBe(false); + }); + }); + + describe("Combined scenarios matching template", () => { + interface TestUser { + name: string; + email: string; + } + + const getDisplayInfo = (user: TestUser) => { + return { + primaryText: user.name || user.email, + showSecondaryEmail: !!user.name, + secondaryText: user.name ? user.email : null, + }; + }; + + it("should correctly display user with name", () => { + const user = { name: "Alice Smith", email: "alice@example.com" }; + const display = getDisplayInfo(user); + + expect(display.primaryText).toBe("Alice Smith"); + expect(display.showSecondaryEmail).toBe(true); + expect(display.secondaryText).toBe("alice@example.com"); + }); + + it("should correctly display user without name", () => { + const user = { name: "", email: "bob@example.com" }; + const display = getDisplayInfo(user); + + expect(display.primaryText).toBe("bob@example.com"); + expect(display.showSecondaryEmail).toBe(false); + expect(display.secondaryText).toBe(null); + }); + + it("should handle multiple users with mixed scenarios", () => { + const users = [ + { name: "User With Name", email: "with@example.com" }, + { name: "", email: "without@example.com" }, + { name: "Another Name", email: "another@example.com" }, + ]; + + const displays = users.map(getDisplayInfo); + + // First user (with name) + expect(displays[0].primaryText).toBe("User With Name"); + expect(displays[0].showSecondaryEmail).toBe(true); + expect(displays[0].secondaryText).toBe("with@example.com"); + + // Second user (without name) + expect(displays[1].primaryText).toBe("without@example.com"); + expect(displays[1].showSecondaryEmail).toBe(false); + expect(displays[1].secondaryText).toBe(null); + + // Third user (with name) + expect(displays[2].primaryText).toBe("Another Name"); + expect(displays[2].showSecondaryEmail).toBe(true); + expect(displays[2].secondaryText).toBe("another@example.com"); + }); + }); + + describe("Template pattern verification", () => { + it("should match the pattern used in Members page", () => { + // This test verifies that our logic matches the Members page pattern: + // Primary: {{ row.name || row.email }} + // Secondary: @if (row.name) { {{ row.email }} } + + const testCases = [ + { + user: { name: "Test User", email: "test@example.com" }, + expectedPrimary: "Test User", + expectedShowSecondary: true, + }, + { + user: { name: "", email: "noname@example.com" }, + expectedPrimary: "noname@example.com", + expectedShowSecondary: false, + }, + ]; + + testCases.forEach(({ user, expectedPrimary, expectedShowSecondary }) => { + const primary = user.name || user.email; + const showSecondary = !!user.name; + + expect(primary).toBe(expectedPrimary); + expect(showSecondary).toBe(expectedShowSecondary); + }); + }); + }); +}); diff --git a/bitwarden_license/bit-web/src/app/dirt/reports/member-access-report/model/member-access-progress.spec.ts b/bitwarden_license/bit-web/src/app/dirt/reports/member-access-report/model/member-access-progress.spec.ts new file mode 100644 index 00000000000..2a93c6aa702 --- /dev/null +++ b/bitwarden_license/bit-web/src/app/dirt/reports/member-access-report/model/member-access-progress.spec.ts @@ -0,0 +1,156 @@ +import { + MemberAccessProgress, + MemberAccessProgressConfig, + MemberAccessProgressState, + calculateProgressPercentage, +} from "./member-access-progress"; + +describe("MemberAccessProgress", () => { + describe("Progress Configuration", () => { + it("should have valid configuration for all progress steps", () => { + // Verify all steps have a config entry + Object.values(MemberAccessProgress).forEach((step) => { + const config = MemberAccessProgressConfig[step]; + expect(config).toBeDefined(); + expect(config.messageKey).toBeDefined(); + expect(config.progress).toBeDefined(); + expect(typeof config.messageKey).toBe("string"); + expect(typeof config.progress).toBe("number"); + }); + }); + + it("should have 'reportGenerationComplete' as the completion message key", () => { + const completeConfig = MemberAccessProgressConfig[MemberAccessProgress.Complete]; + expect(completeConfig.messageKey).toBe("reportGenerationComplete"); + }); + + it("should have 100% progress for Complete step", () => { + const completeConfig = MemberAccessProgressConfig[MemberAccessProgress.Complete]; + expect(completeConfig.progress).toBe(100); + }); + + it("should have sequential progress values", () => { + const progressValues = [ + MemberAccessProgressConfig[MemberAccessProgress.FetchingMembers].progress, + MemberAccessProgressConfig[MemberAccessProgress.FetchingCollections].progress, + MemberAccessProgressConfig[MemberAccessProgress.FetchingGroups].progress, + MemberAccessProgressConfig[MemberAccessProgress.FetchingCipherCounts].progress, + MemberAccessProgressConfig[MemberAccessProgress.BuildingMaps].progress, + MemberAccessProgressConfig[MemberAccessProgress.ProcessingMembers].progress, + MemberAccessProgressConfig[MemberAccessProgress.Complete].progress, + ]; + + // Verify values are in ascending order + for (let i = 0; i < progressValues.length - 1; i++) { + expect(progressValues[i]).toBeLessThanOrEqual(progressValues[i + 1]); + } + }); + + it("should have unique message keys for all steps", () => { + const messageKeys = Object.values(MemberAccessProgressConfig).map((c) => c.messageKey); + const uniqueKeys = new Set(messageKeys); + expect(uniqueKeys.size).toBe(messageKeys.length); + }); + }); + + describe("calculateProgressPercentage", () => { + it("should return static progress for non-ProcessingMembers steps", () => { + const state: MemberAccessProgressState = { + step: MemberAccessProgress.FetchingMembers, + processedMembers: 0, + totalMembers: 100, + message: "", + }; + + const result = calculateProgressPercentage(state); + expect(result).toBe( + MemberAccessProgressConfig[MemberAccessProgress.FetchingMembers].progress, + ); + }); + + it("should calculate dynamic progress for ProcessingMembers step", () => { + const state: MemberAccessProgressState = { + step: MemberAccessProgress.ProcessingMembers, + processedMembers: 50, + totalMembers: 100, + message: "", + }; + + const result = calculateProgressPercentage(state); + // 35% base + (50/100 * 60%) = 35% + 30% = 65% + expect(result).toBe(65); + }); + + it("should return 35% for ProcessingMembers with 0 processed", () => { + const state: MemberAccessProgressState = { + step: MemberAccessProgress.ProcessingMembers, + processedMembers: 0, + totalMembers: 100, + message: "", + }; + + const result = calculateProgressPercentage(state); + expect(result).toBe(35); + }); + + it("should cap at 95% for ProcessingMembers with all processed", () => { + const state: MemberAccessProgressState = { + step: MemberAccessProgress.ProcessingMembers, + processedMembers: 100, + totalMembers: 100, + message: "", + }; + + const result = calculateProgressPercentage(state); + // 35% + (100/100 * 60%) = 35% + 60% = 95% + expect(result).toBe(95); + }); + + it("should handle ProcessingMembers with 0 total members", () => { + const state: MemberAccessProgressState = { + step: MemberAccessProgress.ProcessingMembers, + processedMembers: 0, + totalMembers: 0, + message: "", + }; + + const result = calculateProgressPercentage(state); + // Should use static progress when totalMembers is 0 + expect(result).toBe( + MemberAccessProgressConfig[MemberAccessProgress.ProcessingMembers].progress, + ); + }); + + it("should return 100% for Complete step", () => { + const state: MemberAccessProgressState = { + step: MemberAccessProgress.Complete, + processedMembers: 100, + totalMembers: 100, + message: "", + }; + + const result = calculateProgressPercentage(state); + expect(result).toBe(100); + }); + + it("should calculate correct progress for partial processing", () => { + const testCases = [ + { processed: 10, total: 100, expected: 41 }, // 35 + (10/100 * 60) + { processed: 25, total: 100, expected: 50 }, // 35 + (25/100 * 60) + { processed: 75, total: 100, expected: 80 }, // 35 + (75/100 * 60) + ]; + + testCases.forEach(({ processed, total, expected }) => { + const state: MemberAccessProgressState = { + step: MemberAccessProgress.ProcessingMembers, + processedMembers: processed, + totalMembers: total, + message: "", + }; + + const result = calculateProgressPercentage(state); + expect(result).toBe(expected); + }); + }); + }); +}); diff --git a/bitwarden_license/bit-web/src/app/dirt/reports/member-access-report/model/member-access-progress.ts b/bitwarden_license/bit-web/src/app/dirt/reports/member-access-report/model/member-access-progress.ts index 07d6844e2c3..94fa7ea72bd 100644 --- a/bitwarden_license/bit-web/src/app/dirt/reports/member-access-report/model/member-access-progress.ts +++ b/bitwarden_license/bit-web/src/app/dirt/reports/member-access-report/model/member-access-progress.ts @@ -60,7 +60,7 @@ export const MemberAccessProgressConfig = Object.freeze({ progress: 35, }, [MemberAccessProgress.Complete]: { - messageKey: "complete", + messageKey: "reportGenerationComplete", progress: 100, }, } as const); diff --git a/bitwarden_license/bit-web/src/app/dirt/reports/member-access-report/services/member-access-report.service.spec.ts b/bitwarden_license/bit-web/src/app/dirt/reports/member-access-report/services/member-access-report.service.spec.ts index 615e6d079b2..637566b4228 100644 --- a/bitwarden_license/bit-web/src/app/dirt/reports/member-access-report/services/member-access-report.service.spec.ts +++ b/bitwarden_license/bit-web/src/app/dirt/reports/member-access-report/services/member-access-report.service.spec.ts @@ -1,168 +1,520 @@ import { mock } from "jest-mock-extended"; import { of } from "rxjs"; +import { + CollectionAdminService, + OrganizationUserApiService, +} from "@bitwarden/admin-console/common"; +import { ApiService } from "@bitwarden/common/abstractions/api.service"; +import { CollectionAdminView } from "@bitwarden/common/admin-console/models/collections"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; +import { ListResponse } from "@bitwarden/common/models/response/list.response"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { mockAccountServiceWith } from "@bitwarden/common/spec"; import { OrganizationId, UserId } from "@bitwarden/common/types/guid"; +import { CipherResponse } from "@bitwarden/common/vault/models/response/cipher.response"; import { newGuid } from "@bitwarden/guid"; import { KeyService } from "@bitwarden/key-management"; +import { GroupApiService } from "@bitwarden/web-vault/app/admin-console/organizations/core/services/group/group-api.service"; +import { GroupDetailsView } from "@bitwarden/web-vault/app/admin-console/organizations/core/views/group-details.view"; -import { MemberAccessReportApiService } from "./member-access-report-api.service"; -import { - memberAccessReportsMock, - memberAccessWithoutAccessDetailsReportsMock, -} from "./member-access-report.mock"; import { MemberAccessReportService } from "./member-access-report.service"; -describe("ImportService", () => { +describe("MemberAccessReportService", () => { const mockOrganizationId = "mockOrgId" as OrganizationId; - const reportApiService = mock(); - const mockEncryptService = mock(); const userId = newGuid() as UserId; + + const mockOrganizationUserApiService = mock(); + const mockCollectionAdminService = mock(); + const mockGroupApiService = mock(); + const mockApiService = mock(); + const mockEncryptService = mock(); const mockAccountService = mockAccountServiceWith(userId); const mockKeyService = mock(); - let memberAccessReportService: MemberAccessReportService; - const i18nMock = mock({ + const mockI18nService = mock({ t(key) { return key; }, }); + let service: MemberAccessReportService; + beforeEach(() => { + jest.clearAllMocks(); + mockKeyService.orgKeys$.mockReturnValue( of({ mockOrgId: new SymmetricCryptoKey(new Uint8Array(64)) }), ); - reportApiService.getMemberAccessData.mockImplementation(() => - Promise.resolve(memberAccessReportsMock), - ); - memberAccessReportService = new MemberAccessReportService( - reportApiService, - i18nMock, + + // Mock account service + mockAccountService.activeAccount$ = of({ id: userId } as any); + + service = new MemberAccessReportService( + mockI18nService, mockEncryptService, mockKeyService, mockAccountService, + mockOrganizationUserApiService, + mockCollectionAdminService, + mockGroupApiService, + mockApiService, ); }); - describe("generateMemberAccessReportView", () => { - it("should generate member access report view", async () => { - const result = - await memberAccessReportService.generateMemberAccessReportView(mockOrganizationId); - - expect(result).toEqual([ - { - name: "Sarah Johnson", - email: "sjohnson@email.com", - collectionsCount: 3, - groupsCount: 1, - itemsCount: 0, - userGuid: expect.any(String), - usesKeyConnector: expect.any(Boolean), - }, - { - name: "James Lull", - email: "jlull@email.com", - collectionsCount: 2, - groupsCount: 1, - itemsCount: 0, - userGuid: expect.any(String), - usesKeyConnector: expect.any(Boolean), - }, - { - name: "Beth Williams", - email: "bwilliams@email.com", - collectionsCount: 2, - groupsCount: 1, - itemsCount: 0, - userGuid: expect.any(String), - usesKeyConnector: expect.any(Boolean), - }, - { - name: "Ray Williams", - email: "rwilliams@email.com", - collectionsCount: 3, - groupsCount: 3, - itemsCount: 0, - userGuid: expect.any(String), - usesKeyConnector: expect.any(Boolean), - }, - ]); - }); - }); - describe("generateUserReportExportItems", () => { - it("should generate user report export items", async () => { - const result = - await memberAccessReportService.generateUserReportExportItems(mockOrganizationId); + describe("(No Name) fallback", () => { + it("should use '(No Name)' when user has empty name string", async () => { + // Setup mock data with empty name + mockOrganizationUserApiService.getAllUsers.mockResolvedValue({ + data: [ + { + id: "user1", + name: "", // Empty name + email: "user@example.com", + twoFactorEnabled: false, + resetPasswordEnrolled: false, + usesKeyConnector: false, + groups: ["group1"], + avatarColor: null, + } as any, + ], + } as ListResponse); - const filteredReportItems = result - .filter( - (item) => - (item.name === "Sarah Johnson" && - item.group === "Group 1" && - item.totalItems === "0") || - (item.name === "James Lull" && item.group === "Group 4" && item.totalItems === "0"), - ) - .map((item) => ({ - name: item.name, - email: item.email, - group: item.group, - totalItems: item.totalItems, - accountRecovery: item.accountRecovery, - twoStepLogin: item.twoStepLogin, - })); + mockCollectionAdminService.collectionAdminViews$.mockReturnValue( + of([ + { + id: "col1", + name: "Test Collection", + groups: [{ id: "group1", readOnly: false, hidePasswords: false, manage: false }], + users: [], + } as CollectionAdminView, + ]), + ); - expect(filteredReportItems).toEqual( - expect.arrayContaining([ - expect.objectContaining({ - email: "sjohnson@email.com", - name: "Sarah Johnson", - twoStepLogin: "memberAccessReportTwoFactorEnabledTrue", - accountRecovery: "memberAccessReportAuthenticationEnabledTrue", - group: "Group 1", - totalItems: "0", - }), - expect.objectContaining({ - email: "jlull@email.com", - name: "James Lull", - twoStepLogin: "memberAccessReportTwoFactorEnabledFalse", - accountRecovery: "memberAccessReportAuthenticationEnabledFalse", - group: "Group 4", - totalItems: "0", - }), - ]), - ); + mockGroupApiService.getAllDetails.mockResolvedValue([ + { + id: "group1", + name: "Test Group", + collections: [{ id: "col1", readOnly: false, hidePasswords: false, manage: false }], + } as GroupDetailsView, + ]); + + mockApiService.getCiphersOrganization.mockResolvedValue({ + data: [] as CipherResponse[], + }); + + // Generate report view first (required before export) + await service.generateMemberAccessReportView(mockOrganizationId); + + // Generate export + const result = await service.generateUserReportExportItems(mockOrganizationId); + + // Verify all export items have "(No Name)" instead of empty string + expect(result.length).toBeGreaterThan(0); + result.forEach((item) => { + expect(item.name).toBe("(No Name)"); + }); + }); + + it("should use actual name when user has non-empty name", async () => { + // Setup mock data with actual name + mockOrganizationUserApiService.getAllUsers.mockResolvedValue({ + data: [ + { + id: "user1", + name: "John Doe", + email: "john@example.com", + twoFactorEnabled: false, + resetPasswordEnrolled: false, + usesKeyConnector: false, + groups: ["group1"], + avatarColor: null, + } as any, + ], + } as ListResponse); + + mockCollectionAdminService.collectionAdminViews$.mockReturnValue( + of([ + { + id: "col1", + name: "Test Collection", + groups: [{ id: "group1", readOnly: false, hidePasswords: false, manage: false }], + users: [], + } as CollectionAdminView, + ]), + ); + + mockGroupApiService.getAllDetails.mockResolvedValue([ + { + id: "group1", + name: "Test Group", + collections: [{ id: "col1", readOnly: false, hidePasswords: false, manage: false }], + } as GroupDetailsView, + ]); + + mockApiService.getCiphersOrganization.mockResolvedValue({ + data: [] as CipherResponse[], + }); + + // Generate report view first + await service.generateMemberAccessReportView(mockOrganizationId); + + // Generate export + const result = await service.generateUserReportExportItems(mockOrganizationId); + + // Verify actual name is used + expect(result.length).toBeGreaterThan(0); + result.forEach((item) => { + expect(item.name).toBe("John Doe"); + }); + }); + + it("should use '(No Name)' for users with no collection or group access", async () => { + // Setup mock data with empty name and no access + mockOrganizationUserApiService.getAllUsers.mockResolvedValue({ + data: [ + { + id: "user1", + name: "", // Empty name + email: "user@example.com", + twoFactorEnabled: false, + resetPasswordEnrolled: false, + usesKeyConnector: false, + groups: [], // No groups + avatarColor: null, + } as any, + ], + } as ListResponse); + + mockCollectionAdminService.collectionAdminViews$.mockReturnValue(of([])); + mockGroupApiService.getAllDetails.mockResolvedValue([]); + mockApiService.getCiphersOrganization.mockResolvedValue({ + data: [] as CipherResponse[], + }); + + // Generate report view first + await service.generateMemberAccessReportView(mockOrganizationId); + + // Generate export + const result = await service.generateUserReportExportItems(mockOrganizationId); + + // Should have one row for user with no access + expect(result).toHaveLength(1); + expect(result[0].name).toBe("(No Name)"); + expect(result[0].email).toBe("user@example.com"); + expect(result[0].group).toBe("memberAccessReportNoGroup"); + expect(result[0].collection).toBe("memberAccessReportNoCollection"); + }); }); - it("should generate user report export items and include users with no access", async () => { - reportApiService.getMemberAccessData.mockImplementation(() => - Promise.resolve(memberAccessWithoutAccessDetailsReportsMock), - ); - const result = - await memberAccessReportService.generateUserReportExportItems(mockOrganizationId); + describe("Groups with no collections", () => { + it("should include group membership even when group has no collections", async () => { + // Setup: user in a group, but group has no collections + mockOrganizationUserApiService.getAllUsers.mockResolvedValue({ + data: [ + { + id: "user1", + name: "Jane Doe", + email: "jane@example.com", + twoFactorEnabled: true, + resetPasswordEnrolled: true, + usesKeyConnector: false, + groups: ["group1"], // User is in group1 + avatarColor: null, + } as any, + ], + } as ListResponse); - expect(result).toEqual( - expect.arrayContaining([ - expect.objectContaining({ - email: "asmith@email.com", - name: "Alice Smith", - twoStepLogin: "memberAccessReportTwoFactorEnabledTrue", - accountRecovery: "memberAccessReportAuthenticationEnabledTrue", - group: "Alice Group 1", - totalItems: "0", - }), - expect.objectContaining({ - email: "rbrown@email.com", - name: "Robert Brown", - twoStepLogin: "memberAccessReportTwoFactorEnabledFalse", - accountRecovery: "memberAccessReportAuthenticationEnabledFalse", - group: "memberAccessReportNoGroup", - totalItems: "0", - }), - ]), - ); + // No collections at all + mockCollectionAdminService.collectionAdminViews$.mockReturnValue(of([])); + + // Group exists but has no collections + mockGroupApiService.getAllDetails.mockResolvedValue([ + { + id: "group1", + name: "Empty Group", + collections: [], // No collections + } as GroupDetailsView, + ]); + + mockApiService.getCiphersOrganization.mockResolvedValue({ + data: [] as CipherResponse[], + }); + + // Generate report view first + await service.generateMemberAccessReportView(mockOrganizationId); + + // Generate export + const result = await service.generateUserReportExportItems(mockOrganizationId); + + // Should have one row showing the group membership + expect(result).toHaveLength(1); + expect(result[0].name).toBe("Jane Doe"); + expect(result[0].email).toBe("jane@example.com"); + expect(result[0].group).toBe("Empty Group"); + expect(result[0].collection).toBe("memberAccessReportNoCollection"); + expect(result[0].collectionPermission).toBe("memberAccessReportNoCollectionPermission"); + expect(result[0].totalItems).toBe("0"); + }); + + it("should create separate rows for groups with collections and groups without", async () => { + // Setup: user in two groups, one with collections and one without + mockOrganizationUserApiService.getAllUsers.mockResolvedValue({ + data: [ + { + id: "user1", + name: "Multi Group User", + email: "multi@example.com", + twoFactorEnabled: false, + resetPasswordEnrolled: false, + usesKeyConnector: false, + groups: ["group1", "group2"], // In both groups + avatarColor: null, + } as any, + ], + } as ListResponse); + + // One collection assigned to group1 + mockCollectionAdminService.collectionAdminViews$.mockReturnValue( + of([ + { + id: "col1", + name: "Collection 1", + groups: [{ id: "group1", readOnly: false, hidePasswords: false, manage: true }], + users: [], + } as CollectionAdminView, + ]), + ); + + // group1 has collection, group2 does not + mockGroupApiService.getAllDetails.mockResolvedValue([ + { + id: "group1", + name: "Group With Collection", + collections: [{ id: "col1", readOnly: false, hidePasswords: false, manage: true }], + } as GroupDetailsView, + { + id: "group2", + name: "Group Without Collection", + collections: [], + } as GroupDetailsView, + ]); + + mockApiService.getCiphersOrganization.mockResolvedValue({ + data: [] as CipherResponse[], + }); + + // Generate report view first + await service.generateMemberAccessReportView(mockOrganizationId); + + // Generate export + const result = await service.generateUserReportExportItems(mockOrganizationId); + + // Should have 2 rows: one for group1 with collection, one for group2 without + expect(result).toHaveLength(2); + + // Find rows by group name + const group1Row = result.find((r) => r.group === "Group With Collection"); + const group2Row = result.find((r) => r.group === "Group Without Collection"); + + expect(group1Row).toBeDefined(); + expect(group1Row?.collection).toBe("Collection 1"); + expect(group1Row?.collectionPermission).not.toBe( + "memberAccessReportNoCollectionPermission", + ); + + expect(group2Row).toBeDefined(); + expect(group2Row?.collection).toBe("memberAccessReportNoCollection"); + expect(group2Row?.collectionPermission).toBe("memberAccessReportNoCollectionPermission"); + expect(group2Row?.totalItems).toBe("0"); + }); + + it("should show multiple collections for group with collections", async () => { + // Setup: user in group with multiple collections + mockOrganizationUserApiService.getAllUsers.mockResolvedValue({ + data: [ + { + id: "user1", + name: "User Name", + email: "user@example.com", + twoFactorEnabled: false, + resetPasswordEnrolled: false, + usesKeyConnector: false, + groups: ["group1"], + avatarColor: null, + } as any, + ], + } as ListResponse); + + // Two collections both assigned to group1 + mockCollectionAdminService.collectionAdminViews$.mockReturnValue( + of([ + { + id: "col1", + name: "Collection 1", + groups: [{ id: "group1", readOnly: false, hidePasswords: false, manage: false }], + users: [], + } as CollectionAdminView, + { + id: "col2", + name: "Collection 2", + groups: [{ id: "group1", readOnly: true, hidePasswords: false, manage: false }], + users: [], + } as CollectionAdminView, + ]), + ); + + mockGroupApiService.getAllDetails.mockResolvedValue([ + { + id: "group1", + name: "Multi Collection Group", + collections: [ + { id: "col1", readOnly: false, hidePasswords: false, manage: false }, + { id: "col2", readOnly: true, hidePasswords: false, manage: false }, + ], + } as GroupDetailsView, + ]); + + mockApiService.getCiphersOrganization.mockResolvedValue({ + data: [] as CipherResponse[], + }); + + // Generate report view first + await service.generateMemberAccessReportView(mockOrganizationId); + + // Generate export + const result = await service.generateUserReportExportItems(mockOrganizationId); + + // Should have 2 rows, one for each collection + expect(result).toHaveLength(2); + expect(result[0].group).toBe("Multi Collection Group"); + expect(result[1].group).toBe("Multi Collection Group"); + + const collectionNames = result.map((r) => r.collection).sort(); + expect(collectionNames).toEqual(["Collection 1", "Collection 2"]); + }); + }); + + describe("Combined scenarios", () => { + it("should handle user with empty name in group with no collections", async () => { + // Combines both fixes: empty name + group without collections + mockOrganizationUserApiService.getAllUsers.mockResolvedValue({ + data: [ + { + id: "user1", + name: "", + email: "noname@example.com", + twoFactorEnabled: true, + resetPasswordEnrolled: false, + usesKeyConnector: false, + groups: ["group1"], + avatarColor: null, + } as any, + ], + } as ListResponse); + + mockCollectionAdminService.collectionAdminViews$.mockReturnValue(of([])); + + mockGroupApiService.getAllDetails.mockResolvedValue([ + { + id: "group1", + name: "No Collection Group", + collections: [], + } as GroupDetailsView, + ]); + + mockApiService.getCiphersOrganization.mockResolvedValue({ + data: [] as CipherResponse[], + }); + + // Generate report view first + await service.generateMemberAccessReportView(mockOrganizationId); + + // Generate export + const result = await service.generateUserReportExportItems(mockOrganizationId); + + expect(result).toHaveLength(1); + expect(result[0].name).toBe("(No Name)"); + expect(result[0].group).toBe("No Collection Group"); + expect(result[0].collection).toBe("memberAccessReportNoCollection"); + }); + + it("should handle multiple users with mixed name and group scenarios", async () => { + // Complex scenario with multiple users + mockOrganizationUserApiService.getAllUsers.mockResolvedValue({ + data: [ + { + id: "user1", + name: "Alice", + email: "alice@example.com", + twoFactorEnabled: true, + resetPasswordEnrolled: true, + usesKeyConnector: false, + groups: ["group1"], + avatarColor: null, + } as any, + { + id: "user2", + name: "", + email: "bob@example.com", + twoFactorEnabled: false, + resetPasswordEnrolled: false, + usesKeyConnector: false, + groups: ["group2"], + avatarColor: null, + } as any, + ], + } as ListResponse); + + mockCollectionAdminService.collectionAdminViews$.mockReturnValue( + of([ + { + id: "col1", + name: "Collection A", + groups: [{ id: "group1", readOnly: false, hidePasswords: false, manage: false }], + users: [], + } as CollectionAdminView, + ]), + ); + + mockGroupApiService.getAllDetails.mockResolvedValue([ + { + id: "group1", + name: "Group A", + collections: [{ id: "col1", readOnly: false, hidePasswords: false, manage: false }], + } as GroupDetailsView, + { + id: "group2", + name: "Group B", + collections: [], // No collections + } as GroupDetailsView, + ]); + + mockApiService.getCiphersOrganization.mockResolvedValue({ + data: [] as CipherResponse[], + }); + + // Generate report view first + await service.generateMemberAccessReportView(mockOrganizationId); + + // Generate export + const result = await service.generateUserReportExportItems(mockOrganizationId); + + expect(result).toHaveLength(2); + + // Alice should have actual name with collection + const aliceRow = result.find((r) => r.email === "alice@example.com"); + expect(aliceRow?.name).toBe("Alice"); + expect(aliceRow?.group).toBe("Group A"); + expect(aliceRow?.collection).toBe("Collection A"); + + // Bob should have "(No Name)" and group without collection + const bobRow = result.find((r) => r.email === "bob@example.com"); + expect(bobRow?.name).toBe("(No Name)"); + expect(bobRow?.group).toBe("Group B"); + expect(bobRow?.collection).toBe("memberAccessReportNoCollection"); + }); }); }); }); diff --git a/bitwarden_license/bit-web/src/app/dirt/reports/member-access-report/services/member-access-report.service.ts b/bitwarden_license/bit-web/src/app/dirt/reports/member-access-report/services/member-access-report.service.ts index 0031b2083c6..99121041368 100644 --- a/bitwarden_license/bit-web/src/app/dirt/reports/member-access-report/services/member-access-report.service.ts +++ b/bitwarden_license/bit-web/src/app/dirt/reports/member-access-report/services/member-access-report.service.ts @@ -401,7 +401,7 @@ export class MemberAccessReportService { for (const access of directAccess) { exportItems.push({ email: member.email, - name: member.name, + name: member.name || "(No Name)", twoStepLogin: member.twoFactorEnabled ? this.i18nService.t("memberAccessReportTwoFactorEnabledTrue") : this.i18nService.t("memberAccessReportTwoFactorEnabledFalse"), @@ -421,10 +421,30 @@ export class MemberAccessReportService { const groupCollections = groupCollectionMap.get(groupId) || []; const groupName = groupNameMap.get(groupId) || "Unknown Group"; - for (const access of groupCollections) { + if (groupCollections.length > 0) { + // Group has collection access - create a row for each collection + for (const access of groupCollections) { + exportItems.push({ + email: member.email, + name: member.name || "(No Name)", + twoStepLogin: member.twoFactorEnabled + ? this.i18nService.t("memberAccessReportTwoFactorEnabledTrue") + : this.i18nService.t("memberAccessReportTwoFactorEnabledFalse"), + accountRecovery: member.resetPasswordEnrolled + ? this.i18nService.t("memberAccessReportAuthenticationEnabledTrue") + : this.i18nService.t("memberAccessReportAuthenticationEnabledFalse"), + group: groupName, + collection: + access.collectionName || this.i18nService.t("memberAccessReportNoCollection"), + collectionPermission: this.getPermissionText(access), + totalItems: String(collectionCipherCountMap.get(access.collectionId) || 0), + }); + } + } else { + // Group has no collection access - still show the group membership exportItems.push({ email: member.email, - name: member.name, + name: member.name || "(No Name)", twoStepLogin: member.twoFactorEnabled ? this.i18nService.t("memberAccessReportTwoFactorEnabledTrue") : this.i18nService.t("memberAccessReportTwoFactorEnabledFalse"), @@ -432,10 +452,9 @@ export class MemberAccessReportService { ? this.i18nService.t("memberAccessReportAuthenticationEnabledTrue") : this.i18nService.t("memberAccessReportAuthenticationEnabledFalse"), group: groupName, - collection: - access.collectionName || this.i18nService.t("memberAccessReportNoCollection"), - collectionPermission: this.getPermissionText(access), - totalItems: String(collectionCipherCountMap.get(access.collectionId) || 0), + collection: this.i18nService.t("memberAccessReportNoCollection"), + collectionPermission: this.i18nService.t("memberAccessReportNoCollectionPermission"), + totalItems: "0", }); } } @@ -444,7 +463,7 @@ export class MemberAccessReportService { if (directAccess.length === 0 && memberGroups.length === 0) { exportItems.push({ email: member.email, - name: member.name, + name: member.name || "(No Name)", twoStepLogin: member.twoFactorEnabled ? this.i18nService.t("memberAccessReportTwoFactorEnabledTrue") : this.i18nService.t("memberAccessReportTwoFactorEnabledFalse"),