diff --git a/libs/auth/src/common/abstractions/login-strategy.service.ts b/libs/auth/src/common/abstractions/login-strategy.service.ts index 70cd38597f5..18fc176e72c 100644 --- a/libs/auth/src/common/abstractions/login-strategy.service.ts +++ b/libs/auth/src/common/abstractions/login-strategy.service.ts @@ -5,8 +5,6 @@ import { Observable } from "rxjs"; import { AuthenticationType } from "@bitwarden/common/auth/enums/authentication-type"; 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 { MasterKey } from "@bitwarden/common/types/key"; -import { KdfConfig } from "@bitwarden/key-management"; import { UserApiLoginCredentials, @@ -70,20 +68,6 @@ export abstract class LoginStrategyServiceAbstraction { // TODO: PM-15162 - deprecate captchaResponse captchaResponse: string, ) => Promise; - - // TODO: PM-19273 - Refactor makePrePasswordLoginMasterKey to no longer be on the service - // once PM-18176 removes the Recover2faComponent dependency. - /** - * Creates a master key from the provided master password and email. - * If a KdfConfig is provided, it will be used to generate the key. - * Otherwise, the PrePasswordLogin endpoint will be used to retrieve the user's - * KdfConfig. - */ - makePrePasswordLoginMasterKey: ( - masterPassword: string, - email: string, - kdfConfig?: KdfConfig, - ) => Promise; /** * Emits true if the authentication session has expired. */ diff --git a/libs/auth/src/common/login-strategies/base-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/base-login.strategy.spec.ts index fc3be61fe11..6ea786c049f 100644 --- a/libs/auth/src/common/login-strategies/base-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/base-login.strategy.spec.ts @@ -42,11 +42,10 @@ import { import { CsprngArray } from "@bitwarden/common/types/csprng"; import { UserId } from "@bitwarden/common/types/guid"; import { UserKey, MasterKey } from "@bitwarden/common/types/key"; -import { KdfConfigService, KeyService } from "@bitwarden/key-management"; +import { KdfConfigService, KeyService, PBKDF2KdfConfig } from "@bitwarden/key-management"; -import { LoginStrategyServiceAbstraction } from "../abstractions"; import { InternalUserDecryptionOptionsServiceAbstraction } from "../abstractions/user-decryption-options.service.abstraction"; -import { PasswordLoginCredentials } from "../models"; +import { PasswordHashLoginCredentials } from "../models"; import { UserDecryptionOptions } from "../models/domain/user-decryption-options"; import { PasswordLoginStrategy, PasswordLoginStrategyData } from "./password-login.strategy"; @@ -102,12 +101,11 @@ export function identityTokenResponseFactory( } // TODO: add tests for latest changes to base class for TDE -describe("LoginStrategy", () => { +describe("BaseLoginStrategy", () => { let cache: PasswordLoginStrategyData; let accountService: FakeAccountService; let masterPasswordService: FakeMasterPasswordService; - let loginStrategyService: MockProxy; let keyService: MockProxy; let encryptService: MockProxy; let apiService: MockProxy; @@ -127,13 +125,12 @@ describe("LoginStrategy", () => { let environmentService: MockProxy; let passwordLoginStrategy: PasswordLoginStrategy; - let credentials: PasswordLoginCredentials; + let credentials: PasswordHashLoginCredentials; beforeEach(async () => { accountService = mockAccountServiceWith(userId); masterPasswordService = new FakeMasterPasswordService(); - loginStrategyService = mock(); keyService = mock(); encryptService = mock(); apiService = mock(); @@ -161,7 +158,6 @@ describe("LoginStrategy", () => { cache, passwordStrengthService, policyService, - loginStrategyService, accountService as unknown as AccountService, masterPasswordService, keyService, @@ -180,7 +176,7 @@ describe("LoginStrategy", () => { kdfConfigService, environmentService, ); - credentials = new PasswordLoginCredentials(email, masterPassword); + credentials = new PasswordHashLoginCredentials(email, masterPassword, new PBKDF2KdfConfig()); }); describe("base class", () => { @@ -508,7 +504,6 @@ describe("LoginStrategy", () => { cache, passwordStrengthService, policyService, - loginStrategyService, accountService as AccountService, masterPasswordService, keyService, @@ -572,7 +567,6 @@ describe("LoginStrategy", () => { cache, passwordStrengthService, policyService, - loginStrategyService, accountService as AccountService, masterPasswordService, keyService, diff --git a/libs/auth/src/common/login-strategies/base-login.strategy.ts b/libs/auth/src/common/login-strategies/base-login.strategy.ts index dedffe8f1ad..46b9c1e6631 100644 --- a/libs/auth/src/common/login-strategies/base-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/base-login.strategy.ts @@ -35,12 +35,14 @@ import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/pl import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { Account, AccountProfile } from "@bitwarden/common/platform/models/domain/account"; import { UserId } from "@bitwarden/common/types/guid"; +import { MasterKey } from "@bitwarden/common/types/key"; import { KeyService, Argon2KdfConfig, PBKDF2KdfConfig, KdfConfigService, KdfType, + KdfConfig, } from "@bitwarden/key-management"; import { InternalUserDecryptionOptionsServiceAbstraction } from "../abstractions/user-decryption-options.service.abstraction"; @@ -412,4 +414,23 @@ export abstract class BaseLoginStrategy { this.cache.next({ ...this.cache.value, captchaBypassToken: response.captchaToken ?? null }); return result; } + + /** + * Creates a master key from the provided master password and email, using the provided kdfConfig. + */ + protected async makePrePasswordLoginMasterKey( + masterPassword: string, + email: string, + kdfConfig: KdfConfig, + ): Promise { + email = email.trim().toLowerCase(); + + if (!kdfConfig) { + throw new Error("KDF config is required"); + } + + kdfConfig.validateKdfConfigForPreLogin(); + + return this.keyService.makeMasterKey(masterPassword, email, kdfConfig); + } } diff --git a/libs/auth/src/common/login-strategies/opaque-login.strategy.ts b/libs/auth/src/common/login-strategies/opaque-login.strategy.ts index 19021ecabf7..bd1f6bc0387 100644 --- a/libs/auth/src/common/login-strategies/opaque-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/opaque-login.strategy.ts @@ -21,7 +21,6 @@ import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/pass import { UserId } from "@bitwarden/common/types/guid"; import { MasterKey } from "@bitwarden/common/types/key"; -import { LoginStrategyServiceAbstraction } from "../abstractions"; import { OpaqueLoginCredentials } from "../models/domain/login-credentials"; import { CacheData } from "../services/login-strategies/login-strategy.state"; @@ -77,7 +76,6 @@ export class OpaqueLoginStrategy extends BaseLoginStrategy { data: OpaqueLoginStrategyData, private passwordStrengthService: PasswordStrengthServiceAbstraction, private policyService: PolicyService, - private loginStrategyService: LoginStrategyServiceAbstraction, ...sharedDeps: ConstructorParameters ) { super(...sharedDeps); @@ -97,11 +95,7 @@ export class OpaqueLoginStrategy extends BaseLoginStrategy { // Even though we are completing OPAQUE authN and not logging in with password hash, // we still need to hash the master password for logged in user verification scenarios. - data.masterKey = await this.loginStrategyService.makePrePasswordLoginMasterKey( - masterPassword, - email, - kdfConfig, - ); + data.masterKey = await this.makePrePasswordLoginMasterKey(masterPassword, email, kdfConfig); // Hash the password early (before authentication) so we don't persist it in memory in plaintext data.localMasterKeyHash = await this.keyService.hashMasterKey( diff --git a/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts index d0d479d92bc..0cd48410404 100644 --- a/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts @@ -36,7 +36,6 @@ import { UserId } from "@bitwarden/common/types/guid"; import { MasterKey, UserKey } from "@bitwarden/common/types/key"; import { KdfConfigService, KeyService, PBKDF2KdfConfig } from "@bitwarden/key-management"; -import { LoginStrategyServiceAbstraction } from "../abstractions"; import { InternalUserDecryptionOptionsServiceAbstraction } from "../abstractions/user-decryption-options.service.abstraction"; import { PasswordHashLoginCredentials } from "../models/domain/login-credentials"; @@ -64,7 +63,6 @@ describe("PasswordLoginStrategy", () => { let accountService: FakeAccountService; let masterPasswordService: FakeMasterPasswordService; - let loginStrategyService: MockProxy; let keyService: MockProxy; let encryptService: MockProxy; let apiService: MockProxy; @@ -91,7 +89,6 @@ describe("PasswordLoginStrategy", () => { accountService = mockAccountServiceWith(userId); masterPasswordService = new FakeMasterPasswordService(); - loginStrategyService = mock(); keyService = mock(); encryptService = mock(); apiService = mock(); @@ -115,8 +112,9 @@ describe("PasswordLoginStrategy", () => { sub: userId, }); - loginStrategyService.makePrePasswordLoginMasterKey.mockResolvedValue(masterKey); - + keyService.makeMasterKey + .calledWith(masterPassword, email, expect.any(PBKDF2KdfConfig)) + .mockResolvedValue(masterKey); keyService.hashMasterKey .calledWith(masterPassword, expect.anything(), undefined) .mockResolvedValue(hashedPassword); @@ -130,7 +128,6 @@ describe("PasswordLoginStrategy", () => { cache, passwordStrengthService, policyService, - loginStrategyService, accountService, masterPasswordService, keyService, diff --git a/libs/auth/src/common/login-strategies/password-login.strategy.ts b/libs/auth/src/common/login-strategies/password-login.strategy.ts index ce64b0a547e..a33d3c04e4d 100644 --- a/libs/auth/src/common/login-strategies/password-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/password-login.strategy.ts @@ -19,7 +19,6 @@ import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/pass import { UserId } from "@bitwarden/common/types/guid"; import { MasterKey } from "@bitwarden/common/types/key"; -import { LoginStrategyServiceAbstraction } from "../abstractions"; import { PasswordHashLoginCredentials } from "../models/domain/login-credentials"; import { CacheData } from "../services/login-strategies/login-strategy.state"; @@ -66,7 +65,6 @@ export class PasswordLoginStrategy extends BaseLoginStrategy { data: PasswordLoginStrategyData, private passwordStrengthService: PasswordStrengthServiceAbstraction, private policyService: PolicyService, - private loginStrategyService: LoginStrategyServiceAbstraction, ...sharedDeps: ConstructorParameters ) { super(...sharedDeps); @@ -84,11 +82,7 @@ export class PasswordLoginStrategy extends BaseLoginStrategy { const data = new PasswordLoginStrategyData(); - data.masterKey = await this.loginStrategyService.makePrePasswordLoginMasterKey( - masterPassword, - email, - kdfConfig, - ); + data.masterKey = await this.makePrePasswordLoginMasterKey(masterPassword, email, kdfConfig); data.userEnteredEmail = email; // Hash the password early (before authentication) so we don't persist it in memory in plaintext diff --git a/libs/auth/src/common/services/login-strategies/login-strategy.service.ts b/libs/auth/src/common/services/login-strategies/login-strategy.service.ts index 60a9a9d2fef..1a0ba5b4ad3 100644 --- a/libs/auth/src/common/services/login-strategies/login-strategy.service.ts +++ b/libs/auth/src/common/services/login-strategies/login-strategy.service.ts @@ -36,8 +36,7 @@ import { StateService } from "@bitwarden/common/platform/abstractions/state.serv import { TaskSchedulerService, ScheduledTaskNames } from "@bitwarden/common/platform/scheduling"; import { GlobalState, GlobalStateProvider } from "@bitwarden/common/platform/state"; import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength"; -import { MasterKey } from "@bitwarden/common/types/key"; -import { KeyService, KdfConfig, KdfConfigService } from "@bitwarden/key-management"; +import { KeyService, KdfConfigService } from "@bitwarden/key-management"; import { AuthRequestServiceAbstraction, LoginStrategyServiceAbstraction } from "../../abstractions"; import { InternalUserDecryptionOptionsServiceAbstraction } from "../../abstractions/user-decryption-options.service.abstraction"; @@ -324,35 +323,6 @@ export class LoginStrategyService implements LoginStrategyServiceAbstraction { } } - async makePrePasswordLoginMasterKey( - masterPassword: string, - email: string, - kdfConfig?: KdfConfig, - ): Promise { - email = email.trim().toLowerCase(); - - if (!kdfConfig) { - try { - const preloginResponse = await this.prePasswordLoginApiService.postPrePasswordLogin( - new PrePasswordLoginRequest(email), - ); - kdfConfig = preloginResponse?.toKdfConfig(); - } catch (e: any) { - if (e == null || e.statusCode !== 404) { - throw e; - } - } - - if (!kdfConfig) { - throw new Error("KDF config is required"); - } - } - - kdfConfig.validateKdfConfigForPreLogin(); - - return this.keyService.makeMasterKey(masterPassword, email, kdfConfig); - } - private async clearCache(): Promise { await this.currentAuthnTypeState.update((_) => null); await this.loginStrategyCacheState.update((_) => null); @@ -426,7 +396,6 @@ export class LoginStrategyService implements LoginStrategyServiceAbstraction { data?.password ?? new PasswordLoginStrategyData(), this.passwordStrengthService, this.policyService, - this, ...sharedDeps, ); case AuthenticationType.Sso: @@ -460,11 +429,10 @@ export class LoginStrategyService implements LoginStrategyServiceAbstraction { data?.opaque ?? new OpaqueLoginStrategyData(), this.passwordStrengthService, this.policyService, - this, ...sharedDeps, ); default: - return null; + throw new Error("No matching strategy found for AuthenticationType " + strategy); } } }