1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-21 02:33:46 +00:00

[PM-24105] Remove usage of getUserKey on keyService (#16626)

• prefer undefined over null
• obtain required UserId once per method, before branching
• guards moved to beginning of methods
* lift UserId retrieval to occur once during import
* remove redundant userId retrieval
This commit is contained in:
John Harrington
2025-10-15 07:03:29 -07:00
committed by GitHub
parent 8ff4fb1ed4
commit 64105e64e9
22 changed files with 118 additions and 62 deletions

View File

@@ -88,7 +88,7 @@ export class BitwardenJsonImporter extends BaseImporter implements Importer {
for (const c of results.items) {
const cipher = CipherWithIdExport.toDomain(c);
// reset ids incase they were set for some reason
// reset ids in case they were set for some reason
cipher.id = null;
cipher.organizationId = this.organizationId;
cipher.collectionIds = null;
@@ -131,7 +131,7 @@ export class BitwardenJsonImporter extends BaseImporter implements Importer {
results.items.forEach((c) => {
const cipher = CipherWithIdExport.toView(c);
// reset ids incase they were set for some reason
// reset ids in case they were set for some reason
cipher.id = null;
cipher.organizationId = null;
cipher.collectionIds = null;

View File

@@ -9,7 +9,6 @@ import { Utils } from "@bitwarden/common/platform/misc/utils";
import { emptyGuid, OrganizationId } from "@bitwarden/common/types/guid";
import { OrgKey, UserKey } from "@bitwarden/common/types/key";
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import { newGuid } from "@bitwarden/guid";
import { KdfType, KeyService } from "@bitwarden/key-management";
import { UserId } from "@bitwarden/user-core";
@@ -41,7 +40,7 @@ describe("BitwardenPasswordProtectedImporter", () => {
accountService = mock<AccountService>();
accountService.activeAccount$ = of({
id: newGuid() as UserId,
id: emptyGuid as UserId,
email: "test@example.com",
emailVerified: true,
name: "Test User",
@@ -52,8 +51,8 @@ describe("BitwardenPasswordProtectedImporter", () => {
The key values below are never read, empty objects are cast as types for compilation type checking only.
Tests specific to key contents are in key-service.spec.ts
*/
const mockOrgKey = {} as unknown as OrgKey;
const mockUserKey = {} as unknown as UserKey;
const mockOrgKey = {} as OrgKey;
const mockUserKey = {} as UserKey;
keyService.orgKeys$.mockImplementation(() =>
of({ [mockOrgId]: mockOrgKey } as Record<OrganizationId, OrgKey>),
@@ -99,7 +98,7 @@ describe("BitwardenPasswordProtectedImporter", () => {
beforeEach(() => {
accountService.activeAccount$ = of({
id: newGuid() as UserId,
id: emptyGuid as UserId,
email: "test@example.com",
emailVerified: true,
name: "Test User",

View File

@@ -10,6 +10,7 @@ import {
CollectionView,
} from "@bitwarden/admin-console/common";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { getUserId } from "@bitwarden/common/auth/services/account.service";
import { DeviceType } from "@bitwarden/common/enums";
import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service";
import { PinServiceAbstraction } from "@bitwarden/common/key-management/pin/pin.service.abstraction";
@@ -21,7 +22,7 @@ import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.servic
import { Utils } from "@bitwarden/common/platform/misc/utils";
import { SemanticLogger } from "@bitwarden/common/tools/log";
import { SystemServiceProvider } from "@bitwarden/common/tools/providers";
import { OrganizationId } from "@bitwarden/common/types/guid";
import { OrganizationId, UserId } from "@bitwarden/common/types/guid";
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction";
import { CipherType, toCipherTypeName } from "@bitwarden/common/vault/enums";
@@ -238,10 +239,11 @@ export class ImportService implements ImportServiceAbstraction {
try {
await this.setImportTarget(importResult, organizationId, selectedImportTarget);
const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId));
if (organizationId != null) {
await this.handleOrganizationalImport(importResult, organizationId);
await this.handleOrganizationalImport(importResult, organizationId, userId);
} else {
await this.handleIndividualImport(importResult);
await this.handleIndividualImport(importResult, userId);
}
} catch (error) {
const errorResponse = new ErrorResponse(error, 400);
@@ -419,16 +421,14 @@ export class ImportService implements ImportServiceAbstraction {
}
}
private async handleIndividualImport(importResult: ImportResult) {
private async handleIndividualImport(importResult: ImportResult, userId: UserId) {
const request = new ImportCiphersRequest();
const activeUserId = await firstValueFrom(
this.accountService.activeAccount$.pipe(map((a) => a?.id)),
);
for (let i = 0; i < importResult.ciphers.length; i++) {
const c = await this.cipherService.encrypt(importResult.ciphers[i], activeUserId);
const c = await this.cipherService.encrypt(importResult.ciphers[i], userId);
request.ciphers.push(new CipherRequest(c));
}
const userKey = await this.keyService.getUserKey(activeUserId);
const userKey = await firstValueFrom(this.keyService.userKey$(userId));
if (importResult.folders != null) {
for (let i = 0; i < importResult.folders.length; i++) {
const f = await this.folderService.encrypt(importResult.folders[i], userKey);
@@ -446,20 +446,18 @@ export class ImportService implements ImportServiceAbstraction {
private async handleOrganizationalImport(
importResult: ImportResult,
organizationId: OrganizationId,
userId: UserId,
) {
const request = new ImportOrganizationCiphersRequest();
const activeUserId = await firstValueFrom(
this.accountService.activeAccount$.pipe(map((a) => a?.id)),
);
for (let i = 0; i < importResult.ciphers.length; i++) {
importResult.ciphers[i].organizationId = organizationId;
const c = await this.cipherService.encrypt(importResult.ciphers[i], activeUserId);
const c = await this.cipherService.encrypt(importResult.ciphers[i], userId);
request.ciphers.push(new CipherRequest(c));
}
if (importResult.collections != null) {
for (let i = 0; i < importResult.collections.length; i++) {
importResult.collections[i].organizationId = organizationId;
const c = await this.collectionService.encrypt(importResult.collections[i], activeUserId);
const c = await this.collectionService.encrypt(importResult.collections[i], userId);
request.collections.push(new CollectionWithIdRequest(c));
}
}