diff --git a/libs/angular/src/vault/guards/new-device-verification-notice.guard.spec.ts b/libs/angular/src/vault/guards/new-device-verification-notice.guard.spec.ts index 960590dab53..938d5b32527 100644 --- a/libs/angular/src/vault/guards/new-device-verification-notice.guard.spec.ts +++ b/libs/angular/src/vault/guards/new-device-verification-notice.guard.spec.ts @@ -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(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) => { diff --git a/libs/angular/src/vault/guards/new-device-verification-notice.guard.ts b/libs/angular/src/vault/guards/new-device-verification-notice.guard.ts index 09d6b3313c4..8d4a7742bc5 100644 --- a/libs/angular/src/vault/guards/new-device-verification-notice.guard.ts +++ b/libs/angular/src/vault/guards/new-device-verification-notice.guard.ts @@ -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 */ diff --git a/libs/angular/src/vault/services/vault-profile.service.spec.ts b/libs/angular/src/vault/services/vault-profile.service.spec.ts index 7761503253a..ade34da39a6 100644 --- a/libs/angular/src/vault/services/vault-profile.service.spec.ts +++ b/libs/angular/src/vault/services/vault-profile.service.spec.ts @@ -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(); + }); + }); }); diff --git a/libs/angular/src/vault/services/vault-profile.service.ts b/libs/angular/src/vault/services/vault-profile.service.ts index b368a973781..21f4ecc2285 100644 --- a/libs/angular/src/vault/services/vault-profile.service.ts +++ b/libs/angular/src/vault/services/vault-profile.service.ts @@ -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 { + 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 { + if (this.userIsSsoBoundAdminOwner !== null && userId === this.userId) { + return Promise.resolve(this.userIsSsoBoundAdminOwner); + } + + await this.fetchAndCacheProfile(); + + return !!this.userIsSsoBoundAdminOwner; + } + private async fetchAndCacheProfile(): Promise { 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; }