bitwarden/browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser synced 2025-12-10 05:13:29 +00:00
Code Issues Releases Wiki Activity

[PM-18485] Remove new device verification guard (#14417)

Browse Source 
* remove NewDeviceVerificationGuard and all associated entities. New Device verification feature has rolled out in production, this guard is no longer needed.

* remove unused properties from the vault profile service
This commit is contained in:
Nick Krantz
2025-05-06 13:08:30 -05:00
committed by GitHub
parent 5176345584
commit 46df5279a3
28 changed files with 8 additions and 1774 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
libs/angular/src/services/jslib-services.module.ts
View File

@@ -312,7 +312,7 @@ import {
UserAsymmetricKeysRegenerationService,
} from "@bitwarden/key-management";
import { SafeInjectionToken } from "@bitwarden/ui-common";
import { NewDeviceVerificationNoticeService, PasswordRepromptService } from "@bitwarden/vault";
import { PasswordRepromptService } from "@bitwarden/vault";
import {
IndividualVaultExportService,
IndividualVaultExportServiceAbstraction,
@@ -1459,7 +1459,6 @@ const safeProviders: SafeProvider[] = [
useClass: DefaultLoginDecryptionOptionsService,
deps: [MessagingServiceAbstraction],
}),
safeProvider(NewDeviceVerificationNoticeService),
safeProvider({
provide: UserAsymmetricKeysRegenerationApiService,
useClass: DefaultUserAsymmetricKeysRegenerationApiService,

1
libs/angular/src/vault/guards/index.ts
View File

@@ -1 +0,0 @@
export * from "./new-device-verification-notice.guard";

293
libs/angular/src/vault/guards/new-device-verification-notice.guard.spec.ts
View File

@@ -1,293 +0,0 @@
import { TestBed } from "@angular/core/testing";
import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from "@angular/router";
import { BehaviorSubject } from "rxjs";
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";
import { NewDeviceVerificationNoticeService } from "@bitwarden/vault";
import { VaultProfileService } from "../services/vault-profile.service";
import { NewDeviceVerificationNoticeGuard } from "./new-device-verification-notice.guard";
describe("NewDeviceVerificationNoticeGuard", () => {
const _state = Object.freeze({}) as RouterStateSnapshot;
const emptyRoute = Object.freeze({ queryParams: {} }) as ActivatedRouteSnapshot;
const eightDaysAgo = new Date();
eightDaysAgo.setDate(eightDaysAgo.getDate() - 8);
const account = {
id: "account-id",
} as unknown as Account;
const activeAccount$ = new BehaviorSubject<Account | null>(account);
const createUrlTree = jest.fn();
const getFeatureFlag = jest.fn().mockImplementation((key) => {
if (key === FeatureFlag.NewDeviceVerificationTemporaryDismiss) {
return Promise.resolve(true);
}
return Promise.resolve(false);
});
const isSelfHost = jest.fn().mockReturnValue(false);
const getProfileTwoFactorEnabled = jest.fn().mockResolvedValue(false);
const noticeState$ = jest.fn().mockReturnValue(new BehaviorSubject(null));
const skipState$ = 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();
createUrlTree.mockClear();
hasMasterPasswordAndMasterKeyHash.mockClear();
getUserSSOBound.mockClear();
getUserSSOBoundAdminOwner.mockClear();
skipState$.mockClear();
TestBed.configureTestingModule({
providers: [
{ provide: Router, useValue: { createUrlTree } },
{ provide: ConfigService, useValue: { getFeatureFlag } },
{ provide: NewDeviceVerificationNoticeService, useValue: { noticeState$, skipState$ } },
{ provide: AccountService, useValue: { activeAccount$ } },
{ provide: PlatformUtilsService, useValue: { isSelfHost } },
{ provide: UserVerificationService, useValue: { hasMasterPasswordAndMasterKeyHash } },
{
provide: VaultProfileService,
useValue: {
getProfileCreationDate,
getProfileTwoFactorEnabled,
getUserSSOBound,
getUserSSOBoundAdminOwner,
},
},
],
});
});
function newDeviceGuard(route?: ActivatedRouteSnapshot) {
// Run the guard within injection context so `inject` works as you'd expect
// Pass state object to make TypeScript happy
return TestBed.runInInjectionContext(async () =>
NewDeviceVerificationNoticeGuard(route ?? emptyRoute, _state),
);
}
describe("fromNewDeviceVerification", () => {
const route = {
queryParams: { fromNewDeviceVerification: "true" },
} as unknown as ActivatedRouteSnapshot;
it("returns `true` when `fromNewDeviceVerification` is present", async () => {
expect(await newDeviceGuard(route)).toBe(true);
});
it("does not execute other logic", async () => {
// `fromNewDeviceVerification` param should exit early,
// not foolproof but a quick way to test that other logic isn't executed
await newDeviceGuard(route);
expect(getFeatureFlag).not.toHaveBeenCalled();
expect(isSelfHost).not.toHaveBeenCalled();
expect(getProfileTwoFactorEnabled).not.toHaveBeenCalled();
expect(getProfileCreationDate).not.toHaveBeenCalled();
expect(hasMasterPasswordAndMasterKeyHash).not.toHaveBeenCalled();
});
});
describe("missing current account", () => {
afterAll(() => {
// reset `activeAccount$` observable
activeAccount$.next(account);
});
it("redirects to login when account is missing", async () => {
activeAccount$.next(null);
await newDeviceGuard();
expect(createUrlTree).toHaveBeenCalledWith(["/login"]);
});
});
it("returns `true` when 2FA is enabled", async () => {
getProfileTwoFactorEnabled.mockResolvedValueOnce(true);
expect(await newDeviceGuard()).toBe(true);
});
it("returns `true` when the user is self hosted", async () => {
isSelfHost.mockReturnValueOnce(true);
expect(await newDeviceGuard()).toBe(true);
});
it("returns `true` when the profile was created less than a week ago", async () => {
const sixDaysAgo = new Date();
sixDaysAgo.setDate(sixDaysAgo.getDate() - 6);
getProfileCreationDate.mockResolvedValueOnce(sixDaysAgo);
expect(await newDeviceGuard()).toBe(true);
});
it("returns `true` when the profile service throws an error", async () => {
getProfileCreationDate.mockRejectedValueOnce(new Error("test"));
expect(await newDeviceGuard()).toBe(true);
});
it("returns `true` when the skip state value is set to true", async () => {
skipState$.mockReturnValueOnce(new BehaviorSubject(true));
expect(await newDeviceGuard()).toBe(true);
expect(skipState$.mock.calls[0][0]).toBe("account-id");
expect(skipState$.mock.calls.length).toBe(1);
});
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) => {
if (key === FeatureFlag.NewDeviceVerificationTemporaryDismiss) {
return Promise.resolve(true);
}
return Promise.resolve(false);
});
});
afterAll(() => {
getFeatureFlag.mockReturnValue(false);
});
it("redirects to notice when the user has not dismissed it", async () => {
noticeState$.mockReturnValueOnce(new BehaviorSubject(null));
await newDeviceGuard();
expect(createUrlTree).toHaveBeenCalledWith(["/new-device-notice"]);
expect(noticeState$).toHaveBeenCalledWith(account.id);
});
it("redirects to notice when the user dismissed it more than 7 days ago", async () => {
const eighteenDaysAgo = new Date();
eighteenDaysAgo.setDate(eighteenDaysAgo.getDate() - 18);
noticeState$.mockReturnValueOnce(
new BehaviorSubject({ last_dismissal: eighteenDaysAgo.toISOString() }),
);
await newDeviceGuard();
expect(createUrlTree).toHaveBeenCalledWith(["/new-device-notice"]);
});
it("returns true when the user dismissed less than 7 days ago", async () => {
const fourDaysAgo = new Date();
fourDaysAgo.setDate(fourDaysAgo.getDate() - 4);
noticeState$.mockReturnValueOnce(
new BehaviorSubject({ last_dismissal: fourDaysAgo.toISOString() }),
);
expect(await newDeviceGuard()).toBe(true);
});
});
describe("permanent flag", () => {
beforeEach(() => {
getFeatureFlag.mockImplementation((key) => {
if (key === FeatureFlag.NewDeviceVerificationPermanentDismiss) {
return Promise.resolve(true);
}
return Promise.resolve(false);
});
});
afterAll(() => {
getFeatureFlag.mockReturnValue(false);
});
it("redirects when the user has not dismissed", async () => {
noticeState$.mockReturnValueOnce(new BehaviorSubject(null));
await newDeviceGuard();
expect(createUrlTree).toHaveBeenCalledWith(["/new-device-notice"]);
noticeState$.mockReturnValueOnce(new BehaviorSubject({ permanent_dismissal: null }));
await newDeviceGuard();
expect(createUrlTree).toHaveBeenCalledTimes(2);
expect(createUrlTree).toHaveBeenCalledWith(["/new-device-notice"]);
});
it("returns `true` when the user has dismissed", async () => {
noticeState$.mockReturnValueOnce(new BehaviorSubject({ permanent_dismissal: true }));
expect(await newDeviceGuard()).toBe(true);
});
});
});

