1
0
mirror of https://github.com/bitwarden/browser synced 2026-02-24 00:23:17 +00:00

fix(auth-tech-debt): [PM-24103] Remove Obsolete getUserKey - Made initial fixes. some tests are fixed, need to run back through the rest

This commit is contained in:
Patrick Pimentel
2025-09-17 14:22:23 -06:00
parent 5b167d6748
commit 990e6a9eac
10 changed files with 40 additions and 12 deletions

View File

@@ -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.");
}

View File

@@ -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, {

View File

@@ -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<void> {
const userKey = await this.keyService.getUserKey();
async confirm(
id: string,
granteeId: string,
publicKey: Uint8Array,
activeUserId: UserId,
): Promise<void> {
const userKey = await firstValueFrom(this.keyService.userKey$(activeUserId));
if (!userKey) {
throw new Error("No user key found");
}

View File

@@ -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<any>;
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();

View File

@@ -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,

View File

@@ -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);
}

View File

@@ -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<AuthRequestResponse>;
/**
* Sets the `UserKey` from an auth request. Auth request must have a `UserKey`.

View File

@@ -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");
}

View File

@@ -123,6 +123,7 @@ export class AuthRequestService implements AuthRequestServiceAbstraction {
async approveOrDenyAuthRequest(
approve: boolean,
authRequest: AuthRequestResponse,
userId: UserId,
): Promise<AuthRequestResponse> {
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(

View File

@@ -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);