diff --git a/apps/browser/src/popup/app-routing.module.ts b/apps/browser/src/popup/app-routing.module.ts index 77b5091a43..8766702efe 100644 --- a/apps/browser/src/popup/app-routing.module.ts +++ b/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: [ { diff --git a/apps/desktop/src/app/app-routing.module.ts b/apps/desktop/src/app/app-routing.module.ts index 11d5699414..afcfcd0097 100644 --- a/apps/desktop/src/app/app-routing.module.ts +++ b/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()], }, ], }, diff --git a/apps/web/src/app/oss-routing.module.ts b/apps/web/src/app/oss-routing.module.ts index 5d5846a95b..15cf3fa6de 100644 --- a/apps/web/src/app/oss-routing.module.ts +++ b/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, }, ), - { - path: "2fa", - canActivate: [unauthGuardFn()], - children: [ - ...unauthUiRefreshSwap(TwoFactorComponentV1, TwoFactorAuthComponent, { - path: "", - }), - { - path: "", - component: EnvironmentSelectorComponent, - outlet: "environment-selector", - }, - ], - data: { - pageTitle: { - key: "verifyIdentity", - }, - } satisfies RouteDataProperties & AnonLayoutWrapperData, - }, + ...extensionRefreshSwap( + TwoFactorComponentV1, + TwoFactorAuthComponent, + { + path: "2fa", + canActivate: [unauthGuardFn()], + children: [ + { + path: "", + component: TwoFactorComponentV1, + }, + { + path: "", + component: EnvironmentSelectorComponent, + outlet: "environment-selector", + }, + ], + 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()], diff --git a/libs/auth/src/angular/index.ts b/libs/auth/src/angular/index.ts index 8798679c02..5dec280b50 100644 --- a/libs/auth/src/angular/index.ts +++ b/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"; diff --git a/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.ts b/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.ts index ca7c6b1783..38028d21a0 100644 --- a/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.ts +++ b/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 { - 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"; diff --git a/libs/auth/src/angular/two-factor-auth/two-factor-auth.guard.spec.ts b/libs/auth/src/angular/two-factor-auth/two-factor-auth.guard.spec.ts new file mode 100644 index 0000000000..22cfe0820e --- /dev/null +++ b/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; + const currentAuthTypesSubject = new BehaviorSubject(null); + + let twoFactorService: MockProxy; + let router: Router; + + beforeEach(() => { + loginStrategyService = mock(); + loginStrategyService.currentAuthType$ = currentAuthTypesSubject.asObservable(); + + twoFactorService = mock(); + + 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"); + }); + }); +}); diff --git a/libs/auth/src/angular/two-factor-auth/two-factor-auth.guard.ts b/libs/auth/src/angular/two-factor-auth/two-factor-auth.guard.ts new file mode 100644 index 0000000000..2aec0bae44 --- /dev/null +++ b/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 => { + 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; +};