166
libs/angular/src/vault/guards/new-device-verification-notice.guard.ts
View File

@@ -1,166 +0,0 @@
import { inject } from "@angular/core";
import { ActivatedRouteSnapshot, CanActivateFn, Router } from "@angular/router";
import { firstValueFrom, Observable } from "rxjs";
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";
import { NewDeviceVerificationNoticeService } from "@bitwarden/vault";
import { VaultProfileService } from "../services/vault-profile.service";
export const NewDeviceVerificationNoticeGuard: CanActivateFn = async (
route: ActivatedRouteSnapshot,
) => {
const router = inject(Router);
const configService = inject(ConfigService);
const newDeviceVerificationNoticeService = inject(NewDeviceVerificationNoticeService);
const accountService = inject(AccountService);
const platformUtilsService = inject(PlatformUtilsService);
const vaultProfileService = inject(VaultProfileService);
const userVerificationService = inject(UserVerificationService);
if (route.queryParams["fromNewDeviceVerification"]) {
return true;
}
const tempNoticeFlag = await configService.getFeatureFlag(
FeatureFlag.NewDeviceVerificationTemporaryDismiss,
);
const permNoticeFlag = await configService.getFeatureFlag(
FeatureFlag.NewDeviceVerificationPermanentDismiss,
);
if (!tempNoticeFlag && !permNoticeFlag) {
return true;
}
const currentAcct$: Observable<Account | null> = accountService.activeAccount$;
const currentAcct = await firstValueFrom(currentAcct$);
if (!currentAcct) {
return router.createUrlTree(["/login"]);
}
// Currently used by the auth recovery login flow and will get cleaned up in PM-18485.
if (await firstValueFrom(newDeviceVerificationNoticeService.skipState$(currentAcct.id))) {
return true;
}
try {
const isSelfHosted = platformUtilsService.isSelfHost();
const userIsSSOUser = await ssoAppliesToUser(
userVerificationService,
vaultProfileService,
currentAcct.id,
);
const has2FAEnabled = await hasATwoFactorProviderEnabled(vaultProfileService, currentAcct.id);
const isProfileLessThanWeekOld = await profileIsLessThanWeekOld(
vaultProfileService,
currentAcct.id,
);
// When any of the following are true, the device verification notice is
// 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 {
// Skip showing the notice if there was a problem determining applicability
// The most likely problem to occur is the user not having a network connection
return true;
}
const userItems$ = newDeviceVerificationNoticeService.noticeState$(currentAcct.id);
const userItems = await firstValueFrom(userItems$);
// Show the notice when:
// - The temp notice flag is enabled
// - The user hasn't dismissed the notice or the user dismissed it more than 7 days ago
if (
tempNoticeFlag &&
(!userItems?.last_dismissal || isMoreThan7DaysAgo(userItems?.last_dismissal))
) {
return router.createUrlTree(["/new-device-notice"]);
}
// Show the notice when:
// - The permanent notice flag is enabled
// - The user hasn't dismissed the notice
if (permNoticeFlag && !userItems?.permanent_dismissal) {
return router.createUrlTree(["/new-device-notice"]);
}
return true;
};
/** Returns true has one 2FA provider enabled */
async function hasATwoFactorProviderEnabled(
vaultProfileService: VaultProfileService,
userId: string,
): Promise<boolean> {
return vaultProfileService.getProfileTwoFactorEnabled(userId);
}
/** Returns true when the user's profile is less than a week old */
async function profileIsLessThanWeekOld(
vaultProfileService: VaultProfileService,
userId: string,
): Promise<boolean> {
const creationDate = await vaultProfileService.getProfileCreationDate(userId);
return !isMoreThan7DaysAgo(creationDate);
}
/**
* 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 */
function isMoreThan7DaysAgo(date?: string | Date): boolean {
if (!date) {
return false;
}
const inputDate = new Date(date).getTime();
const today = new Date().getTime();
const differenceInMS = today - inputDate;
const msInADay = 1000 * 60 * 60 * 24;
const differenceInDays = Math.round(differenceInMS / msInADay);
return differenceInDays > 7;
}

