1
0
mirror of https://github.com/bitwarden/browser.git synced 2024-11-21 11:35:34 +01:00

Update decryptUserKeyWithMasterKey to requireUserId (#11560)

* Updated decryptUserKeyWithMasterKey to requireUserId

* Removed unintended extra character.

* Added dependency to LogService.

* Fixed unlock command.
This commit is contained in:
Todd Martin 2024-11-01 11:21:18 -04:00 committed by GitHub
parent 5eae599b81
commit a049b553a6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
19 changed files with 41 additions and 18 deletions

View File

@ -632,6 +632,7 @@ export default class MainBackground {
this.stateService, this.stateService,
this.keyGenerationService, this.keyGenerationService,
this.encryptService, this.encryptService,
this.logService,
); );
this.i18nService = new I18nService(BrowserApi.getUILanguage(), this.globalStateProvider); this.i18nService = new I18nService(BrowserApi.getUILanguage(), this.globalStateProvider);

View File

@ -68,7 +68,7 @@ export class UnlockCommand {
return Response.error(e.message); return Response.error(e.message);
} }
const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(masterKey); const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(masterKey, userId);
await this.keyService.setUserKey(userKey, userId); await this.keyService.setUserKey(userKey, userId);
if (await this.keyConnectorService.getConvertAccountRequired()) { if (await this.keyConnectorService.getConvertAccountRequired()) {

View File

@ -404,6 +404,7 @@ export class ServiceContainer {
this.stateService, this.stateService,
this.keyGenerationService, this.keyGenerationService,
this.encryptService, this.encryptService,
this.logService,
); );
this.kdfConfigService = new KdfConfigService(this.stateProvider); this.kdfConfigService = new KdfConfigService(this.stateProvider);

View File

@ -194,7 +194,7 @@ export class ChangePasswordComponent
HashPurpose.LocalAuthorization, HashPurpose.LocalAuthorization,
); );
const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(masterKey); const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(masterKey, userId);
if (userKey == null) { if (userKey == null) {
this.toastService.showToast({ this.toastService.showToast({
variant: "error", variant: "error",

View File

@ -267,6 +267,7 @@ export class LockComponent implements OnInit, OnDestroy {
const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey( const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(
response.masterKey, response.masterKey,
userId,
); );
await this.setUserKeyAndContinue(userKey, userId, true); await this.setUserKeyAndContinue(userKey, userId, true);
} }

View File

@ -921,7 +921,13 @@ const safeProviders: SafeProvider[] = [
safeProvider({ safeProvider({
provide: InternalMasterPasswordServiceAbstraction, provide: InternalMasterPasswordServiceAbstraction,
useClass: MasterPasswordService, useClass: MasterPasswordService,
deps: [StateProvider, StateServiceAbstraction, KeyGenerationServiceAbstraction, EncryptService], deps: [
StateProvider,
StateServiceAbstraction,
KeyGenerationServiceAbstraction,
EncryptService,
LogService,
],
}), }),
safeProvider({ safeProvider({
provide: MasterPasswordServiceAbstraction, provide: MasterPasswordServiceAbstraction,

View File

@ -483,6 +483,7 @@ export class LockV2Component implements OnInit, OnDestroy {
const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey( const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(
masterPasswordVerificationResponse.masterKey, masterPasswordVerificationResponse.masterKey,
this.activeAccount.id,
); );
await this.setUserKeyAndContinue(userKey, true); await this.setUserKeyAndContinue(userKey, true);
} }

View File

@ -114,7 +114,10 @@ export class AuthRequestLoginStrategy extends LoginStrategy {
private async trySetUserKeyWithMasterKey(userId: UserId): Promise<void> { private async trySetUserKeyWithMasterKey(userId: UserId): Promise<void> {
const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId)); const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId));
if (masterKey) { if (masterKey) {
const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(masterKey); const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(
masterKey,
userId,
);
await this.keyService.setUserKey(userKey, userId); await this.keyService.setUserKey(userKey, userId);
} }
} }

View File

@ -183,7 +183,10 @@ export class PasswordLoginStrategy extends LoginStrategy {
const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId)); const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId));
if (masterKey) { if (masterKey) {
const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(masterKey); const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(
masterKey,
userId,
);
await this.keyService.setUserKey(userKey, userId); await this.keyService.setUserKey(userKey, userId);
} }
} }

View File

@ -496,7 +496,7 @@ describe("SsoLoginStrategy", () => {
expect(masterPasswordService.mock.decryptUserKeyWithMasterKey).toHaveBeenCalledWith( expect(masterPasswordService.mock.decryptUserKeyWithMasterKey).toHaveBeenCalledWith(
masterKey, masterKey,
undefined, userId,
undefined, undefined,
); );
expect(keyService.setUserKey).toHaveBeenCalledWith(userKey, userId); expect(keyService.setUserKey).toHaveBeenCalledWith(userKey, userId);
@ -552,7 +552,7 @@ describe("SsoLoginStrategy", () => {
expect(masterPasswordService.mock.decryptUserKeyWithMasterKey).toHaveBeenCalledWith( expect(masterPasswordService.mock.decryptUserKeyWithMasterKey).toHaveBeenCalledWith(
masterKey, masterKey,
undefined, userId,
undefined, undefined,
); );
expect(keyService.setUserKey).toHaveBeenCalledWith(userKey, userId); expect(keyService.setUserKey).toHaveBeenCalledWith(userKey, userId);

View File

@ -338,7 +338,7 @@ export class SsoLoginStrategy extends LoginStrategy {
return; return;
} }
const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(masterKey); const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(masterKey, userId);
await this.keyService.setUserKey(userKey, userId); await this.keyService.setUserKey(userKey, userId);
} }

View File

@ -213,7 +213,7 @@ describe("UserApiLoginStrategy", () => {
expect(masterPasswordService.mock.decryptUserKeyWithMasterKey).toHaveBeenCalledWith( expect(masterPasswordService.mock.decryptUserKeyWithMasterKey).toHaveBeenCalledWith(
masterKey, masterKey,
undefined, userId,
undefined, undefined,
); );
expect(keyService.setUserKey).toHaveBeenCalledWith(userKey, userId); expect(keyService.setUserKey).toHaveBeenCalledWith(userKey, userId);

View File

@ -69,7 +69,10 @@ export class UserApiLoginStrategy extends LoginStrategy {
if (response.apiUseKeyConnector) { if (response.apiUseKeyConnector) {
const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId)); const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId));
if (masterKey) { if (masterKey) {
const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(masterKey); const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(
masterKey,
userId,
);
await this.keyService.setUserKey(userKey, userId); await this.keyService.setUserKey(userKey, userId);
} }
} }

View File

@ -200,7 +200,7 @@ describe("AuthRequestService", () => {
); );
expect(masterPasswordService.mock.decryptUserKeyWithMasterKey).toHaveBeenCalledWith( expect(masterPasswordService.mock.decryptUserKeyWithMasterKey).toHaveBeenCalledWith(
mockDecryptedMasterKey, mockDecryptedMasterKey,
undefined, mockUserId,
undefined, undefined,
); );
expect(keyService.setUserKey).toHaveBeenCalledWith(mockDecryptedUserKey, mockUserId); expect(keyService.setUserKey).toHaveBeenCalledWith(mockDecryptedUserKey, mockUserId);

View File

@ -150,7 +150,7 @@ export class AuthRequestService implements AuthRequestServiceAbstraction {
); );
// Decrypt and set user key in state // Decrypt and set user key in state
const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(masterKey); const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(masterKey, userId);
// Set masterKey + masterKeyHash in state after decryption (in case decryption fails) // Set masterKey + masterKeyHash in state after decryption (in case decryption fails)
await this.masterPasswordService.setMasterKey(masterKey, userId); await this.masterPasswordService.setMasterKey(masterKey, userId);

