Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโ€™ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auth/PM-17693 - Web - Existing users accepting an org invite are required to update password to meet org policy requirements #13388

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,10 @@ describe("WebLoginComponentService", () => {
expect(service).toBeTruthy();
});

describe("getOrgPolicies", () => {
describe("getOrgPoliciesFromOrgInvite", () => {
it("returns undefined if organization invite is null", async () => {
acceptOrganizationInviteService.getOrganizationInvite.mockResolvedValue(null);
const result = await service.getOrgPolicies();
const result = await service.getOrgPoliciesFromOrgInvite();
expect(result).toBeUndefined();
});

Expand All @@ -94,7 +94,7 @@ describe("WebLoginComponentService", () => {
organizationName: "org-name",
});
policyApiService.getPoliciesByToken.mockRejectedValue(error);
await service.getOrgPolicies();
await service.getOrgPoliciesFromOrgInvite();
expect(logService.error).toHaveBeenCalledWith(error);
});

Expand Down Expand Up @@ -130,7 +130,7 @@ describe("WebLoginComponentService", () => {
of(masterPasswordPolicyOptions),
);

const result = await service.getOrgPolicies();
const result = await service.getOrgPoliciesFromOrgInvite();

expect(result).toEqual({
policies: policies,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export class WebLoginComponentService
this.clientType = this.platformUtilsService.getClientType();
}

async getOrgPolicies(): Promise<PasswordPolicies | null> {
async getOrgPoliciesFromOrgInvite(): Promise<PasswordPolicies | null> {
const orgInvite = await this.acceptOrganizationInviteService.getOrganizationInvite();

if (orgInvite != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,6 @@ describe("DefaultLoginComponentService", () => {
expect(service).toBeTruthy();
});

describe("getOrgPolicies", () => {
it("returns null", async () => {
const result = await service.getOrgPolicies();
expect(result).toBeNull();
});
});

describe("isLoginWithPasskeySupported", () => {
it("returns true when clientType is Web", () => {
service["clientType"] = ClientType.Web;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// @ts-strict-ignore
import { firstValueFrom } from "rxjs";

import { LoginComponentService, PasswordPolicies } from "@bitwarden/auth/angular";
import { LoginComponentService } from "@bitwarden/auth/angular";
import { SsoLoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/sso-login.service.abstraction";
import { ClientType } from "@bitwarden/common/enums";
import { CryptoFunctionService } from "@bitwarden/common/platform/abstractions/crypto-function.service";
Expand All @@ -23,10 +23,6 @@ export class DefaultLoginComponentService implements LoginComponentService {
protected ssoLoginService: SsoLoginServiceAbstraction,
) {}

async getOrgPolicies(): Promise<PasswordPolicies | null> {
return null;
}

isLoginWithPasskeySupported(): boolean {
return this.clientType === ClientType.Web;
}
Expand Down
2 changes: 1 addition & 1 deletion libs/auth/src/angular/login/login-component.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export abstract class LoginComponentService {
* Gets the organization policies if there is an organization invite.
* - Used by: Web
*/
getOrgPolicies: () => Promise<PasswordPolicies | null>;
getOrgPoliciesFromOrgInvite?: () => Promise<PasswordPolicies | null>;

/**
* Indicates whether login with passkey is supported on the given client
Expand Down
120 changes: 64 additions & 56 deletions libs/auth/src/angular/login/login.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
PasswordLoginCredentials,
} from "@bitwarden/auth/common";
import { InternalPolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
import { PolicyData } from "@bitwarden/common/admin-console/models/data/policy.data";

Check warning on line 15 in libs/auth/src/angular/login/login.component.ts

View check run for this annotation

Codecov / codecov/patch

libs/auth/src/angular/login/login.component.ts#L15

Added line #L15 was not covered by tests
import { MasterPasswordPolicyOptions } from "@bitwarden/common/admin-console/models/domain/master-password-policy-options";
import { Policy } from "@bitwarden/common/admin-console/models/domain/policy";
import { DevicesApiServiceAbstraction } from "@bitwarden/common/auth/abstractions/devices-api.service.abstraction";
Expand All @@ -30,6 +31,7 @@
import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service";
import { Utils } from "@bitwarden/common/platform/misc/utils";
import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength";
import { UserId } from "@bitwarden/common/types/guid";
import {
AsyncActionsModule,
ButtonModule,
Expand All @@ -43,7 +45,7 @@
import { AnonLayoutWrapperDataService } from "../anon-layout/anon-layout-wrapper-data.service";
import { VaultIcon, WaveIcon } from "../icons";

import { LoginComponentService } from "./login-component.service";
import { LoginComponentService, PasswordPolicies } from "./login-component.service";

Check warning on line 48 in libs/auth/src/angular/login/login.component.ts

View check run for this annotation

Codecov / codecov/patch

libs/auth/src/angular/login/login.component.ts#L48

Added line #L48 was not covered by tests

const BroadcasterSubscriptionId = "LoginComponent";

Expand Down Expand Up @@ -72,7 +74,6 @@
@ViewChild("masterPasswordInputRef") masterPasswordInputRef: ElementRef | undefined;

private destroy$ = new Subject<void>();
private enforcedMasterPasswordOptions: MasterPasswordPolicyOptions | undefined = undefined;
readonly Icons = { WaveIcon, VaultIcon };

clientType: ClientType;
Expand All @@ -97,11 +98,6 @@
return this.formGroup.controls.email;
}

// Web properties
enforcedPasswordPolicyOptions: MasterPasswordPolicyOptions | undefined;
policies: Policy[] | undefined;
showResetPasswordAutoEnrollWarning = false;

// Desktop properties
deferFocus: boolean | null = null;

Expand Down Expand Up @@ -281,18 +277,39 @@
return;
}

// User logged in successfully so execute side effects
await this.loginSuccessHandlerService.run(authResult.userId);
this.loginEmailService.clearValues();

Check warning on line 282 in libs/auth/src/angular/login/login.component.ts

View check run for this annotation

Codecov / codecov/patch

libs/auth/src/angular/login/login.component.ts#L282

Added line #L282 was not covered by tests

// Determine where to send the user next
if (authResult.forcePasswordReset != ForceSetPasswordReason.None) {
this.loginEmailService.clearValues();
await this.router.navigate(["update-temp-password"]);
return;
}

// If none of the above cases are true, proceed with login...
await this.evaluatePassword();

this.loginEmailService.clearValues();
// TODO: PM-18269 - evaluate if we can combine this with the
// password evaluation done in the password login strategy.
// If there's an existing org invite, use it to get the org's password policies
// so we can evaluate the MP against the org policies
if (this.loginComponentService.getOrgPoliciesFromOrgInvite) {
const orgPolicies: PasswordPolicies | null =
await this.loginComponentService.getOrgPoliciesFromOrgInvite();

Check warning on line 296 in libs/auth/src/angular/login/login.component.ts

View check run for this annotation

Codecov / codecov/patch

libs/auth/src/angular/login/login.component.ts#L296

Added line #L296 was not covered by tests

if (orgPolicies) {
// Since we have retrieved the policies, we can go ahead and set them into state for future use
// e.g., the update-password page currently only references state for policy data and
// doesn't fallback to pulling them from the server like it should if they are null.
await this.setPoliciesIntoState(authResult.userId, orgPolicies.policies);

Check warning on line 302 in libs/auth/src/angular/login/login.component.ts

View check run for this annotation

Codecov / codecov/patch

libs/auth/src/angular/login/login.component.ts#L302

Added line #L302 was not covered by tests

const isPasswordChangeRequired = await this.isPasswordChangeRequiredByOrgPolicy(

Check warning on line 304 in libs/auth/src/angular/login/login.component.ts

View check run for this annotation

Codecov / codecov/patch

libs/auth/src/angular/login/login.component.ts#L304

Added line #L304 was not covered by tests
orgPolicies.enforcedPasswordPolicyOptions,
);
if (isPasswordChangeRequired) {
await this.router.navigate(["update-password"]);
return;

Check warning on line 309 in libs/auth/src/angular/login/login.component.ts

View check run for this annotation

Codecov / codecov/patch

libs/auth/src/angular/login/login.component.ts#L308-L309

Added lines #L308 - L309 were not covered by tests
}
}
}

if (this.clientType === ClientType.Browser) {
await this.router.navigate(["/tabs/vault"]);
Expand All @@ -310,54 +327,51 @@
await this.loginComponentService.launchSsoBrowserWindow(email, clientId);
}

protected async evaluatePassword(): Promise<void> {
/**
* Checks if the master password meets the enforced policy requirements
* and if the user is required to change their password.
*/
private async isPasswordChangeRequiredByOrgPolicy(
enforcedPasswordPolicyOptions: MasterPasswordPolicyOptions,
): Promise<boolean> {
try {
// If we do not have any saved policies, attempt to load them from the service
if (this.enforcedMasterPasswordOptions == undefined) {
this.enforcedMasterPasswordOptions = await firstValueFrom(
this.policyService.masterPasswordPolicyOptions$(),
);
if (enforcedPasswordPolicyOptions == undefined) {
return false;

Check warning on line 339 in libs/auth/src/angular/login/login.component.ts

View check run for this annotation

Codecov / codecov/patch

libs/auth/src/angular/login/login.component.ts#L339

Added line #L339 was not covered by tests
}

if (this.requirePasswordChange()) {
await this.router.navigate(["update-password"]);
return;
// Note: we deliberately do not check enforcedPasswordPolicyOptions.enforceOnLogin
// as existing users who are logging in after getting an org invite should
// always be forced to set a password that meets the org's policy.
// Org Invite -> Registration also works this way for new BW users as well.

const masterPassword = this.formGroup.controls.masterPassword.value;

Check warning on line 347 in libs/auth/src/angular/login/login.component.ts

View check run for this annotation

Codecov / codecov/patch

libs/auth/src/angular/login/login.component.ts#L347

Added line #L347 was not covered by tests

// Return false if masterPassword is null/undefined since this is only evaluated after successful login
if (!masterPassword) {
return false;

Check warning on line 351 in libs/auth/src/angular/login/login.component.ts

View check run for this annotation

Codecov / codecov/patch

libs/auth/src/angular/login/login.component.ts#L351

Added line #L351 was not covered by tests
}

const passwordStrength = this.passwordStrengthService.getPasswordStrength(
masterPassword,
this.formGroup.value.email ?? undefined,
)?.score;

return !this.policyService.evaluateMasterPassword(

Check warning on line 359 in libs/auth/src/angular/login/login.component.ts

View check run for this annotation

Codecov / codecov/patch

libs/auth/src/angular/login/login.component.ts#L359

Added line #L359 was not covered by tests
passwordStrength,
masterPassword,
enforcedPasswordPolicyOptions,
);
} catch (e) {
// Do not prevent unlock if there is an error evaluating policies
this.logService.error(e);
}
}

/**
* Checks if the master password meets the enforced policy requirements
* If not, returns false
*/
private requirePasswordChange(): boolean {
if (
this.enforcedMasterPasswordOptions == undefined ||
!this.enforcedMasterPasswordOptions.enforceOnLogin
) {
return false;
}

const masterPassword = this.formGroup.controls.masterPassword.value;

// Return false if masterPassword is null/undefined since this is only evaluated after successful login
if (!masterPassword) {
return false;
}
}

const passwordStrength = this.passwordStrengthService.getPasswordStrength(
masterPassword,
this.formGroup.value.email ?? undefined,
)?.score;

return !this.policyService.evaluateMasterPassword(
passwordStrength,
masterPassword,
this.enforcedMasterPasswordOptions,
);
private async setPoliciesIntoState(userId: UserId, policies: Policy[]): Promise<void> {
const policiesData: { [id: string]: PolicyData } = {};
policies.map((p) => (policiesData[p.id] = PolicyData.fromPolicy(p)));
await this.policyService.replace(policiesData, userId);

Check warning on line 374 in libs/auth/src/angular/login/login.component.ts

View check run for this annotation

Codecov / codecov/patch

libs/auth/src/angular/login/login.component.ts#L372-L374

Added lines #L372 - L374 were not covered by tests
}

protected async startAuthRequestLogin(): Promise<void> {
Expand Down Expand Up @@ -528,12 +542,6 @@
}

private async defaultOnInit(): Promise<void> {
// If there's an existing org invite, use it to get the password policies
const orgPolicies = await this.loginComponentService.getOrgPolicies();

this.policies = orgPolicies?.policies;
this.showResetPasswordAutoEnrollWarning = orgPolicies?.isPolicyAndAutoEnrollEnabled ?? false;

let paramEmailIsSet = false;

const params = await firstValueFrom(this.activatedRoute.queryParams);
Expand Down
Loading