1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-11 22:03:36 +00:00

Auth/PM-17197 - UnauthGuard Trusted Devices Lock State Refactor (#12938)

* PM-17197 - Refactor DeviceTrustService to deprecate active user state as I need to call with a user id per latest best practice

* PM-17197 - Refactor Unauth Guard to be aware of TDE lock state + use active user best practice.
This commit is contained in:
Jared Snider
2025-01-24 13:20:42 -05:00
committed by GitHub
parent b1744c4e0a
commit 6acaa6c711
5 changed files with 177 additions and 28 deletions

View File

@@ -5,17 +5,48 @@ import { MockProxy, mock } from "jest-mock-extended";
import { BehaviorSubject } from "rxjs"; import { BehaviorSubject } from "rxjs";
import { EmptyComponent } from "@bitwarden/angular/platform/guard/feature-flag.guard.spec"; import { EmptyComponent } from "@bitwarden/angular/platform/guard/feature-flag.guard.spec";
import { Account, AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
import { DeviceTrustServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust.service.abstraction";
import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { UserId } from "@bitwarden/common/types/guid";
import { KeyService } from "@bitwarden/key-management";
import { unauthGuardFn } from "./unauth.guard"; import { unauthGuardFn } from "./unauth.guard";
describe("UnauthGuard", () => { describe("UnauthGuard", () => {
const setup = (authStatus: AuthenticationStatus) => { const activeUser: Account = {
id: "fake_user_id" as UserId,
email: "test@email.com",
emailVerified: true,
name: "Test User",
};
const setup = (
activeUser: Account | null,
authStatus: AuthenticationStatus | null = null,
tdeEnabled: boolean = false,
everHadUserKey: boolean = false,
) => {
const accountService: MockProxy<AccountService> = mock<AccountService>();
const authService: MockProxy<AuthService> = mock<AuthService>(); const authService: MockProxy<AuthService> = mock<AuthService>();
authService.getAuthStatus.mockResolvedValue(authStatus); const keyService: MockProxy<KeyService> = mock<KeyService>();
const deviceTrustService: MockProxy<DeviceTrustServiceAbstraction> =
mock<DeviceTrustServiceAbstraction>();
const logService: MockProxy<LogService> = mock<LogService>();
accountService.activeAccount$ = new BehaviorSubject<Account | null>(activeUser);
if (authStatus !== null) {
const activeAccountStatusObservable = new BehaviorSubject<AuthenticationStatus>(authStatus); const activeAccountStatusObservable = new BehaviorSubject<AuthenticationStatus>(authStatus);
authService.activeAccountStatus$ = activeAccountStatusObservable; authService.authStatusFor$.mockReturnValue(activeAccountStatusObservable);
}
keyService.everHadUserKey$ = new BehaviorSubject<boolean>(everHadUserKey);
deviceTrustService.supportsDeviceTrustByUserId$.mockReturnValue(
new BehaviorSubject<boolean>(tdeEnabled),
);
const testBed = TestBed.configureTestingModule({ const testBed = TestBed.configureTestingModule({
imports: [ imports: [
@@ -30,6 +61,7 @@ describe("UnauthGuard", () => {
{ path: "lock", component: EmptyComponent }, { path: "lock", component: EmptyComponent },
{ path: "testhomepage", component: EmptyComponent }, { path: "testhomepage", component: EmptyComponent },
{ path: "testlocked", component: EmptyComponent }, { path: "testlocked", component: EmptyComponent },
{ path: "login-initiated", component: EmptyComponent },
{ {
path: "testOverrides", path: "testOverrides",
component: EmptyComponent, component: EmptyComponent,
@@ -39,7 +71,13 @@ describe("UnauthGuard", () => {
}, },
]), ]),
], ],
providers: [{ provide: AuthService, useValue: authService }], providers: [
{ provide: AccountService, useValue: accountService },
{ provide: AuthService, useValue: authService },
{ provide: KeyService, useValue: keyService },
{ provide: DeviceTrustServiceAbstraction, useValue: deviceTrustService },
{ provide: LogService, useValue: logService },
],
}); });
return { return {
@@ -48,40 +86,54 @@ describe("UnauthGuard", () => {
}; };
it("should be created", () => { it("should be created", () => {
const { router } = setup(AuthenticationStatus.LoggedOut); const { router } = setup(null, AuthenticationStatus.LoggedOut);
expect(router).toBeTruthy(); expect(router).toBeTruthy();
}); });
it("should redirect to /vault for guarded routes when logged in and unlocked", async () => { it("should redirect to /vault for guarded routes when logged in and unlocked", async () => {
const { router } = setup(AuthenticationStatus.Unlocked); const { router } = setup(activeUser, AuthenticationStatus.Unlocked);
await router.navigateByUrl("unauth-guarded-route"); await router.navigateByUrl("unauth-guarded-route");
expect(router.url).toBe("/vault"); expect(router.url).toBe("/vault");
}); });
it("should allow access to guarded routes when logged out", async () => { it("should allow access to guarded routes when account is null", async () => {
const { router } = setup(AuthenticationStatus.LoggedOut); const { router } = setup(null);
await router.navigateByUrl("unauth-guarded-route"); await router.navigateByUrl("unauth-guarded-route");
expect(router.url).toBe("/unauth-guarded-route"); expect(router.url).toBe("/unauth-guarded-route");
}); });
it("should allow access to guarded routes when logged out", async () => {
const { router } = setup(null, AuthenticationStatus.LoggedOut);
await router.navigateByUrl("unauth-guarded-route");
expect(router.url).toBe("/unauth-guarded-route");
});
it("should redirect to /login-initiated when locked, TDE is enabled, and the user hasn't decrypted yet", async () => {
const { router } = setup(activeUser, AuthenticationStatus.Locked, true, false);
await router.navigateByUrl("unauth-guarded-route");
expect(router.url).toBe("/login-initiated");
});
it("should redirect to /lock for guarded routes when locked", async () => { it("should redirect to /lock for guarded routes when locked", async () => {
const { router } = setup(AuthenticationStatus.Locked); const { router } = setup(activeUser, AuthenticationStatus.Locked);
await router.navigateByUrl("unauth-guarded-route"); await router.navigateByUrl("unauth-guarded-route");
expect(router.url).toBe("/lock"); expect(router.url).toBe("/lock");
}); });
it("should redirect to /testhomepage for guarded routes when testOverrides are provided and the account is unlocked", async () => { it("should redirect to /testhomepage for guarded routes when testOverrides are provided and the account is unlocked", async () => {
const { router } = setup(AuthenticationStatus.Unlocked); const { router } = setup(activeUser, AuthenticationStatus.Unlocked);
await router.navigateByUrl("testOverrides"); await router.navigateByUrl("testOverrides");
expect(router.url).toBe("/testhomepage"); expect(router.url).toBe("/testhomepage");
}); });
it("should redirect to /testlocked for guarded routes when testOverrides are provided and the account is locked", async () => { it("should redirect to /testlocked for guarded routes when testOverrides are provided and the account is locked", async () => {
const { router } = setup(AuthenticationStatus.Locked); const { router } = setup(activeUser, AuthenticationStatus.Locked);
await router.navigateByUrl("testOverrides"); await router.navigateByUrl("testOverrides");
expect(router.url).toBe("/testlocked"); expect(router.url).toBe("/testlocked");

View File

@@ -1,9 +1,13 @@
import { inject } from "@angular/core"; import { inject } from "@angular/core";
import { CanActivateFn, Router, UrlTree } from "@angular/router"; import { ActivatedRouteSnapshot, CanActivateFn, Router, UrlTree } from "@angular/router";
import { Observable, map } from "rxjs"; import { firstValueFrom } from "rxjs";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
import { DeviceTrustServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust.service.abstraction";
import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { KeyService } from "@bitwarden/key-management";
type UnauthRoutes = { type UnauthRoutes = {
homepage: () => string; homepage: () => string;
@@ -15,23 +19,54 @@ const defaultRoutes: UnauthRoutes = {
locked: "/lock", locked: "/lock",
}; };
function unauthGuard(routes: UnauthRoutes): Observable<boolean | UrlTree> { // TODO: PM-17195 - Investigate consolidating unauthGuard and redirectGuard into AuthStatusGuard
async function unauthGuard(
route: ActivatedRouteSnapshot,
routes: UnauthRoutes,
): Promise<boolean | UrlTree> {
const accountService = inject(AccountService);
const authService = inject(AuthService); const authService = inject(AuthService);
const router = inject(Router); const router = inject(Router);
const keyService = inject(KeyService);
const deviceTrustService = inject(DeviceTrustServiceAbstraction);
const logService = inject(LogService);
return authService.activeAccountStatus$.pipe( const activeUser = await firstValueFrom(accountService.activeAccount$);
map((status) => {
if (status == null || status === AuthenticationStatus.LoggedOut) { if (!activeUser) {
return true; return true;
} else if (status === AuthenticationStatus.Locked) { }
return router.createUrlTree([routes.locked]);
} else { const authStatus = await firstValueFrom(authService.authStatusFor$(activeUser.id));
if (authStatus == null || authStatus === AuthenticationStatus.LoggedOut) {
return true;
}
if (authStatus === AuthenticationStatus.Unlocked) {
return router.createUrlTree([routes.homepage()]); return router.createUrlTree([routes.homepage()]);
} }
}),
const tdeEnabled = await firstValueFrom(
deviceTrustService.supportsDeviceTrustByUserId$(activeUser.id),
); );
const everHadUserKey = await firstValueFrom(keyService.everHadUserKey$);
// If locked, TDE is enabled, and the user hasn't decrypted yet, then redirect to the
// login decryption options component.
if (authStatus === AuthenticationStatus.Locked && tdeEnabled && !everHadUserKey) {
logService.info(
"Sending user to TDE decryption options. AuthStatus is %s. TDE support is %s. Ever had user key is %s.",
AuthenticationStatus[authStatus],
tdeEnabled,
everHadUserKey,
);
return router.createUrlTree(["/login-initiated"]);
}
return router.createUrlTree([routes.locked]);
} }
export function unauthGuardFn(overrides: Partial<UnauthRoutes> = {}): CanActivateFn { export function unauthGuardFn(overrides: Partial<UnauthRoutes> = {}): CanActivateFn {
return () => unauthGuard({ ...defaultRoutes, ...overrides }); return async (route) => unauthGuard(route, { ...defaultRoutes, ...overrides });
} }

View File

@@ -9,7 +9,18 @@ import { DeviceKey, UserKey } from "../../types/key";
import { DeviceResponse } from "./devices/responses/device.response"; import { DeviceResponse } from "./devices/responses/device.response";
export abstract class DeviceTrustServiceAbstraction { export abstract class DeviceTrustServiceAbstraction {
/**
* @deprecated - use supportsDeviceTrustByUserId instead as active user state is being deprecated
* by Platform
* @description Checks if the device trust feature is supported for the active user.
*/
supportsDeviceTrust$: Observable<boolean>; supportsDeviceTrust$: Observable<boolean>;
/**
* @description Checks if the device trust feature is supported for the given user.
*/
supportsDeviceTrustByUserId$: (userId: UserId) => Observable<boolean>;
/** /**
* @description Retrieves the users choice to trust the device which can only happen after decryption * @description Retrieves the users choice to trust the device which can only happen after decryption
* Note: this value should only be used once and then reset * Note: this value should only be used once and then reset

View File

@@ -81,7 +81,17 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction {
private configService: ConfigService, private configService: ConfigService,
) { ) {
this.supportsDeviceTrust$ = this.userDecryptionOptionsService.userDecryptionOptions$.pipe( this.supportsDeviceTrust$ = this.userDecryptionOptionsService.userDecryptionOptions$.pipe(
map((options) => options?.trustedDeviceOption != null ?? false), map((options) => {
return options?.trustedDeviceOption != null ?? false;
}),
);
}
supportsDeviceTrustByUserId$(userId: UserId): Observable<boolean> {
return this.userDecryptionOptionsService.userDecryptionOptionsById$(userId).pipe(
map((options) => {
return options?.trustedDeviceOption != null ?? false;
}),
); );
} }

View File

@@ -1,5 +1,7 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import { matches, mock } from "jest-mock-extended"; import { matches, mock } from "jest-mock-extended";
import { BehaviorSubject, of } from "rxjs"; import { BehaviorSubject, firstValueFrom, of } from "rxjs";
import { UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common"; import { UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common";
@@ -74,17 +76,56 @@ describe("deviceTrustService", () => {
userId: mockUserId, userId: mockUserId,
}; };
let userDecryptionOptions: UserDecryptionOptions;
beforeEach(() => { beforeEach(() => {
jest.clearAllMocks(); jest.clearAllMocks();
const supportsSecureStorage = false; // default to false; tests will override as needed const supportsSecureStorage = false; // default to false; tests will override as needed
// By default all the tests will have a mocked active user in state provider. // By default all the tests will have a mocked active user in state provider.
deviceTrustService = createDeviceTrustService(mockUserId, supportsSecureStorage); deviceTrustService = createDeviceTrustService(mockUserId, supportsSecureStorage);
userDecryptionOptions = new UserDecryptionOptions();
}); });
it("instantiates", () => { it("instantiates", () => {
expect(deviceTrustService).not.toBeFalsy(); expect(deviceTrustService).not.toBeFalsy();
}); });
describe("supportsDeviceTrustByUserId$", () => {
it("returns true when the user has a non-null trusted device decryption option", async () => {
// Arrange
userDecryptionOptions.trustedDeviceOption = {
hasAdminApproval: false,
hasLoginApprovingDevice: false,
hasManageResetPasswordPermission: false,
isTdeOffboarding: false,
};
userDecryptionOptionsService.userDecryptionOptionsById$.mockReturnValue(
new BehaviorSubject<UserDecryptionOptions>(userDecryptionOptions),
);
const result = await firstValueFrom(
deviceTrustService.supportsDeviceTrustByUserId$(mockUserId),
);
expect(result).toBe(true);
});
it("returns false when the user has a null trusted device decryption option", async () => {
// Arrange
userDecryptionOptions.trustedDeviceOption = null;
userDecryptionOptionsService.userDecryptionOptionsById$.mockReturnValue(
new BehaviorSubject<UserDecryptionOptions>(userDecryptionOptions),
);
const result = await firstValueFrom(
deviceTrustService.supportsDeviceTrustByUserId$(mockUserId),
);
expect(result).toBe(false);
});
});
describe("User Trust Device Choice For Decryption", () => { describe("User Trust Device Choice For Decryption", () => {
describe("getShouldTrustDevice", () => { describe("getShouldTrustDevice", () => {
it("gets the user trust device choice for decryption", async () => { it("gets the user trust device choice for decryption", async () => {