Skip to content
Draft
Show file tree
Hide file tree
Changes from all 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
1 change: 0 additions & 1 deletion apps/browser/src/background/main.background.ts
Original file line number Diff line number Diff line change
Expand Up @@ -770,7 +770,6 @@ export default class MainBackground {

this.masterPasswordUnlockService = new DefaultMasterPasswordUnlockService(
this.masterPasswordService,
this.keyService,
this.logService,
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -507,8 +507,6 @@ export class DefaultSetInitialPasswordService implements SetInitialPasswordServi
);
await this.kdfConfigService.setKdfConfig(userId, kdfConfig);
// [PM-23246] "Legacy" master key setting path - to be removed once unlock path migration is complete
await this.masterPasswordService.setMasterKey(masterKey, userId);
// [PM-23246] "Legacy" master key setting path - to be removed once unlock path migration is complete
await this.masterPasswordService.setMasterKeyEncryptedUserKey(
masterKeyEncryptedUserKey[1],
userId,
Expand Down Expand Up @@ -538,13 +536,6 @@ export class DefaultSetInitialPasswordService implements SetInitialPasswordServi
await this.kdfConfigService.setKdfConfig(userId, kdfConfig);
// TODO Remove master key memory state https://bitwarden.atlassian.net/browse/PM-23477
await this.masterPasswordService.setMasterKeyEncryptedUserKey(masterKeyWrappedUserKey, userId);

// TODO Removed with https://bitwarden.atlassian.net/browse/PM-30676
await this.masterPasswordService.setLegacyMasterKeyFromUnlockData(
newPassword,
masterPasswordUnlockData,
userId,
);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -410,10 +410,6 @@ describe("DefaultSetInitialPasswordService", () => {
userDecryptionOptions,
);
expect(kdfConfigService.setKdfConfig).toHaveBeenCalledWith(userId, credentials.kdfConfig);
expect(masterPasswordService.setMasterKey).toHaveBeenCalledWith(
credentials.newMasterKey,
userId,
);
expect(keyService.setUserKey).toHaveBeenCalledWith(masterKeyEncryptedUserKey[0], userId);
});

Expand Down Expand Up @@ -644,10 +640,6 @@ describe("DefaultSetInitialPasswordService", () => {
userDecryptionOptions,
);
expect(kdfConfigService.setKdfConfig).toHaveBeenCalledWith(userId, credentials.kdfConfig);
expect(masterPasswordService.setMasterKey).toHaveBeenCalledWith(
credentials.newMasterKey,
userId,
);
expect(masterPasswordService.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith(
masterKeyEncryptedUserKey[1],
userId,
Expand Down Expand Up @@ -1144,12 +1136,6 @@ describe("DefaultSetInitialPasswordService", () => {
new EncString(sdkRegistrationResult.master_password_unlock.masterKeyWrappedUserKey),
userId,
);

expect(masterPasswordService.setLegacyMasterKeyFromUnlockData).toHaveBeenCalledWith(
credentials.newPassword,
MasterPasswordUnlockData.fromSdk(sdkRegistrationResult.master_password_unlock),
userId,
);
});

describe("input validation", () => {
Expand Down Expand Up @@ -1433,11 +1419,6 @@ describe("DefaultSetInitialPasswordService", () => {
new EncString(unlockData.masterKeyWrappedUserKey),
userId,
);
expect(masterPasswordService.setLegacyMasterKeyFromUnlockData).toHaveBeenCalledWith(
credentials.newPassword,
unlockData,
userId,
);
});

describe("given resetPasswordAutoEnroll is false", () => {
Expand Down
6 changes: 3 additions & 3 deletions libs/angular/src/services/jslib-services.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,7 @@ const safeProviders: SafeProvider[] = [
safeProvider({
provide: ChangeKdfService,
useClass: DefaultChangeKdfService,
deps: [ChangeKdfApiService, SdkService, KeyService, InternalMasterPasswordServiceAbstraction],
deps: [ChangeKdfApiService, SdkService],
}),
safeProvider({
provide: EncryptedMigrator,
Expand Down Expand Up @@ -1195,7 +1195,7 @@ const safeProviders: SafeProvider[] = [
safeProvider({
provide: MasterPasswordUnlockService,
useClass: DefaultMasterPasswordUnlockService,
deps: [InternalMasterPasswordServiceAbstraction, KeyService, LogService],
deps: [InternalMasterPasswordServiceAbstraction, LogService],
}),
safeProvider({
provide: KeyConnectorServiceAbstraction,
Expand Down Expand Up @@ -1413,7 +1413,7 @@ const safeProviders: SafeProvider[] = [
safeProvider({
provide: ChangeKdfService,
useClass: DefaultChangeKdfService,
deps: [ChangeKdfApiService, SdkService, KeyService, InternalMasterPasswordServiceAbstraction],
deps: [ChangeKdfApiService, SdkService],
}),
safeProvider({
provide: AuthRequestServiceAbstraction,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ describe("LoginDecryptionOptionsComponent", () => {
jest.mock("@bitwarden/common/platform/abstractions/sdk/sdk.service", () => ({
asUuid: (x: any) => x,
}));
(Symbol as any).dispose = Symbol("dispose");

mockPrivateKey = "mock-private-key";
mockSignedPublicKey = "mock-signed-public-key";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,6 @@ describe("AuthRequestLoginStrategy", () => {
// Call logIn
await authRequestLoginStrategy.logIn(credentials);

// setMasterKey and setMasterKeyHash should not be called
expect(masterPasswordService.mock.setMasterKey).not.toHaveBeenCalled();

// setMasterKeyEncryptedUserKey, setUserKey, and setPrivateKey should still be called
expect(masterPasswordService.mock.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith(
tokenResponse.key,
Expand All @@ -175,7 +172,7 @@ describe("AuthRequestLoginStrategy", () => {
);

// trustDeviceIfRequired should be called
expect(deviceTrustService.trustDeviceIfRequired).not.toHaveBeenCalled();
expect(deviceTrustService.trustDeviceIfRequired).toHaveBeenCalledWith(mockUserId);
});

it("sets account cryptographic state when accountKeysResponseModel is present", async () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import { firstValueFrom, Observable, map, BehaviorSubject } from "rxjs";

Check failure on line 3 in libs/auth/src/common/login-strategies/auth-request-login.strategy.ts

View workflow job for this annotation

GitHub Actions / Lint

'firstValueFrom' is defined but never used
import { Jsonify } from "type-fest";

import { AuthResult } from "@bitwarden/common/auth/models/domain/auth-result";
Expand Down Expand Up @@ -86,25 +86,8 @@
await this.masterPasswordService.setMasterKeyEncryptedUserKey(response.key, userId);
}

if (authRequestCredentials.decryptedUserKey) {
await this.keyService.setUserKey(authRequestCredentials.decryptedUserKey, userId);
} else {
await this.trySetUserKeyWithMasterKey(userId);

// Establish trust if required after setting user key
await this.deviceTrustService.trustDeviceIfRequired(userId);
}
}

private async trySetUserKeyWithMasterKey(userId: UserId): Promise<void> {
const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId));
if (masterKey) {
const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(
masterKey,
userId,
);
await this.keyService.setUserKey(userKey, userId);
}
await this.keyService.setUserKey(authRequestCredentials.decryptedUserKey, userId);
await this.deviceTrustService.trustDeviceIfRequired(userId);
}

protected override async setAccountCryptographicState(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import {
} from "@bitwarden/common/auth/password-prelogin";
import { TwoFactorService } from "@bitwarden/common/auth/two-factor";
import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service";
import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
import { AccountCryptographicStateService } from "@bitwarden/common/key-management/account-cryptography/account-cryptographic-state.service";
import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service";
import { FakeMasterPasswordService } from "@bitwarden/common/key-management/master-password/services/fake-master-password.service";
Expand Down Expand Up @@ -200,76 +199,20 @@ describe("PasswordLoginStrategy", () => {
);
});

it("sets keys after a successful authentication", async () => {
const userKey = new SymmetricCryptoKey(new Uint8Array(64)) as UserKey;

masterPasswordService.masterKeySubject.next(masterKey);
masterPasswordService.mock.decryptUserKeyWithMasterKey.mockResolvedValue(userKey);
tokenService.decodeAccessToken.mockResolvedValue({ sub: userId });

await passwordLoginStrategy.logIn(credentials);

expect(masterPasswordService.mock.setMasterKey).toHaveBeenCalledWith(masterKey, userId);
expect(masterPasswordService.mock.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith(
tokenResponse.key,
userId,
);
expect(keyService.setUserKey).toHaveBeenCalledWith(userKey, userId);
expect(accountCryptographicStateService.setAccountCryptographicState).toHaveBeenCalledWith(
{ V1: { private_key: tokenResponse.privateKey } },
userId,
);
});

it("uses master password unlock service when feature flag is enabled", async () => {
configService.getFeatureFlag.mockImplementation(async (flag: FeatureFlag) => {
if (flag === FeatureFlag.UseUnlockServiceForPasswordLogin) {
return true;
}
return false;
});

// Re-create he strategy and wait a bit to settle the feature flag
passwordLoginStrategy = new PasswordLoginStrategy(
new PasswordLoginStrategyData(),
passwordStrengthService,
policyService,
passwordPreloginService,
unlockService,
accountService,
masterPasswordService,
keyService,
encryptService,
apiService,
tokenService,
appIdService,
platformUtilsService,
messagingService,
logService,
stateService,
twoFactorService,
userDecryptionOptionsService,
billingAccountProfileStateService,
vaultTimeoutSettingsService,
kdfConfigService,
environmentService,
configService,
accountCryptographicStateService,
);

it("unlocks with the master password and sets account cryptographic state after a successful authentication", async () => {
unlockService.unlockWithMasterPassword.mockResolvedValue(undefined);
tokenService.decodeAccessToken.mockResolvedValue({ sub: userId });

await passwordLoginStrategy.logIn(credentials);

expect(configService.getFeatureFlag).toHaveBeenCalledWith(
FeatureFlag.UseUnlockServiceForPasswordLogin,
);
expect(masterPasswordService.mock.setMasterKey).not.toHaveBeenCalled();
expect(unlockService.unlockWithMasterPassword).toHaveBeenCalledWith(userId, masterPassword);
expect(masterPasswordService.mock.setMasterKeyEncryptedUserKey).not.toHaveBeenCalled();
expect(masterPasswordService.mock.decryptUserKeyWithMasterKey).not.toHaveBeenCalled();
expect(keyService.setUserKey).not.toHaveBeenCalled();
expect(passwordLoginStrategy.exportCache().password.unlockServiceForPasswordLogin).toBe(true);
expect(accountCryptographicStateService.setAccountCryptographicState).toHaveBeenCalledWith(
{ V1: { private_key: tokenResponse.privateKey } },
userId,
);
});

describe("makePasswordPreloginMasterKey", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import {
PasswordPreloginData,
PasswordPreloginService,
} from "@bitwarden/common/auth/password-prelogin";
import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key";
import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength";
import { UserId } from "@bitwarden/common/types/guid";
Expand All @@ -38,8 +37,6 @@ export class PasswordLoginStrategyData implements LoginStrategyData {
masterKey: MasterKey;
/** The user's master password */
masterPassword: string;
/** Whether unlock service should be used for this login flow. */
unlockServiceForPasswordLogin = false;
/**
* Tracks if the user needs to update their password due to
* a password that does not meet an organization's master password policy.
Expand Down Expand Up @@ -81,10 +78,6 @@ export class PasswordLoginStrategy extends LoginStrategy {
}

override async logIn(credentials: PasswordLoginCredentials): Promise<AuthResult> {
const unlockServiceForPasswordLogin = await this.configService.getFeatureFlag(
FeatureFlag.UseUnlockServiceForPasswordLogin,
);

const { email, masterPassword, twoFactor, preFetchedPreloginData } = credentials;

const data = new PasswordLoginStrategyData();
Expand All @@ -95,7 +88,6 @@ export class PasswordLoginStrategy extends LoginStrategy {
);
this.passwordPreloginService.clearCache();
data.masterPassword = masterPassword;
data.unlockServiceForPasswordLogin = unlockServiceForPasswordLogin;
data.userEnteredEmail = email;

// Hash the password early (before authentication) so we don't persist it in memory in plaintext
Expand Down Expand Up @@ -123,37 +115,13 @@ export class PasswordLoginStrategy extends LoginStrategy {
return result;
}

protected override async setMasterKey(response: IdentityTokenResponse, userId: UserId) {
if (!this.cache.value.unlockServiceForPasswordLogin) {
const { masterKey } = this.cache.value;
await this.masterPasswordService.setMasterKey(masterKey, userId);
}
}
protected override async setMasterKey(response: IdentityTokenResponse, userId: UserId) {}

protected override async setUserKey(
response: IdentityTokenResponse,
userId: UserId,
): Promise<void> {
if (this.cache.value.unlockServiceForPasswordLogin) {
await this.unlockService.unlockWithMasterPassword(userId, this.cache.value.masterPassword);
} else {
// If migration is required, we won't have a user key to set yet.
if (this.encryptionKeyMigrationRequired(response)) {
return;
}
await this.masterPasswordService.setMasterKeyEncryptedUserKey(response.key, userId);
// Warning: State is accessed right after state is set. This could lead to a race condition
// in some cases where decryptUserKeyWithMasterKey will get a null encrypted user-key!!
// https://github.com/bitwarden/clients/tree/afc45ee0c8fc823301bb361b0dcac581eb0aff0c/libs/state#updating-state-with-update
const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId));
if (masterKey) {
const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(
masterKey,
userId,
);
await this.keyService.setUserKey(userKey, userId);
}
}
await this.unlockService.unlockWithMasterPassword(userId, this.cache.value.masterPassword);
}

protected override async setAccountCryptographicState(
Expand Down
Loading
Loading