1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-06 00:13:28 +00:00

[PM-23614] Remove activeUserOrgKeys$ from the key service (#17045)

* Remove activeUserOrgKeys$ from the key service

* test leftover

* test leftover
This commit is contained in:
Maciej Zieniuk
2025-11-06 19:27:13 +01:00
committed by GitHub
parent c404ee210b
commit ff12e672e6
7 changed files with 42 additions and 36 deletions

View File

@@ -1,6 +1,6 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import { firstValueFrom, map, switchMap } from "rxjs";
import { filter, firstValueFrom, map, switchMap } from "rxjs";
import { CollectionService, CollectionView } from "@bitwarden/admin-console/common";
import { ApiService } from "@bitwarden/common/abstractions/api.service";
@@ -448,7 +448,9 @@ export class GetCommand extends DownloadCommand {
this.collectionService.encryptedCollections$(activeUserId).pipe(getById(id)),
);
if (collection != null) {
const orgKeys = await firstValueFrom(this.keyService.activeUserOrgKeys$);
const orgKeys = await firstValueFrom(
this.keyService.orgKeys$(activeUserId).pipe(filter((orgKeys) => orgKeys != null)),
);
decCollection = await collection.decrypt(
orgKeys[collection.organizationId as OrganizationId],
this.encryptService,

View File

@@ -177,8 +177,7 @@ describe("DefaultCollectionService", () => {
// Arrange dependencies
void setEncryptedState([collection1, collection2]).then(() => {
// Act: emit undefined
cryptoKeys.next(undefined);
keyService.activeUserOrgKeys$ = of(undefined);
cryptoKeys.next(null);
});
});

View File

@@ -1,11 +1,12 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import { concatMap, firstValueFrom, map } from "rxjs";
import { filter, firstValueFrom } from "rxjs";
// 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 { Collection, 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 { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service";
import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string";
import {
@@ -15,7 +16,7 @@ import {
} from "@bitwarden/common/models/export";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key";
import { OrganizationId } from "@bitwarden/common/types/guid";
import { UserId } from "@bitwarden/common/types/guid";
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import { FolderView } from "@bitwarden/common/vault/models/view/folder.view";
import { KeyService } from "@bitwarden/key-management";
@@ -64,13 +65,13 @@ export class BitwardenJsonImporter extends BaseImporter implements Importer {
private async parseEncrypted(
results: BitwardenEncryptedIndividualJsonExport | BitwardenEncryptedOrgJsonExport,
) {
const account = await firstValueFrom(this.accountService.activeAccount$);
const userId = await firstValueFrom(getUserId(this.accountService.activeAccount$));
if (results.encKeyValidation_DO_NOT_EDIT != null) {
const orgKeys = await firstValueFrom(this.keyService.orgKeys$(account.id));
const orgKeys = await firstValueFrom(this.keyService.orgKeys$(userId));
let keyForDecryption: SymmetricCryptoKey = orgKeys?.[this.organizationId];
if (keyForDecryption == null) {
keyForDecryption = await firstValueFrom(this.keyService.userKey$(account.id));
keyForDecryption = await firstValueFrom(this.keyService.userKey$(userId));
}
const encKeyValidation = new EncString(results.encKeyValidation_DO_NOT_EDIT);
try {
@@ -83,7 +84,7 @@ export class BitwardenJsonImporter extends BaseImporter implements Importer {
}
const groupingsMap = this.organization
? await this.parseCollections(results as BitwardenEncryptedOrgJsonExport)
? await this.parseCollections(userId, results as BitwardenEncryptedOrgJsonExport)
: await this.parseFolders(results as BitwardenEncryptedIndividualJsonExport);
for (const c of results.items) {
@@ -114,7 +115,7 @@ export class BitwardenJsonImporter extends BaseImporter implements Importer {
});
}
const view = await this.cipherService.decrypt(cipher, account.id);
const view = await this.cipherService.decrypt(cipher, userId);
this.cleanupCipher(view);
this.result.ciphers.push(view);
}
@@ -125,8 +126,10 @@ export class BitwardenJsonImporter extends BaseImporter implements Importer {
private async parseDecrypted(
results: BitwardenUnEncryptedIndividualJsonExport | BitwardenUnEncryptedOrgJsonExport,
) {
const userId = await firstValueFrom(getUserId(this.accountService.activeAccount$));
const groupingsMap = this.organization
? await this.parseCollections(results as BitwardenUnEncryptedOrgJsonExport)
? await this.parseCollections(userId, results as BitwardenUnEncryptedOrgJsonExport)
: await this.parseFolders(results as BitwardenUnEncryptedIndividualJsonExport);
results.items.forEach((c) => {
@@ -193,12 +196,17 @@ export class BitwardenJsonImporter extends BaseImporter implements Importer {
}
private async parseCollections(
userId: UserId,
data: BitwardenUnEncryptedOrgJsonExport | BitwardenEncryptedOrgJsonExport,
): Promise<Map<string, number>> | null {
if (data.collections == null) {
return null;
}
const orgKeys = await firstValueFrom(
this.keyService.orgKeys$(userId).pipe(filter((orgKeys) => orgKeys != null)),
);
const groupingsMap = new Map<string, number>();
for (const c of data.collections) {
@@ -212,12 +220,9 @@ export class BitwardenJsonImporter extends BaseImporter implements Importer {
organizationId: this.organizationId,
}),
);
const collection$ = this.keyService.activeUserOrgKeys$.pipe(
// FIXME: replace type assertion with narrowing
map((keys) => keys[c.organizationId as OrganizationId]),
concatMap((key) => collection.decrypt(key, this.encryptService)),
);
collectionView = await firstValueFrom(collection$);
const orgKey = orgKeys[c.organizationId];
collectionView = await collection.decrypt(orgKey, this.encryptService);
} else {
collectionView = CollectionWithIdExport.toView(c);
collectionView.organizationId = null;

View File

@@ -47,7 +47,7 @@ describe("BitwardenPasswordProtectedImporter", () => {
});
const mockOrgId = emptyGuid as OrganizationId;
/*
/*
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
*/
@@ -58,9 +58,6 @@ describe("BitwardenPasswordProtectedImporter", () => {
of({ [mockOrgId]: mockOrgKey } as Record<OrganizationId, OrgKey>),
);
keyService.userKey$.mockImplementation(() => of(mockUserKey));
(keyService as any).activeUserOrgKeys$ = of({
[mockOrgId]: mockOrgKey,
} as Record<OrganizationId, OrgKey>);
/*
Crypto isnt under test here; keys are just placeholders.

View File

@@ -220,13 +220,6 @@ export abstract class KeyService {
providerOrgs: ProfileProviderOrganizationResponse[],
userId: UserId,
): Promise<void>;
/**
* Retrieves a stream of the active users organization keys,
* will NOT emit any value if there is no active user.
*
* @deprecated Use {@link orgKeys$} with a required {@link UserId} instead.
*/
abstract activeUserOrgKeys$: Observable<Record<OrganizationId, OrgKey>>;
/**
* Returns the organization's symmetric key

View File

@@ -68,7 +68,14 @@ import {
import { KdfConfig } from "./models/kdf-config";
export class DefaultKeyService implements KeyServiceAbstraction {
readonly activeUserOrgKeys$: Observable<Record<OrganizationId, OrgKey>>;
/**
* Retrieves a stream of the active users organization keys,
* will NOT emit any value if there is no active user.
*
* @deprecated Use {@link orgKeys$} with a required {@link UserId} instead.
* TODO to be removed with https://bitwarden.atlassian.net/browse/PM-23623
*/
private readonly activeUserOrgKeys$: Observable<Record<OrganizationId, OrgKey>>;
constructor(
protected masterPasswordService: InternalMasterPasswordServiceAbstraction,

View File

@@ -1,7 +1,7 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import * as papa from "papaparse";
import { firstValueFrom, map } from "rxjs";
import { filter, firstValueFrom, map } from "rxjs";
import {
CollectionService,
@@ -137,6 +137,10 @@ export class OrganizationVaultExportService
const decCiphers: CipherView[] = [];
const promises = [];
const orgKeys = await firstValueFrom(
this.keyService.orgKeys$(activeUserId).pipe(filter((orgKeys) => orgKeys != null)),
);
const restrictions = await firstValueFrom(this.restrictedItemTypesService.restricted$);
promises.push(
@@ -148,12 +152,11 @@ export class OrganizationVaultExportService
const collection = Collection.fromCollectionData(
new CollectionData(c as CollectionDetailsResponse),
);
const orgKey = orgKeys[organizationId];
exportPromises.push(
firstValueFrom(this.keyService.activeUserOrgKeys$)
.then((keys) => collection.decrypt(keys[organizationId], this.encryptService))
.then((decCol) => {
decCollections.push(decCol);
}),
collection.decrypt(orgKey, this.encryptService).then((decCol) => {
decCollections.push(decCol);
}),
);
});
}