mirror of
https://github.com/bitwarden/browser
synced 2025-12-10 13:23:34 +00:00
[PM-14347][PM-14348] New Device Verification Logic (#12451)
* add account created date to the account information * set permanent dismissal flag when the user selects that they can access their email * update the logic of device verification notice * add service to cache the profile creation date to avoid calling the API multiple times * update step one logic for new device verification + add tests * update step two logic for new device verification + add tests - remove remind me later link for permanent logic * migrate 2FA check to use the profile property rather than hitting the API directly. The API for 2FA providers is only available on web so it didn't work for browser & native. * remove unneeded account related changes - profile creation is used from other sources * remove obsolete test * store the profile id within the vault service * remove unused map * store the associated profile id so account for profile switching in the extension * add comment for temporary service and ticket number to remove * formatting * move up logic for feature flags
This commit is contained in:
@@ -0,0 +1,225 @@
|
||||
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";
|
||||
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
|
||||
|
||||
import { NewDeviceVerificationNoticeService } from "../../../../vault/src/services/new-device-verification-notice.service";
|
||||
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().mockResolvedValue(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);
|
||||
|
||||
beforeEach(() => {
|
||||
getFeatureFlag.mockClear();
|
||||
isSelfHost.mockClear();
|
||||
getProfileCreationDate.mockClear();
|
||||
getProfileTwoFactorEnabled.mockClear();
|
||||
policyAppliesToActiveUser$.mockClear();
|
||||
createUrlTree.mockClear();
|
||||
|
||||
TestBed.configureTestingModule({
|
||||
providers: [
|
||||
{ provide: Router, useValue: { createUrlTree } },
|
||||
{ provide: ConfigService, useValue: { getFeatureFlag } },
|
||||
{ provide: NewDeviceVerificationNoticeService, useValue: { noticeState$ } },
|
||||
{ provide: AccountService, useValue: { activeAccount$ } },
|
||||
{ provide: PlatformUtilsService, useValue: { isSelfHost } },
|
||||
{ provide: PolicyService, useValue: { policyAppliesToActiveUser$ } },
|
||||
{
|
||||
provide: VaultProfileService,
|
||||
useValue: { getProfileCreationDate, getProfileTwoFactorEnabled },
|
||||
},
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
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(policyAppliesToActiveUser$).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` 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);
|
||||
|
||||
getProfileCreationDate.mockResolvedValueOnce(sixDaysAgo);
|
||||
|
||||
expect(await newDeviceGuard()).toBe(true);
|
||||
});
|
||||
|
||||
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);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -1,12 +1,16 @@
|
||||
import { inject } from "@angular/core";
|
||||
import { ActivatedRouteSnapshot, CanActivateFn, Router } from "@angular/router";
|
||||
import { Observable, firstValueFrom, map } from "rxjs";
|
||||
import { Observable, firstValueFrom } 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";
|
||||
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
|
||||
|
||||
import { NewDeviceVerificationNoticeService } from "../../../../vault/src/services/new-device-verification-notice.service";
|
||||
import { VaultProfileService } from "../services/vault-profile.service";
|
||||
|
||||
export const NewDeviceVerificationNoticeGuard: CanActivateFn = async (
|
||||
route: ActivatedRouteSnapshot,
|
||||
@@ -15,6 +19,9 @@ export const NewDeviceVerificationNoticeGuard: CanActivateFn = async (
|
||||
const configService = inject(ConfigService);
|
||||
const newDeviceVerificationNoticeService = inject(NewDeviceVerificationNoticeService);
|
||||
const accountService = inject(AccountService);
|
||||
const platformUtilsService = inject(PlatformUtilsService);
|
||||
const policyService = inject(PolicyService);
|
||||
const vaultProfileService = inject(VaultProfileService);
|
||||
|
||||
if (route.queryParams["fromNewDeviceVerification"]) {
|
||||
return true;
|
||||
@@ -27,25 +34,88 @@ export const NewDeviceVerificationNoticeGuard: CanActivateFn = async (
|
||||
FeatureFlag.NewDeviceVerificationPermanentDismiss,
|
||||
);
|
||||
|
||||
const currentAcct$: Observable<Account | null> = accountService.activeAccount$.pipe(
|
||||
map((acct) => acct),
|
||||
);
|
||||
if (!tempNoticeFlag && !permNoticeFlag) {
|
||||
return true;
|
||||
}
|
||||
|
||||
const currentAcct$: Observable<Account | null> = accountService.activeAccount$;
|
||||
const currentAcct = await firstValueFrom(currentAcct$);
|
||||
|
||||
if (!currentAcct) {
|
||||
return router.createUrlTree(["/login"]);
|
||||
}
|
||||
|
||||
const has2FAEnabled = await hasATwoFactorProviderEnabled(vaultProfileService, currentAcct.id);
|
||||
const isSelfHosted = await platformUtilsService.isSelfHost();
|
||||
const requiresSSO = await isSSORequired(policyService);
|
||||
const isProfileLessThanWeekOld = await profileIsLessThanWeekOld(
|
||||
vaultProfileService,
|
||||
currentAcct.id,
|
||||
);
|
||||
|
||||
// When any of the following are true, the device verification notice is
|
||||
// not applicable for the user.
|
||||
if (has2FAEnabled || isSelfHosted || requiresSSO || isProfileLessThanWeekOld) {
|
||||
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 (
|
||||
userItems?.last_dismissal == null &&
|
||||
(userItems?.permanent_dismissal == null || !userItems?.permanent_dismissal) &&
|
||||
(tempNoticeFlag || permNoticeFlag)
|
||||
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 the user is required to login via SSO */
|
||||
async function isSSORequired(policyService: PolicyService) {
|
||||
return firstValueFrom(policyService.policyAppliesToActiveUser$(PolicyType.RequireSso));
|
||||
}
|
||||
|
||||
/** 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;
|
||||
}
|
||||
|
||||
@@ -0,0 +1,94 @@
|
||||
import { TestBed } from "@angular/core/testing";
|
||||
|
||||
import { ApiService } from "@bitwarden/common/abstractions/api.service";
|
||||
|
||||
import { VaultProfileService } from "./vault-profile.service";
|
||||
|
||||
describe("VaultProfileService", () => {
|
||||
let service: VaultProfileService;
|
||||
const userId = "profile-id";
|
||||
const hardcodedDateString = "2024-02-24T12:00:00Z";
|
||||
|
||||
const getProfile = jest.fn().mockResolvedValue({
|
||||
creationDate: hardcodedDateString,
|
||||
twoFactorEnabled: true,
|
||||
id: "new-user-id",
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
getProfile.mockClear();
|
||||
|
||||
TestBed.configureTestingModule({
|
||||
providers: [{ provide: ApiService, useValue: { getProfile } }],
|
||||
});
|
||||
|
||||
jest.useFakeTimers();
|
||||
jest.setSystemTime(new Date("2024-02-22T00:00:00Z"));
|
||||
service = TestBed.runInInjectionContext(() => new VaultProfileService());
|
||||
service["userId"] = userId;
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
jest.useRealTimers();
|
||||
});
|
||||
|
||||
describe("getProfileCreationDate", () => {
|
||||
it("calls `getProfile` when stored profile date is not set", async () => {
|
||||
expect(service["profileCreatedDate"]).toBeNull();
|
||||
|
||||
const date = await service.getProfileCreationDate(userId);
|
||||
|
||||
expect(date.toISOString()).toBe("2024-02-24T12:00:00.000Z");
|
||||
expect(getProfile).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("calls `getProfile` when stored profile id does not match", async () => {
|
||||
service["profileCreatedDate"] = hardcodedDateString;
|
||||
service["userId"] = "old-user-id";
|
||||
|
||||
const date = await service.getProfileCreationDate(userId);
|
||||
|
||||
expect(date.toISOString()).toBe("2024-02-24T12:00:00.000Z");
|
||||
expect(getProfile).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not call `getProfile` when the date is already stored", async () => {
|
||||
service["profileCreatedDate"] = hardcodedDateString;
|
||||
|
||||
const date = await service.getProfileCreationDate(userId);
|
||||
|
||||
expect(date.toISOString()).toBe("2024-02-24T12:00:00.000Z");
|
||||
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();
|
||||
});
|
||||
});
|
||||
});
|
||||
64
libs/angular/src/vault/services/vault-profile.service.ts
Normal file
64
libs/angular/src/vault/services/vault-profile.service.ts
Normal file
@@ -0,0 +1,64 @@
|
||||
import { Injectable, inject } from "@angular/core";
|
||||
|
||||
import { ApiService } from "@bitwarden/common/abstractions/api.service";
|
||||
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
|
||||
*/
|
||||
export class VaultProfileService {
|
||||
private apiService = inject(ApiService);
|
||||
|
||||
private userId: string | null = null;
|
||||
|
||||
/** Profile creation stored as a string. */
|
||||
private profileCreatedDate: string | null = null;
|
||||
|
||||
/** True when 2FA is enabled on the profile. */
|
||||
private profile2FAEnabled: boolean | null = null;
|
||||
|
||||
/**
|
||||
* Returns the creation date of the profile.
|
||||
* Note: `Date`s are mutable in JS, creating a new
|
||||
* instance is important to avoid unwanted changes.
|
||||
*/
|
||||
async getProfileCreationDate(userId: string): Promise<Date> {
|
||||
if (this.profileCreatedDate && userId === this.userId) {
|
||||
return Promise.resolve(new Date(this.profileCreatedDate));
|
||||
}
|
||||
|
||||
const profile = await this.fetchAndCacheProfile();
|
||||
|
||||
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;
|
||||
}
|
||||
|
||||
private async fetchAndCacheProfile(): Promise<ProfileResponse> {
|
||||
const profile = await this.apiService.getProfile();
|
||||
|
||||
this.userId = profile.id;
|
||||
this.profileCreatedDate = profile.creationDate;
|
||||
this.profile2FAEnabled = profile.twoFactorEnabled;
|
||||
|
||||
return profile;
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user