mirror of
https://github.com/bitwarden/browser
synced 2025-12-11 13:53:34 +00:00
[PM-17776] New Device - SSO Check (#13177)
* refactor SSO policy check to check for SSO users that have `ssoBound` true on any of their organizations
* Revert "refactor SSO policy check to check for SSO users that have `ssoBound` true on any of their organizations"
This reverts commit 419c26fbbc.
* update new device verification guard to check for master password usage
* add sso check for new device verification guard
This commit is contained in:
@@ -2,9 +2,8 @@ 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 { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction";
|
||||
import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
|
||||
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
|
||||
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
|
||||
@@ -36,17 +35,21 @@ 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 hasMasterPasswordAndMasterKeyHash = jest.fn().mockResolvedValue(true);
|
||||
const getUserSSOBound = jest.fn().mockResolvedValue(false);
|
||||
const getUserSSOBoundAdminOwner = jest.fn().mockResolvedValue(false);
|
||||
|
||||
beforeEach(() => {
|
||||
getFeatureFlag.mockClear();
|
||||
isSelfHost.mockClear();
|
||||
getProfileCreationDate.mockClear();
|
||||
getProfileTwoFactorEnabled.mockClear();
|
||||
policyAppliesToActiveUser$.mockClear();
|
||||
createUrlTree.mockClear();
|
||||
hasMasterPasswordAndMasterKeyHash.mockClear();
|
||||
getUserSSOBound.mockClear();
|
||||
getUserSSOBoundAdminOwner.mockClear();
|
||||
|
||||
TestBed.configureTestingModule({
|
||||
providers: [
|
||||
@@ -55,10 +58,15 @@ describe("NewDeviceVerificationNoticeGuard", () => {
|
||||
{ provide: NewDeviceVerificationNoticeService, useValue: { noticeState$ } },
|
||||
{ provide: AccountService, useValue: { activeAccount$ } },
|
||||
{ provide: PlatformUtilsService, useValue: { isSelfHost } },
|
||||
{ provide: PolicyService, useValue: { policyAppliesToActiveUser$ } },
|
||||
{ provide: UserVerificationService, useValue: { hasMasterPasswordAndMasterKeyHash } },
|
||||
{
|
||||
provide: VaultProfileService,
|
||||
useValue: { getProfileCreationDate, getProfileTwoFactorEnabled },
|
||||
useValue: {
|
||||
getProfileCreationDate,
|
||||
getProfileTwoFactorEnabled,
|
||||
getUserSSOBound,
|
||||
getUserSSOBoundAdminOwner,
|
||||
},
|
||||
},
|
||||
],
|
||||
});
|
||||
@@ -90,7 +98,7 @@ describe("NewDeviceVerificationNoticeGuard", () => {
|
||||
expect(isSelfHost).not.toHaveBeenCalled();
|
||||
expect(getProfileTwoFactorEnabled).not.toHaveBeenCalled();
|
||||
expect(getProfileCreationDate).not.toHaveBeenCalled();
|
||||
expect(policyAppliesToActiveUser$).not.toHaveBeenCalled();
|
||||
expect(hasMasterPasswordAndMasterKeyHash).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -121,13 +129,6 @@ describe("NewDeviceVerificationNoticeGuard", () => {
|
||||
expect(await newDeviceGuard()).toBe(true);
|
||||
});
|
||||
|
||||
it("returns `true` SSO is required", async () => {
|
||||
policyAppliesToActiveUser$.mockReturnValueOnce(new BehaviorSubject(true));
|
||||
|
||||
expect(await newDeviceGuard()).toBe(true);
|
||||
expect(policyAppliesToActiveUser$).toHaveBeenCalledWith(PolicyType.RequireSso);
|
||||
});
|
||||
|
||||
it("returns `true` when the profile was created less than a week ago", async () => {
|
||||
const sixDaysAgo = new Date();
|
||||
sixDaysAgo.setDate(sixDaysAgo.getDate() - 6);
|
||||
@@ -143,6 +144,57 @@ describe("NewDeviceVerificationNoticeGuard", () => {
|
||||
expect(await newDeviceGuard()).toBe(true);
|
||||
});
|
||||
|
||||
describe("SSO bound", () => {
|
||||
beforeEach(() => {
|
||||
getFeatureFlag.mockImplementation((key) => {
|
||||
if (key === FeatureFlag.NewDeviceVerificationPermanentDismiss) {
|
||||
return Promise.resolve(true);
|
||||
}
|
||||
|
||||
return Promise.resolve(false);
|
||||
});
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
getFeatureFlag.mockReturnValue(false);
|
||||
});
|
||||
|
||||
it('returns "true" when the user is SSO bound and not an admin or owner', async () => {
|
||||
getUserSSOBound.mockResolvedValueOnce(true);
|
||||
getUserSSOBoundAdminOwner.mockResolvedValueOnce(false);
|
||||
|
||||
expect(await newDeviceGuard()).toBe(true);
|
||||
});
|
||||
|
||||
it('returns "true" when the user is an admin or owner of an SSO bound organization and has not logged in with their master password', async () => {
|
||||
getUserSSOBound.mockResolvedValueOnce(true);
|
||||
getUserSSOBoundAdminOwner.mockResolvedValueOnce(true);
|
||||
hasMasterPasswordAndMasterKeyHash.mockResolvedValueOnce(false);
|
||||
|
||||
expect(await newDeviceGuard()).toBe(true);
|
||||
});
|
||||
|
||||
it("shows notice when the user is an admin or owner of an SSO bound organization and logged in with their master password", async () => {
|
||||
getUserSSOBound.mockResolvedValueOnce(true);
|
||||
getUserSSOBoundAdminOwner.mockResolvedValueOnce(true);
|
||||
hasMasterPasswordAndMasterKeyHash.mockResolvedValueOnce(true);
|
||||
|
||||
await newDeviceGuard();
|
||||
|
||||
expect(createUrlTree).toHaveBeenCalledWith(["/new-device-notice"]);
|
||||
});
|
||||
|
||||
it("shows notice when the user that is not in an SSO bound organization", async () => {
|
||||
getUserSSOBound.mockResolvedValueOnce(false);
|
||||
getUserSSOBoundAdminOwner.mockResolvedValueOnce(false);
|
||||
hasMasterPasswordAndMasterKeyHash.mockResolvedValueOnce(true);
|
||||
|
||||
await newDeviceGuard();
|
||||
|
||||
expect(createUrlTree).toHaveBeenCalledWith(["/new-device-notice"]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("temp flag", () => {
|
||||
beforeEach(() => {
|
||||
getFeatureFlag.mockImplementation((key) => {
|
||||
|
||||
@@ -2,9 +2,8 @@ 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 { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction";
|
||||
import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
|
||||
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
|
||||
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
|
||||
@@ -20,8 +19,8 @@ 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);
|
||||
const userVerificationService = inject(UserVerificationService);
|
||||
|
||||
if (route.queryParams["fromNewDeviceVerification"]) {
|
||||
return true;
|
||||
@@ -47,7 +46,11 @@ export const NewDeviceVerificationNoticeGuard: CanActivateFn = async (
|
||||
|
||||
try {
|
||||
const isSelfHosted = platformUtilsService.isSelfHost();
|
||||
const requiresSSO = await isSSORequired(policyService);
|
||||
const userIsSSOUser = await ssoAppliesToUser(
|
||||
userVerificationService,
|
||||
vaultProfileService,
|
||||
currentAcct.id,
|
||||
);
|
||||
const has2FAEnabled = await hasATwoFactorProviderEnabled(vaultProfileService, currentAcct.id);
|
||||
const isProfileLessThanWeekOld = await profileIsLessThanWeekOld(
|
||||
vaultProfileService,
|
||||
@@ -55,8 +58,9 @@ 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) {
|
||||
// not applicable for the user. When the user has *not* logged in with their
|
||||
// master password, assume they logged in with SSO.
|
||||
if (has2FAEnabled || isSelfHosted || userIsSSOUser || isProfileLessThanWeekOld) {
|
||||
return true;
|
||||
}
|
||||
} catch {
|
||||
@@ -105,9 +109,39 @@ 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 when either:
|
||||
* - The user is SSO bound to an organization and is not an Admin or Owner
|
||||
* - The user is an Admin or Owner of an organization with SSO bound and has not logged in with their master password
|
||||
*
|
||||
* NOTE: There are edge cases where this does not satisfy the original requirement of showing the notice to
|
||||
* users who are subject to the SSO required policy. When Owners and Admins log in with their MP they will see the notice
|
||||
* when they log in with SSO they will not. This is a concession made because the original logic references policies would not work for TDE users.
|
||||
* When this guard is run for those users a sync hasn't occurred and thus the policies are not available.
|
||||
*/
|
||||
async function ssoAppliesToUser(
|
||||
userVerificationService: UserVerificationService,
|
||||
vaultProfileService: VaultProfileService,
|
||||
userId: string,
|
||||
) {
|
||||
const userSSOBound = await vaultProfileService.getUserSSOBound(userId);
|
||||
const userSSOBoundAdminOwner = await vaultProfileService.getUserSSOBoundAdminOwner(userId);
|
||||
const userLoggedInWithMP = await userLoggedInWithMasterPassword(userVerificationService, userId);
|
||||
|
||||
const nonOwnerAdminSsoUser = userSSOBound && !userSSOBoundAdminOwner;
|
||||
const ssoAdminOwnerLoggedInWithMP = userSSOBoundAdminOwner && !userLoggedInWithMP;
|
||||
|
||||
return nonOwnerAdminSsoUser || ssoAdminOwnerLoggedInWithMP;
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns true when the user logged in with their master password.
|
||||
*/
|
||||
async function userLoggedInWithMasterPassword(
|
||||
userVerificationService: UserVerificationService,
|
||||
userId: string,
|
||||
) {
|
||||
return userVerificationService.hasMasterPasswordAndMasterKeyHash(userId);
|
||||
}
|
||||
|
||||
/** Returns the true when the date given is older than 7 days */
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { TestBed } from "@angular/core/testing";
|
||||
|
||||
import { ApiService } from "@bitwarden/common/abstractions/api.service";
|
||||
import { OrganizationUserType } from "@bitwarden/common/admin-console/enums";
|
||||
|
||||
import { VaultProfileService } from "./vault-profile.service";
|
||||
|
||||
@@ -13,6 +14,12 @@ describe("VaultProfileService", () => {
|
||||
creationDate: hardcodedDateString,
|
||||
twoFactorEnabled: true,
|
||||
id: "new-user-id",
|
||||
organizations: [
|
||||
{
|
||||
ssoBound: true,
|
||||
type: OrganizationUserType.Admin,
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
@@ -91,4 +98,64 @@ 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();
|
||||
});
|
||||
});
|
||||
|
||||
describe("getUserSSOBoundAdminOwner", () => {
|
||||
it("calls `getProfile` when stored userIsSsoBoundAdminOwner property is not stored", async () => {
|
||||
expect(service["userIsSsoBoundAdminOwner"]).toBeNull();
|
||||
|
||||
const userIsSsoBoundAdminOwner = await service.getUserSSOBoundAdminOwner(userId);
|
||||
|
||||
expect(userIsSsoBoundAdminOwner).toBe(true);
|
||||
expect(getProfile).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("calls `getProfile` when stored profile id does not match", async () => {
|
||||
service["userIsSsoBoundAdminOwner"] = false;
|
||||
service["userId"] = "old-user-id";
|
||||
|
||||
const userIsSsoBoundAdminOwner = await service.getUserSSOBoundAdminOwner(userId);
|
||||
|
||||
expect(userIsSsoBoundAdminOwner).toBe(true);
|
||||
expect(getProfile).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not call `getProfile` when userIsSsoBoundAdminOwner property is already stored", async () => {
|
||||
service["userIsSsoBoundAdminOwner"] = false;
|
||||
|
||||
const userIsSsoBoundAdminOwner = await service.getUserSSOBoundAdminOwner(userId);
|
||||
|
||||
expect(userIsSsoBoundAdminOwner).toBe(false);
|
||||
expect(getProfile).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { Injectable, inject } from "@angular/core";
|
||||
|
||||
import { ApiService } from "@bitwarden/common/abstractions/api.service";
|
||||
import { OrganizationUserType } from "@bitwarden/common/admin-console/enums";
|
||||
import { ProfileResponse } from "@bitwarden/common/models/response/profile.response";
|
||||
|
||||
@Injectable({
|
||||
@@ -24,6 +25,12 @@ 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;
|
||||
|
||||
/** True when the user is an admin or owner of the ssoBound organization */
|
||||
private userIsSsoBoundAdminOwner: boolean | null = null;
|
||||
|
||||
/**
|
||||
* Returns the creation date of the profile.
|
||||
* Note: `Date`s are mutable in JS, creating a new
|
||||
@@ -52,12 +59,43 @@ 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;
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns true when the user is an Admin or Owner of an organization with `ssoBound` true.
|
||||
*/
|
||||
async getUserSSOBoundAdminOwner(userId: string): Promise<boolean> {
|
||||
if (this.userIsSsoBoundAdminOwner !== null && userId === this.userId) {
|
||||
return Promise.resolve(this.userIsSsoBoundAdminOwner);
|
||||
}
|
||||
|
||||
await this.fetchAndCacheProfile();
|
||||
|
||||
return !!this.userIsSsoBoundAdminOwner;
|
||||
}
|
||||
|
||||
private async fetchAndCacheProfile(): Promise<ProfileResponse> {
|
||||
const profile = await this.apiService.getProfile();
|
||||
|
||||
this.userId = profile.id;
|
||||
this.profileCreatedDate = profile.creationDate;
|
||||
this.profile2FAEnabled = profile.twoFactorEnabled;
|
||||
const ssoBoundOrg = profile.organizations.find((org) => org.ssoBound);
|
||||
this.userIsSsoBound = !!ssoBoundOrg;
|
||||
this.userIsSsoBoundAdminOwner =
|
||||
ssoBoundOrg?.type === OrganizationUserType.Admin ||
|
||||
ssoBoundOrg?.type === OrganizationUserType.Owner;
|
||||
|
||||
return profile;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user