Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโ€™ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PM-11764] Implement account switching and sdk initialization #11472

Merged
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions libs/angular/src/services/jslib-services.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1334,6 +1334,9 @@ const safeProviders: SafeProvider[] = [
SdkClientFactory,
EnvironmentService,
PlatformUtilsServiceAbstraction,
AccountServiceAbstraction,
KdfConfigServiceAbstraction,
CryptoServiceAbstraction,
ApiServiceAbstraction,
],
}),
Expand Down
7 changes: 5 additions & 2 deletions libs/common/src/auth/abstractions/kdf-config.service.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import { Observable } from "rxjs";

import { UserId } from "../../types/guid";
import { KdfConfig } from "../models/domain/kdf-config";

export abstract class KdfConfigService {
setKdfConfig: (userId: UserId, KdfConfig: KdfConfig) => Promise<void>;
getKdfConfig: () => Promise<KdfConfig>;
abstract setKdfConfig(userId: UserId, KdfConfig: KdfConfig): Promise<void>;
abstract getKdfConfig(): Promise<KdfConfig>;
abstract getKdfConfig$(userId: UserId): Observable<KdfConfig>;
}
6 changes: 5 additions & 1 deletion libs/common/src/auth/services/kdf-config.service.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { firstValueFrom } from "rxjs";
import { firstValueFrom, Observable } from "rxjs";

import { KdfType } from "../../platform/enums/kdf-type.enum";
import { KDF_CONFIG_DISK, StateProvider, UserKeyDefinition } from "../../platform/state";
Expand Down Expand Up @@ -38,4 +38,8 @@ export class KdfConfigService implements KdfConfigServiceAbstraction {
}
return state;
}

getKdfConfig$(userId: UserId): Observable<KdfConfig> {
return this.stateProvider.getUser(userId, KDF_CONFIG).state$;
}
}
26 changes: 25 additions & 1 deletion libs/common/src/platform/abstractions/crypto.service.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Observable } from "rxjs";

import { EncryptedOrganizationKeyData } from "../../admin-console/models/data/encrypted-organization-key.data";
import { ProfileOrganizationResponse } from "../../admin-console/models/response/profile-organization.response";
import { ProfileProviderOrganizationResponse } from "../../admin-console/models/response/profile-provider-organization.response";
import { ProfileProviderResponse } from "../../admin-console/models/response/profile-provider.response";
Expand All @@ -15,7 +16,7 @@ import {
UserPublicKey,
} from "../../types/key";
import { KeySuffixOptions, HashPurpose } from "../enums";
import { EncString } from "../models/domain/enc-string";
import { EncryptedString, EncString } from "../models/domain/enc-string";
import { SymmetricCryptoKey } from "../models/domain/symmetric-crypto-key";