90
libs/angular/src/vault/services/vault-profile.service.spec.ts
View File

@@ -68,94 +68,4 @@ describe("VaultProfileService", () => {
expect(getProfile).not.toHaveBeenCalled();
});
});
describe("getProfileTwoFactorEnabled", () => {
it("calls `getProfile` when stored 2FA property is not stored", async () => {
expect(service["profile2FAEnabled"]).toBeNull();
const twoFactorEnabled = await service.getProfileTwoFactorEnabled(userId);
expect(twoFactorEnabled).toBe(true);
expect(getProfile).toHaveBeenCalled();
});
it("calls `getProfile` when stored profile id does not match", async () => {
service["profile2FAEnabled"] = false;
service["userId"] = "old-user-id";
const twoFactorEnabled = await service.getProfileTwoFactorEnabled(userId);
expect(twoFactorEnabled).toBe(true);
expect(getProfile).toHaveBeenCalled();
});
it("does not call `getProfile` when 2FA property is already stored", async () => {
service["profile2FAEnabled"] = false;
const twoFactorEnabled = await service.getProfileTwoFactorEnabled(userId);
expect(twoFactorEnabled).toBe(false);
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();
});
});
});

61
libs/angular/src/vault/services/vault-profile.service.ts
View File

@@ -1,18 +1,13 @@
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({
providedIn: "root",
})
/**
* Class to provide profile level details without having to call the API each time.
* NOTE: This is a temporary service and can be replaced once the `UnauthenticatedExtensionUIRefresh` flag goes live.
* The `UnauthenticatedExtensionUIRefresh` introduces a sync that takes place upon logging in. These details can then
* be added to account object and retrieved from there.
* TODO: PM-16202
* Class to provide profile level details to vault entities without having to call the API each time.
*/
export class VaultProfileService {
private apiService = inject(ApiService);
@@ -22,15 +17,6 @@ export class VaultProfileService {
/** Profile creation stored as a string. */
private profileCreatedDate: string | null = null;
/** 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
@@ -46,56 +32,11 @@ export class VaultProfileService {
return new Date(profile.creationDate);
}
/**
* Returns whether there is a 2FA provider on the profile.
*/
async getProfileTwoFactorEnabled(userId: string): Promise<boolean> {
if (this.profile2FAEnabled !== null && userId === this.userId) {
return Promise.resolve(this.profile2FAEnabled);
}
const profile = await this.fetchAndCacheProfile();
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;
}