Skip to content

Commit

Permalink
Merge branch 'main' into km/tmp-ownership-2
Browse files Browse the repository at this point in the history
  • Loading branch information
quexten authored Jan 24, 2025
2 parents 9a46b61 + 315e133 commit 23343fd
Show file tree
Hide file tree
Showing 14 changed files with 336 additions and 35 deletions.
2 changes: 1 addition & 1 deletion apps/browser/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@bitwarden/browser",
"version": "2025.1.2",
"version": "2025.1.3",
"scripts": {
"build": "npm run build:chrome",
"build:chrome": "cross-env BROWSER=chrome MANIFEST_VERSION=3 NODE_OPTIONS=\"--max-old-space-size=8192\" webpack",
Expand Down
2 changes: 1 addition & 1 deletion apps/browser/src/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"manifest_version": 2,
"name": "__MSG_extName__",
"short_name": "__MSG_appName__",
"version": "2025.1.2",
"version": "2025.1.3",
"description": "__MSG_extDesc__",
"default_locale": "en",
"author": "Bitwarden Inc.",
Expand Down
2 changes: 1 addition & 1 deletion apps/browser/src/manifest.v3.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"minimum_chrome_version": "102.0",
"name": "__MSG_extName__",
"short_name": "__MSG_appName__",
"version": "2025.1.2",
"version": "2025.1.3",
"description": "__MSG_extDesc__",
"default_locale": "en",
"author": "Bitwarden Inc.",
Expand Down
74 changes: 63 additions & 11 deletions libs/angular/src/auth/guards/unauth.guard.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,48 @@ import { MockProxy, mock } from "jest-mock-extended";
import { BehaviorSubject } from "rxjs";

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 { DeviceTrustServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust.service.abstraction";
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";

describe("UnauthGuard", () => {
const setup = (authStatus: AuthenticationStatus) => {
const activeUser: Account = {
id: "fake_user_id" as UserId,
email: "[email protected]",
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>();
authService.getAuthStatus.mockResolvedValue(authStatus);
const activeAccountStatusObservable = new BehaviorSubject<AuthenticationStatus>(authStatus);
authService.activeAccountStatus$ = activeAccountStatusObservable;
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);
authService.authStatusFor$.mockReturnValue(activeAccountStatusObservable);
}

keyService.everHadUserKey$ = new BehaviorSubject<boolean>(everHadUserKey);
deviceTrustService.supportsDeviceTrustByUserId$.mockReturnValue(
new BehaviorSubject<boolean>(tdeEnabled),
);

const testBed = TestBed.configureTestingModule({
imports: [
Expand All @@ -30,6 +61,7 @@ describe("UnauthGuard", () => {
{ path: "lock", component: EmptyComponent },
{ path: "testhomepage", component: EmptyComponent },
{ path: "testlocked", component: EmptyComponent },
{ path: "login-initiated", component: EmptyComponent },
{
path: "testOverrides",
component: EmptyComponent,
Expand All @@ -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 {
Expand All @@ -48,40 +86,54 @@ describe("UnauthGuard", () => {
};

it("should be created", () => {
const { router } = setup(AuthenticationStatus.LoggedOut);
const { router } = setup(null, AuthenticationStatus.LoggedOut);
expect(router).toBeTruthy();
});

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");
expect(router.url).toBe("/vault");
});

it("should allow access to guarded routes when account is null", async () => {
const { router } = setup(null);

await router.navigateByUrl("unauth-guarded-route");
expect(router.url).toBe("/unauth-guarded-route");
});

it("should allow access to guarded routes when logged out", async () => {
const { router } = setup(AuthenticationStatus.LoggedOut);
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 () => {
const { router } = setup(AuthenticationStatus.Locked);
const { router } = setup(activeUser, AuthenticationStatus.Locked);

await router.navigateByUrl("unauth-guarded-route");
expect(router.url).toBe("/lock");
});

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");
expect(router.url).toBe("/testhomepage");
});

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");
expect(router.url).toBe("/testlocked");
Expand Down
63 changes: 49 additions & 14 deletions libs/angular/src/auth/guards/unauth.guard.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
import { inject } from "@angular/core";
import { CanActivateFn, Router, UrlTree } from "@angular/router";
import { Observable, map } from "rxjs";
import { ActivatedRouteSnapshot, CanActivateFn, Router, UrlTree } from "@angular/router";
import { firstValueFrom } from "rxjs";

import { AccountService } from "@bitwarden/common/auth/abstractions/account.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 { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { KeyService } from "@bitwarden/key-management";

type UnauthRoutes = {
homepage: () => string;
Expand All @@ -15,23 +19,54 @@ const defaultRoutes: UnauthRoutes = {
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 router = inject(Router);
const keyService = inject(KeyService);
const deviceTrustService = inject(DeviceTrustServiceAbstraction);
const logService = inject(LogService);

return authService.activeAccountStatus$.pipe(
map((status) => {
if (status == null || status === AuthenticationStatus.LoggedOut) {
return true;
} else if (status === AuthenticationStatus.Locked) {
return router.createUrlTree([routes.locked]);
} else {
return router.createUrlTree([routes.homepage()]);
}
}),
const activeUser = await firstValueFrom(accountService.activeAccount$);

if (!activeUser) {
return true;
}

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()]);
}

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 {
return () => unauthGuard({ ...defaultRoutes, ...overrides });
return async (route) => unauthGuard(route, { ...defaultRoutes, ...overrides });
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,11 @@ import { LogService } from "@bitwarden/common/platform/abstractions/log.service"
import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service";
import { ToastService } from "@bitwarden/components";

import { LoginStrategyServiceAbstraction, PasswordLoginCredentials } from "../../../common";
import {
LoginStrategyServiceAbstraction,
LoginSuccessHandlerService,
PasswordLoginCredentials,
} from "../../../common";
import { AnonLayoutWrapperDataService } from "../../anon-layout/anon-layout-wrapper-data.service";
import { InputPasswordComponent } from "../../input-password/input-password.component";
import { PasswordInputResult } from "../../input-password/password-input-result";
Expand Down Expand Up @@ -68,6 +72,7 @@ export class RegistrationFinishComponent implements OnInit, OnDestroy {
private loginStrategyService: LoginStrategyServiceAbstraction,
private logService: LogService,
private anonLayoutWrapperDataService: AnonLayoutWrapperDataService,
private loginSuccessHandlerService: LoginSuccessHandlerService,
) {}

async ngOnInit() {
Expand Down Expand Up @@ -189,6 +194,8 @@ export class RegistrationFinishComponent implements OnInit, OnDestroy {
message: this.i18nService.t("youHaveBeenLoggedIn"),
});

await this.loginSuccessHandlerService.run(authenticationResult.userId);

await this.router.navigate(["/vault"]);
} catch (e) {
// If login errors, redirect to login page per product. Don't show error
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,18 @@ import { DeviceKey, UserKey } from "../../types/key";
import { DeviceResponse } from "./devices/responses/device.response";

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>;

/**
* @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
* Note: this value should only be used once and then reset
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,17 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction {
private configService: ConfigService,
) {
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;
}),
);
}

Expand Down
43 changes: 42 additions & 1 deletion libs/common/src/auth/services/device-trust.service.spec.ts
Original file line number Diff line number Diff line change
@@ -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 { BehaviorSubject, of } from "rxjs";
import { BehaviorSubject, firstValueFrom, of } from "rxjs";

import { UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common";

Expand Down Expand Up @@ -74,17 +76,56 @@ describe("deviceTrustService", () => {
userId: mockUserId,
};

let userDecryptionOptions: UserDecryptionOptions;

beforeEach(() => {
jest.clearAllMocks();
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.
deviceTrustService = createDeviceTrustService(mockUserId, supportsSecureStorage);

userDecryptionOptions = new UserDecryptionOptions();
});

it("instantiates", () => {
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("getShouldTrustDevice", () => {
it("gets the user trust device choice for decryption", async () => {
Expand Down
Loading

0 comments on commit 23343fd

Please sign in to comment.