[PM-14813] - Implement server API to user member cipher details instead of mock data (#11978)

* use cipher details data * fix failing tests * fix failing tests
2024-11-14 10:26:19 +01:00 · 2024-11-13 08:37:29 -08:00 · 2024-11-13 08:37:29 -08:00 · 0b11596f08
commit 0b11596f08
parent 379efb1326
9 changed files with 140 additions and 109 deletions
--- a/apps/web/src/app/tools/risk-insights/password-health-members-uri.component.spec.ts
+++ b/apps/web/src/app/tools/risk-insights/password-health-members-uri.component.spec.ts
@ -4,7 +4,11 @@ import { mock, MockProxy } from "jest-mock-extended";
 import { of } from "rxjs";

 // eslint-disable-next-line no-restricted-imports
-import { PasswordHealthService } from "@bitwarden/bit-common/tools/reports/risk-insights";
+import {
+  MemberCipherDetailsApiService,
+  PasswordHealthService,
+} from "@bitwarden/bit-common/tools/reports/risk-insights";
+import { ApiService } from "@bitwarden/common/abstractions/api.service";
 import { AuditService } from "@bitwarden/common/abstractions/audit.service";
 import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
@ -46,6 +50,14 @@ describe("PasswordHealthMembersUriComponent", () => {
            url: of([]),
          },
        },
+        {
+          provide: MemberCipherDetailsApiService,
+          useValue: mock<MemberCipherDetailsApiService>(),
+        },
+        {
+          provide: ApiService,
+          useValue: mock<ApiService>(),
+        },
      ],
    }).compileComponents();
  });
--- a/apps/web/src/app/tools/risk-insights/password-health-members-uri.component.ts
+++ b/apps/web/src/app/tools/risk-insights/password-health-members-uri.component.ts
@ -6,7 +6,10 @@ import { map } from "rxjs";

 import { JslibModule } from "@bitwarden/angular/jslib.module";
 // eslint-disable-next-line no-restricted-imports
-import { PasswordHealthService } from "@bitwarden/bit-common/tools/reports/risk-insights";
+import {
+  MemberCipherDetailsApiService,
+  PasswordHealthService,
+} from "@bitwarden/bit-common/tools/reports/risk-insights";
 import { AuditService } from "@bitwarden/common/abstractions/audit.service";
 import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
 import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