View File

@ -418,6 +418,7 @@ export class PinService implements PinServiceAbstraction {
const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey( const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(
masterKey, masterKey,
userId,
encUserKey ? new EncString(encUserKey) : undefined, encUserKey ? new EncString(encUserKey) : undefined,
); );

View File

@ -33,16 +33,16 @@ export abstract class MasterPasswordServiceAbstraction {
/** /**
* Decrypts the user key with the provided master key * Decrypts the user key with the provided master key
* @param masterKey The user's master key * @param masterKey The user's master key
* * @param userId The desired user
* @param userKey The user's encrypted symmetric key * @param userKey The user's encrypted symmetric key
* @param userId The desired user
* @throws If either the MasterKey or UserKey are not resolved, or if the UserKey encryption type * @throws If either the MasterKey or UserKey are not resolved, or if the UserKey encryption type
* is neither AesCbc256_B64 nor AesCbc256_HmacSha256_B64 * is neither AesCbc256_B64 nor AesCbc256_HmacSha256_B64
* @returns The user key * @returns The user key
*/ */
abstract decryptUserKeyWithMasterKey: ( abstract decryptUserKeyWithMasterKey: (
masterKey: MasterKey, masterKey: MasterKey,
userId: string,
userKey?: EncString, userKey?: EncString,
userId?: string,
) => Promise<UserKey>; ) => Promise<UserKey>;
} }

