[PM-23627] Require publicKey for keyService getFingerprint (#15933)

* require public key on keyService getFingerprint

* Update consumers and add error handling & logging

apps/browser/src/auth/popup/settings/account-security.component.spec.ts

@@ -24,7 +24,6 @@ import { EnvironmentService } from "@bitwarden/common/platform/abstractions/envi
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
 import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
-import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
 import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service";
 import { MessageSender } from "@bitwarden/common/platform/messaging";
 import { Utils } from "@bitwarden/common/platform/misc/utils";
@@ -79,7 +78,6 @@ describe("AccountSecurityComponent", () => {
         { provide: PlatformUtilsService, useValue: platformUtilsService },
         { provide: PolicyService, useValue: policyService },
         { provide: PopupRouterCacheService, useValue: mock<PopupRouterCacheService>() },
-        { provide: StateService, useValue: mock<StateService>() },
         { provide: ToastService, useValue: mock<ToastService>() },
         { provide: UserVerificationService, useValue: mock<UserVerificationService>() },
         { provide: VaultTimeoutService, useValue: mock<VaultTimeoutService>() },

apps/browser/src/auth/popup/settings/account-security.component.ts

@@ -44,9 +44,9 @@ import {
 import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
 import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.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 { StateService } from "@bitwarden/common/platform/abstractions/state.service";
 import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service";
 import {
   DialogRef,
@@ -149,7 +149,6 @@ export class AccountSecurityComponent implements OnInit, OnDestroy {
     public messagingService: MessagingService,
     private environmentService: EnvironmentService,
     private keyService: KeyService,
-    private stateService: StateService,
     private userVerificationService: UserVerificationService,
     private dialogService: DialogService,
     private changeDetectorRef: ChangeDetectorRef,
@@ -159,6 +158,7 @@ export class AccountSecurityComponent implements OnInit, OnDestroy {
     private vaultNudgesService: NudgesService,
     private validationService: ValidationService,
     private configService: ConfigService,
+    private logService: LogService,
   ) {}
 
   async ngOnInit() {
@@ -683,10 +683,16 @@ export class AccountSecurityComponent implements OnInit, OnDestroy {
   }
 
   async openAcctFingerprintDialog() {
-    const activeUserId = await firstValueFrom(
+    const activeUserId = await firstValueFrom(getUserId(this.accountService.activeAccount$));
-      this.accountService.activeAccount$.pipe(map((a) => a?.id)),
-    );
     const publicKey = await firstValueFrom(this.keyService.userPublicKey$(activeUserId));
+    if (publicKey == null) {
+      this.logService.error(
+        "[AccountSecurityComponent] No public key available for the user: " +
+          activeUserId +
+          " fingerprint can't be displayed.",
+      );
+      return;
+    }
     const fingerprint = await this.keyService.getFingerprint(activeUserId, publicKey);
 
     const dialogRef = FingerprintDialogComponent.open(this.dialogService, {

apps/cli/src/commands/get.command.ts

@@ -606,6 +606,9 @@ export class GetCommand extends DownloadCommand {
     if (id === "me") {
       const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId));
       const publicKey = await firstValueFrom(this.keyService.userPublicKey$(activeUserId));
+      if (publicKey == null) {
+        return Response.error("No public key available for the active user.");
+      }
       fingerprint = await this.keyService.getFingerprint(activeUserId, publicKey);
     } else if (Utils.isGuid(id)) {
       try {

apps/desktop/src/app/app.component.ts

@@ -299,12 +299,22 @@ export class AppComponent implements OnInit, OnDestroy {
             break;
           case "showFingerprintPhrase": {
             const activeUserId = await firstValueFrom(
-              this.accountService.activeAccount$.pipe(map((a) => a?.id)),
+              getUserId(this.accountService.activeAccount$),
             );
             const publicKey = await firstValueFrom(this.keyService.userPublicKey$(activeUserId));
-            const fingerprint = await this.keyService.getFingerprint(activeUserId, publicKey);
+            if (publicKey == null) {
-            const dialogRef = FingerprintDialogComponent.open(this.dialogService, { fingerprint });
+              this.logService.error(
-            await firstValueFrom(dialogRef.closed);
+                "[AppComponent] No public key available for the user: " +
+                  activeUserId +
+                  " fingerprint can't be displayed.",
+              );
+            } else {
+              const fingerprint = await this.keyService.getFingerprint(activeUserId, publicKey);
+              const dialogRef = FingerprintDialogComponent.open(this.dialogService, {
+                fingerprint,
+              });
+              await firstValueFrom(dialogRef.closed);
+            }
             break;
           }
           case "deleteAccount":

apps/web/src/app/auth/settings/account/profile.component.html

@@ -46,11 +46,14 @@
           <i class="bwi bwi-question-circle" aria-hidden="true"></i>
         </a>
       </div>
-      <app-account-fingerprint
+      @if (fingerprintMaterial && userPublicKey) {
-        [fingerprintMaterial]="fingerprintMaterial"
+        <app-account-fingerprint
-        fingerprintLabel="{{ 'yourAccountsFingerprint' | i18n }}"
+          [fingerprintMaterial]="fingerprintMaterial"
-      >
+          [publicKeyBuffer]="userPublicKey"
-      </app-account-fingerprint>
+          fingerprintLabel="{{ 'yourAccountsFingerprint' | i18n }}"
+        >
+        </app-account-fingerprint>
+      }
     </div>
   </div>
   <button bitButton bitFormButton type="submit" buttonType="primary">{{ "save" | i18n }}</button>

apps/web/src/app/auth/settings/account/profile.component.ts

@@ -12,7 +12,10 @@ import { UpdateProfileRequest } from "@bitwarden/common/auth/models/request/upda
 import { getUserId } from "@bitwarden/common/auth/services/account.service";
 import { ProfileResponse } from "@bitwarden/common/models/response/profile.response";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
+import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
+import { UserPublicKey } from "@bitwarden/common/types/key";
 import { DialogService, ToastService } from "@bitwarden/components";
+import { KeyService } from "@bitwarden/key-management";
 
 import { DynamicAvatarComponent } from "../../../components/dynamic-avatar.component";
 import { SharedModule } from "../../../shared";
@@ -29,6 +32,7 @@ export class ProfileComponent implements OnInit, OnDestroy {
   loading = true;
   profile: ProfileResponse;
   fingerprintMaterial: string;
+  userPublicKey: UserPublicKey;
   managingOrganization$: Observable<Organization>;
   private destroy$ = new Subject<void>();
 
@@ -44,16 +48,24 @@ export class ProfileComponent implements OnInit, OnDestroy {
     private dialogService: DialogService,
     private toastService: ToastService,
     private organizationService: OrganizationService,
+    private keyService: KeyService,
+    private logService: LogService,
   ) {}
 
   async ngOnInit() {
     this.profile = await this.apiService.getProfile();
-    this.loading = false;
-    this.fingerprintMaterial = await firstValueFrom(
-      this.accountService.activeAccount$.pipe(map((a) => a?.id)),
-    );
-
     const userId = await firstValueFrom(getUserId(this.accountService.activeAccount$));
+    this.fingerprintMaterial = userId;
+    const publicKey = await firstValueFrom(this.keyService.userPublicKey$(userId));
+    if (publicKey == null) {
+      this.logService.error(
+        "[ProfileComponent] No public key available for the user: " +
+          userId +
+          " fingerprint can't be displayed.",
+      );
+    } else {
+      this.userPublicKey = publicKey;
+    }
 
     this.managingOrganization$ = this.organizationService
       .organizations$(userId)
@@ -70,6 +82,8 @@ export class ProfileComponent implements OnInit, OnDestroy {
       .subscribe((name) => {
         this.profile.name = name;
       });
+
+    this.loading = false;
   }
 
   openChangeAvatar = async () => {

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

@@ -318,14 +318,14 @@ export abstract class KeyService {
   ): Observable<{ privateKey: UserPrivateKey; publicKey: UserPublicKey } | null>;
 
   /**
-   * Generates a fingerprint phrase for the user based on their public key
+   * Generates a fingerprint phrase for the public key provided.
    *
-   * @throws Error when publicKey is null and there is no active user, or the active user does not have a public key
+   * @throws Error when publicKey is null or undefined.
    * @param fingerprintMaterial Fingerprint material
-   * @param publicKey The user's public key
+   * @param publicKey The public key to generate the fingerprint phrase for.
-   * @returns The user's fingerprint phrase
+   * @returns The fingerprint phrase
    */
-  abstract getFingerprint(fingerprintMaterial: string, publicKey?: Uint8Array): Promise<string[]>;
+  abstract getFingerprint(fingerprintMaterial: string, publicKey: Uint8Array): Promise<string[]>;
   /**
    * Generates a new keypair
    * @param key A key to encrypt the private key with. If not provided,

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

@@ -1227,6 +1227,63 @@ describe("keyService", () => {
     });
   });
 
+  describe("getFingerprint", () => {
+    const mockFingerprintMaterial = "test@example.com";
+    const mockPublicKey = new Uint8Array(256);
+    const mockKeyFingerprint = Utils.fromB64ToArray("nfG2jTrJilBEsSrg7ffe9exE9PlClem4P2bxlQ6rNbs=");
+    const mockUserFingerprint = Utils.fromB64ToArray(
+      "V5AQSk83YXd6kZqCncC6d9J72R7UZ60Xl1eIoDoWgTc=",
+    );
+    const expectedFingerprint = ["predefine", "hunting", "pastime", "enrich", "unhearing"];
+
+    beforeEach(() => {
+      cryptoFunctionService.hash.mockResolvedValue(mockKeyFingerprint);
+      cryptoFunctionService.hkdfExpand.mockResolvedValue(mockUserFingerprint);
+    });
+
+    test.each([null as unknown as Uint8Array, undefined as unknown as Uint8Array])(
+      "throws when publicKey is %s",
+      async (publicKey) => {
+        await expect(keyService.getFingerprint(mockFingerprintMaterial, publicKey)).rejects.toThrow(
+          "Public key is required to generate a fingerprint.",
+        );
+        expect(cryptoFunctionService.hash).not.toHaveBeenCalled();
+        expect(cryptoFunctionService.hkdfExpand).not.toHaveBeenCalled();
+      },
+    );
+
+    it("generates fingerprint successfully", async () => {
+      const result = await keyService.getFingerprint(mockFingerprintMaterial, mockPublicKey);
+
+      expect(result).toEqual(expectedFingerprint);
+      expect(cryptoFunctionService.hash).toHaveBeenCalledWith(mockPublicKey, "sha256");
+      expect(cryptoFunctionService.hkdfExpand).toHaveBeenCalledWith(
+        mockKeyFingerprint,
+        mockFingerprintMaterial,
+        32,
+        "sha256",
+      );
+    });
+
+    it("throws when entropy of hash function is too small", async () => {
+      const keyFingerprint = new Uint8Array(3);
+      cryptoFunctionService.hash.mockResolvedValue(keyFingerprint);
+      cryptoFunctionService.hkdfExpand.mockResolvedValue(new Uint8Array(3));
+
+      await expect(
+        keyService.getFingerprint(mockFingerprintMaterial, mockPublicKey),
+      ).rejects.toThrow("Output entropy of hash function is too small");
+
+      expect(cryptoFunctionService.hash).toHaveBeenCalledWith(mockPublicKey, "sha256");
+      expect(cryptoFunctionService.hkdfExpand).toHaveBeenCalledWith(
+        keyFingerprint,
+        mockFingerprintMaterial,
+        32,
+        "sha256",
+      );
+    });
+  });
+
   describe("makeKeyPair", () => {
     test.each([null as unknown as SymmetricCryptoKey, undefined as unknown as SymmetricCryptoKey])(
       "throws when the provided key is %s",

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

@@ -473,19 +473,11 @@ export class DefaultKeyService implements KeyServiceAbstraction {
       .update(() => encPrivateKey);
   }
 
-  // TODO: Make public key required
+  async getFingerprint(fingerprintMaterial: string, publicKey: Uint8Array): Promise<string[]> {
-  async getFingerprint(fingerprintMaterial: string, publicKey?: Uint8Array): Promise<string[]> {
     if (publicKey == null) {
-      const activeUserId = await firstValueFrom(this.stateProvider.activeUserId$);
+      throw new Error("Public key is required to generate a fingerprint.");
-      if (activeUserId == null) {
-        throw new Error("No active user found.");
-      }
-      publicKey = (await firstValueFrom(this.userPublicKey$(activeUserId))) as Uint8Array;
     }
 
-    if (publicKey === null) {
-      throw new Error("No public key available.");
-    }
     const keyFingerprint = await this.cryptoFunctionService.hash(publicKey, "sha256");
     const userFingerprint = await this.cryptoFunctionService.hkdfExpand(
       keyFingerprint,