bitwarden/browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser synced 2025-12-18 09:13:33 +00:00
Code Issues Releases Wiki Activity

Revert "[PM-5362]Create MP Service for state provider migration (#7623)" (#8617)

Browse Source 
This reverts commit b1abfb0a5c.
This commit is contained in:
Jake Fink
2024-04-04 12:17:09 -04:00
committed by GitHub
parent e2e593c0fe
commit 775c8a1bbe
79 changed files with 498 additions and 1340 deletions
Download Patch File Download Diff File Expand all files Collapse all files

67
libs/common/src/auth/abstractions/master-password.service.abstraction.ts
View File

@@ -1,67 +0,0 @@
import { Observable } from "rxjs";
import { EncString } from "../../platform/models/domain/enc-string";
import { UserId } from "../../types/guid";
import { MasterKey } from "../../types/key";
import { ForceSetPasswordReason } from "../models/domain/force-set-password-reason";
export abstract class MasterPasswordServiceAbstraction {
/**
* An observable that emits if the user is being forced to set a password on login and why.
* @param userId The user ID.
* @throws If the user ID is missing.
*/
abstract forceSetPasswordReason$: (userId: UserId) => Observable<ForceSetPasswordReason>;
/**
* An observable that emits the master key for the user.
* @param userId The user ID.
* @throws If the user ID is missing.
*/
abstract masterKey$: (userId: UserId) => Observable<MasterKey>;
/**
* An observable that emits the master key hash for the user.
* @param userId The user ID.
* @throws If the user ID is missing.
*/
abstract masterKeyHash$: (userId: UserId) => Observable<string>;
/**
* Returns the master key encrypted user key for the user.
* @param userId The user ID.
* @throws If the user ID is missing.
*/
abstract getMasterKeyEncryptedUserKey: (userId: UserId) => Promise<EncString>;
}
export abstract class InternalMasterPasswordServiceAbstraction extends MasterPasswordServiceAbstraction {
/**
* Set the master key for the user.
* @param masterKey The master key.
* @param userId The user ID.
* @throws If the user ID or master key is missing.
*/
abstract setMasterKey: (masterKey: MasterKey, userId: UserId) => Promise<void>;
/**
* Set the master key hash for the user.
* @param masterKeyHash The master key hash.
* @param userId The user ID.
* @throws If the user ID or master key hash is missing.
*/
abstract setMasterKeyHash: (masterKeyHash: string, userId: UserId) => Promise<void>;
/**
* Set the master key encrypted user key for the user.
* @param encryptedKey The master key encrypted user key.
* @param userId The user ID.
* @throws If the user ID or encrypted key is missing.
*/
abstract setMasterKeyEncryptedUserKey: (encryptedKey: EncString, userId: UserId) => Promise<void>;
/**
* Set the force set password reason for the user.
* @param reason The reason the user is being forced to set a password.
* @param userId The user ID.
* @throws If the user ID or reason is missing.
*/
abstract setForceSetPasswordReason: (
reason: ForceSetPasswordReason,
userId: UserId,
) => Promise<void>;
}

16
libs/common/src/auth/services/key-connector.service.spec.ts
View File

@@ -21,7 +21,6 @@ import {
CONVERT_ACCOUNT_TO_KEY_CONNECTOR,
KeyConnectorService,
} from "./key-connector.service";
import { FakeMasterPasswordService } from "./master-password/fake-master-password.service";
import { TokenService } from "./token.service";
describe("KeyConnectorService", () => {
@@ -37,7 +36,6 @@ describe("KeyConnectorService", () => {
let stateProvider: FakeStateProvider;
let accountService: FakeAccountService;
let masterPasswordService: FakeMasterPasswordService;
const mockUserId = Utils.newGuid() as UserId;
const mockOrgId = Utils.newGuid() as OrganizationId;
@@ -49,13 +47,10 @@ describe("KeyConnectorService", () => {
beforeEach(() => {
jest.clearAllMocks();
masterPasswordService = new FakeMasterPasswordService();
accountService = mockAccountServiceWith(mockUserId);
stateProvider = new FakeStateProvider(accountService);
keyConnectorService = new KeyConnectorService(
accountService,
masterPasswordService,
cryptoService,
apiService,
tokenService,
@@ -219,10 +214,7 @@ describe("KeyConnectorService", () => {
// Assert
expect(apiService.getMasterKeyFromKeyConnector).toHaveBeenCalledWith(url);
expect(masterPasswordService.mock.setMasterKey).toHaveBeenCalledWith(
masterKey,
expect.any(String),
);
expect(cryptoService.setMasterKey).toHaveBeenCalledWith(masterKey);
});
it("should handle errors thrown during the process", async () => {
@@ -249,10 +241,10 @@ describe("KeyConnectorService", () => {
// Arrange
const organization = organizationData(true, true, "https://key-connector-url.com", 2, false);
const masterKey = getMockMasterKey();
masterPasswordService.masterKeySubject.next(masterKey);
const keyConnectorRequest = new KeyConnectorUserKeyRequest(masterKey.encKeyB64);
jest.spyOn(keyConnectorService, "getManagingOrganization").mockResolvedValue(organization);
jest.spyOn(cryptoService, "getMasterKey").mockResolvedValue(masterKey);
jest.spyOn(apiService, "postUserKeyToKeyConnector").mockResolvedValue();
// Act
@@ -260,6 +252,7 @@ describe("KeyConnectorService", () => {
// Assert
expect(keyConnectorService.getManagingOrganization).toHaveBeenCalled();
expect(cryptoService.getMasterKey).toHaveBeenCalled();
expect(apiService.postUserKeyToKeyConnector).toHaveBeenCalledWith(
organization.keyConnectorUrl,
keyConnectorRequest,
@@ -275,8 +268,8 @@ describe("KeyConnectorService", () => {
const error = new Error("Failed to post user key to key connector");
organizationService.getAll.mockResolvedValue([organization]);
masterPasswordService.masterKeySubject.next(masterKey);
jest.spyOn(keyConnectorService, "getManagingOrganization").mockResolvedValue(organization);
jest.spyOn(cryptoService, "getMasterKey").mockResolvedValue(masterKey);
jest.spyOn(apiService, "postUserKeyToKeyConnector").mockRejectedValue(error);
jest.spyOn(logService, "error");
@@ -287,6 +280,7 @@ describe("KeyConnectorService", () => {
// Assert
expect(logService.error).toHaveBeenCalledWith(error);
expect(keyConnectorService.getManagingOrganization).toHaveBeenCalled();
expect(cryptoService.getMasterKey).toHaveBeenCalled();
expect(apiService.postUserKeyToKeyConnector).toHaveBeenCalledWith(
organization.keyConnectorUrl,
keyConnectorRequest,

13
libs/common/src/auth/services/key-connector.service.ts
View File

@@ -16,9 +16,7 @@ import {
UserKeyDefinition,
} from "../../platform/state";
import { MasterKey } from "../../types/key";
import { AccountService } from "../abstractions/account.service";
import { KeyConnectorService as KeyConnectorServiceAbstraction } from "../abstractions/key-connector.service";
import { InternalMasterPasswordServiceAbstraction } from "../abstractions/master-password.service.abstraction";
import { TokenService } from "../abstractions/token.service";
import { KdfConfig } from "../models/domain/kdf-config";
import { KeyConnectorUserKeyRequest } from "../models/request/key-connector-user-key.request";
@@ -47,8 +45,6 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction {
private usesKeyConnectorState: ActiveUserState<boolean>;
private convertAccountToKeyConnectorState: ActiveUserState<boolean>;
constructor(
private accountService: AccountService,
private masterPasswordService: InternalMasterPasswordServiceAbstraction,
private cryptoService: CryptoService,
private apiService: ApiService,
private tokenService: TokenService,
@@ -82,8 +78,7 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction {
async migrateUser() {
const organization = await this.getManagingOrganization();
const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id;
const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId));
const masterKey = await this.cryptoService.getMasterKey();
const keyConnectorRequest = new KeyConnectorUserKeyRequest(masterKey.encKeyB64);
try {
@@ -104,8 +99,7 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction {
const masterKeyResponse = await this.apiService.getMasterKeyFromKeyConnector(url);
const keyArr = Utils.fromB64ToArray(masterKeyResponse.key);
const masterKey = new SymmetricCryptoKey(keyArr) as MasterKey;
const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id;
await this.masterPasswordService.setMasterKey(masterKey, userId);
await this.cryptoService.setMasterKey(masterKey);
} catch (e) {
this.handleKeyConnectorError(e);
}
@@ -142,8 +136,7 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction {
kdfConfig,
);
const keyConnectorRequest = new KeyConnectorUserKeyRequest(masterKey.encKeyB64);
const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id;
await this.masterPasswordService.setMasterKey(masterKey, userId);
await this.cryptoService.setMasterKey(masterKey);
const userKey = await this.cryptoService.makeUserKey(masterKey);
await this.cryptoService.setUserKey(userKey[0]);

56
libs/common/src/auth/services/master-password/fake-master-password.service.ts
View File

@@ -1,56 +0,0 @@
import { mock } from "jest-mock-extended";
import { ReplaySubject, Observable } from "rxjs";
import { EncString } from "../../../platform/models/domain/enc-string";
import { UserId } from "../../../types/guid";
import { MasterKey } from "../../../types/key";
import { InternalMasterPasswordServiceAbstraction } from "../../abstractions/master-password.service.abstraction";
import { ForceSetPasswordReason } from "../../models/domain/force-set-password-reason";
export class FakeMasterPasswordService implements InternalMasterPasswordServiceAbstraction {
mock = mock<InternalMasterPasswordServiceAbstraction>();
// eslint-disable-next-line rxjs/no-exposed-subjects -- test class
masterKeySubject = new ReplaySubject<MasterKey>(1);
// eslint-disable-next-line rxjs/no-exposed-subjects -- test class
masterKeyHashSubject = new ReplaySubject<string>(1);
// eslint-disable-next-line rxjs/no-exposed-subjects -- test class
forceSetPasswordReasonSubject = new ReplaySubject<ForceSetPasswordReason>(1);
constructor(initialMasterKey?: MasterKey, initialMasterKeyHash?: string) {
this.masterKeySubject.next(initialMasterKey);
this.masterKeyHashSubject.next(initialMasterKeyHash);
}
masterKey$(userId: UserId): Observable<MasterKey> {
return this.masterKeySubject.asObservable();
}
setMasterKey(masterKey: MasterKey, userId: UserId): Promise<void> {
return this.mock.setMasterKey(masterKey, userId);
}
masterKeyHash$(userId: UserId): Observable<string> {
return this.masterKeyHashSubject.asObservable();
}
getMasterKeyEncryptedUserKey(userId: UserId): Promise<EncString> {
return this.mock.getMasterKeyEncryptedUserKey(userId);
}
setMasterKeyEncryptedUserKey(encryptedKey: EncString, userId: UserId): Promise<void> {
return this.mock.setMasterKeyEncryptedUserKey(encryptedKey, userId);
}
setMasterKeyHash(masterKeyHash: string, userId: UserId): Promise<void> {
return this.mock.setMasterKeyHash(masterKeyHash, userId);
}
forceSetPasswordReason$(userId: UserId): Observable<ForceSetPasswordReason> {
return this.forceSetPasswordReasonSubject.asObservable();
}
setForceSetPasswordReason(reason: ForceSetPasswordReason, userId: UserId): Promise<void> {
return this.mock.setForceSetPasswordReason(reason, userId);
}
}

125
libs/common/src/auth/services/master-password/master-password.service.ts
View File

@@ -1,125 +0,0 @@
import { firstValueFrom, map, Observable } from "rxjs";
import { EncString } from "../../../platform/models/domain/enc-string";
import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-crypto-key";
import {
MASTER_PASSWORD_DISK,
MASTER_PASSWORD_MEMORY,
StateProvider,
UserKeyDefinition,
} from "../../../platform/state";
import { UserId } from "../../../types/guid";
import { MasterKey } from "../../../types/key";
import { InternalMasterPasswordServiceAbstraction } from "../../abstractions/master-password.service.abstraction";
import { ForceSetPasswordReason } from "../../models/domain/force-set-password-reason";
/** Memory since master key shouldn't be available on lock */
const MASTER_KEY = new UserKeyDefinition<MasterKey>(MASTER_PASSWORD_MEMORY, "masterKey", {
deserializer: (masterKey) => SymmetricCryptoKey.fromJSON(masterKey) as MasterKey,
clearOn: ["lock", "logout"],
});
/** Disk since master key hash is used for unlock */
const MASTER_KEY_HASH = new UserKeyDefinition<string>(MASTER_PASSWORD_DISK, "masterKeyHash", {
deserializer: (masterKeyHash) => masterKeyHash,
clearOn: ["logout"],
});
const MASTER_KEY_ENCRYPTED_USER_KEY = new UserKeyDefinition<EncString>(
MASTER_PASSWORD_DISK,
"masterKeyEncryptedUserKey",
{
deserializer: (key) => EncString.fromJSON(key),
clearOn: ["logout"],
},
);
/** Disk to persist through lock and account switches */
const FORCE_SET_PASSWORD_REASON = new UserKeyDefinition<ForceSetPasswordReason>(
MASTER_PASSWORD_DISK,
"forceSetPasswordReason",
{
deserializer: (reason) => reason,
clearOn: ["logout"],
},
);
export class MasterPasswordService implements InternalMasterPasswordServiceAbstraction {
constructor(private stateProvider: StateProvider) {}
masterKey$(userId: UserId): Observable<MasterKey> {
if (userId == null) {
throw new Error("User ID is required.");
}
return this.stateProvider.getUser(userId, MASTER_KEY).state$;
}
masterKeyHash$(userId: UserId): Observable<string> {
if (userId == null) {
throw new Error("User ID is required.");
}
return this.stateProvider.getUser(userId, MASTER_KEY_HASH).state$;
}
forceSetPasswordReason$(userId: UserId): Observable<ForceSetPasswordReason> {
if (userId == null) {
throw new Error("User ID is required.");
}
return this.stateProvider
.getUser(userId, FORCE_SET_PASSWORD_REASON)
.state$.pipe(map((reason) => reason ?? ForceSetPasswordReason.None));
}
// TODO: Remove this method and decrypt directly in the service instead
async getMasterKeyEncryptedUserKey(userId: UserId): Promise<EncString> {
if (userId == null) {
throw new Error("User ID is required.");
}
const key = await firstValueFrom(
this.stateProvider.getUser(userId, MASTER_KEY_ENCRYPTED_USER_KEY).state$,
);
return key;
}
async setMasterKey(masterKey: MasterKey, userId: UserId): Promise<void> {
if (masterKey == null) {
throw new Error("Master key is required.");
}
if (userId == null) {
throw new Error("User ID is required.");
}
await this.stateProvider.getUser(userId, MASTER_KEY).update((_) => masterKey);
}
async setMasterKeyHash(masterKeyHash: string, userId: UserId): Promise<void> {
if (masterKeyHash == null) {
throw new Error("Master key hash is required.");
}
if (userId == null) {
throw new Error("User ID is required.");
}
await this.stateProvider.getUser(userId, MASTER_KEY_HASH).update((_) => masterKeyHash);
}
async setMasterKeyEncryptedUserKey(encryptedKey: EncString, userId: UserId): Promise<void> {
if (encryptedKey == null) {
throw new Error("Encrypted Key is required.");
}
if (userId == null) {
throw new Error("User ID is required.");
}
await this.stateProvider
.getUser(userId, MASTER_KEY_ENCRYPTED_USER_KEY)
.update((_) => encryptedKey);
}
async setForceSetPasswordReason(reason: ForceSetPasswordReason, userId: UserId): Promise<void> {
if (reason == null) {
throw new Error("Reason is required.");
}
if (userId == null) {
throw new Error("User ID is required.");
}
await this.stateProvider.getUser(userId, FORCE_SET_PASSWORD_REASON).update((_) => reason);
}
}

16
libs/common/src/auth/services/user-verification/user-verification.service.ts
View File

@@ -10,10 +10,7 @@ import { LogService } from "../../../platform/abstractions/log.service";
import { PlatformUtilsService } from "../../../platform/abstractions/platform-utils.service";
import { StateService } from "../../../platform/abstractions/state.service";
import { KeySuffixOptions } from "../../../platform/enums/key-suffix-options.enum";
import { UserId } from "../../../types/guid";
import { UserKey } from "../../../types/key";
import { AccountService } from "../../abstractions/account.service";
import { InternalMasterPasswordServiceAbstraction } from "../../abstractions/master-password.service.abstraction";
import { UserVerificationApiServiceAbstraction } from "../../abstractions/user-verification/user-verification-api.service.abstraction";
import { UserVerificationService as UserVerificationServiceAbstraction } from "../../abstractions/user-verification/user-verification.service.abstraction";
import { VerificationType } from "../../enums/verification-type";
@@ -38,8 +35,6 @@ export class UserVerificationService implements UserVerificationServiceAbstracti
constructor(
private stateService: StateService,
private cryptoService: CryptoService,
private accountService: AccountService,
private masterPasswordService: InternalMasterPasswordServiceAbstraction,
private i18nService: I18nService,
private userVerificationApiService: UserVerificationApiServiceAbstraction,
private userDecryptionOptionsService: UserDecryptionOptionsServiceAbstraction,
@@ -112,8 +107,7 @@ export class UserVerificationService implements UserVerificationServiceAbstracti
if (verification.type === VerificationType.OTP) {
request.otp = verification.secret;
} else {
const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id;
let masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId));
let masterKey = await this.cryptoService.getMasterKey();
if (!masterKey && !alreadyHashed) {
masterKey = await this.cryptoService.makeMasterKey(
verification.secret,
@@ -170,8 +164,7 @@ export class UserVerificationService implements UserVerificationServiceAbstracti
private async verifyUserByMasterPassword(
verification: MasterPasswordVerification,
): Promise<boolean> {
const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id;
let masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId));
let masterKey = await this.cryptoService.getMasterKey();
if (!masterKey) {
masterKey = await this.cryptoService.makeMasterKey(
verification.secret,
@@ -188,7 +181,7 @@ export class UserVerificationService implements UserVerificationServiceAbstracti
throw new Error(this.i18nService.t("invalidMasterPassword"));
}
// TODO: we should re-evaluate later on if user verification should have the side effect of modifying state. Probably not.
await this.masterPasswordService.setMasterKey(masterKey, userId);
await this.cryptoService.setMasterKey(masterKey);
return true;
}
@@ -237,10 +230,9 @@ export class UserVerificationService implements UserVerificationServiceAbstracti
}
async hasMasterPasswordAndMasterKeyHash(userId?: string): Promise<boolean> {
userId ??= (await firstValueFrom(this.accountService.activeAccount$))?.id;
return (
(await this.hasMasterPassword(userId)) &&
(await firstValueFrom(this.masterPasswordService.masterKeyHash$(userId as UserId))) != null
(await this.cryptoService.getMasterKeyHash()) != null
);
}