View File

@ -64,9 +64,9 @@ export class FakeMasterPasswordService implements InternalMasterPasswordServiceA
decryptUserKeyWithMasterKey( decryptUserKeyWithMasterKey(
masterKey: MasterKey, masterKey: MasterKey,
userId: string,
userKey?: EncString, userKey?: EncString,
userId?: string,
): Promise<UserKey> { ): Promise<UserKey> {
return this.mock.decryptUserKeyWithMasterKey(masterKey, userKey, userId); return this.mock.decryptUserKeyWithMasterKey(masterKey, userId, userKey);
} }
} }

View File

@ -1,5 +1,7 @@
import { firstValueFrom, map, Observable } from "rxjs"; import { firstValueFrom, map, Observable } from "rxjs";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { EncryptService } from "../../../platform/abstractions/encrypt.service"; import { EncryptService } from "../../../platform/abstractions/encrypt.service";
import { KeyGenerationService } from "../../../platform/abstractions/key-generation.service"; import { KeyGenerationService } from "../../../platform/abstractions/key-generation.service";
import { StateService } from "../../../platform/abstractions/state.service"; import { StateService } from "../../../platform/abstractions/state.service";
@ -55,6 +57,7 @@ export class MasterPasswordService implements InternalMasterPasswordServiceAbstr
private stateService: StateService, private stateService: StateService,
private keyGenerationService: KeyGenerationService, private keyGenerationService: KeyGenerationService,
private encryptService: EncryptService, private encryptService: EncryptService,
private logService: LogService,
) {} ) {}
masterKey$(userId: UserId): Observable<MasterKey> { masterKey$(userId: UserId): Observable<MasterKey> {
@ -149,10 +152,9 @@ export class MasterPasswordService implements InternalMasterPasswordServiceAbstr
async decryptUserKeyWithMasterKey( async decryptUserKeyWithMasterKey(
masterKey: MasterKey, masterKey: MasterKey,
userId: UserId,
userKey?: EncString, userKey?: EncString,
userId?: UserId,
): Promise<UserKey> { ): Promise<UserKey> {
userId ??= await firstValueFrom(this.stateProvider.activeUserId$);
userKey ??= await this.getMasterKeyEncryptedUserKey(userId); userKey ??= await this.getMasterKeyEncryptedUserKey(userId);
masterKey ??= await firstValueFrom(this.masterKey$(userId)); masterKey ??= await firstValueFrom(this.masterKey$(userId));
@ -185,6 +187,7 @@ export class MasterPasswordService implements InternalMasterPasswordServiceAbstr
} }
if (decUserKey == null) { if (decUserKey == null) {
this.logService.warning("Failed to decrypt user key with master key.");
return null; return null;
} }