update changePassword(), submitOld(), DI, and variable naming

2026-02-09 13:10:17 +00:00 · 2025-04-01 23:06:13 -07:00
parent 60066830a0
commit 198aa7d9bd
12 changed files with 166 additions and 118 deletions
--- a/apps/web/src/app/auth/core/services/change-password/web-change-password.service.ts
+++ b/apps/web/src/app/auth/core/services/change-password/web-change-password.service.ts
@@ -14,13 +14,13 @@ export class WebChangePasswordService
    currentPassword: string,
    newPassword: string,
    user: Account,
-    hint: string,
+    newPasswordHint: string,
  ): Promise<void | null> {
    await this.userKeyRotationService.rotateUserKeyMasterPasswordAndEncryptedData(
      currentPassword,
      newPassword,
      user,
-      hint,
+      newPasswordHint,
    );
  }

--- a/apps/web/src/app/core/core.module.ts
+++ b/apps/web/src/app/core/core.module.ts
@@ -51,7 +51,10 @@ import {
 import { AccountApiService as AccountApiServiceAbstraction } from "@bitwarden/common/auth/abstractions/account-api.service";
 import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
 import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
-import { MasterPasswordApiService } from "@bitwarden/common/auth/abstractions/master-password-api.service.abstraction";
+import {
+  MasterPasswordApiService,
+  MasterPasswordApiService as MasterPasswordApiServiceAbstraction,
+} from "@bitwarden/common/auth/abstractions/master-password-api.service.abstraction";
 import { SsoLoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/sso-login.service.abstraction";
 import { ClientType } from "@bitwarden/common/enums";
 import { ProcessReloadServiceAbstraction } from "@bitwarden/common/key-management/abstractions/process-reload.service";
@@ -385,7 +388,14 @@ const safeProviders: SafeProvider[] = [
  safeProvider({
    provide: ChangePasswordService,
    useClass: WebChangePasswordService,
-    deps: [],
+    deps: [
+      AccountService,
+      I18nServiceAbstraction,
+      KeyServiceAbstraction,
+      MasterPasswordApiServiceAbstraction,
+      InternalMasterPasswordServiceAbstraction,
+      ToastService,
+    ],
  }),
 ];

--- a/libs/angular/src/services/jslib-services.module.ts
+++ b/libs/angular/src/services/jslib-services.module.ts
@@ -1510,7 +1510,14 @@ const safeProviders: SafeProvider[] = [
  safeProvider({
    provide: ChangePasswordService,
    useClass: DefaultChangePasswordService,
-    deps: [],
+    deps: [
+      AccountServiceAbstraction,
+      I18nServiceAbstraction,
+      KeyService,
+      MasterPasswordApiServiceAbstraction,
+      InternalMasterPasswordServiceAbstraction,
+      ToastService,
+    ],
  }),
 ];

--- a/libs/auth/src/angular/change-existing-password/change-existing-password.component.ts
+++ b/libs/auth/src/angular/change-existing-password/change-existing-password.component.ts
@@ -13,11 +13,10 @@ import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/key-
 import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
-import { HashPurpose } from "@bitwarden/common/platform/enums";
 import { SyncService } from "@bitwarden/common/platform/sync";
 import { UserId } from "@bitwarden/common/types/guid";
 import { ToastService } from "@bitwarden/components";
-import { KdfConfigService, KeyService } from "@bitwarden/key-management";
+import { KeyService } from "@bitwarden/key-management";

 import {
  InputPasswordComponent,
@@ -44,7 +43,6 @@ export class ChangeExistingPasswordComponent implements OnInit {
    private changePasswordService: ChangePasswordService,
    private configService: ConfigService,
    private i18nService: I18nService,
-    private kdfConfigService: KdfConfigService,
    private keyService: KeyService,
    private masterPasswordApiService: MasterPasswordApiService,
    private masterPasswordService: InternalMasterPasswordServiceAbstraction,
@@ -77,21 +75,33 @@ export class ChangeExistingPasswordComponent implements OnInit {
  }

  async submitNew(passwordInputResult: PasswordInputResult) {
-    const { currentPassword, newPassword, hint, rotateUserKey } = passwordInputResult;
-
    try {
-      if (rotateUserKey) {
+      if (passwordInputResult.rotateUserKey) {
        await this.syncService.fullSync(true);
        const user = await firstValueFrom(this.accountService.activeAccount$);

        await this.changePasswordService.rotateUserKeyMasterPasswordAndEncryptedData(
-          currentPassword,
-          newPassword,
+          passwordInputResult.currentPassword,
+          passwordInputResult.newPassword,
          user,
-          hint,
+          passwordInputResult.newPasswordHint,
        );
      } else {
-        await this.updatePassword(currentPassword, newPassword, hint);
+        await this.changePasswordService.changePassword(
+          passwordInputResult.currentMasterKey,
+          passwordInputResult.currentServerMasterKeyHash,
+          passwordInputResult.newPasswordHint,
+          passwordInputResult.newMasterKey,
+          passwordInputResult.newServerMasterKeyHash,
+        );
+
+        this.toastService.showToast({
+          variant: "success",
+          title: this.i18nService.t("masterPasswordChanged"),
+          message: this.i18nService.t("masterPasswordChangedDesc"),
+        });
+
+        this.messagingService.send("logout"); // TODO-rr-bw: verify
      }
    } catch (e) {
      this.toastService.showToast({
@@ -102,19 +112,23 @@ export class ChangeExistingPasswordComponent implements OnInit {
    }
  }

-  // todo: move this to a service
-  // https://bitwarden.atlassian.net/browse/PM-17108
-  async updatePassword(currentPassword: string, newPassword: string, hint: string) {
-    const { userId, email } = await firstValueFrom(
-      this.accountService.activeAccount$.pipe(map((a) => ({ userId: a?.id, email: a?.email }))),
-    );
-    const kdfConfig = await firstValueFrom(this.kdfConfigService.getKdfConfig$(userId));
+  async submitOld(passwordInputResult: PasswordInputResult) {
+    if (passwordInputResult.rotateUserKey) {
+      await this.syncService.fullSync(true);
+    }
+
+    const userId = await firstValueFrom(getUserId(this.accountService.activeAccount$));

-    const currentMasterKey = await this.keyService.makeMasterKey(currentPassword, email, kdfConfig);
    const decryptedUserKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(
-      currentMasterKey,
+      passwordInputResult.currentMasterKey,
      userId,
    );
+
+    if (decryptedUserKey == null) {
+      throw new Error("Could not decrypt user key");
+    }
+
+    // TODO-rr-bw: do we still need this check/toast if it is handled in InputPasswordComponent?
    if (decryptedUserKey == null) {
      this.toastService.showToast({
        variant: "error",
@@ -124,82 +138,25 @@ export class ChangeExistingPasswordComponent implements OnInit {
      return;
    }

-    const newMasterKey = await this.keyService.makeMasterKey(newPassword, email, kdfConfig);
    const newMasterKeyEncryptedUserKey = await this.keyService.encryptUserKeyWithMasterKey(
-      newMasterKey,
+      passwordInputResult.newMasterKey,
      decryptedUserKey,
    );

    const request = new PasswordRequest();
-    request.masterPasswordHash = await this.keyService.hashMasterKey(
-      currentPassword,
-      currentMasterKey,
-    );
-    request.masterPasswordHint = hint;
-    request.newMasterPasswordHash = await this.keyService.hashMasterKey(newPassword, newMasterKey);
+    request.masterPasswordHash = passwordInputResult.currentServerMasterKeyHash;
+    request.masterPasswordHint = passwordInputResult.newPasswordHint;
+    request.newMasterPasswordHash = passwordInputResult.newServerMasterKeyHash;
    request.key = newMasterKeyEncryptedUserKey[1].encryptedString;

-    try {
-      await this.masterPasswordApiService.postPassword(request);
-
-      this.toastService.showToast({
-        variant: "success",
-        title: this.i18nService.t("masterPasswordChanged"),
-        message: this.i18nService.t("masterPasswordChangedDesc"),
-      });
-
-      this.messagingService.send("logout");
-    } catch {
-      this.toastService.showToast({
-        variant: "error",
-        title: null,
-        message: this.i18nService.t("errorOccurred"),
-      });
-    }
-  }
-
-  async submitOld(passwordInputResult: PasswordInputResult) {
-    if (passwordInputResult.rotateUserKey) {
-      await this.syncService.fullSync(true);
-    }
-
-    const masterKey = await this.keyService.makeMasterKey(
-      passwordInputResult.currentPassword,
-      await firstValueFrom(this.accountService.activeAccount$.pipe(map((a) => a?.email))),
-      await this.kdfConfigService.getKdfConfig(),
-    );
-
-    const userId = await firstValueFrom(getUserId(this.accountService.activeAccount$));
-    const newLocalKeyHash = await this.keyService.hashMasterKey(
-      passwordInputResult.newPassword,
-      passwordInputResult.newMasterKey,
-      HashPurpose.LocalAuthorization,
-    );
-
-    const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(masterKey, userId);
-    if (userKey == null) {
-      this.toastService.showToast({
-        variant: "error",
-        title: null,
-        message: this.i18nService.t("invalidMasterPassword"),
-      });
-      return;
-    }
-
-    const request = new PasswordRequest();
-    request.masterPasswordHash = await this.keyService.hashMasterKey(
-      passwordInputResult.currentPassword,
-      masterKey,
-    );
-    request.masterPasswordHint = passwordInputResult.hint;
-    request.newMasterPasswordHash = passwordInputResult.serverMasterKeyHash;
-    // request.key = newUserKey[1].encryptedString;
-
    try {
      if (passwordInputResult.rotateUserKey) {
        this.formPromise = this.masterPasswordApiService.postPassword(request).then(async () => {
          // we need to save this for local masterkey verification during rotation
-          await this.masterPasswordService.setMasterKeyHash(newLocalKeyHash, userId as UserId);
+          await this.masterPasswordService.setMasterKeyHash(
+            passwordInputResult.newLocalMasterKeyHash,
+            userId as UserId,
+          );
          await this.masterPasswordService.setMasterKey(
            passwordInputResult.newMasterKey,
            userId as UserId,
--- a/libs/auth/src/angular/input-password/input-password.component.html
+++ b/libs/auth/src/angular/input-password/input-password.component.html
@@ -76,7 +76,7 @@

  <bit-form-field>
    <bit-label>{{ "masterPassHintLabel" | i18n }}</bit-label>
-    <input bitInput formControlName="hint" />
+    <input bitInput formControlName="newPasswordHint" />
    <bit-hint>
      {{
        "masterPassHintText"
--- a/libs/auth/src/angular/registration/registration-finish/default-registration-finish.service.spec.ts
+++ b/libs/auth/src/angular/registration/registration-finish/default-registration-finish.service.spec.ts
@@ -60,11 +60,11 @@ describe("DefaultRegistrationFinishService", () => {
      masterKey = new SymmetricCryptoKey(new Uint8Array(64).buffer as CsprngArray) as MasterKey;
      passwordInputResult = {
        newMasterKey: masterKey,
-        serverMasterKeyHash: "serverMasterKeyHash",
-        localMasterKeyHash: "localMasterKeyHash",
+        newServerMasterKeyHash: "newServerMasterKeyHash",
+        newLocalMasterKeyHash: "newLocalMasterKeyHash",
        kdfConfig: DEFAULT_KDF_CONFIG,
-        hint: "hint",
-        newPassword: "password",
+        newPasswordHint: "newPasswordHint",
+        newPassword: "newPassword",
      };

      userKey = new SymmetricCryptoKey(new Uint8Array(64).buffer as CsprngArray) as UserKey;
@@ -101,8 +101,8 @@ describe("DefaultRegistrationFinishService", () => {
        expect.objectContaining({
          email,
          emailVerificationToken: emailVerificationToken,
-          masterPasswordHash: passwordInputResult.serverMasterKeyHash,
-          masterPasswordHint: passwordInputResult.hint,
+          masterPasswordHash: passwordInputResult.newServerMasterKeyHash,
+          masterPasswordHint: passwordInputResult.newPasswordHint,
          userSymmetricKey: userKeyEncString.encryptedString,
          userAsymmetricKeys: {
            publicKey: userKeyPair[0],
--- a/libs/auth/src/angular/registration/registration-finish/default-registration-finish.service.ts
+++ b/libs/auth/src/angular/registration/registration-finish/default-registration-finish.service.ts
@@ -81,8 +81,8 @@ export class DefaultRegistrationFinishService implements RegistrationFinishServi

    const registerFinishRequest = new RegisterFinishRequest(
      email,
-      passwordInputResult.serverMasterKeyHash,
-      passwordInputResult.hint,
+      passwordInputResult.newServerMasterKeyHash,
+      passwordInputResult.newPasswordHint,
      encryptedUserKey,
      userAsymmetricKeysRequest,
      passwordInputResult.kdfConfig.kdfType,
--- a/libs/auth/src/angular/set-password-jit/default-set-password-jit.service.spec.ts
+++ b/libs/auth/src/angular/set-password-jit/default-set-password-jit.service.spec.ts
@@ -112,11 +112,11 @@ describe("DefaultSetPasswordJitService", () => {

      passwordInputResult = {
        newMasterKey: masterKey,
-        serverMasterKeyHash: "serverMasterKeyHash",
-        localMasterKeyHash: "localMasterKeyHash",
-        hint: "hint",
+        newServerMasterKeyHash: "newServerMasterKeyHash",
+        newLocalMasterKeyHash: "newLocalMasterKeyHash",
+        newPasswordHint: "newPasswordHint",
        kdfConfig: DEFAULT_KDF_CONFIG,
-        newPassword: "password",
+        newPassword: "newPassword",
      };

      credentials = {
@@ -131,9 +131,9 @@ describe("DefaultSetPasswordJitService", () => {
      userDecryptionOptionsService.userDecryptionOptions$ = userDecryptionOptionsSubject;

      setPasswordRequest = new SetPasswordRequest(
-        passwordInputResult.serverMasterKeyHash,
+        passwordInputResult.newServerMasterKeyHash,
        protectedUserKey[1].encryptedString,
-        passwordInputResult.hint,
+        passwordInputResult.newPasswordHint,
        orgSsoIdentifier,
        keysRequest,
        passwordInputResult.kdfConfig.kdfType,
--- a/libs/auth/src/angular/set-password-jit/default-set-password-jit.service.ts
+++ b/libs/auth/src/angular/set-password-jit/default-set-password-jit.service.ts
@@ -44,9 +44,9 @@ export class DefaultSetPasswordJitService implements SetPasswordJitService {
  async setPassword(credentials: SetPasswordCredentials): Promise<void> {
    const {
      newMasterKey,
-      serverMasterKeyHash,
-      localMasterKeyHash,
-      hint,
+      newServerMasterKeyHash,
+      newLocalMasterKeyHash,
+      newPasswordHint,
      kdfConfig,
      orgSsoIdentifier,
      orgId,
@@ -70,9 +70,9 @@ export class DefaultSetPasswordJitService implements SetPasswordJitService {
    const [keyPair, keysRequest] = await this.makeKeyPairAndRequest(protectedUserKey);

    const request = new SetPasswordRequest(
-      serverMasterKeyHash,
+      newServerMasterKeyHash,
      protectedUserKey[1].encryptedString,
-      hint,
+      newPasswordHint,
      orgSsoIdentifier,
      keysRequest,
      kdfConfig.kdfType,
@@ -89,10 +89,10 @@ export class DefaultSetPasswordJitService implements SetPasswordJitService {

    await this.keyService.setPrivateKey(keyPair[1].encryptedString, userId);

-    await this.masterPasswordService.setMasterKeyHash(localMasterKeyHash, userId);
+    await this.masterPasswordService.setMasterKeyHash(newLocalMasterKeyHash, userId);

    if (resetPasswordAutoEnroll) {
-      await this.handleResetPasswordAutoEnroll(serverMasterKeyHash, orgId, userId);
+      await this.handleResetPasswordAutoEnroll(newServerMasterKeyHash, orgId, userId);
    }
  }

--- a/libs/auth/src/angular/set-password-jit/set-password-jit.service.abstraction.ts
+++ b/libs/auth/src/angular/set-password-jit/set-password-jit.service.abstraction.ts
@@ -6,10 +6,10 @@ import { KdfConfig } from "@bitwarden/key-management";

 export interface SetPasswordCredentials {
  newMasterKey: MasterKey;
-  serverMasterKeyHash: string;
-  localMasterKeyHash: string;
+  newServerMasterKeyHash: string;
+  newLocalMasterKeyHash: string;
+  newPasswordHint: string;
  kdfConfig: KdfConfig;
-  hint: string;
  orgSsoIdentifier: string;
  orgId: string;
  resetPasswordAutoEnroll: boolean;
--- a/libs/auth/src/common/abstractions/change-password.service.abstraction.ts
+++ b/libs/auth/src/common/abstractions/change-password.service.abstraction.ts
@@ -1,15 +1,24 @@
 import { Account } from "@bitwarden/common/auth/abstractions/account.service";
+import { MasterKey } from "@bitwarden/common/types/key";

 export abstract class ChangePasswordService {
  abstract rotateUserKeyMasterPasswordAndEncryptedData(
    currentPassword: string,
    newPassword: string,
    user: Account,
-    hint: string,
+    newPasswordHint: string,
  ): Promise<void | null>;

  abstract rotateUserKeyAndEncryptedDataLegacy(
    newPassword: string,
    user: Account,
  ): Promise<void | null>;
+
+  abstract changePassword(
+    currentMasterKey: MasterKey,
+    currentServerMasterKeyHash: string,
+    newPasswordHint: string,
+    newMasterKey: MasterKey,
+    newServerMasterKeyHash: string,
+  ): Promise<void>;
 }
--- a/libs/auth/src/common/services/change-password/default-change-password.service.ts
+++ b/libs/auth/src/common/services/change-password/default-change-password.service.ts
@@ -1,21 +1,86 @@
-import { Account } from "@bitwarden/common/auth/abstractions/account.service";
+import { firstValueFrom } from "rxjs";
+
+import { Account, AccountService } from "@bitwarden/common/auth/abstractions/account.service";
+import { MasterPasswordApiService } from "@bitwarden/common/auth/abstractions/master-password-api.service.abstraction";
+import { PasswordRequest } from "@bitwarden/common/auth/models/request/password.request";
+import { getUserId } from "@bitwarden/common/auth/services/account.service";
+import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction";
+import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
+import { MasterKey } from "@bitwarden/common/types/key";
+import { ToastService } from "@bitwarden/components";
+import { KeyService } from "@bitwarden/key-management";

 import { ChangePasswordService } from "../../abstractions";

 export class DefaultChangePasswordService implements ChangePasswordService {
+  constructor(
+    private accountService: AccountService,
+    private i18nService: I18nService,
+    private keyService: KeyService,
+    private masterPasswordApiService: MasterPasswordApiService,
+    private masterPasswordService: InternalMasterPasswordServiceAbstraction,
+    private toastService: ToastService,
+  ) {}
+
  async rotateUserKeyMasterPasswordAndEncryptedData(
    currentPassword: string,
    newPassword: string,
    user: Account,
    hint: string,
  ): Promise<void | null> {
-    return null;
+    return null; // implemented in Web
  }

  async rotateUserKeyAndEncryptedDataLegacy(
    newPassword: string,
    user: Account,
  ): Promise<void | null> {
-    return null;
+    return null; // implemented in Web
+  }
+
+  async changePassword(
+    currentMasterKey: MasterKey,
+    currentServerMasterKeyHash: string,
+    newPasswordHint: string,
+    newMasterKey: MasterKey,
+    newServerMasterKeyHash: string,
+  ) {
+    const userId = await firstValueFrom(getUserId(this.accountService.activeAccount$));
+
+    const decryptedUserKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(
+      currentMasterKey,
+      userId,
+    );
+
+    if (decryptedUserKey == null) {
+      throw new Error("Could not decrypt user key");
+    }
+
+    // TODO-rr-bw: do we still need this check/toast if it is handled in InputPasswordComponent?
+    if (decryptedUserKey == null) {
+      this.toastService.showToast({
+        variant: "error",
+        title: null,
+        message: this.i18nService.t("invalidMasterPassword"),
+      });
+      return;
+    }
+
+    const newMasterKeyEncryptedUserKey = await this.keyService.encryptUserKeyWithMasterKey(
+      newMasterKey,
+      decryptedUserKey,
+    );
+
+    const request = new PasswordRequest();
+    request.masterPasswordHash = currentServerMasterKeyHash;
+    request.masterPasswordHint = newPasswordHint;
+    request.newMasterPasswordHash = newServerMasterKeyHash;
+    request.key = newMasterKeyEncryptedUserKey[1].encryptedString;
+
+    try {
+      await this.masterPasswordApiService.postPassword(request);
+    } catch (e) {
+      throw new Error(e);
+    }
  }
 }