@ -25,8 +28,6 @@ import {
 // eslint-disable-next-line no-restricted-imports
 import { HeaderModule } from "../../layouts/header/header.module";
 // eslint-disable-next-line no-restricted-imports
-import { OrganizationBadgeModule } from "../../vault/individual-vault/organization-badge/organization-badge.module";
-// eslint-disable-next-line no-restricted-imports
 import { PipesModule } from "../../vault/individual-vault/pipes/pipes.module";

@Component({
@ -35,7 +36,6 @@ import { PipesModule } from "../../vault/individual-vault/pipes/pipes.module";
  templateUrl: "password-health-members-uri.component.html",
  imports: [
    BadgeModule,
-    OrganizationBadgeModule,
    CommonModule,
    ContainerComponent,
    PipesModule,
@ -43,7 +43,7 @@ import { PipesModule } from "../../vault/individual-vault/pipes/pipes.module";
    HeaderModule,
    TableModule,
  ],
-  providers: [PasswordHealthService],
+  providers: [PasswordHealthService, MemberCipherDetailsApiService],
 })
 export class PasswordHealthMembersURIComponent implements OnInit {
  passwordStrengthMap = new Map<string, [string, BadgeVariant]>();
@ -74,6 +74,7 @@ export class PasswordHealthMembersURIComponent implements OnInit {
    protected auditService: AuditService,
    protected i18nService: I18nService,
    protected activatedRoute: ActivatedRoute,
+    protected memberCipherDetailsApiService: MemberCipherDetailsApiService,
  ) {}

  ngOnInit() {
@ -93,6 +94,7 @@ export class PasswordHealthMembersURIComponent implements OnInit {
      this.passwordStrengthService,
      this.auditService,
      this.cipherService,
+      this.memberCipherDetailsApiService,
      organizationId,
    );

--- a/apps/web/src/app/tools/risk-insights/password-health-members.component.ts
+++ b/apps/web/src/app/tools/risk-insights/password-health-members.component.ts
@ -5,7 +5,10 @@ import { ActivatedRoute } from "@angular/router";
 import { debounceTime, map } from "rxjs";

 // eslint-disable-next-line no-restricted-imports
-import { PasswordHealthService } from "@bitwarden/bit-common/tools/reports/risk-insights";
+import {
+  MemberCipherDetailsApiService,
+  PasswordHealthService,
+} from "@bitwarden/bit-common/tools/reports/risk-insights";
 import { AuditService } from "@bitwarden/common/abstractions/audit.service";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength";
@ -18,12 +21,10 @@ import {
  TableModule,
  ToastService,
 } from "@bitwarden/components";
-import { CardComponent } from "@bitwarden/tools-card";

 import { HeaderModule } from "../../layouts/header/header.module";
 // eslint-disable-next-line no-restricted-imports
 import { SharedModule } from "../../shared";
-import { OrganizationBadgeModule } from "../../vault/individual-vault/organization-badge/organization-badge.module";
 // eslint-disable-next-line no-restricted-imports
 import { PipesModule } from "../../vault/individual-vault/pipes/pipes.module";

@ -31,17 +32,8 @@ import { PipesModule } from "../../vault/individual-vault/pipes/pipes.module";
  standalone: true,
  selector: "tools-password-health-members",
  templateUrl: "password-health-members.component.html",
-  imports: [
-    CardComponent,
-    OrganizationBadgeModule,
-    PipesModule,
-    HeaderModule,
-    SearchModule,
-    FormsModule,
-    SharedModule,
-    TableModule,
-  ],
-  providers: [PasswordHealthService],
+  imports: [PipesModule, HeaderModule, SearchModule, FormsModule, SharedModule, TableModule],
+  providers: [PasswordHealthService, MemberCipherDetailsApiService],
 })
 export class PasswordHealthMembersComponent implements OnInit {
  passwordStrengthMap = new Map<string, [string, BadgeVariant]>();
@ -69,6 +61,7 @@ export class PasswordHealthMembersComponent implements OnInit {
    protected i18nService: I18nService,
    protected activatedRoute: ActivatedRoute,
    protected toastService: ToastService,
+    protected memberCipherDetailsApiService: MemberCipherDetailsApiService,
  ) {
    this.searchControl.valueChanges
      .pipe(debounceTime(200), takeUntilDestroyed())
@ -92,6 +85,7 @@ export class PasswordHealthMembersComponent implements OnInit {
      this.passwordStrengthService,
      this.auditService,
      this.cipherService,
+      this.memberCipherDetailsApiService,
      organizationId,
    );

--- a/apps/web/src/app/tools/risk-insights/password-health.component.spec.ts
+++ b/apps/web/src/app/tools/risk-insights/password-health.component.spec.ts
@ -4,7 +4,11 @@ import { mock } from "jest-mock-extended";
 import { of } from "rxjs";

 // eslint-disable-next-line no-restricted-imports
-import { PasswordHealthService } from "@bitwarden/bit-common/tools/reports/risk-insights";
+import {
+  MemberCipherDetailsApiService,
+  PasswordHealthService,
+} from "@bitwarden/bit-common/tools/reports/risk-insights";
+import { ApiService } from "@bitwarden/common/abstractions/api.service";
 import { AuditService } from "@bitwarden/common/abstractions/audit.service";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength";
@ -30,6 +34,8 @@ describe("PasswordHealthComponent", () => {
        { provide: CipherService, useValue: mock<CipherService>() },
        { provide: I18nService, useValue: mock<I18nService>() },
        { provide: AuditService, useValue: mock<AuditService>() },
+        { provide: ApiService, useValue: mock<ApiService>() },
+        { provide: MemberCipherDetailsApiService, useValue: mock<MemberCipherDetailsApiService>() },
        {
          provide: PasswordStrengthServiceAbstraction,
          useValue: mock<PasswordStrengthServiceAbstraction>(),
--- a/apps/web/src/app/tools/risk-insights/password-health.component.ts
+++ b/apps/web/src/app/tools/risk-insights/password-health.component.ts
@ -6,7 +6,10 @@ import { map } from "rxjs";

 import { JslibModule } from "@bitwarden/angular/jslib.module";
 // eslint-disable-next-line no-restricted-imports
-import { PasswordHealthService } from "@bitwarden/bit-common/tools/reports/risk-insights";
+import {
+  MemberCipherDetailsApiService,
+  PasswordHealthService,
+} from "@bitwarden/bit-common/tools/reports/risk-insights";
 import { AuditService } from "@bitwarden/common/abstractions/audit.service";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength";
@ -41,7 +44,7 @@ import { PipesModule } from "../../vault/individual-vault/pipes/pipes.module";
    HeaderModule,
    TableModule,
  ],
-  providers: [PasswordHealthService],
+  providers: [PasswordHealthService, MemberCipherDetailsApiService],
 })
 export class PasswordHealthComponent implements OnInit {
  passwordStrengthMap = new Map<string, [string, BadgeVariant]>();
@ -62,6 +65,7 @@ export class PasswordHealthComponent implements OnInit {
    protected auditService: AuditService,
    protected i18nService: I18nService,
    protected activatedRoute: ActivatedRoute,
+    protected memberCipherDetailsApiService: MemberCipherDetailsApiService,
  ) {}

  ngOnInit() {
@ -81,6 +85,7 @@ export class PasswordHealthComponent implements OnInit {
      this.passwordStrengthService,
      this.auditService,
      this.cipherService,
+      this.memberCipherDetailsApiService,
      organizationId,
    );

--- a/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/member-cipher-details-api.service.spec.ts
+++ b/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/member-cipher-details-api.service.spec.ts
@ -4,74 +4,72 @@ import { ApiService } from "@bitwarden/common/abstractions/api.service";

 import { MemberCipherDetailsApiService } from "./member-cipher-details-api.service";

-const mockMemberCipherDetails: any = {
-  data: [
-    {
-      userName: "David Brent",
-      email: "david.brent@wernhamhogg.uk",
-      usesKeyConnector: true,
-      cipherIds: [
-        "cbea34a8-bde4-46ad-9d19-b05001228ab1",
-        "cbea34a8-bde4-46ad-9d19-b05001228ab2",
-        "cbea34a8-bde4-46ad-9d19-b05001228xy4",
-        "cbea34a8-bde4-46ad-9d19-b05001227nm5",
-      ],
-    },
-    {
-      userName: "Tim Canterbury",
-      email: "tim.canterbury@wernhamhogg.uk",
-      usesKeyConnector: false,
-      cipherIds: [
-        "cbea34a8-bde4-46ad-9d19-b05001228ab2",
-        "cbea34a8-bde4-46ad-9d19-b05001228cd3",
-        "cbea34a8-bde4-46ad-9d19-b05001228xy4",
-        "cbea34a8-bde4-46ad-9d19-b05001227nm5",
-      ],
-    },
-    {
-      userName: "Gareth Keenan",
-      email: "gareth.keenan@wernhamhogg.uk",
-      usesKeyConnector: true,
-      cipherIds: [
-        "cbea34a8-bde4-46ad-9d19-b05001228cd3",
-        "cbea34a8-bde4-46ad-9d19-b05001228xy4",
-        "cbea34a8-bde4-46ad-9d19-b05001227nm5",
-        "cbea34a8-bde4-46ad-9d19-b05001227nm7",
-      ],
-    },
-    {
-      userName: "Dawn Tinsley",
-      email: "dawn.tinsley@wernhamhogg.uk",
-      usesKeyConnector: true,
-      cipherIds: [
-        "cbea34a8-bde4-46ad-9d19-b05001228ab2",
-        "cbea34a8-bde4-46ad-9d19-b05001228cd3",
-        "cbea34a8-bde4-46ad-9d19-b05001228xy4",
-      ],
-    },
-    {
-      userName: "Keith Bishop",
-      email: "keith.bishop@wernhamhogg.uk",
-      usesKeyConnector: false,
-      cipherIds: [
-        "cbea34a8-bde4-46ad-9d19-b05001228ab1",
-        "cbea34a8-bde4-46ad-9d19-b05001228cd3",
-        "cbea34a8-bde4-46ad-9d19-b05001228xy4",
-        "cbea34a8-bde4-46ad-9d19-b05001227nm5",
-      ],
-    },
-    {
-      userName: "Chris Finch",
-      email: "chris.finch@wernhamhogg.uk",
-      usesKeyConnector: true,
-      cipherIds: [
-        "cbea34a8-bde4-46ad-9d19-b05001228ab2",
-        "cbea34a8-bde4-46ad-9d19-b05001228cd3",
-        "cbea34a8-bde4-46ad-9d19-b05001228xy4",
-      ],
-    },
-  ],
-};
+export const mockMemberCipherDetails: any = [
+  {
+    userName: "David Brent",
+    email: "david.brent@wernhamhogg.uk",
+    usesKeyConnector: true,
+    cipherIds: [
+      "cbea34a8-bde4-46ad-9d19-b05001228ab1",
+      "cbea34a8-bde4-46ad-9d19-b05001228ab2",
+      "cbea34a8-bde4-46ad-9d19-b05001228xy4",
+      "cbea34a8-bde4-46ad-9d19-b05001227nm5",
+    ],
+  },
+  {
+    userName: "Tim Canterbury",
+    email: "tim.canterbury@wernhamhogg.uk",
+    usesKeyConnector: false,
+    cipherIds: [
+      "cbea34a8-bde4-46ad-9d19-b05001228ab2",
+      "cbea34a8-bde4-46ad-9d19-b05001228cd3",
+      "cbea34a8-bde4-46ad-9d19-b05001228xy4",
+      "cbea34a8-bde4-46ad-9d19-b05001227nm5",
+    ],
+  },
+  {
+    userName: "Gareth Keenan",
+    email: "gareth.keenan@wernhamhogg.uk",
+    usesKeyConnector: true,
+    cipherIds: [
+      "cbea34a8-bde4-46ad-9d19-b05001228cd3",
+      "cbea34a8-bde4-46ad-9d19-b05001228xy4",
+      "cbea34a8-bde4-46ad-9d19-b05001227nm5",
+      "cbea34a8-bde4-46ad-9d19-b05001227nm7",
+    ],
+  },
+  {
+    userName: "Dawn Tinsley",
+    email: "dawn.tinsley@wernhamhogg.uk",
+    usesKeyConnector: true,
+    cipherIds: [
+      "cbea34a8-bde4-46ad-9d19-b05001228ab2",
+      "cbea34a8-bde4-46ad-9d19-b05001228cd3",
+      "cbea34a8-bde4-46ad-9d19-b05001228xy4",
+    ],
+  },
+  {
+    userName: "Keith Bishop",
+    email: "keith.bishop@wernhamhogg.uk",
+    usesKeyConnector: false,
+    cipherIds: [
+      "cbea34a8-bde4-46ad-9d19-b05001228ab1",
+      "cbea34a8-bde4-46ad-9d19-b05001228cd3",
+      "cbea34a8-bde4-46ad-9d19-b05001228xy4",
+      "cbea34a8-bde4-46ad-9d19-b05001227nm5",
+    ],
+  },
+  {
+    userName: "Chris Finch",
+    email: "chris.finch@wernhamhogg.uk",
+    usesKeyConnector: true,
+    cipherIds: [
+      "cbea34a8-bde4-46ad-9d19-b05001228ab2",
+      "cbea34a8-bde4-46ad-9d19-b05001228cd3",
+      "cbea34a8-bde4-46ad-9d19-b05001228xy4",
+    ],
+  },
+];

 describe("Member Cipher Details API Service", () => {
  let memberCipherDetailsApiService: MemberCipherDetailsApiService;
--- a/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/member-cipher-details-api.service.ts
+++ b/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/member-cipher-details-api.service.ts
@ -1,8 +1,10 @@
+import { Injectable } from "@angular/core";
+
 import { ApiService } from "@bitwarden/common/abstractions/api.service";
-import { ListResponse } from "@bitwarden/common/models/response/list.response";

 import { MemberCipherDetailsResponse } from "../response/member-cipher-details.response";

+@Injectable()
 export class MemberCipherDetailsApiService {
  constructor(private apiService: ApiService) {}

@ -21,7 +23,6 @@ export class MemberCipherDetailsApiService {
      true,
    );

-    const listResponse = new ListResponse(response, MemberCipherDetailsResponse);
-    return listResponse.data.map((r) => new MemberCipherDetailsResponse(r));
+    return response;
  }
 }
--- a/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/password-health.service.spec.ts
+++ b/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/password-health.service.spec.ts
@ -3,15 +3,17 @@ import { TestBed } from "@angular/core/testing";
 import { AuditService } from "@bitwarden/common/abstractions/audit.service";
 import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength";
 import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
-import { CipherData } from "@bitwarden/common/vault/models/data/cipher.data";
 import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";

 import { mockCiphers } from "./ciphers.mock";
+import { MemberCipherDetailsApiService } from "./member-cipher-details-api.service";
+import { mockMemberCipherDetails } from "./member-cipher-details-api.service.spec";
 import { PasswordHealthService } from "./password-health.service";

 describe("PasswordHealthService", () => {
  let service: PasswordHealthService;
  let cipherService: CipherService;
+  let memberCipherDetailsApiService: MemberCipherDetailsApiService;

  beforeEach(() => {
    TestBed.configureTestingModule({
@ -35,7 +37,13 @@ describe("PasswordHealthService", () => {
        {
          provide: CipherService,
          useValue: {
-            getAllFromApiForOrganization: jest.fn().mockResolvedValue(CipherData),
+            getAllFromApiForOrganization: jest.fn().mockResolvedValue(mockCiphers),
+          },
+        },
+        {
+          provide: MemberCipherDetailsApiService,
+          useValue: {
+            getMemberCipherDetails: jest.fn().mockResolvedValue(mockMemberCipherDetails),
          },
        },
        { provide: "organizationId", useValue: "org1" },
@ -44,6 +52,7 @@ describe("PasswordHealthService", () => {

    service = TestBed.inject(PasswordHealthService);
    cipherService = TestBed.inject(CipherService);
+    memberCipherDetailsApiService = TestBed.inject(MemberCipherDetailsApiService);
  });

  it("should be created", () => {
@ -68,6 +77,10 @@ describe("PasswordHealthService", () => {
      expect(cipherService.getAllFromApiForOrganization).toHaveBeenCalledWith("org1");
    });

+    it("should fetch member cipher details", () => {
+      expect(memberCipherDetailsApiService.getMemberCipherDetails).toHaveBeenCalledWith("org1");
+    });
+
    it("should populate reportCiphers with ciphers that have issues", () => {
      expect(service.reportCiphers.length).toBeGreaterThan(0);
    });
@ -99,12 +112,12 @@ describe("PasswordHealthService", () => {

    it("should calculate total members per cipher", () => {
      expect(service.totalMembersMap.size).toBeGreaterThan(0);
-      expect(service.totalMembersMap.get("cbea34a8-bde4-46ad-9d19-b05001228ab1")).toBe(3);
-      expect(service.totalMembersMap.get("cbea34a8-bde4-46ad-9d19-b05001228ab2")).toBe(5);
-      expect(service.totalMembersMap.get("cbea34a8-bde4-46ad-9d19-b05001228cd3")).toBe(6);
+      expect(service.totalMembersMap.get("cbea34a8-bde4-46ad-9d19-b05001228ab1")).toBe(2);
+      expect(service.totalMembersMap.get("cbea34a8-bde4-46ad-9d19-b05001228ab2")).toBe(4);
+      expect(service.totalMembersMap.get("cbea34a8-bde4-46ad-9d19-b05001228cd3")).toBe(5);
      expect(service.totalMembersMap.get("cbea34a8-bde4-46ad-9d19-b05001227nm5")).toBe(4);
      expect(service.totalMembersMap.get("cbea34a8-bde4-46ad-9d19-b05001227nm7")).toBe(1);
-      expect(service.totalMembersMap.get("cbea34a8-bde4-46ad-9d19-b05001228xy4")).toBe(7);
+      expect(service.totalMembersMap.get("cbea34a8-bde4-46ad-9d19-b05001228xy4")).toBe(6);
    });
  });

--- a/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/password-health.service.ts
+++ b/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/password-health.service.ts
@ -1,9 +1,5 @@
 import { Inject, Injectable } from "@angular/core";

-// eslint-disable-next-line no-restricted-imports
-import { mockCiphers } from "@bitwarden/bit-common/tools/reports/risk-insights/services/ciphers.mock";
-// eslint-disable-next-line no-restricted-imports
-import { mockMemberCipherDetailsResponse } from "@bitwarden/bit-common/tools/reports/risk-insights/services/member-cipher-details-response.mock";
 import { AuditService } from "@bitwarden/common/abstractions/audit.service";
 import { Utils } from "@bitwarden/common/platform/misc/utils";
 import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength";
@ -12,6 +8,8 @@ import { CipherType } from "@bitwarden/common/vault/enums";
 import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
 import { BadgeVariant } from "@bitwarden/components";

+import { MemberCipherDetailsApiService } from "./member-cipher-details-api.service";
+
@Injectable()
 export class PasswordHealthService {
  reportCiphers: CipherView[] = [];
@ -30,21 +28,23 @@ export class PasswordHealthService {
    private passwordStrengthService: PasswordStrengthServiceAbstraction,
    private auditService: AuditService,
    private cipherService: CipherService,
+    private memberCipherDetailsApiService: MemberCipherDetailsApiService,
    @Inject("organizationId") private organizationId: string,
  ) {}

  async generateReport() {
-    let allCiphers = await this.cipherService.getAllFromApiForOrganization(this.organizationId);
-    // TODO remove when actual user member data is available
-    allCiphers = mockCiphers;
+    const allCiphers = await this.cipherService.getAllFromApiForOrganization(this.organizationId);
    allCiphers.forEach(async (cipher) => {
      this.findWeakPassword(cipher);
      this.findReusedPassword(cipher);
      await this.findExposedPassword(cipher);
    });

-    // TODO - fetch actual user member when data is available
-    mockMemberCipherDetailsResponse.data.forEach((user) => {
+    const memberCipherDetails = await this.memberCipherDetailsApiService.getMemberCipherDetails(
+      this.organizationId,
+    );
+
+    memberCipherDetails.forEach((user) => {
      user.cipherIds.forEach((cipherId: string) => {
        if (this.totalMembersMap.has(cipherId)) {
          this.totalMembersMap.set(cipherId, (this.totalMembersMap.get(cipherId) || 0) + 1);