1
0
mirror of https://github.com/bitwarden/browser.git synced 2025-03-12 13:39:14 +01:00

fix(auth-routing): [PM-19018] SSO TDE Routing Fix - Fixed routing logic. (#13778)

* fix(auth-routing): [PM-19018] SSO TDE Routing Fix - Fixed routing logic.

* PM-19018 - TwoFactorAuthTests - remove tests that are no longer applicable as 2FA comp isn't responsible for setting admin account recovery flag into state.

* PM-19018 - LoginStrategyTests - add test for processing forcePasswordReset response

---------

Co-authored-by: Jared Snider <jsnider@bitwarden.com>
This commit is contained in:
Patrick-Pimentel-Bitwarden 2025-03-10 21:20:11 -04:00 committed by GitHub
parent 992be1d054
commit 3b9be21fd7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 44 additions and 88 deletions

View File

@ -226,20 +226,6 @@ describe("TwoFactorAuthComponent", () => {
}); });
}; };
const testForceResetOnSuccessfulLogin = (reasonString: string) => {
it(`navigates to the component's defined forcePasswordResetRoute route when response.forcePasswordReset is ${reasonString}`, async () => {
// Act
await component.submit("testToken");
// expect(mockRouter.navigate).toHaveBeenCalledTimes(1);
expect(mockRouter.navigate).toHaveBeenCalledWith(["update-temp-password"], {
queryParams: {
identifier: component.orgSsoIdentifier,
},
});
});
};
describe("Standard 2FA scenarios", () => { describe("Standard 2FA scenarios", () => {
describe("submit", () => { describe("submit", () => {
const token = "testToken"; const token = "testToken";
@ -311,26 +297,6 @@ describe("TwoFactorAuthComponent", () => {
}); });
}); });
describe("Force Master Password Reset scenarios", () => {
[
ForceSetPasswordReason.AdminForcePasswordReset,
ForceSetPasswordReason.WeakMasterPassword,
].forEach((forceResetPasswordReason) => {
const reasonString = ForceSetPasswordReason[forceResetPasswordReason];
beforeEach(() => {
// use standard user with MP because this test is not concerned with password reset.
selectedUserDecryptionOptions.next(mockUserDecryptionOpts.withMasterPassword);
const authResult = new AuthResult();
authResult.forcePasswordReset = forceResetPasswordReason;
mockLoginStrategyService.logInTwoFactor.mockResolvedValue(authResult);
});
testForceResetOnSuccessfulLogin(reasonString);
});
});
it("navigates to the component's defined success route (vault is default) when the login is successful", async () => { it("navigates to the component's defined success route (vault is default) when the login is successful", async () => {
mockLoginStrategyService.logInTwoFactor.mockResolvedValue(new AuthResult()); mockLoginStrategyService.logInTwoFactor.mockResolvedValue(new AuthResult());
@ -407,29 +373,7 @@ describe("TwoFactorAuthComponent", () => {
}); });
}); });
describe("Given Trusted Device Encryption is enabled, user doesn't need to set a MP, and forcePasswordReset is required", () => { describe("Given Trusted Device Encryption is enabled and user doesn't need to set a MP", () => {
[
ForceSetPasswordReason.AdminForcePasswordReset,
ForceSetPasswordReason.WeakMasterPassword,
].forEach((forceResetPasswordReason) => {
const reasonString = ForceSetPasswordReason[forceResetPasswordReason];
beforeEach(() => {
// use standard user with MP because this test is not concerned with password reset.
selectedUserDecryptionOptions.next(
mockUserDecryptionOpts.withMasterPasswordAndTrustedDevice,
);
const authResult = new AuthResult();
authResult.forcePasswordReset = forceResetPasswordReason;
mockLoginStrategyService.logInTwoFactor.mockResolvedValue(authResult);
});
testForceResetOnSuccessfulLogin(reasonString);
});
});
describe("Given Trusted Device Encryption is enabled, user doesn't need to set a MP, and forcePasswordReset is not required", () => {
let authResult; let authResult;
beforeEach(() => { beforeEach(() => {
selectedUserDecryptionOptions.next( selectedUserDecryptionOptions.next(
@ -437,7 +381,6 @@ describe("TwoFactorAuthComponent", () => {
); );
authResult = new AuthResult(); authResult = new AuthResult();
authResult.forcePasswordReset = ForceSetPasswordReason.None;
mockLoginStrategyService.logInTwoFactor.mockResolvedValue(authResult); mockLoginStrategyService.logInTwoFactor.mockResolvedValue(authResult);
}); });

View File

@ -396,11 +396,6 @@ export class TwoFactorAuthComponent implements OnInit, OnDestroy {
); );
} }
// note: this flow affects both TDE & standard users
if (this.isForcePasswordResetRequired(authResult)) {
return await this.handleForcePasswordReset(this.orgSsoIdentifier);
}
const userDecryptionOpts = await firstValueFrom( const userDecryptionOpts = await firstValueFrom(
this.userDecryptionOptionsService.userDecryptionOptions$, this.userDecryptionOptionsService.userDecryptionOptions$,
); );
@ -415,6 +410,7 @@ export class TwoFactorAuthComponent implements OnInit, OnDestroy {
const requireSetPassword = const requireSetPassword =
!userDecryptionOpts.hasMasterPassword && userDecryptionOpts.keyConnectorOption === undefined; !userDecryptionOpts.hasMasterPassword && userDecryptionOpts.keyConnectorOption === undefined;
// New users without a master password must set a master password before advancing.
if (requireSetPassword || authResult.resetMasterPassword) { if (requireSetPassword || authResult.resetMasterPassword) {
// Change implies going no password -> password in this case // Change implies going no password -> password in this case
return await this.handleChangePasswordRequired(this.orgSsoIdentifier); return await this.handleChangePasswordRequired(this.orgSsoIdentifier);
@ -524,14 +520,6 @@ export class TwoFactorAuthComponent implements OnInit, OnDestroy {
return forceResetReasons.includes(authResult.forcePasswordReset); return forceResetReasons.includes(authResult.forcePasswordReset);
} }
private async handleForcePasswordReset(orgIdentifier: string | undefined) {
await this.router.navigate(["update-temp-password"], {
queryParams: {
identifier: orgIdentifier,
},
});
}
showContinueButton() { showContinueButton() {
return ( return (
this.selectedProviderType != null && this.selectedProviderType != null &&

View File

@ -306,6 +306,31 @@ describe("LoginStrategy", () => {
expect(result).toEqual(expected); expect(result).toEqual(expected);
}); });
it("processes a forcePasswordReset response properly", async () => {
const tokenResponse = identityTokenResponseFactory();
tokenResponse.forcePasswordReset = true;
apiService.postIdentityToken.mockResolvedValue(tokenResponse);
const result = await passwordLoginStrategy.logIn(credentials);
const expected = new AuthResult();
expected.userId = userId;
expected.forcePasswordReset = ForceSetPasswordReason.AdminForcePasswordReset;
expected.resetMasterPassword = false;
expected.twoFactorProviders = {} as Partial<
Record<TwoFactorProviderType, Record<string, string>>
>;
expected.captchaSiteKey = "";
expected.twoFactorProviders = null;
expect(result).toEqual(expected);
expect(masterPasswordService.mock.setForceSetPasswordReason).toHaveBeenCalledWith(
ForceSetPasswordReason.AdminForcePasswordReset,
userId,
);
});
it("rejects login if CAPTCHA is required", async () => { it("rejects login if CAPTCHA is required", async () => {
// Sample CAPTCHA response // Sample CAPTCHA response
const tokenResponse = new IdentityCaptchaResponse({ const tokenResponse = new IdentityCaptchaResponse({

View File

@ -271,17 +271,24 @@ export abstract class LoginStrategy {
} }
} }
result.resetMasterPassword = response.resetMasterPassword; // Must come before setting keys, user key needs email to update additional keys.
// Convert boolean to enum
if (response.forcePasswordReset) {
result.forcePasswordReset = ForceSetPasswordReason.AdminForcePasswordReset;
}
// Must come before setting keys, user key needs email to update additional keys
const userId = await this.saveAccountInformation(response); const userId = await this.saveAccountInformation(response);
result.userId = userId; result.userId = userId;
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,
);
}
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
const userEmail = await this.tokenService.getEmail(); const userEmail = await this.tokenService.getEmail();
@ -300,7 +307,9 @@ export abstract class LoginStrategy {
// The keys comes from different sources depending on the login strategy // The keys comes from different sources depending on the login strategy
protected abstract setMasterKey(response: IdentityTokenResponse, userId: UserId): Promise<void>; protected abstract setMasterKey(response: IdentityTokenResponse, userId: UserId): Promise<void>;
protected abstract setUserKey(response: IdentityTokenResponse, userId: UserId): Promise<void>; protected abstract setUserKey(response: IdentityTokenResponse, userId: UserId): Promise<void>;
protected abstract setPrivateKey(response: IdentityTokenResponse, userId: UserId): Promise<void>; protected abstract setPrivateKey(response: IdentityTokenResponse, userId: UserId): Promise<void>;
// Old accounts used master key for encryption. We are forcing migrations but only need to // Old accounts used master key for encryption. We are forcing migrations but only need to

View File

@ -6,7 +6,6 @@ import { Jsonify } from "type-fest";
import { DeviceTrustServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust.service.abstraction"; import { DeviceTrustServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust.service.abstraction";
import { KeyConnectorService } from "@bitwarden/common/auth/abstractions/key-connector.service"; import { KeyConnectorService } from "@bitwarden/common/auth/abstractions/key-connector.service";
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";
@ -108,14 +107,6 @@ export class SsoLoginStrategy extends LoginStrategy {
const email = ssoAuthResult.email; const email = ssoAuthResult.email;
const ssoEmail2FaSessionToken = ssoAuthResult.ssoEmail2FaSessionToken; const ssoEmail2FaSessionToken = ssoAuthResult.ssoEmail2FaSessionToken;
// Auth guard currently handles redirects for this.
if (ssoAuthResult.forcePasswordReset == ForceSetPasswordReason.AdminForcePasswordReset) {
await this.masterPasswordService.setForceSetPasswordReason(
ssoAuthResult.forcePasswordReset,
ssoAuthResult.userId,
);
}
this.cache.next({ this.cache.next({
...this.cache.value, ...this.cache.value,
email, email,