PM-8113 - Refactor 2FA Guard logic out of ngOnInit and into own tested guard. Updated all routes.

apps/browser/src/popup/app-routing.module.ts

@@ -41,6 +41,7 @@ import {
   TwoFactorTimeoutIcon,
   TwoFactorAuthComponent,
   TwoFactorTimeoutComponent,
+  TwoFactorAuthGuard,
 } from "@bitwarden/auth/angular";
 import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
 
@@ -182,7 +183,7 @@ const routes: Routes = [
   },
   ...unauthUiRefreshSwap(
     TwoFactorComponentV1,
-    AnonLayoutWrapperComponent,
+    ExtensionAnonLayoutWrapperComponent,
     {
       path: "2fa",
       canActivate: [unauthGuardFn(unauthRouteOverrides)],
@@ -190,7 +191,7 @@ const routes: Routes = [
     },
     {
       path: "2fa",
-      canActivate: [unauthGuardFn(unauthRouteOverrides)],
+      canActivate: [unauthGuardFn(unauthRouteOverrides), TwoFactorAuthGuard],
       data: { elevation: 1 } satisfies RouteDataProperties,
       children: [
         {

apps/desktop/src/app/app-routing.module.ts

@@ -38,6 +38,7 @@ import {
   TwoFactorTimeoutIcon,
   TwoFactorAuthComponent,
   TwoFactorTimeoutComponent,
+  TwoFactorAuthGuard,
 } from "@bitwarden/auth/angular";
 import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
 
@@ -96,11 +97,11 @@ const routes: Routes = [
     {
       path: "2fa",
       component: AnonLayoutWrapperComponent,
+      canActivate: [unauthGuardFn(), TwoFactorAuthGuard],
       children: [
         {
           path: "",
           component: TwoFactorAuthComponent,
-          canActivate: [unauthGuardFn()],
         },
       ],
     },

apps/web/src/app/oss-routing.module.ts

@@ -37,6 +37,7 @@ import {
   LoginDecryptionOptionsComponent,
   TwoFactorAuthComponent,
   TwoFactorTimeoutComponent,
+  TwoFactorAuthGuard,
 } from "@bitwarden/auth/angular";
 import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
 
@@ -508,25 +509,51 @@ const routes: Routes = [
           } satisfies AnonLayoutWrapperData,
         },
       ),
-      {
+      ...extensionRefreshSwap(
-        path: "2fa",
+        TwoFactorComponentV1,
-        canActivate: [unauthGuardFn()],
+        TwoFactorAuthComponent,
-        children: [
+        {
-          ...unauthUiRefreshSwap(TwoFactorComponentV1, TwoFactorAuthComponent, {
+          path: "2fa",
-            path: "",
+          canActivate: [unauthGuardFn()],
-          }),
+          children: [
-          {
+            {
-            path: "",
+              path: "",
-            component: EnvironmentSelectorComponent,
+              component: TwoFactorComponentV1,
-            outlet: "environment-selector",
+            },
-          },
+            {
-        ],
+              path: "",
-        data: {
+              component: EnvironmentSelectorComponent,
-          pageTitle: {
+              outlet: "environment-selector",
-            key: "verifyIdentity",
+            },
-          },
+          ],
-        } satisfies RouteDataProperties & AnonLayoutWrapperData,
+          data: {
-      },
+            pageTitle: {
+              key: "verifyIdentity",
+            },
+          } satisfies RouteDataProperties & AnonLayoutWrapperData,
+        },
+        {
+          path: "2fa",
+          canActivate: [unauthGuardFn(), TwoFactorAuthGuard],
+          children: [
+            {
+              path: "",
+              component: TwoFactorAuthComponent,
+            },
+            {
+              path: "",
+              component: EnvironmentSelectorComponent,
+              outlet: "environment-selector",
+            },
+          ],
+          data: {
+            pageTitle: {
+              key: "verifyIdentity",
+            },
+          } satisfies RouteDataProperties & AnonLayoutWrapperData,
+        },
+      ),
+
       {
         path: "2fa-timeout",
         canActivate: [unauthGuardFn()],

libs/auth/src/angular/index.ts

@@ -76,3 +76,4 @@ export * from "./two-factor-auth/two-factor-auth-component.service";
 export * from "./two-factor-auth/default-two-factor-auth-component.service";
 export * from "./two-factor-auth/two-factor-auth.component";
 export * from "./two-factor-auth/two-factor-auth-expired.component";
+export * from "./two-factor-auth/two-factor-auth.guard";

libs/auth/src/angular/two-factor-auth/two-factor-auth.component.ts

@@ -145,14 +145,6 @@ export class TwoFactorAuthComponent implements OnInit, OnDestroy {
   }
 
   async ngOnInit() {
-    // TODO: should this be in a guard instead of here?
-    const userIsAuthenticating = await this.userIsAuthenticating();
-    const twoFactorProviders = await this.twoFactorService.getProviders();
-    if (!userIsAuthenticating || twoFactorProviders == null) {
-      await this.router.navigate([this.loginRoute]);
-      return;
-    }
-
     this.orgSsoIdentifier = this.activatedRoute.snapshot.queryParamMap.get("identifier");
     this.inSsoFlow = this.activatedRoute.snapshot.queryParamMap.get("sso") === "true";
 
@@ -193,11 +185,6 @@ export class TwoFactorAuthComponent implements OnInit, OnDestroy {
       return;
     }
 
-    // TODO: refactor into service
-    // if (await BrowserPopupUtils.inPopout(this.win)) {
-    //   this.selectedProviderType = TwoFactorProviderType.Email;
-    // }
-
     // WebAuthn prompt appears inside the popup on linux, and requires a larger popup width
     // than usual to avoid cutting off the dialog.
     if (this.selectedProviderType === TwoFactorProviderType.WebAuthn && (await this.isLinux())) {
@@ -496,10 +483,6 @@ export class TwoFactorAuthComponent implements OnInit, OnDestroy {
     );
   }
 
-  private async userIsAuthenticating(): Promise<boolean> {
-    return (await firstValueFrom(this.loginStrategyService.currentAuthType$)) !== null;
-  }
-
   async isLinux() {
     // TODO: this was extension logic and must be moved to service if platform utils service doesn't have support for this
     // return (await BrowserApi.getPlatformInfo()).os === "linux";

libs/auth/src/angular/two-factor-auth/two-factor-auth.guard.spec.ts

@@ -0,0 +1,74 @@
+import { Component } from "@angular/core";
+import { TestBed } from "@angular/core/testing";
+import { provideRouter, Router } from "@angular/router";
+import { mock, MockProxy } from "jest-mock-extended";
+import { BehaviorSubject } from "rxjs";
+
+import { TwoFactorService } from "@bitwarden/common/auth/abstractions/two-factor.service";
+import { AuthenticationType } from "@bitwarden/common/auth/enums/authentication-type";
+
+import { LoginStrategyServiceAbstraction } from "../../common";
+
+import { TwoFactorAuthGuard } from "./two-factor-auth.guard";
+
+@Component({ template: "" })
+export class EmptyComponent {}
+
+describe("TwoFactorAuthGuard", () => {
+  let loginStrategyService: MockProxy<LoginStrategyServiceAbstraction>;
+  const currentAuthTypesSubject = new BehaviorSubject<AuthenticationType | null>(null);
+
+  let twoFactorService: MockProxy<TwoFactorService>;
+  let router: Router;
+
+  beforeEach(() => {
+    loginStrategyService = mock<LoginStrategyServiceAbstraction>();
+    loginStrategyService.currentAuthType$ = currentAuthTypesSubject.asObservable();
+
+    twoFactorService = mock<TwoFactorService>();
+
+    TestBed.configureTestingModule({
+      providers: [
+        provideRouter([
+          { path: "login", component: EmptyComponent },
+          { path: "protected", component: EmptyComponent, canActivate: [TwoFactorAuthGuard] },
+        ]),
+        { provide: LoginStrategyServiceAbstraction, useValue: loginStrategyService },
+        { provide: TwoFactorService, useValue: twoFactorService },
+      ],
+    });
+
+    router = TestBed.inject(Router);
+  });
+
+  it("should redirect to /login if the user is not authenticating", async () => {
+    // Arrange
+    currentAuthTypesSubject.next(null);
+    twoFactorService.getProviders.mockResolvedValue(null);
+
+    // Act
+    await router.navigateByUrl("/protected");
+
+    // Assert
+    expect(router.url).toBe("/login");
+  });
+
+  const authenticationTypes = Object.entries(AuthenticationType)
+    // filter out reverse mappings (e.g., "0": "Password")
+    .filter(([key, value]) => typeof value === "number")
+    .map(([key, value]) => [value, key]) as [AuthenticationType, string][];
+
+  authenticationTypes.forEach(([authType, authTypeName]) => {
+    it(`should redirect to /login if the user is authenticating with ${authTypeName} but no two-factor providers exist`, async () => {
+      // Arrange
+      currentAuthTypesSubject.next(authType);
+      twoFactorService.getProviders.mockResolvedValue(null);
+
+      // Act
+      await router.navigateByUrl("/protected");
+
+      // Assert
+      expect(router.url).toBe("/login");
+    });
+  });
+});

libs/auth/src/angular/two-factor-auth/two-factor-auth.guard.ts

@@ -0,0 +1,33 @@
+import { inject } from "@angular/core";
+import {
+  ActivatedRouteSnapshot,
+  CanActivateFn,
+  Router,
+  RouterStateSnapshot,
+  UrlTree,
+} from "@angular/router";
+import { firstValueFrom } from "rxjs";
+
+import { TwoFactorService } from "@bitwarden/common/auth/abstractions/two-factor.service";
+
+import { LoginStrategyServiceAbstraction } from "../../common";
+
+export const TwoFactorAuthGuard: CanActivateFn = async (
+  route: ActivatedRouteSnapshot,
+  routerState: RouterStateSnapshot,
+): Promise<boolean | UrlTree> => {
+  const loginStrategyService = inject(LoginStrategyServiceAbstraction);
+  const twoFactorService = inject(TwoFactorService);
+  const router = inject(Router);
+
+  const currentAuthType = await firstValueFrom(loginStrategyService.currentAuthType$);
+  const userIsAuthenticating = currentAuthType !== null;
+
+  const twoFactorProviders = await twoFactorService.getProviders();
+
+  if (!userIsAuthenticating || twoFactorProviders == null) {
+    return router.createUrlTree(["/login"]);
+  }
+
+  return true;
+};