export class UserPrivateKeyDecryptionFailedError extends Error {
Expand Down Expand Up @@ -288,6 +289,17 @@ export abstract class CryptoService {
*/
abstract userPrivateKey$(userId: UserId): Observable<UserPrivateKey>;

/**
* Gets an observable stream of the given users encrypted private key, will emit null if the user
* doesn't have an encrypted private key at all.
*
* @param userId The user id of the user to get the data for.
*
* @deprecated Temporary function to allow the SDK to be initialized after the login process, it
* will be removed when auth has been migrated to the SDK.
*/
abstract userEncryptedPrivateKey$(userId: UserId): Observable<EncryptedString>;

/**
* Gets an observable stream of the given users decrypted private key with legacy support,
* will emit null if the user doesn't have a UserKey to decrypt the encrypted private key
Expand Down Expand Up @@ -381,6 +393,18 @@ export abstract class CryptoService {
*/
abstract orgKeys$(userId: UserId): Observable<Record<OrganizationId, OrgKey> | null>;

/**
* Gets an observable stream of the given users encrypted organisation keys.
*
* @param userId The user id of the user to get the data for.
*
* @deprecated Temporary function to allow the SDK to be initialized after the login process, it
* will be removed when auth has been migrated to the SDK.
*/
abstract encryptedOrgKeys$(
userId: UserId,
): Observable<Record<OrganizationId, EncryptedOrganizationKeyData>>;

/**
* Gets an observable stream of the users public key. If the user is does not have
* a {@link UserKey} or {@link UserPrivateKey} that is decryptable, this will emit null.
Expand Down
20 changes: 19 additions & 1 deletion libs/common/src/platform/abstractions/sdk/sdk.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,27 @@ import { Observable } from "rxjs";

import { BitwardenClient } from "@bitwarden/sdk-internal";

import { UserId } from "../../../types/guid";

export abstract class SdkService {
client$: Observable<BitwardenClient>;
/**
* Check if the SDK is supported in the current environment.
*/
supported$: Observable<boolean>;

/**
* Retrieve a client initialized without a user.
* This client can only be used for operations that don't require a user context.
*/
client$: Observable<BitwardenClient | undefined>;

/**
* Retrieve a client initialized for a specific user.
* This client can be used for operations that require a user context, such as retrieving ciphers
* and operations involving crypto. It can also be used for operations that don't require a user context.
* @param userId
*/
abstract userClient$(userId: UserId): Observable<BitwardenClient>;

abstract failedToInitialize(): Promise<void>;
}
10 changes: 10 additions & 0 deletions libs/common/src/platform/services/crypto.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -841,6 +841,10 @@ export class CryptoService implements CryptoServiceAbstraction {
return this.userPrivateKeyHelper$(userId, false).pipe(map((keys) => keys?.userPrivateKey));
}

userEncryptedPrivateKey$(userId: UserId): Observable<EncryptedString> {
return this.stateProvider.getUser(userId, USER_ENCRYPTED_PRIVATE_KEY).state$;
}

userPrivateKeyWithLegacySupport$(userId: UserId): Observable<UserPrivateKey> {
return this.userPrivateKeyHelper$(userId, true).pipe(map((keys) => keys?.userPrivateKey));
}
Expand Down Expand Up @@ -929,6 +933,12 @@ export class CryptoService implements CryptoServiceAbstraction {
return this.cipherDecryptionKeys$(userId, true).pipe(map((keys) => keys?.orgKeys));
}

encryptedOrgKeys$(
userId: UserId,
): Observable<Record<OrganizationId, EncryptedOrganizationKeyData>> {
return this.stateProvider.getUser(userId, USER_ENCRYPTED_ORGANIZATION_KEYS).state$;
}

cipherDecryptionKeys$(
userId: UserId,
legacySupport: boolean = false,
Expand Down
125 changes: 115 additions & 10 deletions libs/common/src/platform/services/sdk/default-sdk.service.ts
Original file line number Diff line number Diff line change
@@ -1,24 +1,40 @@
import { concatMap, firstValueFrom, shareReplay } from "rxjs";
import {
combineLatest,
concatMap,
firstValueFrom,
Observable,
shareReplay,
map,
distinctUntilChanged,
tap,
} from "rxjs";

import { LogLevel, DeviceType as SdkDeviceType } from "@bitwarden/sdk-internal";
import {
BitwardenClient,
ClientSettings,
LogLevel,
DeviceType as SdkDeviceType,
} from "@bitwarden/sdk-internal";

import { ApiService } from "../../../abstractions/api.service";
import { AccountService } from "../../../auth/abstractions/account.service";
import { KdfConfigService } from "../../../auth/abstractions/kdf-config.service";
import { DeviceType } from "../../../enums/device-type.enum";
import { EnvironmentService } from "../../abstractions/environment.service";
import { UserId } from "../../../types/guid";
import { CryptoService } from "../../abstractions/crypto.service";
import { Environment, EnvironmentService } from "../../abstractions/environment.service";
import { PlatformUtilsService } from "../../abstractions/platform-utils.service";
import { SdkClientFactory } from "../../abstractions/sdk/sdk-client-factory";
import { SdkService } from "../../abstractions/sdk/sdk.service";
import { KdfType } from "../../enums";
import { compareValues } from "../../misc/compare-values";

export class DefaultSdkService implements SdkService {
private sdkClientCache = new Map<UserId, Observable<BitwardenClient>>();

client$ = this.environmentService.environment$.pipe(
concatMap(async (env) => {
const settings = {
apiUrl: env.getApiUrl(),
identityUrl: env.getIdentityUrl(),
deviceType: this.toDevice(this.platformUtilsService.getDevice()),
userAgent: this.userAgent ?? navigator.userAgent,
};

const settings = this.toSettings(env);
return await this.sdkClientFactory.createSdkClient(settings, LogLevel.Info);
}),
shareReplay({ refCount: true, bufferSize: 1 }),
Expand All @@ -34,10 +50,90 @@
private sdkClientFactory: SdkClientFactory,
private environmentService: EnvironmentService,
private platformUtilsService: PlatformUtilsService,
private accountService: AccountService,
private kdfConfigService: KdfConfigService,
private cryptoService: CryptoService,
private apiService: ApiService, // Yes we shouldn't import ApiService, but it's temporary
private userAgent: string = null,
) {}

userClient$(userId: UserId): Observable<BitwardenClient | undefined> {
// TODO: Figure out what happens when the user logs out
if (this.sdkClientCache.has(userId)) {
return this.sdkClientCache.get(userId);
}

const account$ = this.accountService.accounts$.pipe(
map((accounts) => accounts[userId]),
distinctUntilChanged(),
);
const kdfParams$ = this.kdfConfigService.getKdfConfig$(userId).pipe(distinctUntilChanged());
const privateKey$ = this.cryptoService
.userEncryptedPrivateKey$(userId)
.pipe(distinctUntilChanged());
const userKey$ = this.cryptoService.userKey$(userId).pipe(distinctUntilChanged());
const orgKeys$ = this.cryptoService.encryptedOrgKeys$(userId).pipe(
distinctUntilChanged(compareValues), // The upstream observable emits different objects with the same values
);

const client$ = combineLatest([
this.environmentService.environment$,
account$,
kdfParams$,
privateKey$,
userKey$,
orgKeys$,
]).pipe(
concatMap(async ([env, account, kdfParams, privateKey, userKey, orgKeys]) => {
if (privateKey == null || userKey == null || orgKeys == null) {
return undefined;
}

const settings = this.toSettings(env);
const client = await this.sdkClientFactory.createSdkClient(settings, LogLevel.Info);

await client.crypto().initialize_user_crypto({

Check failure on line 95 in libs/common/src/platform/services/sdk/default-sdk.service.ts

View workflow job for this annotation

GitHub Actions / Chromatic

Property 'crypto' does not exist on type 'BitwardenClient'.
email: account.email,
method: { decryptedKey: { decrypted_user_key: userKey.keyB64 } },
kdfParams:
kdfParams.kdfType === KdfType.PBKDF2_SHA256
? {
pBKDF2: { iterations: kdfParams.iterations },
}
: {
argon2id: {
iterations: kdfParams.iterations,
memory: kdfParams.memory,
parallelism: kdfParams.parallelism,
},
},
privateKey,
});

await client.crypto().initialize_org_crypto({

Check failure on line 113 in libs/common/src/platform/services/sdk/default-sdk.service.ts

View workflow job for this annotation

GitHub Actions / Chromatic

Property 'crypto' does not exist on type 'BitwardenClient'.
organizationKeys: new Map(
Object.entries(orgKeys)
.filter(([_, v]) => v.type === "organization")
.map(([k, v]) => [k, v.key]),
),
});

return client;
}),
tap({
finalize: () => {
if (this.sdkClientCache.has(userId)) {
this.sdkClientCache.delete(userId);
}
coroiu marked this conversation as resolved.
Show resolved Hide resolved
},
}),
Copy link
Contributor Author

@coroiu coroiu Oct 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought: I'm not 100% happy with this, but I can't really see any better solutions short-of removing the cache all together or maybe writing some advanced separate utility for it. Caching is hard.

An alternative would be to remove this and let the cache grow (leak memory). I don't think it would ever be an issue since nobody logs in and out of so many accounts that it will actually become an issue, but @JaredSnider-Bitwarden challenged me on this approach and in the end I leaned towards not leaking.

shareReplay({ refCount: true, bufferSize: 1 }),
);

this.sdkClientCache.set(userId, client$);
return client$;
}

async failedToInitialize(): Promise<void> {
// Only log on cloud instances
if (
Expand All @@ -52,6 +148,15 @@
});
}

private toSettings(env: Environment): ClientSettings {
return {
apiUrl: env.getApiUrl(),
identityUrl: env.getIdentityUrl(),
deviceType: this.toDevice(this.platformUtilsService.getDevice()),
userAgent: this.userAgent ?? navigator.userAgent,
};
}

private toDevice(device: DeviceType): SdkDeviceType {
switch (device) {
case DeviceType.Android:
Expand Down
Loading