mirror of
https://github.com/bitwarden/browser
synced 2026-01-05 10:03:21 +00:00
[PM-5362]Create MP Service for state provider migration (#7623)
* create mp and kdf service * update mp service interface to not rely on active user * rename observable methods * update crypto service with new MP service * add master password service to login strategies - make fake service for easier testing - fix crypto service tests * update auth service and finish strategies * auth request refactors * more service refactors and constructor updates * setMasterKey refactors * remove master key methods from crypto service * remove master key and hash from state service * missed fixes * create migrations and fix references * fix master key imports * default force set password reason to none * add password reset reason observable factory to service * remove kdf changes and migrate only disk data * update migration number * fix sync service deps * use disk for force set password state * fix desktop migration * fix sso test * fix tests * fix more tests * fix even more tests * fix even more tests * fix cli * remove kdf service abstraction * add missing deps for browser * fix merge conflicts * clear reset password reason on lock or logout * fix tests * fix other tests * add jsdocs to abstraction * use state provider in crypto service * inverse master password service factory * add clearOn to master password service * add parameter validation to master password service * add component level userId * add missed userId * migrate key hash * fix login strategy service * delete crypto master key from account * migrate master key encrypted user key * rename key hash to master key hash * use mp service for getMasterKeyEncryptedUserKey * fix tests
This commit is contained in:
Jake Fink
GitHub
@@ -2,13 +2,15 @@ import { mock } from "jest-mock-extended";
|
||||
|
||||
import { ApiService } from "@bitwarden/common/abstractions/api.service";
|
||||
import { AuthRequestResponse } from "@bitwarden/common/auth/models/response/auth-request.response";
|
||||
import { FakeMasterPasswordService } from "@bitwarden/common/auth/services/master-password/fake-master-password.service";
|
||||
import { AuthRequestPushNotification } from "@bitwarden/common/models/response/notification.response";
|
||||
import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service";
|
||||
import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service";
|
||||
import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
|
||||
import { Utils } from "@bitwarden/common/platform/misc/utils";
|
||||
import { EncString } from "@bitwarden/common/platform/models/domain/enc-string";
|
||||
import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key";
|
||||
import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/spec";
|
||||
import { UserId } from "@bitwarden/common/types/guid";
|
||||
import { MasterKey, UserKey } from "@bitwarden/common/types/key";
|
||||
|
||||
import { AuthRequestService } from "./auth-request.service";
|
||||
@@ -16,17 +18,27 @@ import { AuthRequestService } from "./auth-request.service";
|
||||
describe("AuthRequestService", () => {
|
||||
let sut: AuthRequestService;
|
||||
|
||||
let accountService: FakeAccountService;
|
||||
let masterPasswordService: FakeMasterPasswordService;
|
||||
const appIdService = mock<AppIdService>();
|
||||
const cryptoService = mock<CryptoService>();
|
||||
const apiService = mock<ApiService>();
|
||||
const stateService = mock<StateService>();
|
||||
|
||||
let mockPrivateKey: Uint8Array;
|
||||
const mockUserId = Utils.newGuid() as UserId;
|
||||
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
accountService = mockAccountServiceWith(mockUserId);
|
||||
masterPasswordService = new FakeMasterPasswordService();
|
||||
|
||||
sut = new AuthRequestService(appIdService, cryptoService, apiService, stateService);
|
||||
sut = new AuthRequestService(
|
||||
appIdService,
|
||||
accountService,
|
||||
masterPasswordService,
|
||||
cryptoService,
|
||||
apiService,
|
||||
);
|
||||
|
||||
mockPrivateKey = new Uint8Array(64);
|
||||
});
|
||||
@@ -67,8 +79,8 @@ describe("AuthRequestService", () => {
|
||||
});
|
||||
|
||||
it("should use the master key and hash if they exist", async () => {
|
||||
cryptoService.getMasterKey.mockResolvedValueOnce({ encKey: new Uint8Array(64) } as MasterKey);
|
||||
stateService.getKeyHash.mockResolvedValueOnce("KEY_HASH");
|
||||
masterPasswordService.masterKeySubject.next({ encKey: new Uint8Array(64) } as MasterKey);
|
||||
masterPasswordService.masterKeyHashSubject.next("MASTER_KEY_HASH");
|
||||
|
||||
await sut.approveOrDenyAuthRequest(
|
||||
true,
|
||||
@@ -130,8 +142,8 @@ describe("AuthRequestService", () => {
|
||||
masterKeyHash: mockDecryptedMasterKeyHash,
|
||||
});
|
||||
|
||||
cryptoService.setMasterKey.mockResolvedValueOnce(undefined);
|
||||
cryptoService.setMasterKeyHash.mockResolvedValueOnce(undefined);
|
||||
masterPasswordService.masterKeySubject.next(undefined);
|
||||
masterPasswordService.masterKeyHashSubject.next(undefined);
|
||||
cryptoService.decryptUserKeyWithMasterKey.mockResolvedValueOnce(mockDecryptedUserKey);
|
||||
cryptoService.setUserKey.mockResolvedValueOnce(undefined);
|
||||
|
||||
@@ -144,10 +156,18 @@ describe("AuthRequestService", () => {
|
||||
mockAuthReqResponse.masterPasswordHash,
|
||||
mockPrivateKey,
|
||||
);
|
||||
expect(cryptoService.setMasterKey).toBeCalledWith(mockDecryptedMasterKey);
|
||||
expect(cryptoService.setMasterKeyHash).toBeCalledWith(mockDecryptedMasterKeyHash);
|
||||
expect(cryptoService.decryptUserKeyWithMasterKey).toBeCalledWith(mockDecryptedMasterKey);
|
||||
expect(cryptoService.setUserKey).toBeCalledWith(mockDecryptedUserKey);
|
||||
expect(masterPasswordService.mock.setMasterKey).toHaveBeenCalledWith(
|
||||
mockDecryptedMasterKey,
|
||||
mockUserId,
|
||||
);
|
||||
expect(masterPasswordService.mock.setMasterKeyHash).toHaveBeenCalledWith(
|
||||
mockDecryptedMasterKeyHash,
|
||||
mockUserId,
|
||||
);
|
||||
expect(cryptoService.decryptUserKeyWithMasterKey).toHaveBeenCalledWith(
|
||||
mockDecryptedMasterKey,
|
||||
);
|
||||
expect(cryptoService.setUserKey).toHaveBeenCalledWith(mockDecryptedUserKey);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -1,12 +1,13 @@
|
||||
import { Observable, Subject } from "rxjs";
|
||||
import { firstValueFrom, Observable, Subject } from "rxjs";
|
||||
|
||||
import { ApiService } from "@bitwarden/common/abstractions/api.service";
|
||||
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
|
||||
import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/auth/abstractions/master-password.service.abstraction";
|
||||
import { PasswordlessAuthRequest } from "@bitwarden/common/auth/models/request/passwordless-auth.request";
|
||||
import { AuthRequestResponse } from "@bitwarden/common/auth/models/response/auth-request.response";
|
||||
import { AuthRequestPushNotification } from "@bitwarden/common/models/response/notification.response";
|
||||
import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service";
|
||||
import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service";
|
||||
import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
|
||||
import { Utils } from "@bitwarden/common/platform/misc/utils";
|
||||
import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key";
|
||||
import { MasterKey, UserKey } from "@bitwarden/common/types/key";
|
||||
@@ -19,9 +20,10 @@ export class AuthRequestService implements AuthRequestServiceAbstraction {
|
||||
|
||||
constructor(
|
||||
private appIdService: AppIdService,
|
||||
private accountService: AccountService,
|
||||
private masterPasswordService: InternalMasterPasswordServiceAbstraction,
|
||||
private cryptoService: CryptoService,
|
||||
private apiService: ApiService,
|
||||
private stateService: StateService,
|
||||
) {
|
||||
this.authRequestPushNotification$ = this.authRequestPushNotificationSubject.asObservable();
|
||||
}
|
||||
@@ -38,8 +40,9 @@ export class AuthRequestService implements AuthRequestServiceAbstraction {
|
||||
}
|
||||
const pubKey = Utils.fromB64ToArray(authRequest.publicKey);
|
||||
|
||||
const masterKey = await this.cryptoService.getMasterKey();
|
||||
const masterKeyHash = await this.stateService.getKeyHash();
|
||||
const userId = (await firstValueFrom(this.accountService.activeAccount$)).id;
|
||||
const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId));
|
||||
const masterKeyHash = await firstValueFrom(this.masterPasswordService.masterKeyHash$(userId));
|
||||
let encryptedMasterKeyHash;
|
||||
let keyToEncrypt;
|
||||
|
||||
@@ -92,8 +95,9 @@ export class AuthRequestService implements AuthRequestServiceAbstraction {
|
||||
const userKey = await this.cryptoService.decryptUserKeyWithMasterKey(masterKey);
|
||||
|
||||
// Set masterKey + masterKeyHash in state after decryption (in case decryption fails)
|
||||
await this.cryptoService.setMasterKey(masterKey);
|
||||
await this.cryptoService.setMasterKeyHash(masterKeyHash);
|
||||
const userId = (await firstValueFrom(this.accountService.activeAccount$)).id;
|
||||
await this.masterPasswordService.setMasterKey(masterKey, userId);
|
||||
await this.masterPasswordService.setMasterKeyHash(masterKeyHash, userId);
|
||||
|
||||
await this.cryptoService.setUserKey(userKey);
|
||||
}
|
||||
|
||||
@@ -11,6 +11,7 @@ import { AuthResult } from "@bitwarden/common/auth/models/domain/auth-result";
|
||||
import { TokenTwoFactorRequest } from "@bitwarden/common/auth/models/request/identity-token/token-two-factor.request";
|
||||
import { IdentityTokenResponse } from "@bitwarden/common/auth/models/response/identity-token.response";
|
||||
import { IdentityTwoFactorResponse } from "@bitwarden/common/auth/models/response/identity-two-factor.response";
|
||||
import { FakeMasterPasswordService } from "@bitwarden/common/auth/services/master-password/fake-master-password.service";
|
||||
import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service";
|
||||
import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service";
|
||||
import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service";
|
||||
@@ -22,8 +23,14 @@ import { MessagingService } from "@bitwarden/common/platform/abstractions/messag
|
||||
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
|
||||
import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
|
||||
import { KdfType } from "@bitwarden/common/platform/enums";
|
||||
import { FakeGlobalState, FakeGlobalStateProvider } from "@bitwarden/common/spec";
|
||||
import {
|
||||
FakeAccountService,
|
||||
FakeGlobalState,
|
||||
FakeGlobalStateProvider,
|
||||
mockAccountServiceWith,
|
||||
} from "@bitwarden/common/spec";
|
||||
import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength";
|
||||
import { UserId } from "@bitwarden/common/types/guid";
|
||||
|
||||
import {
|
||||
AuthRequestServiceAbstraction,
|
||||
@@ -38,6 +45,8 @@ import { CACHE_EXPIRATION_KEY } from "./login-strategy.state";
|
||||
describe("LoginStrategyService", () => {
|
||||
let sut: LoginStrategyService;
|
||||
|
||||
let accountService: FakeAccountService;
|
||||
let masterPasswordService: FakeMasterPasswordService;
|
||||
let cryptoService: MockProxy<CryptoService>;
|
||||
let apiService: MockProxy<ApiService>;
|
||||
let tokenService: MockProxy<TokenService>;
|
||||
@@ -61,7 +70,11 @@ describe("LoginStrategyService", () => {
|
||||
let stateProvider: FakeGlobalStateProvider;
|
||||
let loginStrategyCacheExpirationState: FakeGlobalState<Date | null>;
|
||||
|
||||
const userId = "USER_ID" as UserId;
|
||||
|
||||
beforeEach(() => {
|
||||
accountService = mockAccountServiceWith(userId);
|
||||
masterPasswordService = new FakeMasterPasswordService();
|
||||
cryptoService = mock<CryptoService>();
|
||||
apiService = mock<ApiService>();
|
||||
tokenService = mock<TokenService>();
|
||||
@@ -84,6 +97,8 @@ describe("LoginStrategyService", () => {
|
||||
stateProvider = new FakeGlobalStateProvider();
|
||||
|
||||
sut = new LoginStrategyService(
|
||||
accountService,
|
||||
masterPasswordService,
|
||||
cryptoService,
|
||||
apiService,
|
||||
tokenService,
|
||||
|
||||
@@ -9,8 +9,10 @@ import {
|
||||
|
||||
import { ApiService } from "@bitwarden/common/abstractions/api.service";
|
||||
import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
|
||||
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
|
||||
import { DeviceTrustCryptoServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust-crypto.service.abstraction";
|
||||
import { KeyConnectorService } from "@bitwarden/common/auth/abstractions/key-connector.service";
|
||||
import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/auth/abstractions/master-password.service.abstraction";
|
||||
import { TokenService } from "@bitwarden/common/auth/abstractions/token.service";
|
||||
import { TwoFactorService } from "@bitwarden/common/auth/abstractions/two-factor.service";
|
||||
import { AuthenticationType } from "@bitwarden/common/auth/enums/authentication-type";
|
||||
@@ -81,6 +83,8 @@ export class LoginStrategyService implements LoginStrategyServiceAbstraction {
|
||||
currentAuthType$: Observable<AuthenticationType | null>;
|
||||
|
||||
constructor(
|
||||
protected accountService: AccountService,
|
||||
protected masterPasswordService: InternalMasterPasswordServiceAbstraction,
|
||||
protected cryptoService: CryptoService,
|
||||
protected apiService: ApiService,
|
||||
protected tokenService: TokenService,
|
||||
@@ -257,7 +261,8 @@ export class LoginStrategyService implements LoginStrategyServiceAbstraction {
|
||||
): Promise<AuthRequestResponse> {
|
||||
const pubKey = Utils.fromB64ToArray(key);
|
||||
|
||||
const masterKey = await this.cryptoService.getMasterKey();
|
||||
const userId = (await firstValueFrom(this.accountService.activeAccount$)).id;
|
||||
const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId));
|
||||
let keyToEncrypt;
|
||||
let encryptedMasterKeyHash = null;
|
||||
|
||||
@@ -266,7 +271,7 @@ export class LoginStrategyService implements LoginStrategyServiceAbstraction {
|
||||
|
||||
// Only encrypt the master password hash if masterKey exists as
|
||||
// we won't have a masterKeyHash without a masterKey
|
||||
const masterKeyHash = await this.stateService.getKeyHash();
|
||||
const masterKeyHash = await firstValueFrom(this.masterPasswordService.masterKeyHash$(userId));
|
||||
if (masterKeyHash != null) {
|
||||
encryptedMasterKeyHash = await this.cryptoService.rsaEncrypt(
|
||||
Utils.fromUtf8ToArray(masterKeyHash),
|
||||
@@ -333,6 +338,8 @@ export class LoginStrategyService implements LoginStrategyServiceAbstraction {
|
||||
case AuthenticationType.Password:
|
||||
return new PasswordLoginStrategy(
|
||||
data?.password,
|
||||
this.accountService,
|
||||
this.masterPasswordService,
|
||||
this.cryptoService,
|
||||
this.apiService,
|
||||
this.tokenService,
|
||||
@@ -351,6 +358,8 @@ export class LoginStrategyService implements LoginStrategyServiceAbstraction {
|
||||
case AuthenticationType.Sso:
|
||||
return new SsoLoginStrategy(
|
||||
data?.sso,
|
||||
this.accountService,
|
||||
this.masterPasswordService,
|
||||
this.cryptoService,
|
||||
this.apiService,
|
||||
this.tokenService,
|
||||
@@ -370,6 +379,8 @@ export class LoginStrategyService implements LoginStrategyServiceAbstraction {
|
||||
case AuthenticationType.UserApiKey:
|
||||
return new UserApiLoginStrategy(
|
||||
data?.userApiKey,
|
||||
this.accountService,
|
||||
this.masterPasswordService,
|
||||
this.cryptoService,
|
||||
this.apiService,
|
||||
this.tokenService,
|
||||
@@ -387,6 +398,8 @@ export class LoginStrategyService implements LoginStrategyServiceAbstraction {
|
||||
case AuthenticationType.AuthRequest:
|
||||
return new AuthRequestLoginStrategy(
|
||||
data?.authRequest,
|
||||
this.accountService,
|
||||
this.masterPasswordService,
|
||||
this.cryptoService,
|
||||
this.apiService,
|
||||
this.tokenService,
|
||||
@@ -403,6 +416,8 @@ export class LoginStrategyService implements LoginStrategyServiceAbstraction {
|
||||
case AuthenticationType.WebAuthn:
|
||||
return new WebAuthnLoginStrategy(
|
||||
data?.webAuthn,
|
||||
this.accountService,
|
||||
this.masterPasswordService,
|
||||
this.cryptoService,
|
||||
this.apiService,
|
||||
this.tokenService,
|
||||
|
||||
Reference in New Issue
Block a user