1
0
mirror of https://github.com/bitwarden/browser synced 2026-02-16 00:24:52 +00:00

refactor SSO policy check to check for SSO users that have ssoBound true on any of their organizations

This commit is contained in:
Nick Krantz
2025-01-30 16:18:25 -06:00
parent f17cb61183
commit 419c26fbbc
4 changed files with 73 additions and 17 deletions

View File

@@ -2,8 +2,6 @@ import { TestBed } from "@angular/core/testing";
import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from "@angular/router";
import { BehaviorSubject } from "rxjs";
import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
import { PolicyType } from "@bitwarden/common/admin-console/enums";
import { Account, AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
@@ -38,16 +36,16 @@ describe("NewDeviceVerificationNoticeGuard", () => {
});
const isSelfHost = jest.fn().mockReturnValue(false);
const getProfileTwoFactorEnabled = jest.fn().mockResolvedValue(false);
const policyAppliesToActiveUser$ = jest.fn().mockReturnValue(new BehaviorSubject<boolean>(false));
const noticeState$ = jest.fn().mockReturnValue(new BehaviorSubject(null));
const getProfileCreationDate = jest.fn().mockResolvedValue(eightDaysAgo);
const getUserSSOBound = jest.fn().mockResolvedValue(false);
beforeEach(() => {
getFeatureFlag.mockClear();
isSelfHost.mockClear();
getProfileCreationDate.mockClear();
getUserSSOBound.mockClear();
getProfileTwoFactorEnabled.mockClear();
policyAppliesToActiveUser$.mockClear();
createUrlTree.mockClear();
TestBed.configureTestingModule({
@@ -57,10 +55,9 @@ describe("NewDeviceVerificationNoticeGuard", () => {
{ provide: NewDeviceVerificationNoticeService, useValue: { noticeState$ } },
{ provide: AccountService, useValue: { activeAccount$ } },
{ provide: PlatformUtilsService, useValue: { isSelfHost } },
{ provide: PolicyService, useValue: { policyAppliesToActiveUser$ } },
{
provide: VaultProfileService,
useValue: { getProfileCreationDate, getProfileTwoFactorEnabled },
useValue: { getProfileCreationDate, getProfileTwoFactorEnabled, getUserSSOBound },
},
],
});
@@ -92,7 +89,7 @@ describe("NewDeviceVerificationNoticeGuard", () => {
expect(isSelfHost).not.toHaveBeenCalled();
expect(getProfileTwoFactorEnabled).not.toHaveBeenCalled();
expect(getProfileCreationDate).not.toHaveBeenCalled();
expect(policyAppliesToActiveUser$).not.toHaveBeenCalled();
expect(getUserSSOBound).not.toHaveBeenCalled();
});
});
@@ -124,10 +121,10 @@ describe("NewDeviceVerificationNoticeGuard", () => {
});
it("returns `true` SSO is required", async () => {
policyAppliesToActiveUser$.mockReturnValueOnce(new BehaviorSubject(true));
getUserSSOBound.mockReturnValueOnce(new BehaviorSubject(true));
expect(await newDeviceGuard()).toBe(true);
expect(policyAppliesToActiveUser$).toHaveBeenCalledWith(PolicyType.RequireSso);
expect(getUserSSOBound).toHaveBeenCalledWith(account.id);
});
it("returns `true` when the profile was created less than a week ago", async () => {

View File

@@ -2,8 +2,6 @@ import { inject } from "@angular/core";
import { ActivatedRouteSnapshot, CanActivateFn, Router } from "@angular/router";
import { firstValueFrom, Observable } from "rxjs";
import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
import { PolicyType } from "@bitwarden/common/admin-console/enums";
import { Account, AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
@@ -22,7 +20,6 @@ export const NewDeviceVerificationNoticeGuard: CanActivateFn = async (
const newDeviceVerificationNoticeService = inject(NewDeviceVerificationNoticeService);
const accountService = inject(AccountService);
const platformUtilsService = inject(PlatformUtilsService);
const policyService = inject(PolicyService);
const vaultProfileService = inject(VaultProfileService);
if (route.queryParams["fromNewDeviceVerification"]) {
@@ -49,7 +46,7 @@ export const NewDeviceVerificationNoticeGuard: CanActivateFn = async (
try {
const isSelfHosted = platformUtilsService.isSelfHost();
const requiresSSO = await isSSORequired(policyService);
const usesSSO = await userIsSsoBound(vaultProfileService, currentAcct.id);
const has2FAEnabled = await hasATwoFactorProviderEnabled(vaultProfileService, currentAcct.id);
const isProfileLessThanWeekOld = await profileIsLessThanWeekOld(
vaultProfileService,
@@ -58,7 +55,7 @@ export const NewDeviceVerificationNoticeGuard: CanActivateFn = async (
// When any of the following are true, the device verification notice is
// not applicable for the user.
if (has2FAEnabled || isSelfHosted || requiresSSO || isProfileLessThanWeekOld) {
if (has2FAEnabled || isSelfHosted || usesSSO || isProfileLessThanWeekOld) {
return true;
}
} catch {
@@ -107,9 +104,19 @@ async function profileIsLessThanWeekOld(
return !isMoreThan7DaysAgo(creationDate);
}
/** Returns true when the user is required to login via SSO */
async function isSSORequired(policyService: PolicyService) {
return firstValueFrom(policyService.policyAppliesToActiveUser$(PolicyType.RequireSso));
/**
* Returns true if the user logs in via SSO.
*
* This is used instead of the PolicyService because the policy service relies on a full
* sync to complete before the policies are populated. This isn't guaranteed for all users
* by the time the guard is run (TDE users for example).
* Waiting for a sync in the guard was also considered but could take too long to complete,
* leaving the UI in an odd "waiting" state with no indication to the user.
*/
async function userIsSsoBound(vaultProfileService: VaultProfileService, userId: string) {
const userLogsInWithSSO = await vaultProfileService.getUserSSOBound(userId);
return userLogsInWithSSO;
}
/** Returns the true when the date given is older than 7 days */

View File

@@ -13,6 +13,11 @@ describe("VaultProfileService", () => {
creationDate: hardcodedDateString,
twoFactorEnabled: true,
id: "new-user-id",
organizations: [
{
ssoBound: true,
},
],
});
beforeEach(() => {
@@ -91,4 +96,34 @@ describe("VaultProfileService", () => {
expect(getProfile).not.toHaveBeenCalled();
});
});
describe("getUserSSOBound", () => {
it("calls `getProfile` when stored ssoBound property is not stored", async () => {
expect(service["userIsSsoBound"]).toBeNull();
const userIsSsoBound = await service.getUserSSOBound(userId);
expect(userIsSsoBound).toBe(true);
expect(getProfile).toHaveBeenCalled();
});
it("calls `getProfile` when stored profile id does not match", async () => {
service["userIsSsoBound"] = false;
service["userId"] = "old-user-id";
const userIsSsoBound = await service.getUserSSOBound(userId);
expect(userIsSsoBound).toBe(true);
expect(getProfile).toHaveBeenCalled();
});
it("does not call `getProfile` when ssoBound property is already stored", async () => {
service["userIsSsoBound"] = false;
const userIsSsoBound = await service.getUserSSOBound(userId);
expect(userIsSsoBound).toBe(false);
expect(getProfile).not.toHaveBeenCalled();
});
});
});

View File

@@ -24,6 +24,9 @@ export class VaultProfileService {
/** True when 2FA is enabled on the profile. */
private profile2FAEnabled: boolean | null = null;
/** True when ssoBound is true for any of the users organizations */
private userIsSsoBound: boolean | null = null;
/**
* Returns the creation date of the profile.
* Note: `Date`s are mutable in JS, creating a new
@@ -52,12 +55,26 @@ export class VaultProfileService {
return profile.twoFactorEnabled;
}
/**
* Returns whether the user logs in with SSO for any organization.
*/
async getUserSSOBound(userId: string): Promise<boolean> {
if (this.userIsSsoBound !== null && userId === this.userId) {
return Promise.resolve(this.userIsSsoBound);
}
await this.fetchAndCacheProfile();
return !!this.userIsSsoBound;
}
private async fetchAndCacheProfile(): Promise<ProfileResponse> {
const profile = await this.apiService.getProfile();
this.userId = profile.id;
this.profileCreatedDate = profile.creationDate;
this.profile2FAEnabled = profile.twoFactorEnabled;
this.userIsSsoBound = profile.organizations.find((org) => org.ssoBound) !== undefined;
return profile;
}