1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-12 14:23:32 +00:00

[PM-19212] Consolidate password set routing to AuthGuard using ForceSetPasswordReason (#14356)

* Consolidates component routing, removing routing to update-temp-password from components. All routing to update-temp-password should happen in the AuthGuard now.

---------

Co-authored-by: Jared Snider <jsnider@bitwarden.com>
Co-authored-by: Todd Martin <tmartin@bitwarden.com>
This commit is contained in:
Alec Rippberger
2025-05-08 11:24:52 -05:00
committed by GitHub
parent 78dafe2265
commit 3030eb7552
23 changed files with 324 additions and 165 deletions

View File

@@ -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 { 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 { UserVerificationService as UserVerificationServiceAbstraction } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction";
import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; 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 { AccountServiceImplementation } from "@bitwarden/common/auth/services/account.service";
import { AuthService } from "@bitwarden/common/auth/services/auth.service"; import { AuthService } from "@bitwarden/common/auth/services/auth.service";
import { AvatarService } from "@bitwarden/common/auth/services/avatar.service"; import { AvatarService } from "@bitwarden/common/auth/services/avatar.service";
@@ -1511,9 +1510,6 @@ export default class MainBackground {
} }
nextAccountStatus = await this.authService.getAuthStatus(userId); nextAccountStatus = await this.authService.getAuthStatus(userId);
const forcePasswordReset =
(await firstValueFrom(this.masterPasswordService.forceSetPasswordReason$(userId))) !=
ForceSetPasswordReason.None;
await this.systemService.clearPendingClipboard(); await this.systemService.clearPendingClipboard();
@@ -1521,8 +1517,6 @@ export default class MainBackground {
this.messagingService.send("goHome"); this.messagingService.send("goHome");
} else if (nextAccountStatus === AuthenticationStatus.Locked) { } else if (nextAccountStatus === AuthenticationStatus.Locked) {
this.messagingService.send("locked", { userId: userId }); this.messagingService.send("locked", { userId: userId });
} else if (forcePasswordReset) {
this.messagingService.send("update-temp-password", { userId: userId });
} else { } else {
this.messagingService.send("unlocked", { userId: userId }); this.messagingService.send("unlocked", { userId: userId });
await this.refreshBadge(); await this.refreshBadge();

View File

@@ -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. // 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 // eslint-disable-next-line @typescript-eslint/no-floating-promises
this.router.navigate(["/remove-password"]); 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$), takeUntil(this.destroy$),

View File

@@ -32,6 +32,7 @@ import { UpdateTempPasswordRequest } from "@bitwarden/common/auth/models/request
import { ClientType } from "@bitwarden/common/enums"; import { ClientType } from "@bitwarden/common/enums";
import { CryptoFunctionService } from "@bitwarden/common/key-management/crypto/abstractions/crypto-function.service"; 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 { 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 { ErrorResponse } from "@bitwarden/common/models/response/error.response";
import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
@@ -77,6 +78,7 @@ export class LoginCommand {
protected logoutCallback: () => Promise<void>, protected logoutCallback: () => Promise<void>,
protected kdfConfigService: KdfConfigService, protected kdfConfigService: KdfConfigService,
protected ssoUrlService: SsoUrlService, protected ssoUrlService: SsoUrlService,
protected masterPasswordService: MasterPasswordServiceAbstraction,
) {} ) {}
async run(email: string, password: string, options: OptionValues) { async run(email: string, password: string, options: OptionValues) {
@@ -361,14 +363,14 @@ export class LoginCommand {
await this.syncService.fullSync(true); await this.syncService.fullSync(true);
// Handle updating passwords if NOT using an API Key for authentication // Handle updating passwords if NOT using an API Key for authentication
if ( if (clientId == null && clientSecret == null) {
response.forcePasswordReset != ForceSetPasswordReason.None && const forceSetPasswordReason = await firstValueFrom(
clientId == null && this.masterPasswordService.forceSetPasswordReason$(response.userId),
clientSecret == null );
) {
if (response.forcePasswordReset === ForceSetPasswordReason.AdminForcePasswordReset) { if (forceSetPasswordReason === ForceSetPasswordReason.AdminForcePasswordReset) {
return await this.updateTempPassword(response.userId); return await this.updateTempPassword(response.userId);
} else if (response.forcePasswordReset === ForceSetPasswordReason.WeakMasterPassword) { } else if (forceSetPasswordReason === ForceSetPasswordReason.WeakMasterPassword) {
return await this.updateWeakPassword(response.userId, password); return await this.updateWeakPassword(response.userId, password);
} }
} }

View File

@@ -172,6 +172,7 @@ export class Program extends BaseProgram {
async () => await this.serviceContainer.logout(), async () => await this.serviceContainer.logout(),
this.serviceContainer.kdfConfigService, this.serviceContainer.kdfConfigService,
this.serviceContainer.ssoUrlService, this.serviceContainer.ssoUrlService,
this.serviceContainer.masterPasswordService,
); );
const response = await command.run(email, password, options); const response = await command.run(email, password, options);
this.processResponse(response, true); this.processResponse(response, true);

View File

@@ -27,7 +27,6 @@ import { AccountService } from "@bitwarden/common/auth/abstractions/account.serv
import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction";
import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; 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 { getUserId } from "@bitwarden/common/auth/services/account.service";
import { ProcessReloadServiceAbstraction } from "@bitwarden/common/key-management/abstractions/process-reload.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"; 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 = const locked =
(await this.authService.getAuthStatus(message.userId)) === (await this.authService.getAuthStatus(message.userId)) ===
AuthenticationStatus.Locked; AuthenticationStatus.Locked;
const forcedPasswordReset =
(await firstValueFrom(
this.masterPasswordService.forceSetPasswordReason$(message.userId),
)) != ForceSetPasswordReason.None;
if (locked) { if (locked) {
this.modalService.closeAll(); this.modalService.closeAll();
await this.router.navigate(["lock"]); 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 { } else {
this.messagingService.send("unlocked"); this.messagingService.send("unlocked");
this.loading = true; this.loading = true;

View File

@@ -6,7 +6,6 @@ import { firstValueFrom } from "rxjs";
import { LoginSuccessHandlerService } from "@bitwarden/auth/common"; import { LoginSuccessHandlerService } from "@bitwarden/auth/common";
import { WebAuthnLoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/webauthn/webauthn-login.service.abstraction"; 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 { WebAuthnLoginCredentialAssertionView } from "@bitwarden/common/auth/models/view/webauthn-login/webauthn-login-credential-assertion.view";
import { ErrorResponse } from "@bitwarden/common/models/response/error.response"; import { ErrorResponse } from "@bitwarden/common/models/response/error.response";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
@@ -21,7 +20,6 @@ export class BaseLoginViaWebAuthnComponent implements OnInit {
protected currentState: State = "assert"; protected currentState: State = "assert";
protected successRoute = "/vault"; protected successRoute = "/vault";
protected forcePasswordResetRoute = "/update-temp-password";
constructor( constructor(
private webAuthnLoginService: WebAuthnLoginServiceAbstraction, private webAuthnLoginService: WebAuthnLoginServiceAbstraction,
@@ -73,11 +71,6 @@ export class BaseLoginViaWebAuthnComponent implements OnInit {
await this.loginSuccessHandlerService.run(authResult.userId); await this.loginSuccessHandlerService.run(authResult.userId);
} }
if (authResult.forcePasswordReset == ForceSetPasswordReason.AdminForcePasswordReset) {
await this.router.navigate([this.forcePasswordResetRoute]);
return;
}
await this.router.navigate([this.successRoute]); await this.router.navigate([this.successRoute]);
} catch (error) { } catch (error) {
if (error instanceof ErrorResponse) { if (error instanceof ErrorResponse) {

View File

@@ -19,7 +19,6 @@ import { AuthRequestType } from "@bitwarden/common/auth/enums/auth-request-type"
import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
import { AdminAuthRequestStorable } from "@bitwarden/common/auth/models/domain/admin-auth-req-storable"; import { AdminAuthRequestStorable } from "@bitwarden/common/auth/models/domain/admin-auth-req-storable";
import { AuthResult } from "@bitwarden/common/auth/models/domain/auth-result"; 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 { AuthRequest } from "@bitwarden/common/auth/models/request/auth.request";
import { AuthRequestResponse } from "@bitwarden/common/auth/models/response/auth-request.response"; import { AuthRequestResponse } from "@bitwarden/common/auth/models/response/auth-request.response";
import { LoginViaAuthRequestView } from "@bitwarden/common/auth/models/view/login-via-auth-request.view"; 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) { private async handlePostLoginNavigation(loginResponse: AuthResult) {
if (loginResponse.requiresTwoFactor) { if (loginResponse.requiresTwoFactor) {
await this.router.navigate(["2fa"]); await this.router.navigate(["2fa"]);
} else if (loginResponse.forcePasswordReset != ForceSetPasswordReason.None) {
await this.router.navigate(["update-temp-password"]);
} else { } else {
await this.handleSuccessfulLoginNavigation(loginResponse.userId); await this.handleSuccessfulLoginNavigation(loginResponse.userId);
} }

View File

@@ -17,7 +17,6 @@ import { MasterPasswordPolicyOptions } from "@bitwarden/common/admin-console/mod
import { Policy } from "@bitwarden/common/admin-console/models/domain/policy"; import { Policy } from "@bitwarden/common/admin-console/models/domain/policy";
import { DevicesApiServiceAbstraction } from "@bitwarden/common/auth/abstractions/devices-api.service.abstraction"; import { DevicesApiServiceAbstraction } from "@bitwarden/common/auth/abstractions/devices-api.service.abstraction";
import { AuthResult } from "@bitwarden/common/auth/models/domain/auth-result"; 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 { ClientType, HttpStatusCode } from "@bitwarden/common/enums";
import { ErrorResponse } from "@bitwarden/common/models/response/error.response"; import { ErrorResponse } from "@bitwarden/common/models/response/error.response";
import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service"; 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); await this.loginSuccessHandlerService.run(authResult.userId);
// Determine where to send the user next // Determine where to send the user next
if (authResult.forcePasswordReset != ForceSetPasswordReason.None) { // The AuthGuard will handle routing to update-temp-password based on state
await this.router.navigate(["update-temp-password"]);
return;
}
// TODO: PM-18269 - evaluate if we can combine this with the // TODO: PM-18269 - evaluate if we can combine this with the
// password evaluation done in the password login strategy. // password evaluation done in the password login strategy.

View File

@@ -136,11 +136,6 @@ export class NewDeviceVerificationComponent implements OnInit, OnDestroy {
return; return;
} }
if (authResult.forcePasswordReset) {
await this.router.navigate(["/update-temp-password"]);
return;
}
this.loginSuccessHandlerService.run(authResult.userId); this.loginSuccessHandlerService.run(authResult.userId);
// If verification succeeds, navigate to vault // If verification succeeds, navigate to vault

View File

@@ -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() { private async handleSuccessfulLogin() {
await this.router.navigate(["lock"]); await this.router.navigate(["lock"]);
} }

View File

@@ -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() { showContinueButton() {
return ( return (
this.selectedProviderType != null && this.selectedProviderType != null &&

View File

@@ -296,13 +296,9 @@ describe("LoginStrategy", () => {
const expected = new AuthResult(); const expected = new AuthResult();
expected.userId = userId; expected.userId = userId;
expected.forcePasswordReset = ForceSetPasswordReason.AdminForcePasswordReset;
expected.resetMasterPassword = true; expected.resetMasterPassword = true;
expected.twoFactorProviders = {} as Partial<
Record<TwoFactorProviderType, Record<string, string>>
>;
expected.captchaSiteKey = "";
expected.twoFactorProviders = null; expected.twoFactorProviders = null;
expected.captchaSiteKey = "";
expect(result).toEqual(expected); expect(result).toEqual(expected);
}); });
@@ -316,13 +312,9 @@ describe("LoginStrategy", () => {
const expected = new AuthResult(); const expected = new AuthResult();
expected.userId = userId; expected.userId = userId;
expected.forcePasswordReset = ForceSetPasswordReason.AdminForcePasswordReset;
expected.resetMasterPassword = false; expected.resetMasterPassword = false;
expected.twoFactorProviders = {} as Partial<
Record<TwoFactorProviderType, Record<string, string>>
>;
expected.captchaSiteKey = "";
expected.twoFactorProviders = null; expected.twoFactorProviders = null;
expected.captchaSiteKey = "";
expect(result).toEqual(expected); expect(result).toEqual(expected);
expect(masterPasswordService.mock.setForceSetPasswordReason).toHaveBeenCalledWith( expect(masterPasswordService.mock.setForceSetPasswordReason).toHaveBeenCalledWith(

View File

@@ -277,17 +277,7 @@ export abstract class LoginStrategy {
result.resetMasterPassword = response.resetMasterPassword; result.resetMasterPassword = response.resetMasterPassword;
// Convert boolean to enum and set the state for the master password service to await this.processForceSetPasswordReason(response.forcePasswordReset, userId);
// 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,
);
}
if (response.twoFactorToken != null) { if (response.twoFactorToken != null) {
// note: we can read email from access token b/c it was saved in saveAccountInformation // 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; 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<boolean> {
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) { protected async createKeyPairForOldAccount(userId: UserId) {
try { try {
const userKey = await this.keyService.getUserKeyWithLegacySupport(userId); const userKey = await this.keyService.getUserKeyWithLegacySupport(userId);

View File

@@ -211,20 +211,18 @@ describe("PasswordLoginStrategy", () => {
it("does not force the user to update their master password when there are no requirements", async () => { it("does not force the user to update their master password when there are no requirements", async () => {
apiService.postIdentityToken.mockResolvedValueOnce(identityTokenResponseFactory()); apiService.postIdentityToken.mockResolvedValueOnce(identityTokenResponseFactory());
const result = await passwordLoginStrategy.logIn(credentials); await passwordLoginStrategy.logIn(credentials);
expect(policyService.evaluateMasterPassword).not.toHaveBeenCalled(); 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 () => { it("does not force the user to update their master password when it meets requirements", async () => {
passwordStrengthService.getPasswordStrength.mockReturnValue({ score: 5 } as any); passwordStrengthService.getPasswordStrength.mockReturnValue({ score: 5 } as any);
policyService.evaluateMasterPassword.mockReturnValue(true); policyService.evaluateMasterPassword.mockReturnValue(true);
const result = await passwordLoginStrategy.logIn(credentials); await passwordLoginStrategy.logIn(credentials);
expect(policyService.evaluateMasterPassword).toHaveBeenCalled(); 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 () => { 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); policyService.evaluateMasterPassword.mockReturnValue(false);
tokenService.decodeAccessToken.mockResolvedValue({ sub: userId }); tokenService.decodeAccessToken.mockResolvedValue({ sub: userId });
const result = await passwordLoginStrategy.logIn(credentials); await passwordLoginStrategy.logIn(credentials);
expect(policyService.evaluateMasterPassword).toHaveBeenCalled(); expect(policyService.evaluateMasterPassword).toHaveBeenCalled();
expect(masterPasswordService.mock.setForceSetPasswordReason).toHaveBeenCalledWith( expect(masterPasswordService.mock.setForceSetPasswordReason).toHaveBeenCalledWith(
ForceSetPasswordReason.WeakMasterPassword, ForceSetPasswordReason.WeakMasterPassword,
userId, 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 () => { 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 // First login request fails requiring 2FA
apiService.postIdentityToken.mockResolvedValueOnce(token2FAResponse); apiService.postIdentityToken.mockResolvedValueOnce(token2FAResponse);
const firstResult = await passwordLoginStrategy.logIn(credentials); await passwordLoginStrategy.logIn(credentials);
// Second login request succeeds // Second login request succeeds
apiService.postIdentityToken.mockResolvedValueOnce( apiService.postIdentityToken.mockResolvedValueOnce(
identityTokenResponseFactory(masterPasswordPolicy), identityTokenResponseFactory(masterPasswordPolicy),
); );
const secondResult = await passwordLoginStrategy.logInTwoFactor( await passwordLoginStrategy.logInTwoFactor(
{ {
provider: TwoFactorProviderType.Authenticator, provider: TwoFactorProviderType.Authenticator,
token: "123456", token: "123456",
@@ -272,15 +269,11 @@ describe("PasswordLoginStrategy", () => {
"", "",
); );
// First login attempt should not save the force password reset options // Second login attempt should 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
expect(masterPasswordService.mock.setForceSetPasswordReason).toHaveBeenCalledWith( expect(masterPasswordService.mock.setForceSetPasswordReason).toHaveBeenCalledWith(
ForceSetPasswordReason.WeakMasterPassword, ForceSetPasswordReason.WeakMasterPassword,
userId, userId,
); );
expect(secondResult.forcePasswordReset).toEqual(ForceSetPasswordReason.WeakMasterPassword);
}); });
it("handles new device verification login with OTP", async () => { it("handles new device verification login with OTP", async () => {
@@ -298,7 +291,6 @@ describe("PasswordLoginStrategy", () => {
newDeviceOtp: deviceVerificationOtp, newDeviceOtp: deviceVerificationOtp,
}), }),
); );
expect(result.forcePasswordReset).toBe(ForceSetPasswordReason.None);
expect(result.resetMasterPassword).toBe(false); expect(result.resetMasterPassword).toBe(false);
expect(result.userId).toBe(userId); expect(result.userId).toBe(userId);
}); });

View File

@@ -109,35 +109,8 @@ export class PasswordLoginStrategy extends LoginStrategy {
return authResult; return authResult;
} }
const masterPasswordPolicyOptions = await this.evaluateMasterPasswordIfRequired(identityResponse, credentials, authResult);
this.getMasterPasswordPolicyOptionsFromResponse(identityResponse);
// 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; return authResult;
} }
@@ -151,20 +124,6 @@ export class PasswordLoginStrategy extends LoginStrategy {
const result = await super.logInTwoFactor(twoFactor); 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; return result;
} }
@@ -208,13 +167,58 @@ export class PasswordLoginStrategy extends LoginStrategy {
return !response.key; return !response.key;
} }
private getMasterPasswordPolicyOptionsFromResponse( private async evaluateMasterPasswordIfRequired(
response: identityResponse:
| IdentityTokenResponse | IdentityTokenResponse
| IdentityTwoFactorResponse | IdentityTwoFactorResponse
| IdentityDeviceVerificationResponse, | IdentityDeviceVerificationResponse,
): MasterPasswordPolicyOptions { credentials: PasswordLoginCredentials,
if (response == null || response instanceof IdentityDeviceVerificationResponse) { authResult: AuthResult,
): Promise<void> {
// 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 null;
} }
return MasterPasswordPolicyOptions.fromResponse(response.masterPasswordPolicy); return MasterPasswordPolicyOptions.fromResponse(response.masterPasswordPolicy);
@@ -246,4 +250,35 @@ export class PasswordLoginStrategy extends LoginStrategy {
const [authResult] = await this.startLogIn(); const [authResult] = await this.startLogIn();
return authResult; 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<boolean> {
// 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;
}
} }

View File

@@ -1,5 +1,5 @@
import { mock, MockProxy } from "jest-mock-extended"; 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 { ApiService } from "@bitwarden/common/abstractions/api.service";
import { TokenService } from "@bitwarden/common/auth/abstractions/token.service"; import { TokenService } from "@bitwarden/common/auth/abstractions/token.service";
@@ -37,10 +37,11 @@ import {
AuthRequestServiceAbstraction, AuthRequestServiceAbstraction,
InternalUserDecryptionOptionsServiceAbstraction, InternalUserDecryptionOptionsServiceAbstraction,
} from "../abstractions"; } from "../abstractions";
import { UserDecryptionOptions } from "../models";
import { SsoLoginCredentials } from "../models/domain/login-credentials"; import { SsoLoginCredentials } from "../models/domain/login-credentials";
import { identityTokenResponseFactory } from "./login.strategy.spec"; import { identityTokenResponseFactory } from "./login.strategy.spec";
import { SsoLoginStrategy } from "./sso-login.strategy"; import { SsoLoginStrategy, SsoLoginStrategyData } from "./sso-login.strategy";
describe("SsoLoginStrategy", () => { describe("SsoLoginStrategy", () => {
let accountService: FakeAccountService; let accountService: FakeAccountService;
@@ -123,8 +124,11 @@ describe("SsoLoginStrategy", () => {
mockVaultTimeoutBSub.asObservable(), mockVaultTimeoutBSub.asObservable(),
); );
const userDecryptionOptions = new UserDecryptionOptions();
userDecryptionOptionsService.userDecryptionOptions$ = of(userDecryptionOptions);
ssoLoginStrategy = new SsoLoginStrategy( ssoLoginStrategy = new SsoLoginStrategy(
null, {} as SsoLoginStrategyData,
keyConnectorService, keyConnectorService,
deviceTrustService, deviceTrustService,
authRequestService, authRequestService,

View File

@@ -4,6 +4,7 @@ import { firstValueFrom, Observable, map, BehaviorSubject } from "rxjs";
import { Jsonify } from "type-fest"; import { Jsonify } from "type-fest";
import { AuthResult } from "@bitwarden/common/auth/models/domain/auth-result"; 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 { SsoTokenRequest } from "@bitwarden/common/auth/models/request/identity-token/sso-token.request";
import { AuthRequestResponse } from "@bitwarden/common/auth/models/response/auth-request.response"; import { AuthRequestResponse } from "@bitwarden/common/auth/models/response/auth-request.response";
import { IdentityTokenResponse } from "@bitwarden/common/auth/models/response/identity-token.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, 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<boolean> {
// 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;
}
} }

View File

@@ -209,7 +209,6 @@ describe("WebAuthnLoginStrategy", () => {
expect(authResult).toBeInstanceOf(AuthResult); expect(authResult).toBeInstanceOf(AuthResult);
expect(authResult).toMatchObject({ expect(authResult).toMatchObject({
captchaSiteKey: "", captchaSiteKey: "",
forcePasswordReset: 0,
resetMasterPassword: false, resetMasterPassword: false,
twoFactorProviders: null, twoFactorProviders: null,
requiresTwoFactor: false, requiresTwoFactor: false,

View File

@@ -4,8 +4,6 @@ import { Utils } from "../../../platform/misc/utils";
import { UserId } from "../../../types/guid"; import { UserId } from "../../../types/guid";
import { TwoFactorProviderType } from "../../enums/two-factor-provider-type"; import { TwoFactorProviderType } from "../../enums/two-factor-provider-type";
import { ForceSetPasswordReason } from "./force-set-password-reason";
export class AuthResult { export class AuthResult {
userId: UserId; userId: UserId;
captchaSiteKey = ""; captchaSiteKey = "";
@@ -17,7 +15,6 @@ export class AuthResult {
* */ * */
resetMasterPassword = false; resetMasterPassword = false;
forcePasswordReset: ForceSetPasswordReason = ForceSetPasswordReason.None;
twoFactorProviders: Partial<Record<TwoFactorProviderType, Record<string, string>>> = null; twoFactorProviders: Partial<Record<TwoFactorProviderType, Record<string, string>>> = null;
ssoEmail2FaSessionToken?: string; ssoEmail2FaSessionToken?: string;
email: string; email: string;

View File

@@ -31,4 +31,9 @@ export enum ForceSetPasswordReason {
* Occurs when TDE is disabled and master password has to be set. * Occurs when TDE is disabled and master password has to be set.
*/ */
TdeOffboarding, TdeOffboarding,
/**
* Occurs when a new SSO user is JIT provisioned and needs to set their master password.
*/
SsoNewJitProvisionedUser,
} }

View File

@@ -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<StateProvider>;
let stateService: MockProxy<StateService>;
let keyGenerationService: MockProxy<KeyGenerationService>;
let encryptService: MockProxy<EncryptService>;
let logService: MockProxy<LogService>;
const userId = "user-id" as UserId;
const mockUserState = {
state$: of(null),
update: jest.fn().mockResolvedValue(null),
};
beforeEach(() => {
stateProvider = mock<StateProvider>();
stateService = mock<StateService>();
keyGenerationService = mock<KeyGenerationService>();
encryptService = mock<EncryptService>();
logService = mock<LogService>();
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();
});
});
});

View File

@@ -148,6 +148,17 @@ export class MasterPasswordService implements InternalMasterPasswordServiceAbstr
if (userId == null) { if (userId == null) {
throw new Error("User ID is required."); 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); await this.stateProvider.getUser(userId, FORCE_SET_PASSWORD_REASON).update((_) => reason);
} }

View File

@@ -121,8 +121,6 @@ export class LockComponent implements OnInit, OnDestroy {
showPassword = false; showPassword = false;
private enforcedMasterPasswordOptions?: MasterPasswordPolicyOptions = undefined; private enforcedMasterPasswordOptions?: MasterPasswordPolicyOptions = undefined;
forcePasswordResetRoute = "update-temp-password";
formGroup: FormGroup | null = null; formGroup: FormGroup | null = null;
// Browser extension properties: // Browser extension properties:
@@ -605,8 +603,6 @@ export class LockComponent implements OnInit, OnDestroy {
ForceSetPasswordReason.WeakMasterPassword, ForceSetPasswordReason.WeakMasterPassword,
userId, userId,
); );
await this.router.navigate([this.forcePasswordResetRoute]);
return;
} }
} catch (e) { } catch (e) {
// Do not prevent unlock if there is an error evaluating policies // Do not prevent unlock if there is an error evaluating policies