diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index da47542ee6b..d295fddda52 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -39,7 +39,6 @@ import { TokenService as TokenServiceAbstraction } from "@bitwarden/common/auth/ import { UserVerificationApiServiceAbstraction } from "@bitwarden/common/auth/abstractions/user-verification/user-verification-api.service.abstraction"; import { UserVerificationService as UserVerificationServiceAbstraction } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; -import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; import { AccountServiceImplementation } from "@bitwarden/common/auth/services/account.service"; import { AuthService } from "@bitwarden/common/auth/services/auth.service"; import { AvatarService } from "@bitwarden/common/auth/services/avatar.service"; @@ -1511,9 +1510,6 @@ export default class MainBackground { } nextAccountStatus = await this.authService.getAuthStatus(userId); - const forcePasswordReset = - (await firstValueFrom(this.masterPasswordService.forceSetPasswordReason$(userId))) != - ForceSetPasswordReason.None; await this.systemService.clearPendingClipboard(); @@ -1521,8 +1517,6 @@ export default class MainBackground { this.messagingService.send("goHome"); } else if (nextAccountStatus === AuthenticationStatus.Locked) { this.messagingService.send("locked", { userId: userId }); - } else if (forcePasswordReset) { - this.messagingService.send("update-temp-password", { userId: userId }); } else { this.messagingService.send("unlocked", { userId: userId }); await this.refreshBadge(); diff --git a/apps/browser/src/popup/app.component.ts b/apps/browser/src/popup/app.component.ts index 6a08bf007bb..49579f889b3 100644 --- a/apps/browser/src/popup/app.component.ts +++ b/apps/browser/src/popup/app.component.ts @@ -160,10 +160,6 @@ export class AppComponent implements OnInit, OnDestroy { // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. // eslint-disable-next-line @typescript-eslint/no-floating-promises this.router.navigate(["/remove-password"]); - } else if (msg.command == "update-temp-password") { - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.router.navigate(["/update-temp-password"]); } }), takeUntil(this.destroy$), diff --git a/apps/cli/src/auth/commands/login.command.ts b/apps/cli/src/auth/commands/login.command.ts index 8a94cc4175a..b26ce94207d 100644 --- a/apps/cli/src/auth/commands/login.command.ts +++ b/apps/cli/src/auth/commands/login.command.ts @@ -32,6 +32,7 @@ import { UpdateTempPasswordRequest } from "@bitwarden/common/auth/models/request import { ClientType } from "@bitwarden/common/enums"; import { CryptoFunctionService } from "@bitwarden/common/key-management/crypto/abstractions/crypto-function.service"; import { KeyConnectorService } from "@bitwarden/common/key-management/key-connector/abstractions/key-connector.service"; +import { MasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; import { ErrorResponse } from "@bitwarden/common/models/response/error.response"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; @@ -77,6 +78,7 @@ export class LoginCommand { protected logoutCallback: () => Promise, protected kdfConfigService: KdfConfigService, protected ssoUrlService: SsoUrlService, + protected masterPasswordService: MasterPasswordServiceAbstraction, ) {} async run(email: string, password: string, options: OptionValues) { @@ -361,14 +363,14 @@ export class LoginCommand { await this.syncService.fullSync(true); // Handle updating passwords if NOT using an API Key for authentication - if ( - response.forcePasswordReset != ForceSetPasswordReason.None && - clientId == null && - clientSecret == null - ) { - if (response.forcePasswordReset === ForceSetPasswordReason.AdminForcePasswordReset) { + if (clientId == null && clientSecret == null) { + const forceSetPasswordReason = await firstValueFrom( + this.masterPasswordService.forceSetPasswordReason$(response.userId), + ); + + if (forceSetPasswordReason === ForceSetPasswordReason.AdminForcePasswordReset) { return await this.updateTempPassword(response.userId); - } else if (response.forcePasswordReset === ForceSetPasswordReason.WeakMasterPassword) { + } else if (forceSetPasswordReason === ForceSetPasswordReason.WeakMasterPassword) { return await this.updateWeakPassword(response.userId, password); } } diff --git a/apps/cli/src/program.ts b/apps/cli/src/program.ts index c6b79c7dff2..dca4effcdc9 100644 --- a/apps/cli/src/program.ts +++ b/apps/cli/src/program.ts @@ -172,6 +172,7 @@ export class Program extends BaseProgram { async () => await this.serviceContainer.logout(), this.serviceContainer.kdfConfigService, this.serviceContainer.ssoUrlService, + this.serviceContainer.masterPasswordService, ); const response = await command.run(email, password, options); this.processResponse(response, true); diff --git a/apps/desktop/src/app/app.component.ts b/apps/desktop/src/app/app.component.ts index 3ce4d57e25b..c3cfdf49c2c 100644 --- a/apps/desktop/src/app/app.component.ts +++ b/apps/desktop/src/app/app.component.ts @@ -27,7 +27,6 @@ import { AccountService } from "@bitwarden/common/auth/abstractions/account.serv import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; -import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { ProcessReloadServiceAbstraction } from "@bitwarden/common/key-management/abstractions/process-reload.service"; import { KeyConnectorService } from "@bitwarden/common/key-management/key-connector/abstractions/key-connector.service"; @@ -409,17 +408,9 @@ export class AppComponent implements OnInit, OnDestroy { const locked = (await this.authService.getAuthStatus(message.userId)) === AuthenticationStatus.Locked; - const forcedPasswordReset = - (await firstValueFrom( - this.masterPasswordService.forceSetPasswordReason$(message.userId), - )) != ForceSetPasswordReason.None; if (locked) { this.modalService.closeAll(); await this.router.navigate(["lock"]); - } else if (forcedPasswordReset) { - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.router.navigate(["update-temp-password"]); } else { this.messagingService.send("unlocked"); this.loading = true; diff --git a/libs/angular/src/auth/components/base-login-via-webauthn.component.ts b/libs/angular/src/auth/components/base-login-via-webauthn.component.ts index 1ad4829767a..5d30fc997dc 100644 --- a/libs/angular/src/auth/components/base-login-via-webauthn.component.ts +++ b/libs/angular/src/auth/components/base-login-via-webauthn.component.ts @@ -6,7 +6,6 @@ import { firstValueFrom } from "rxjs"; import { LoginSuccessHandlerService } from "@bitwarden/auth/common"; import { WebAuthnLoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/webauthn/webauthn-login.service.abstraction"; -import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; import { WebAuthnLoginCredentialAssertionView } from "@bitwarden/common/auth/models/view/webauthn-login/webauthn-login-credential-assertion.view"; import { ErrorResponse } from "@bitwarden/common/models/response/error.response"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; @@ -21,7 +20,6 @@ export class BaseLoginViaWebAuthnComponent implements OnInit { protected currentState: State = "assert"; protected successRoute = "/vault"; - protected forcePasswordResetRoute = "/update-temp-password"; constructor( private webAuthnLoginService: WebAuthnLoginServiceAbstraction, @@ -73,11 +71,6 @@ export class BaseLoginViaWebAuthnComponent implements OnInit { await this.loginSuccessHandlerService.run(authResult.userId); } - if (authResult.forcePasswordReset == ForceSetPasswordReason.AdminForcePasswordReset) { - await this.router.navigate([this.forcePasswordResetRoute]); - return; - } - await this.router.navigate([this.successRoute]); } catch (error) { if (error instanceof ErrorResponse) { diff --git a/libs/auth/src/angular/login-via-auth-request/login-via-auth-request.component.ts b/libs/auth/src/angular/login-via-auth-request/login-via-auth-request.component.ts index 5de2339bda1..ab5b0c09b32 100644 --- a/libs/auth/src/angular/login-via-auth-request/login-via-auth-request.component.ts +++ b/libs/auth/src/angular/login-via-auth-request/login-via-auth-request.component.ts @@ -19,7 +19,6 @@ import { AuthRequestType } from "@bitwarden/common/auth/enums/auth-request-type" import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { AdminAuthRequestStorable } from "@bitwarden/common/auth/models/domain/admin-auth-req-storable"; import { AuthResult } from "@bitwarden/common/auth/models/domain/auth-result"; -import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; import { AuthRequest } from "@bitwarden/common/auth/models/request/auth.request"; import { AuthRequestResponse } from "@bitwarden/common/auth/models/response/auth-request.response"; import { LoginViaAuthRequestView } from "@bitwarden/common/auth/models/view/login-via-auth-request.view"; @@ -820,8 +819,6 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { private async handlePostLoginNavigation(loginResponse: AuthResult) { if (loginResponse.requiresTwoFactor) { await this.router.navigate(["2fa"]); - } else if (loginResponse.forcePasswordReset != ForceSetPasswordReason.None) { - await this.router.navigate(["update-temp-password"]); } else { await this.handleSuccessfulLoginNavigation(loginResponse.userId); } diff --git a/libs/auth/src/angular/login/login.component.ts b/libs/auth/src/angular/login/login.component.ts index eb2bdcee291..cd226cddcec 100644 --- a/libs/auth/src/angular/login/login.component.ts +++ b/libs/auth/src/angular/login/login.component.ts @@ -17,7 +17,6 @@ import { MasterPasswordPolicyOptions } from "@bitwarden/common/admin-console/mod import { Policy } from "@bitwarden/common/admin-console/models/domain/policy"; import { DevicesApiServiceAbstraction } from "@bitwarden/common/auth/abstractions/devices-api.service.abstraction"; import { AuthResult } from "@bitwarden/common/auth/models/domain/auth-result"; -import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; import { ClientType, HttpStatusCode } from "@bitwarden/common/enums"; import { ErrorResponse } from "@bitwarden/common/models/response/error.response"; import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service"; @@ -307,10 +306,7 @@ export class LoginComponent implements OnInit, OnDestroy { await this.loginSuccessHandlerService.run(authResult.userId); // Determine where to send the user next - if (authResult.forcePasswordReset != ForceSetPasswordReason.None) { - await this.router.navigate(["update-temp-password"]); - return; - } + // The AuthGuard will handle routing to update-temp-password based on state // TODO: PM-18269 - evaluate if we can combine this with the // password evaluation done in the password login strategy. diff --git a/libs/auth/src/angular/new-device-verification/new-device-verification.component.ts b/libs/auth/src/angular/new-device-verification/new-device-verification.component.ts index c083643c9b4..a2b0b23d05c 100644 --- a/libs/auth/src/angular/new-device-verification/new-device-verification.component.ts +++ b/libs/auth/src/angular/new-device-verification/new-device-verification.component.ts @@ -136,11 +136,6 @@ export class NewDeviceVerificationComponent implements OnInit, OnDestroy { return; } - if (authResult.forcePasswordReset) { - await this.router.navigate(["/update-temp-password"]); - return; - } - this.loginSuccessHandlerService.run(authResult.userId); // If verification succeeds, navigate to vault diff --git a/libs/auth/src/angular/sso/sso.component.ts b/libs/auth/src/angular/sso/sso.component.ts index 311ebf174ef..a91a8ed20e9 100644 --- a/libs/auth/src/angular/sso/sso.component.ts +++ b/libs/auth/src/angular/sso/sso.component.ts @@ -541,14 +541,6 @@ export class SsoComponent implements OnInit { }); } - private async handleForcePasswordReset(orgIdentifier: string) { - await this.router.navigate(["update-temp-password"], { - queryParams: { - identifier: orgIdentifier, - }, - }); - } - private async handleSuccessfulLogin() { await this.router.navigate(["lock"]); } diff --git a/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.ts b/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.ts index f09d7163667..e7e62260b49 100644 --- a/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.ts +++ b/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.ts @@ -575,25 +575,6 @@ export class TwoFactorAuthComponent implements OnInit, OnDestroy { }); } - /** - * Determines if a user needs to reset their password based on certain conditions. - * Users can be forced to reset their password via an admin or org policy disallowing weak passwords. - * Note: this is different from the SSO component login flow as a user can - * login with MP and then have to pass 2FA to finish login and we can actually - * evaluate if they have a weak password at that time. - * - * @param {AuthResult} authResult - The authentication result. - * @returns {boolean} Returns true if a password reset is required, false otherwise. - */ - private isForcePasswordResetRequired(authResult: AuthResult): boolean { - const forceResetReasons = [ - ForceSetPasswordReason.AdminForcePasswordReset, - ForceSetPasswordReason.WeakMasterPassword, - ]; - - return forceResetReasons.includes(authResult.forcePasswordReset); - } - showContinueButton() { return ( this.selectedProviderType != null && diff --git a/libs/auth/src/common/login-strategies/login.strategy.spec.ts b/libs/auth/src/common/login-strategies/login.strategy.spec.ts index fc3be61fe11..5a5a9dc2575 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.spec.ts @@ -296,13 +296,9 @@ describe("LoginStrategy", () => { const expected = new AuthResult(); expected.userId = userId; - expected.forcePasswordReset = ForceSetPasswordReason.AdminForcePasswordReset; expected.resetMasterPassword = true; - expected.twoFactorProviders = {} as Partial< - Record> - >; - expected.captchaSiteKey = ""; expected.twoFactorProviders = null; + expected.captchaSiteKey = ""; expect(result).toEqual(expected); }); @@ -316,13 +312,9 @@ describe("LoginStrategy", () => { const expected = new AuthResult(); expected.userId = userId; - expected.forcePasswordReset = ForceSetPasswordReason.AdminForcePasswordReset; expected.resetMasterPassword = false; - expected.twoFactorProviders = {} as Partial< - Record> - >; - expected.captchaSiteKey = ""; expected.twoFactorProviders = null; + expected.captchaSiteKey = ""; expect(result).toEqual(expected); expect(masterPasswordService.mock.setForceSetPasswordReason).toHaveBeenCalledWith( diff --git a/libs/auth/src/common/login-strategies/login.strategy.ts b/libs/auth/src/common/login-strategies/login.strategy.ts index 96d7b6b0f74..e252d50a5ab 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.ts @@ -277,17 +277,7 @@ export abstract class LoginStrategy { result.resetMasterPassword = response.resetMasterPassword; - // Convert boolean to enum and set the state for the master password service to - // so we know when we reach the auth guard that we need to guide them properly to admin - // password reset. - if (response.forcePasswordReset) { - result.forcePasswordReset = ForceSetPasswordReason.AdminForcePasswordReset; - - await this.masterPasswordService.setForceSetPasswordReason( - ForceSetPasswordReason.AdminForcePasswordReset, - userId, - ); - } + await this.processForceSetPasswordReason(response.forcePasswordReset, userId); if (response.twoFactorToken != null) { // note: we can read email from access token b/c it was saved in saveAccountInformation @@ -318,6 +308,30 @@ export abstract class LoginStrategy { return false; } + /** + * Checks if adminForcePasswordReset is true and sets the ForceSetPasswordReason.AdminForcePasswordReset flag in the master password service. + * @param adminForcePasswordReset - The admin force password reset flag + * @param userId - The user ID + * @returns a promise that resolves to a boolean indicating whether the admin force password reset flag was set + */ + async processForceSetPasswordReason( + adminForcePasswordReset: boolean, + userId: UserId, + ): Promise { + if (!adminForcePasswordReset) { + return false; + } + + // set the flag in the master password service so we know when we reach the auth guard + // that we need to guide them properly to admin password reset. + await this.masterPasswordService.setForceSetPasswordReason( + ForceSetPasswordReason.AdminForcePasswordReset, + userId, + ); + + return true; + } + protected async createKeyPairForOldAccount(userId: UserId) { try { const userKey = await this.keyService.getUserKeyWithLegacySupport(userId); 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 3752960fc47..2923908fb7b 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 @@ -211,20 +211,18 @@ describe("PasswordLoginStrategy", () => { it("does not force the user to update their master password when there are no requirements", async () => { apiService.postIdentityToken.mockResolvedValueOnce(identityTokenResponseFactory()); - const result = await passwordLoginStrategy.logIn(credentials); + await passwordLoginStrategy.logIn(credentials); expect(policyService.evaluateMasterPassword).not.toHaveBeenCalled(); - expect(result.forcePasswordReset).toEqual(ForceSetPasswordReason.None); }); it("does not force the user to update their master password when it meets requirements", async () => { passwordStrengthService.getPasswordStrength.mockReturnValue({ score: 5 } as any); policyService.evaluateMasterPassword.mockReturnValue(true); - const result = await passwordLoginStrategy.logIn(credentials); + await passwordLoginStrategy.logIn(credentials); expect(policyService.evaluateMasterPassword).toHaveBeenCalled(); - expect(result.forcePasswordReset).toEqual(ForceSetPasswordReason.None); }); it("forces the user to update their master password on successful login when it does not meet master password policy requirements", async () => { @@ -232,14 +230,13 @@ describe("PasswordLoginStrategy", () => { policyService.evaluateMasterPassword.mockReturnValue(false); tokenService.decodeAccessToken.mockResolvedValue({ sub: userId }); - const result = await passwordLoginStrategy.logIn(credentials); + await passwordLoginStrategy.logIn(credentials); expect(policyService.evaluateMasterPassword).toHaveBeenCalled(); expect(masterPasswordService.mock.setForceSetPasswordReason).toHaveBeenCalledWith( ForceSetPasswordReason.WeakMasterPassword, userId, ); - expect(result.forcePasswordReset).toEqual(ForceSetPasswordReason.WeakMasterPassword); }); it("forces the user to update their master password on successful 2FA login when it does not meet master password policy requirements", async () => { @@ -257,13 +254,13 @@ describe("PasswordLoginStrategy", () => { // First login request fails requiring 2FA apiService.postIdentityToken.mockResolvedValueOnce(token2FAResponse); - const firstResult = await passwordLoginStrategy.logIn(credentials); + await passwordLoginStrategy.logIn(credentials); // Second login request succeeds apiService.postIdentityToken.mockResolvedValueOnce( identityTokenResponseFactory(masterPasswordPolicy), ); - const secondResult = await passwordLoginStrategy.logInTwoFactor( + await passwordLoginStrategy.logInTwoFactor( { provider: TwoFactorProviderType.Authenticator, token: "123456", @@ -272,15 +269,11 @@ describe("PasswordLoginStrategy", () => { "", ); - // First login attempt should not save the force password reset options - expect(firstResult.forcePasswordReset).toEqual(ForceSetPasswordReason.None); - - // Second login attempt should save the force password reset options and return in result + // Second login attempt should save the force password reset options expect(masterPasswordService.mock.setForceSetPasswordReason).toHaveBeenCalledWith( ForceSetPasswordReason.WeakMasterPassword, userId, ); - expect(secondResult.forcePasswordReset).toEqual(ForceSetPasswordReason.WeakMasterPassword); }); it("handles new device verification login with OTP", async () => { @@ -298,7 +291,6 @@ describe("PasswordLoginStrategy", () => { newDeviceOtp: deviceVerificationOtp, }), ); - expect(result.forcePasswordReset).toBe(ForceSetPasswordReason.None); expect(result.resetMasterPassword).toBe(false); expect(result.userId).toBe(userId); }); 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 f0a8d40f914..6af9d8dbb6b 100644 --- a/libs/auth/src/common/login-strategies/password-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/password-login.strategy.ts @@ -109,35 +109,8 @@ export class PasswordLoginStrategy extends LoginStrategy { return authResult; } - const masterPasswordPolicyOptions = - this.getMasterPasswordPolicyOptionsFromResponse(identityResponse); + await this.evaluateMasterPasswordIfRequired(identityResponse, credentials, authResult); - // The identity result can contain master password policies for the user's organizations - if (masterPasswordPolicyOptions?.enforceOnLogin) { - // If there is a policy active, evaluate the supplied password before its no longer in memory - const meetsRequirements = this.evaluateMasterPassword( - credentials, - masterPasswordPolicyOptions, - ); - if (meetsRequirements) { - return authResult; - } - - if (identityResponse instanceof IdentityTwoFactorResponse) { - // Save the flag to this strategy for use in 2fa login as the master password is about to pass out of scope - this.cache.next({ - ...this.cache.value, - forcePasswordResetReason: ForceSetPasswordReason.WeakMasterPassword, - }); - } else { - // Authentication was successful, save the force update password options with the state service - await this.masterPasswordService.setForceSetPasswordReason( - ForceSetPasswordReason.WeakMasterPassword, - authResult.userId, // userId is only available on successful login - ); - authResult.forcePasswordReset = ForceSetPasswordReason.WeakMasterPassword; - } - } return authResult; } @@ -151,20 +124,6 @@ export class PasswordLoginStrategy extends LoginStrategy { const result = await super.logInTwoFactor(twoFactor); - // 2FA was successful, save the force update password options with the state service if defined - const forcePasswordResetReason = this.cache.value.forcePasswordResetReason; - if ( - !result.requiresTwoFactor && - !result.requiresCaptcha && - forcePasswordResetReason != ForceSetPasswordReason.None - ) { - await this.masterPasswordService.setForceSetPasswordReason( - forcePasswordResetReason, - result.userId, - ); - result.forcePasswordReset = forcePasswordResetReason; - } - return result; } @@ -208,13 +167,58 @@ export class PasswordLoginStrategy extends LoginStrategy { return !response.key; } - private getMasterPasswordPolicyOptionsFromResponse( - response: + private async evaluateMasterPasswordIfRequired( + identityResponse: | IdentityTokenResponse | IdentityTwoFactorResponse | IdentityDeviceVerificationResponse, - ): MasterPasswordPolicyOptions { - if (response == null || response instanceof IdentityDeviceVerificationResponse) { + credentials: PasswordLoginCredentials, + authResult: AuthResult, + ): Promise { + // TODO: PM-21084 - investigate if we should be sending down masterPasswordPolicy on the IdentityDeviceVerificationResponse like we do for the IdentityTwoFactorResponse + // If the response is a device verification response, we don't need to evaluate the password + if (identityResponse instanceof IdentityDeviceVerificationResponse) { + return; + } + + // The identity result can contain master password policies for the user's organizations + const masterPasswordPolicyOptions = + this.getMasterPasswordPolicyOptionsFromResponse(identityResponse); + + if (!masterPasswordPolicyOptions?.enforceOnLogin) { + return; + } + + // If there is a policy active, evaluate the supplied password before its no longer in memory + const meetsRequirements = this.evaluateMasterPassword(credentials, masterPasswordPolicyOptions); + if (meetsRequirements) { + return; + } + + if (identityResponse instanceof IdentityTwoFactorResponse) { + // Save the flag to this strategy for use in 2fa as the master password is about to pass out of scope + this.cache.next({ + ...this.cache.value, + forcePasswordResetReason: ForceSetPasswordReason.WeakMasterPassword, + }); + } + + // Authentication was successful, save the force update password options with the state service + // if there isn't already a reason set (this would only be AdminForcePasswordReset as that can be set server side + // and would have already been processed in the base login strategy processForceSetPasswordReason method) + // Note: masterPasswordService.setForceSetPasswordReason will not allow overwriting + // AdminForcePasswordReset with any other reason except for None. This is because + // an AdminForcePasswordReset will always force a user to update their password to a password that meets the policy. + await this.masterPasswordService.setForceSetPasswordReason( + ForceSetPasswordReason.WeakMasterPassword, + authResult.userId, // userId is only available on successful login + ); + } + + private getMasterPasswordPolicyOptionsFromResponse( + response: IdentityTokenResponse | IdentityTwoFactorResponse, + ): MasterPasswordPolicyOptions | null { + if (response == null) { return null; } return MasterPasswordPolicyOptions.fromResponse(response.masterPasswordPolicy); @@ -246,4 +250,35 @@ export class PasswordLoginStrategy extends LoginStrategy { const [authResult] = await this.startLogIn(); return authResult; } + + /** + * Override to handle the WeakMasterPassword reason if no other reason is set. + * @param authResult - The authentication result + * @param userId - The user ID + */ + override async processForceSetPasswordReason( + adminForcePasswordReset: boolean, + userId: UserId, + ): Promise { + // handle any existing reasons + const adminForcePasswordResetFlagSet = await super.processForceSetPasswordReason( + adminForcePasswordReset, + userId, + ); + + // If we are already processing an admin force password reset, don't process other reasons + if (adminForcePasswordResetFlagSet) { + return false; + } + + // If we have a cached weak password reason from login/logInTwoFactor apply it + const cachedReason = this.cache.value.forcePasswordResetReason; + if (cachedReason !== ForceSetPasswordReason.None) { + await this.masterPasswordService.setForceSetPasswordReason(cachedReason, userId); + return true; + } + + // If none of the conditions are met, return false + return false; + } } diff --git a/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts index 546fa0c5fa7..d743a71f160 100644 --- a/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts @@ -1,5 +1,5 @@ import { mock, MockProxy } from "jest-mock-extended"; -import { BehaviorSubject } from "rxjs"; +import { BehaviorSubject, of } from "rxjs"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { TokenService } from "@bitwarden/common/auth/abstractions/token.service"; @@ -37,10 +37,11 @@ import { AuthRequestServiceAbstraction, InternalUserDecryptionOptionsServiceAbstraction, } from "../abstractions"; +import { UserDecryptionOptions } from "../models"; import { SsoLoginCredentials } from "../models/domain/login-credentials"; import { identityTokenResponseFactory } from "./login.strategy.spec"; -import { SsoLoginStrategy } from "./sso-login.strategy"; +import { SsoLoginStrategy, SsoLoginStrategyData } from "./sso-login.strategy"; describe("SsoLoginStrategy", () => { let accountService: FakeAccountService; @@ -123,8 +124,11 @@ describe("SsoLoginStrategy", () => { mockVaultTimeoutBSub.asObservable(), ); + const userDecryptionOptions = new UserDecryptionOptions(); + userDecryptionOptionsService.userDecryptionOptions$ = of(userDecryptionOptions); + ssoLoginStrategy = new SsoLoginStrategy( - null, + {} as SsoLoginStrategyData, keyConnectorService, deviceTrustService, authRequestService, diff --git a/libs/auth/src/common/login-strategies/sso-login.strategy.ts b/libs/auth/src/common/login-strategies/sso-login.strategy.ts index 1dd01d6fc75..d81284a960e 100644 --- a/libs/auth/src/common/login-strategies/sso-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/sso-login.strategy.ts @@ -4,6 +4,7 @@ import { firstValueFrom, Observable, map, BehaviorSubject } from "rxjs"; import { Jsonify } from "type-fest"; import { AuthResult } from "@bitwarden/common/auth/models/domain/auth-result"; +import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; import { SsoTokenRequest } from "@bitwarden/common/auth/models/request/identity-token/sso-token.request"; import { AuthRequestResponse } from "@bitwarden/common/auth/models/response/auth-request.response"; import { IdentityTokenResponse } from "@bitwarden/common/auth/models/response/identity-token.response"; @@ -355,4 +356,75 @@ export class SsoLoginStrategy extends LoginStrategy { sso: this.cache.value, }; } + + /** + * Override to handle SSO-specific ForceSetPasswordReason flags,including TdeOffboarding, + * TdeUserWithoutPasswordHasPasswordResetPermission, and SsoNewJitProvisionedUser cases. + * @param authResult - The authentication result + * @param userId - The user ID + */ + override async processForceSetPasswordReason( + adminForcePasswordReset: boolean, + userId: UserId, + ): Promise { + // handle any existing reasons + const adminForcePasswordResetFlagSet = await super.processForceSetPasswordReason( + adminForcePasswordReset, + userId, + ); + + // If we are already processing an admin force password reset, don't process other reasons + if (adminForcePasswordResetFlagSet) { + return false; + } + + // Check for TDE-related conditions + const userDecryptionOptions = await firstValueFrom( + this.userDecryptionOptionsService.userDecryptionOptions$, + ); + + if (!userDecryptionOptions) { + return false; + } + + // Check for TDE offboarding - user is being offboarded from TDE and needs to set a password + if (userDecryptionOptions.trustedDeviceOption?.isTdeOffboarding) { + await this.masterPasswordService.setForceSetPasswordReason( + ForceSetPasswordReason.TdeOffboarding, + userId, + ); + return true; + } + + // Check if user has permission to set password but hasn't yet + if ( + !userDecryptionOptions.hasMasterPassword && + userDecryptionOptions.trustedDeviceOption?.hasManageResetPasswordPermission + ) { + await this.masterPasswordService.setForceSetPasswordReason( + ForceSetPasswordReason.TdeUserWithoutPasswordHasPasswordResetPermission, + userId, + ); + + return true; + } + + // Check for new SSO JIT provisioned user + // If a user logs in via SSO but has no master password and no alternative encryption methods + // Then they must be a newly provisioned user who needs to set up their encryption + if ( + !userDecryptionOptions.hasMasterPassword && + !userDecryptionOptions.keyConnectorOption?.keyConnectorUrl && + !userDecryptionOptions.trustedDeviceOption + ) { + await this.masterPasswordService.setForceSetPasswordReason( + ForceSetPasswordReason.SsoNewJitProvisionedUser, + userId, + ); + return true; + } + + // If none of the conditions are met, return false + return false; + } } diff --git a/libs/auth/src/common/login-strategies/webauthn-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/webauthn-login.strategy.spec.ts index ebbe2f3b6cb..fb4cbd55ad9 100644 --- a/libs/auth/src/common/login-strategies/webauthn-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/webauthn-login.strategy.spec.ts @@ -209,7 +209,6 @@ describe("WebAuthnLoginStrategy", () => { expect(authResult).toBeInstanceOf(AuthResult); expect(authResult).toMatchObject({ captchaSiteKey: "", - forcePasswordReset: 0, resetMasterPassword: false, twoFactorProviders: null, requiresTwoFactor: false, diff --git a/libs/common/src/auth/models/domain/auth-result.ts b/libs/common/src/auth/models/domain/auth-result.ts index fdc8c963a1b..5177363ac89 100644 --- a/libs/common/src/auth/models/domain/auth-result.ts +++ b/libs/common/src/auth/models/domain/auth-result.ts @@ -4,8 +4,6 @@ import { Utils } from "../../../platform/misc/utils"; import { UserId } from "../../../types/guid"; import { TwoFactorProviderType } from "../../enums/two-factor-provider-type"; -import { ForceSetPasswordReason } from "./force-set-password-reason"; - export class AuthResult { userId: UserId; captchaSiteKey = ""; @@ -17,7 +15,6 @@ export class AuthResult { * */ resetMasterPassword = false; - forcePasswordReset: ForceSetPasswordReason = ForceSetPasswordReason.None; twoFactorProviders: Partial>> = null; ssoEmail2FaSessionToken?: string; email: string; diff --git a/libs/common/src/auth/models/domain/force-set-password-reason.ts b/libs/common/src/auth/models/domain/force-set-password-reason.ts index 011ef7fff8d..56d52860443 100644 --- a/libs/common/src/auth/models/domain/force-set-password-reason.ts +++ b/libs/common/src/auth/models/domain/force-set-password-reason.ts @@ -31,4 +31,9 @@ export enum ForceSetPasswordReason { * Occurs when TDE is disabled and master password has to be set. */ TdeOffboarding, + + /** + * Occurs when a new SSO user is JIT provisioned and needs to set their master password. + */ + SsoNewJitProvisionedUser, } diff --git a/libs/common/src/key-management/master-password/services/master-password.service.spec.ts b/libs/common/src/key-management/master-password/services/master-password.service.spec.ts new file mode 100644 index 00000000000..93439ac8caa --- /dev/null +++ b/libs/common/src/key-management/master-password/services/master-password.service.spec.ts @@ -0,0 +1,104 @@ +import { mock, MockProxy } from "jest-mock-extended"; +import { of } from "rxjs"; +import * as rxjs from "rxjs"; + +import { ForceSetPasswordReason } from "../../../auth/models/domain/force-set-password-reason"; +import { KeyGenerationService } from "../../../platform/abstractions/key-generation.service"; +import { LogService } from "../../../platform/abstractions/log.service"; +import { StateService } from "../../../platform/abstractions/state.service"; +import { StateProvider } from "../../../platform/state"; +import { UserId } from "../../../types/guid"; +import { EncryptService } from "../../crypto/abstractions/encrypt.service"; + +import { MasterPasswordService } from "./master-password.service"; + +describe("MasterPasswordService", () => { + let sut: MasterPasswordService; + + let stateProvider: MockProxy; + let stateService: MockProxy; + let keyGenerationService: MockProxy; + let encryptService: MockProxy; + let logService: MockProxy; + + const userId = "user-id" as UserId; + const mockUserState = { + state$: of(null), + update: jest.fn().mockResolvedValue(null), + }; + + beforeEach(() => { + stateProvider = mock(); + stateService = mock(); + keyGenerationService = mock(); + encryptService = mock(); + logService = mock(); + + stateProvider.getUser.mockReturnValue(mockUserState as any); + + mockUserState.update.mockReset(); + + sut = new MasterPasswordService( + stateProvider, + stateService, + keyGenerationService, + encryptService, + logService, + ); + }); + + describe("setForceSetPasswordReason", () => { + it("calls stateProvider with the provided reason and user ID", async () => { + const reason = ForceSetPasswordReason.WeakMasterPassword; + + await sut.setForceSetPasswordReason(reason, userId); + + expect(stateProvider.getUser).toHaveBeenCalled(); + expect(mockUserState.update).toHaveBeenCalled(); + + // Call the update function to verify it returns the correct reason + const updateFn = mockUserState.update.mock.calls[0][0]; + expect(updateFn(null)).toBe(reason); + }); + + it("throws an error if reason is null", async () => { + await expect( + sut.setForceSetPasswordReason(null as unknown as ForceSetPasswordReason, userId), + ).rejects.toThrow("Reason is required."); + }); + + it("throws an error if user ID is null", async () => { + await expect( + sut.setForceSetPasswordReason(ForceSetPasswordReason.None, null as unknown as UserId), + ).rejects.toThrow("User ID is required."); + }); + + it("does not overwrite AdminForcePasswordReset with other reasons except None", async () => { + jest + .spyOn(sut, "forceSetPasswordReason$") + .mockReturnValue(of(ForceSetPasswordReason.AdminForcePasswordReset)); + + jest + .spyOn(rxjs, "firstValueFrom") + .mockResolvedValue(ForceSetPasswordReason.AdminForcePasswordReset); + + await sut.setForceSetPasswordReason(ForceSetPasswordReason.WeakMasterPassword, userId); + + expect(mockUserState.update).not.toHaveBeenCalled(); + }); + + it("allows overwriting AdminForcePasswordReset with None", async () => { + jest + .spyOn(sut, "forceSetPasswordReason$") + .mockReturnValue(of(ForceSetPasswordReason.AdminForcePasswordReset)); + + jest + .spyOn(rxjs, "firstValueFrom") + .mockResolvedValue(ForceSetPasswordReason.AdminForcePasswordReset); + + await sut.setForceSetPasswordReason(ForceSetPasswordReason.None, userId); + + expect(mockUserState.update).toHaveBeenCalled(); + }); + }); +}); diff --git a/libs/common/src/key-management/master-password/services/master-password.service.ts b/libs/common/src/key-management/master-password/services/master-password.service.ts index 9ed01cf0c83..72b18d8bfba 100644 --- a/libs/common/src/key-management/master-password/services/master-password.service.ts +++ b/libs/common/src/key-management/master-password/services/master-password.service.ts @@ -148,6 +148,17 @@ export class MasterPasswordService implements InternalMasterPasswordServiceAbstr if (userId == null) { throw new Error("User ID is required."); } + + // Don't overwrite AdminForcePasswordReset with any other reasons other than None + // as we must allow a reset when the user has completed admin account recovery + const currentReason = await firstValueFrom(this.forceSetPasswordReason$(userId)); + if ( + currentReason === ForceSetPasswordReason.AdminForcePasswordReset && + reason !== ForceSetPasswordReason.None + ) { + return; + } + await this.stateProvider.getUser(userId, FORCE_SET_PASSWORD_REASON).update((_) => reason); } diff --git a/libs/key-management-ui/src/lock/components/lock.component.ts b/libs/key-management-ui/src/lock/components/lock.component.ts index 125bf6ab6af..80d64e17b84 100644 --- a/libs/key-management-ui/src/lock/components/lock.component.ts +++ b/libs/key-management-ui/src/lock/components/lock.component.ts @@ -121,8 +121,6 @@ export class LockComponent implements OnInit, OnDestroy { showPassword = false; private enforcedMasterPasswordOptions?: MasterPasswordPolicyOptions = undefined; - forcePasswordResetRoute = "update-temp-password"; - formGroup: FormGroup | null = null; // Browser extension properties: @@ -605,8 +603,6 @@ export class LockComponent implements OnInit, OnDestroy { ForceSetPasswordReason.WeakMasterPassword, userId, ); - await this.router.navigate([this.forcePasswordResetRoute]); - return; } } catch (e) { // Do not prevent unlock if there is an error evaluating policies