From 990e6a9eac521d354d45dda2d319c57bab635b51 Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Wed, 17 Sep 2025 14:22:23 -0600 Subject: [PATCH] fix(auth-tech-debt): [PM-24103] Remove Obsolete getUserKey - Made initial fixes. some tests are fixed, need to run back through the rest --- apps/cli/src/auth/commands/login.command.ts | 2 +- .../services/emergency-access.service.spec.ts | 5 ++++- .../services/emergency-access.service.ts | 12 +++++++++--- .../emergency-access/emergency-access.component.ts | 14 +++++++++++--- .../src/auth/components/set-pin.component.ts | 2 +- .../login-approval-dialog.component.ts | 2 ++ .../auth-request.service.abstraction.ts | 2 ++ .../src/common/login-strategies/login.strategy.ts | 7 ++++++- .../services/auth-request/auth-request.service.ts | 3 ++- ...word-reset-enrollment.service.implementation.ts | 3 ++- 10 files changed, 40 insertions(+), 12 deletions(-) diff --git a/apps/cli/src/auth/commands/login.command.ts b/apps/cli/src/auth/commands/login.command.ts index 133c9658ae7..d6c104faf44 100644 --- a/apps/cli/src/auth/commands/login.command.ts +++ b/apps/cli/src/auth/commands/login.command.ts @@ -611,7 +611,7 @@ export class LoginCommand { const newPasswordHash = await this.keyService.hashMasterKey(masterPassword, newMasterKey); // Grab user key - const userKey = await this.keyService.getUserKey(); + const userKey = await firstValueFrom(this.keyService.userKey$(userId)); if (!userKey) { throw new Error("User key not found."); } diff --git a/apps/web/src/app/auth/emergency-access/services/emergency-access.service.spec.ts b/apps/web/src/app/auth/emergency-access/services/emergency-access.service.spec.ts index 2ff38f6eab0..b4dc2cae388 100644 --- a/apps/web/src/app/auth/emergency-access/services/emergency-access.service.spec.ts +++ b/apps/web/src/app/auth/emergency-access/services/emergency-access.service.spec.ts @@ -17,6 +17,7 @@ import { CsprngArray } from "@bitwarden/common/types/csprng"; import { UserId } from "@bitwarden/common/types/guid"; import { UserKey, MasterKey, UserPrivateKey } from "@bitwarden/common/types/key"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { newGuid } from "@bitwarden/guid"; import { Argon2KdfConfig, KdfType, KeyService, PBKDF2KdfConfig } from "@bitwarden/key-management"; import { EmergencyAccessStatusType } from "../enums/emergency-access-status-type"; @@ -125,6 +126,8 @@ describe("EmergencyAccessService", () => { "mockUserPublicKeyEncryptedUserKey", ); + const mockUserId = newGuid(); + keyService.getUserKey.mockResolvedValueOnce(mockUserKey); encryptService.encapsulateKeyUnsigned.mockResolvedValueOnce( @@ -134,7 +137,7 @@ describe("EmergencyAccessService", () => { emergencyAccessApiService.postEmergencyAccessConfirm.mockResolvedValueOnce(); // Act - await emergencyAccessService.confirm(id, granteeId, publicKey); + await emergencyAccessService.confirm(id, granteeId, publicKey, mockUserId as UserId); // Assert expect(emergencyAccessApiService.postEmergencyAccessConfirm).toHaveBeenCalledWith(id, { diff --git a/apps/web/src/app/auth/emergency-access/services/emergency-access.service.ts b/apps/web/src/app/auth/emergency-access/services/emergency-access.service.ts index cce8d9345b2..8ed90592859 100644 --- a/apps/web/src/app/auth/emergency-access/services/emergency-access.service.ts +++ b/apps/web/src/app/auth/emergency-access/services/emergency-access.service.ts @@ -175,11 +175,17 @@ export class EmergencyAccessService * Step 3 of the 3 step setup flow. * Intended for grantor. * @param id emergency access id - * @param token secret token provided in email + * @param granteeId token secret token provided in email * @param publicKey public key of grantee + * @param activeUserId current user's id */ - async confirm(id: string, granteeId: string, publicKey: Uint8Array): Promise { - const userKey = await this.keyService.getUserKey(); + async confirm( + id: string, + granteeId: string, + publicKey: Uint8Array, + activeUserId: UserId, + ): Promise { + const userKey = await firstValueFrom(this.keyService.userKey$(activeUserId)); if (!userKey) { throw new Error("No user key found"); } diff --git a/apps/web/src/app/auth/settings/emergency-access/emergency-access.component.ts b/apps/web/src/app/auth/settings/emergency-access/emergency-access.component.ts index de30205e6fe..9a2ff31ab21 100644 --- a/apps/web/src/app/auth/settings/emergency-access/emergency-access.component.ts +++ b/apps/web/src/app/auth/settings/emergency-access/emergency-access.component.ts @@ -18,6 +18,7 @@ import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/pl import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { DialogService, ToastService } from "@bitwarden/components"; +import { UserId } from "@bitwarden/user-core"; import { HeaderModule } from "../../../layouts/header/header.module"; import { SharedModule } from "../../../shared/shared.module"; @@ -55,6 +56,7 @@ export class EmergencyAccessComponent implements OnInit { emergencyAccessStatusType = EmergencyAccessStatusType; actionPromise: Promise; isOrganizationOwner: boolean; + userId: UserId; constructor( private emergencyAccessService: EmergencyAccessService, @@ -80,8 +82,8 @@ export class EmergencyAccessComponent implements OnInit { } async ngOnInit() { - const userId = await firstValueFrom(getUserId(this.accountService.activeAccount$)); - const orgs = await firstValueFrom(this.organizationService.organizations$(userId)); + this.userId = await firstValueFrom(getUserId(this.accountService.activeAccount$)); + const orgs = await firstValueFrom(this.organizationService.organizations$(this.userId)); this.isOrganizationOwner = orgs.some((o) => o.isOwner); // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. // eslint-disable-next-line @typescript-eslint/no-floating-promises @@ -165,7 +167,12 @@ export class EmergencyAccessComponent implements OnInit { }); const result = await lastValueFrom(dialogRef.closed); if (result === EmergencyAccessConfirmDialogResult.Confirmed) { - await this.emergencyAccessService.confirm(contact.id, contact.granteeId, publicKey); + await this.emergencyAccessService.confirm( + contact.id, + contact.granteeId, + publicKey, + this.userId, + ); updateUser(); this.toastService.showToast({ variant: "success", @@ -180,6 +187,7 @@ export class EmergencyAccessComponent implements OnInit { contact.id, contact.granteeId, publicKey, + this.userId, ); await this.actionPromise; updateUser(); diff --git a/libs/angular/src/auth/components/set-pin.component.ts b/libs/angular/src/auth/components/set-pin.component.ts index 9e351990fff..ba816d5ad34 100644 --- a/libs/angular/src/auth/components/set-pin.component.ts +++ b/libs/angular/src/auth/components/set-pin.component.ts @@ -47,7 +47,7 @@ export class SetPinComponent implements OnInit { } const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; - const userKey = await this.keyService.getUserKey(); + const userKey = await firstValueFrom(this.keyService.userKey$(userId)); const userKeyEncryptedPin = await this.pinService.createUserKeyEncryptedPin( pinFormControl.value, diff --git a/libs/angular/src/auth/login-approval/login-approval-dialog.component.ts b/libs/angular/src/auth/login-approval/login-approval-dialog.component.ts index 19dc3f519c6..e198e41619d 100644 --- a/libs/angular/src/auth/login-approval/login-approval-dialog.component.ts +++ b/libs/angular/src/auth/login-approval/login-approval-dialog.component.ts @@ -10,6 +10,7 @@ import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { DevicesServiceAbstraction } from "@bitwarden/common/auth/abstractions/devices/devices.service.abstraction"; import { AuthRequestResponse } from "@bitwarden/common/auth/models/response/auth-request.response"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; @@ -146,6 +147,7 @@ export class LoginApprovalDialogComponent implements OnInit, OnDestroy { const loginResponse = await this.authRequestService.approveOrDenyAuthRequest( approve, this.authRequestResponse, + await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)), ); this.showResultToast(loginResponse); } diff --git a/libs/auth/src/common/abstractions/auth-request.service.abstraction.ts b/libs/auth/src/common/abstractions/auth-request.service.abstraction.ts index 10cc643fd45..4b7fb277429 100644 --- a/libs/auth/src/common/abstractions/auth-request.service.abstraction.ts +++ b/libs/auth/src/common/abstractions/auth-request.service.abstraction.ts @@ -55,6 +55,7 @@ export abstract class AuthRequestServiceAbstraction { * Approve or deny an auth request. * @param approve True to approve, false to deny. * @param authRequest The auth request to approve or deny, must have an id and key. + * @param userId The ID of the user for whose account we are approving or denying an auth request for. * @returns The updated auth request, the `requestApproved` field will be true if * approval was successful. * @throws If the auth request is missing an id or key. @@ -62,6 +63,7 @@ export abstract class AuthRequestServiceAbstraction { abstract approveOrDenyAuthRequest( approve: boolean, authRequest: AuthRequestResponse, + userId: UserId, ): Promise; /** * Sets the `UserKey` from an auth request. Auth request must have a `UserKey`. diff --git a/libs/auth/src/common/login-strategies/login.strategy.ts b/libs/auth/src/common/login-strategies/login.strategy.ts index 93198f992cb..0f03a4af0ab 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.ts @@ -306,7 +306,12 @@ export abstract class LoginStrategy { protected async createKeyPairForOldAccount(userId: UserId) { try { - const userKey = await this.keyService.getUserKey(userId); + const userKey = await firstValueFrom(this.keyService.userKey$(userId)); + + if (userKey === null) { + throw new Error(`No user key found by id: ${userId}`); + } + if (userKey.inner().type == EncryptionType.CoseEncrypt0) { throw new Error("Cannot create key pair for account on V2 encryption"); } diff --git a/libs/auth/src/common/services/auth-request/auth-request.service.ts b/libs/auth/src/common/services/auth-request/auth-request.service.ts index 5fc28e960a8..b666ef5ad90 100644 --- a/libs/auth/src/common/services/auth-request/auth-request.service.ts +++ b/libs/auth/src/common/services/auth-request/auth-request.service.ts @@ -123,6 +123,7 @@ export class AuthRequestService implements AuthRequestServiceAbstraction { async approveOrDenyAuthRequest( approve: boolean, authRequest: AuthRequestResponse, + userId: UserId, ): Promise { if (!authRequest.id) { throw new Error("Auth request has no id"); @@ -132,7 +133,7 @@ export class AuthRequestService implements AuthRequestServiceAbstraction { } const pubKey = Utils.fromB64ToArray(authRequest.publicKey); - const keyToEncrypt = await this.keyService.getUserKey(); + const keyToEncrypt = await firstValueFrom(this.keyService.userKey$(userId)); const encryptedKey = await this.encryptService.encapsulateKeyUnsigned(keyToEncrypt, pubKey); const response = new PasswordlessAuthRequest( diff --git a/libs/common/src/auth/services/password-reset-enrollment.service.implementation.ts b/libs/common/src/auth/services/password-reset-enrollment.service.implementation.ts index f491d7d5eb0..0e2567181c7 100644 --- a/libs/common/src/auth/services/password-reset-enrollment.service.implementation.ts +++ b/libs/common/src/auth/services/password-reset-enrollment.service.implementation.ts @@ -11,6 +11,7 @@ import { // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. // eslint-disable-next-line no-restricted-imports import { KeyService } from "@bitwarden/key-management"; +import { UserId } from "@bitwarden/user-core"; import { OrganizationApiServiceAbstraction } from "../../admin-console/abstractions/organization/organization-api.service.abstraction"; import { EncryptService } from "../../key-management/crypto/abstractions/encrypt.service"; @@ -53,7 +54,7 @@ export class PasswordResetEnrollmentServiceImplementation userId = userId ?? (await firstValueFrom(this.accountService.activeAccount$.pipe(map((a) => a?.id)))); - userKey = userKey ?? (await this.keyService.getUserKey(userId)); + userKey = userKey ?? (await firstValueFrom(this.keyService.userKey$(userId as UserId))); // RSA Encrypt user's userKey.key with organization public key const encryptedKey = await this.encryptService.encapsulateKeyUnsigned(userKey, orgPublicKey);