[PM-19172] Passes UserId through change email component and adds tests (#13686)

* add tests and pass userid

* add tests for getOrDeriveMasterKey

* remove extra coalesce for token

apps/web/src/app/auth/settings/account/change-email.component.spec.ts

@@ -0,0 +1,197 @@
+import { ComponentFixture, TestBed } from "@angular/core/testing";
+import { FormBuilder, ReactiveFormsModule } from "@angular/forms";
+import mock, { MockProxy } from "jest-mock-extended/lib/Mock";
+import { firstValueFrom, of } from "rxjs";
+
+import { ApiService } from "@bitwarden/common/abstractions/api.service";
+import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
+import { TwoFactorProviderType } from "@bitwarden/common/auth/enums/two-factor-provider-type";
+import { TwoFactorProviderResponse } from "@bitwarden/common/auth/models/response/two-factor-provider.response";
+import { ListResponse } from "@bitwarden/common/models/response/list.response";
+import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
+import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
+import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/spec";
+import { UserId } from "@bitwarden/common/types/guid";
+import { ToastService } from "@bitwarden/components";
+import { KdfConfigService, KeyService } from "@bitwarden/key-management";
+import { ChangeEmailComponent } from "@bitwarden/web-vault/app/auth/settings/account/change-email.component";
+import { SharedModule } from "@bitwarden/web-vault/app/shared";
+
+describe("ChangeEmailComponent", () => {
+  let component: ChangeEmailComponent;
+  let fixture: ComponentFixture<ChangeEmailComponent>;
+
+  let apiService: MockProxy<ApiService>;
+  let accountService: FakeAccountService;
+  let keyService: MockProxy<KeyService>;
+  let kdfConfigService: MockProxy<KdfConfigService>;
+
+  beforeEach(async () => {
+    apiService = mock<ApiService>();
+    keyService = mock<KeyService>();
+    kdfConfigService = mock<KdfConfigService>();
+    accountService = mockAccountServiceWith("UserId" as UserId);
+
+    await TestBed.configureTestingModule({
+      declarations: [ChangeEmailComponent],
+      imports: [ReactiveFormsModule, SharedModule],
+      providers: [
+        { provide: AccountService, useValue: accountService },
+        { provide: ApiService, useValue: apiService },
+        { provide: I18nService, useValue: { t: (key: string) => key } },
+        { provide: KeyService, useValue: keyService },
+        { provide: MessagingService, useValue: mock<MessagingService>() },
+        { provide: KdfConfigService, useValue: kdfConfigService },
+        { provide: ToastService, useValue: mock<ToastService>() },
+        { provide: FormBuilder, useClass: FormBuilder },
+      ],
+    }).compileComponents();
+
+    fixture = TestBed.createComponent(ChangeEmailComponent);
+    component = fixture.componentInstance;
+    fixture.detectChanges();
+  });
+
+  it("creates component", () => {
+    expect(component).toBeTruthy();
+  });
+
+  describe("ngOnInit", () => {
+    beforeEach(() => {
+      apiService.getTwoFactorProviders.mockResolvedValue({
+        data: [{ type: TwoFactorProviderType.Email, enabled: true } as TwoFactorProviderResponse],
+      } as ListResponse<TwoFactorProviderResponse>);
+    });
+
+    it("initializes userId", async () => {
+      await component.ngOnInit();
+      expect(component.userId).toBe("UserId");
+    });
+
+    it("errors if there is no active user", async () => {
+      // clear active account
+      await firstValueFrom(accountService.activeAccount$);
+      accountService.activeAccountSubject.next(null);
+
+      await expect(() => component.ngOnInit()).rejects.toThrow("Null or undefined account");
+    });
+
+    it("initializes showTwoFactorEmailWarning", async () => {
+      await component.ngOnInit();
+      expect(component.showTwoFactorEmailWarning).toBe(true);
+    });
+  });
+
+  describe("submit", () => {
+    beforeEach(() => {
+      component.formGroup.controls.step1.setValue({
+        masterPassword: "password",
+        newEmail: "test@example.com",
+      });
+
+      keyService.getOrDeriveMasterKey
+        .calledWith("password", "UserId")
+        .mockResolvedValue("getOrDeriveMasterKey" as any);
+      keyService.hashMasterKey
+        .calledWith("password", "getOrDeriveMasterKey" as any)
+        .mockResolvedValue("existingHash");
+    });
+
+    it("throws if userId is null on submit", async () => {
+      component.userId = undefined;
+
+      await expect(component.submit()).rejects.toThrow("Can't find user");
+    });
+
+    describe("step 1", () => {
+      it("does not submit if step 1 is invalid", async () => {
+        component.formGroup.controls.step1.setValue({
+          masterPassword: "",
+          newEmail: "",
+        });
+
+        await component.submit();
+
+        expect(apiService.postEmailToken).not.toHaveBeenCalled();
+      });
+
+      it("sends email token in step 1 if tokenSent is false", async () => {
+        await component.submit();
+
+        expect(apiService.postEmailToken).toHaveBeenCalledWith({
+          newEmail: "test@example.com",
+          masterPasswordHash: "existingHash",
+        });
+        // should activate step 2
+        expect(component.tokenSent).toBe(true);
+        expect(component.formGroup.controls.step1.disabled).toBe(true);
+        expect(component.formGroup.controls.token.enabled).toBe(true);
+      });
+    });
+
+    describe("step 2", () => {
+      beforeEach(() => {
+        component.tokenSent = true;
+        component.formGroup.controls.step1.disable();
+        component.formGroup.controls.token.enable();
+        component.formGroup.controls.token.setValue("token");
+
+        kdfConfigService.getKdfConfig$
+          .calledWith("UserId" as any)
+          .mockReturnValue(of("kdfConfig" as any));
+        keyService.userKey$.calledWith("UserId" as any).mockReturnValue(of("userKey" as any));
+
+        keyService.makeMasterKey
+          .calledWith("password", "test@example.com", "kdfConfig" as any)
+          .mockResolvedValue("newMasterKey" as any);
+        keyService.hashMasterKey
+          .calledWith("password", "newMasterKey" as any)
+          .mockResolvedValue("newMasterKeyHash");
+
+        // Important: make sure this is called with new master key, not existing
+        keyService.encryptUserKeyWithMasterKey
+          .calledWith("newMasterKey" as any, "userKey" as any)
+          .mockResolvedValue(["userKey" as any, { encryptedString: "newEncryptedUserKey" } as any]);
+      });
+
+      it("does not post email if token is missing on submit", async () => {
+        component.formGroup.controls.token.setValue("");
+
+        await component.submit();
+
+        expect(apiService.postEmail).not.toHaveBeenCalled();
+      });
+
+      it("throws if kdfConfig is missing on submit", async () => {
+        kdfConfigService.getKdfConfig$.mockReturnValue(of(null));
+
+        await expect(component.submit()).rejects.toThrow("Missing kdf config");
+      });
+
+      it("throws if userKey can't be found", async () => {
+        keyService.userKey$.mockReturnValue(of(null));
+
+        await expect(component.submit()).rejects.toThrow("Can't find UserKey");
+      });
+
+      it("throws if encryptedUserKey is missing", async () => {
+        keyService.encryptUserKeyWithMasterKey.mockResolvedValue(["userKey" as any, null as any]);
+
+        await expect(component.submit()).rejects.toThrow("Missing Encrypted User Key");
+      });
+
+      it("submits if step 2 is valid", async () => {
+        await component.submit();
+
+        // validate that hashes are correct
+        expect(apiService.postEmail).toHaveBeenCalledWith({
+          masterPasswordHash: "existingHash",
+          newMasterPasswordHash: "newMasterKeyHash",
+          token: "token",
+          newEmail: "test@example.com",
+          key: "newEncryptedUserKey",
+        });
+      });
+    });
+  });
+});

apps/web/src/app/auth/settings/account/change-email.component.ts

@@ -1,17 +1,16 @@
-// FIXME: Update this file to be type safe and remove this and next line
-// @ts-strict-ignore
 import { Component, OnInit } from "@angular/core";
 import { FormBuilder, Validators } from "@angular/forms";
+import { firstValueFrom } from "rxjs";
 
 import { ApiService } from "@bitwarden/common/abstractions/api.service";
+import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
 import { TwoFactorProviderType } from "@bitwarden/common/auth/enums/two-factor-provider-type";
 import { EmailTokenRequest } from "@bitwarden/common/auth/models/request/email-token.request";
 import { EmailRequest } from "@bitwarden/common/auth/models/request/email.request";
+import { getUserId } from "@bitwarden/common/auth/services/account.service";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
-import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
 import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
-import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
+import { UserId } from "@bitwarden/common/types/guid";
-import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
 import { ToastService } from "@bitwarden/components";
 import { KdfConfigService, KeyService } from "@bitwarden/key-management";
 
@@ -22,8 +21,9 @@ import { KdfConfigService, KeyService } from "@bitwarden/key-management";
 export class ChangeEmailComponent implements OnInit {
   tokenSent = false;
   showTwoFactorEmailWarning = false;
+  userId: UserId | undefined;
 
-  protected formGroup = this.formBuilder.group({
+  formGroup = this.formBuilder.group({
     step1: this.formBuilder.group({
       masterPassword: ["", [Validators.required]],
       newEmail: ["", [Validators.required, Validators.email]],
@@ -32,26 +32,30 @@ export class ChangeEmailComponent implements OnInit {
   });
 
   constructor(
+    private accountService: AccountService,
     private apiService: ApiService,
     private i18nService: I18nService,
-    private platformUtilsService: PlatformUtilsService,
     private keyService: KeyService,
     private messagingService: MessagingService,
-    private logService: LogService,
-    private stateService: StateService,
     private formBuilder: FormBuilder,
     private kdfConfigService: KdfConfigService,
     private toastService: ToastService,
   ) {}
 
   async ngOnInit() {
+    this.userId = await firstValueFrom(getUserId(this.accountService.activeAccount$));
+
     const twoFactorProviders = await this.apiService.getTwoFactorProviders();
     this.showTwoFactorEmailWarning = twoFactorProviders.data.some(
       (p) => p.type === TwoFactorProviderType.Email && p.enabled,
     );
   }
 
-  protected submit = async () => {
+  submit = async () => {
+    if (this.userId == null) {
+      throw new Error("Can't find user");
+    }
+
     // This form has multiple steps, so we need to mark all the groups as touched.
     this.formGroup.controls.step1.markAllAsTouched();
 
@@ -65,37 +69,54 @@ export class ChangeEmailComponent implements OnInit {
     }
 
     const step1Value = this.formGroup.controls.step1.value;
-    const newEmail = step1Value.newEmail.trim().toLowerCase();
+    const newEmail = step1Value.newEmail?.trim().toLowerCase();
+    const masterPassword = step1Value.masterPassword;
+
+    if (newEmail == null || masterPassword == null) {
+      throw new Error("Missing email or password");
+    }
+
+    const existingHash = await this.keyService.hashMasterKey(
+      masterPassword,
+      await this.keyService.getOrDeriveMasterKey(masterPassword, this.userId),
+    );
 
     if (!this.tokenSent) {
       const request = new EmailTokenRequest();
       request.newEmail = newEmail;
-      request.masterPasswordHash = await this.keyService.hashMasterKey(
+      request.masterPasswordHash = existingHash;
-        step1Value.masterPassword,
-        await this.keyService.getOrDeriveMasterKey(step1Value.masterPassword),
-      );
       await this.apiService.postEmailToken(request);
       this.activateStep2();
     } else {
+      const token = this.formGroup.value.token;
+      if (token == null) {
+        throw new Error("Missing token");
+      }
       const request = new EmailRequest();
-      request.token = this.formGroup.value.token;
+      request.token = token;
       request.newEmail = newEmail;
-      request.masterPasswordHash = await this.keyService.hashMasterKey(
+      request.masterPasswordHash = existingHash;
-        step1Value.masterPassword,
+
-        await this.keyService.getOrDeriveMasterKey(step1Value.masterPassword),
+      const kdfConfig = await firstValueFrom(this.kdfConfigService.getKdfConfig$(this.userId));
-      );
+      if (kdfConfig == null) {
-      const kdfConfig = await this.kdfConfigService.getKdfConfig();
+        throw new Error("Missing kdf config");
-      const newMasterKey = await this.keyService.makeMasterKey(
+      }
-        step1Value.masterPassword,
+      const newMasterKey = await this.keyService.makeMasterKey(masterPassword, newEmail, kdfConfig);
-        newEmail,
-        kdfConfig,
-      );
       request.newMasterPasswordHash = await this.keyService.hashMasterKey(
-        step1Value.masterPassword,
+        masterPassword,
         newMasterKey,
       );
-      const newUserKey = await this.keyService.encryptUserKeyWithMasterKey(newMasterKey);
+
-      request.key = newUserKey[1].encryptedString;
+      const userKey = await firstValueFrom(this.keyService.userKey$(this.userId));
+      if (userKey == null) {
+        throw new Error("Can't find UserKey");
+      }
+      const newUserKey = await this.keyService.encryptUserKeyWithMasterKey(newMasterKey, userKey);
+      const encryptedUserKey = newUserKey[1]?.encryptedString;
+      if (encryptedUserKey == null) {
+        throw new Error("Missing Encrypted User Key");
+      }
+      request.key = encryptedUserKey;
 
       await this.apiService.postEmail(request);
       this.reset();

libs/key-management/src/key.service.spec.ts

@@ -736,6 +736,52 @@ describe("keyService", () => {
     });
   });
 
+  describe("getOrDeriveMasterKey", () => {
+    it("returns the master key if it is already available", async () => {
+      const getMasterKey = jest
+        .spyOn(masterPasswordService, "masterKey$")
+        .mockReturnValue(of("masterKey" as any));
+
+      const result = await keyService.getOrDeriveMasterKey("password", mockUserId);
+
+      expect(getMasterKey).toHaveBeenCalledWith(mockUserId);
+      expect(result).toEqual("masterKey");
+    });
+
+    it("derives the master key if it is not available", async () => {
+      const getMasterKey = jest
+        .spyOn(masterPasswordService, "masterKey$")
+        .mockReturnValue(of(null as any));
+
+      const deriveKeyFromPassword = jest
+        .spyOn(keyGenerationService, "deriveKeyFromPassword")
+        .mockResolvedValue("mockMasterKey" as any);
+
+      kdfConfigService.getKdfConfig$.mockReturnValue(of("mockKdfConfig" as any));
+
+      const result = await keyService.getOrDeriveMasterKey("password", mockUserId);
+
+      expect(getMasterKey).toHaveBeenCalledWith(mockUserId);
+      expect(deriveKeyFromPassword).toHaveBeenCalledWith("password", "email", "mockKdfConfig");
+      expect(result).toEqual("mockMasterKey");
+    });
+
+    it("throws an error if no user is found", async () => {
+      accountService.activeAccountSubject.next(null);
+
+      await expect(keyService.getOrDeriveMasterKey("password")).rejects.toThrow("No user found");
+    });
+
+    it("throws an error if no kdf config is found", async () => {
+      jest.spyOn(masterPasswordService, "masterKey$").mockReturnValue(of(null as any));
+      kdfConfigService.getKdfConfig$.mockReturnValue(of(null));
+
+      await expect(keyService.getOrDeriveMasterKey("password", mockUserId)).rejects.toThrow(
+        "No kdf found for user",
+      );
+    });
+  });
+
   describe("compareKeyHash", () => {
     type TestCase = {
       masterKey: MasterKey;

libs/key-management/src/key.service.ts

@@ -287,10 +287,15 @@ export class DefaultKeyService implements KeyServiceAbstraction {
       ),
     );
     const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(resolvedUserId));
-    return (
+    if (masterKey != null) {
-      masterKey ||
+      return masterKey;
-      (await this.makeMasterKey(password, email, await this.kdfConfigService.getKdfConfig()))
+    }
-    );
+
+    const kdf = await firstValueFrom(this.kdfConfigService.getKdfConfig$(resolvedUserId));
+    if (kdf == null) {
+      throw new Error("No kdf found for user");
+    }
+    return await this.makeMasterKey(password, email, kdf);
   }
 
   /**