mirror of
https://github.com/bitwarden/browser
synced 2025-12-10 13:23:34 +00:00
fix(passkeys): [PM-28324] Add a guard that conditionally forces a popout depending on platform
* Add a guard that conditionally forces a popout depending on platform * Test the routeguard * Use mockImplementation instead. * autoclose popout
This commit is contained in:
193
apps/browser/src/auth/popup/guards/platform-popout.guard.spec.ts
Normal file
193
apps/browser/src/auth/popup/guards/platform-popout.guard.spec.ts
Normal file
@@ -0,0 +1,193 @@
|
|||||||
|
import { TestBed } from "@angular/core/testing";
|
||||||
|
import { ActivatedRouteSnapshot, RouterStateSnapshot } from "@angular/router";
|
||||||
|
|
||||||
|
import { BrowserApi } from "../../../platform/browser/browser-api";
|
||||||
|
import BrowserPopupUtils from "../../../platform/browser/browser-popup-utils";
|
||||||
|
|
||||||
|
import { platformPopoutGuard } from "./platform-popout.guard";
|
||||||
|
|
||||||
|
describe("platformPopoutGuard", () => {
|
||||||
|
let getPlatformInfoSpy: jest.SpyInstance;
|
||||||
|
let inPopoutSpy: jest.SpyInstance;
|
||||||
|
let inSidebarSpy: jest.SpyInstance;
|
||||||
|
let openPopoutSpy: jest.SpyInstance;
|
||||||
|
let closePopupSpy: jest.SpyInstance;
|
||||||
|
|
||||||
|
const mockRoute = {} as ActivatedRouteSnapshot;
|
||||||
|
const mockState: RouterStateSnapshot = {
|
||||||
|
url: "/login-with-passkey?param=value",
|
||||||
|
} as RouterStateSnapshot;
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
getPlatformInfoSpy = jest.spyOn(BrowserApi, "getPlatformInfo");
|
||||||
|
inPopoutSpy = jest.spyOn(BrowserPopupUtils, "inPopout");
|
||||||
|
inSidebarSpy = jest.spyOn(BrowserPopupUtils, "inSidebar");
|
||||||
|
openPopoutSpy = jest.spyOn(BrowserPopupUtils, "openPopout").mockImplementation();
|
||||||
|
closePopupSpy = jest.spyOn(BrowserApi, "closePopup").mockImplementation();
|
||||||
|
|
||||||
|
TestBed.configureTestingModule({});
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
jest.clearAllMocks();
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("when platform matches", () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
getPlatformInfoSpy.mockResolvedValue({ os: "linux" });
|
||||||
|
inPopoutSpy.mockReturnValue(false);
|
||||||
|
inSidebarSpy.mockReturnValue(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should open popout and block navigation when not already in popout or sidebar", async () => {
|
||||||
|
const guard = platformPopoutGuard(["linux"]);
|
||||||
|
const result = await TestBed.runInInjectionContext(() => guard(mockRoute, mockState));
|
||||||
|
|
||||||
|
expect(getPlatformInfoSpy).toHaveBeenCalled();
|
||||||
|
expect(inPopoutSpy).toHaveBeenCalledWith(window);
|
||||||
|
expect(inSidebarSpy).toHaveBeenCalledWith(window);
|
||||||
|
expect(openPopoutSpy).toHaveBeenCalledWith(
|
||||||
|
"popup/index.html#/login-with-passkey?param=value&autoClosePopout=true",
|
||||||
|
);
|
||||||
|
expect(closePopupSpy).toHaveBeenCalledWith(window);
|
||||||
|
expect(result).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should allow navigation when already in popout", async () => {
|
||||||
|
inPopoutSpy.mockReturnValue(true);
|
||||||
|
|
||||||
|
const guard = platformPopoutGuard(["linux"]);
|
||||||
|
const result = await TestBed.runInInjectionContext(() => guard(mockRoute, mockState));
|
||||||
|
|
||||||
|
expect(openPopoutSpy).not.toHaveBeenCalled();
|
||||||
|
expect(closePopupSpy).not.toHaveBeenCalled();
|
||||||
|
expect(result).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should allow navigation when already in sidebar", async () => {
|
||||||
|
inSidebarSpy.mockReturnValue(true);
|
||||||
|
|
||||||
|
const guard = platformPopoutGuard(["linux"]);
|
||||||
|
const result = await TestBed.runInInjectionContext(() => guard(mockRoute, mockState));
|
||||||
|
|
||||||
|
expect(openPopoutSpy).not.toHaveBeenCalled();
|
||||||
|
expect(closePopupSpy).not.toHaveBeenCalled();
|
||||||
|
expect(result).toBe(true);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("when platform does not match", () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
getPlatformInfoSpy.mockResolvedValue({ os: "win" });
|
||||||
|
inPopoutSpy.mockReturnValue(false);
|
||||||
|
inSidebarSpy.mockReturnValue(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should allow navigation without opening popout", async () => {
|
||||||
|
const guard = platformPopoutGuard(["linux"]);
|
||||||
|
const result = await TestBed.runInInjectionContext(() => guard(mockRoute, mockState));
|
||||||
|
|
||||||
|
expect(getPlatformInfoSpy).toHaveBeenCalled();
|
||||||
|
expect(openPopoutSpy).not.toHaveBeenCalled();
|
||||||
|
expect(result).toBe(true);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("when forcePopout is true", () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
getPlatformInfoSpy.mockResolvedValue({ os: "win" });
|
||||||
|
inPopoutSpy.mockReturnValue(false);
|
||||||
|
inSidebarSpy.mockReturnValue(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should open popout regardless of platform", async () => {
|
||||||
|
const guard = platformPopoutGuard(["linux"], true);
|
||||||
|
const result = await TestBed.runInInjectionContext(() => guard(mockRoute, mockState));
|
||||||
|
|
||||||
|
expect(openPopoutSpy).toHaveBeenCalledWith(
|
||||||
|
"popup/index.html#/login-with-passkey?param=value&autoClosePopout=true",
|
||||||
|
);
|
||||||
|
expect(closePopupSpy).toHaveBeenCalledWith(window);
|
||||||
|
expect(result).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should not open popout when already in popout", async () => {
|
||||||
|
inPopoutSpy.mockReturnValue(true);
|
||||||
|
|
||||||
|
const guard = platformPopoutGuard(["linux"], true);
|
||||||
|
const result = await TestBed.runInInjectionContext(() => guard(mockRoute, mockState));
|
||||||
|
|
||||||
|
expect(openPopoutSpy).not.toHaveBeenCalled();
|
||||||
|
expect(result).toBe(true);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("with multiple platforms", () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
inPopoutSpy.mockReturnValue(false);
|
||||||
|
inSidebarSpy.mockReturnValue(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it.each(["linux", "mac", "win"])(
|
||||||
|
"should open popout when platform is %s and included in platforms array",
|
||||||
|
async (platform) => {
|
||||||
|
getPlatformInfoSpy.mockResolvedValue({ os: platform });
|
||||||
|
|
||||||
|
const guard = platformPopoutGuard(["linux", "mac", "win"]);
|
||||||
|
const result = await TestBed.runInInjectionContext(() => guard(mockRoute, mockState));
|
||||||
|
|
||||||
|
expect(openPopoutSpy).toHaveBeenCalledWith(
|
||||||
|
"popup/index.html#/login-with-passkey?param=value&autoClosePopout=true",
|
||||||
|
);
|
||||||
|
expect(closePopupSpy).toHaveBeenCalledWith(window);
|
||||||
|
expect(result).toBe(false);
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
it("should not open popout when platform is not in the array", async () => {
|
||||||
|
getPlatformInfoSpy.mockResolvedValue({ os: "android" });
|
||||||
|
|
||||||
|
const guard = platformPopoutGuard(["linux", "mac"]);
|
||||||
|
const result = await TestBed.runInInjectionContext(() => guard(mockRoute, mockState));
|
||||||
|
|
||||||
|
expect(openPopoutSpy).not.toHaveBeenCalled();
|
||||||
|
expect(result).toBe(true);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("url handling", () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
getPlatformInfoSpy.mockResolvedValue({ os: "linux" });
|
||||||
|
inPopoutSpy.mockReturnValue(false);
|
||||||
|
inSidebarSpy.mockReturnValue(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should preserve query parameters in the popout url", async () => {
|
||||||
|
const stateWithQuery: RouterStateSnapshot = {
|
||||||
|
url: "/path?foo=bar&baz=qux",
|
||||||
|
} as RouterStateSnapshot;
|
||||||
|
|
||||||
|
const guard = platformPopoutGuard(["linux"]);
|
||||||
|
await TestBed.runInInjectionContext(() => guard(mockRoute, stateWithQuery));
|
||||||
|
|
||||||
|
expect(openPopoutSpy).toHaveBeenCalledWith(
|
||||||
|
"popup/index.html#/path?foo=bar&baz=qux&autoClosePopout=true",
|
||||||
|
);
|
||||||
|
expect(closePopupSpy).toHaveBeenCalledWith(window);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should handle urls without query parameters", async () => {
|
||||||
|
const stateWithoutQuery: RouterStateSnapshot = {
|
||||||
|
url: "/simple-path",
|
||||||
|
} as RouterStateSnapshot;
|
||||||
|
|
||||||
|
const guard = platformPopoutGuard(["linux"]);
|
||||||
|
await TestBed.runInInjectionContext(() => guard(mockRoute, stateWithoutQuery));
|
||||||
|
|
||||||
|
expect(openPopoutSpy).toHaveBeenCalledWith(
|
||||||
|
"popup/index.html#/simple-path?autoClosePopout=true",
|
||||||
|
);
|
||||||
|
expect(closePopupSpy).toHaveBeenCalledWith(window);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
46
apps/browser/src/auth/popup/guards/platform-popout.guard.ts
Normal file
46
apps/browser/src/auth/popup/guards/platform-popout.guard.ts
Normal file
@@ -0,0 +1,46 @@
|
|||||||
|
import { ActivatedRouteSnapshot, CanActivateFn, RouterStateSnapshot } from "@angular/router";
|
||||||
|
|
||||||
|
import { BrowserApi } from "../../../platform/browser/browser-api";
|
||||||
|
import BrowserPopupUtils from "../../../platform/browser/browser-popup-utils";
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Guard that forces a popout window for specific platforms.
|
||||||
|
* Useful when popup context would close during operations (e.g., WebAuthn on Linux).
|
||||||
|
*
|
||||||
|
* @param platforms - Array of platform OS strings (e.g., ["linux", "mac", "win"])
|
||||||
|
* @param forcePopout - If true, always force popout regardless of platform (useful for testing)
|
||||||
|
* @returns CanActivateFn that opens popout and blocks navigation if conditions met
|
||||||
|
*/
|
||||||
|
export function platformPopoutGuard(
|
||||||
|
platforms: string[],
|
||||||
|
forcePopout: boolean = false,
|
||||||
|
): CanActivateFn {
|
||||||
|
return async (route: ActivatedRouteSnapshot, state: RouterStateSnapshot) => {
|
||||||
|
// Check if current platform matches
|
||||||
|
const platformInfo = await BrowserApi.getPlatformInfo();
|
||||||
|
const isPlatformMatch = platforms.includes(platformInfo.os);
|
||||||
|
|
||||||
|
// Check if already in popout/sidebar
|
||||||
|
const inPopout = BrowserPopupUtils.inPopout(window);
|
||||||
|
const inSidebar = BrowserPopupUtils.inSidebar(window);
|
||||||
|
|
||||||
|
// Open popout if conditions met
|
||||||
|
if ((isPlatformMatch || forcePopout) && !inPopout && !inSidebar) {
|
||||||
|
// Add autoClosePopout query param to signal the popout should close after completion
|
||||||
|
const [path, existingQuery] = state.url.split("?");
|
||||||
|
const params = new URLSearchParams(existingQuery || "");
|
||||||
|
params.set("autoClosePopout", "true");
|
||||||
|
const urlWithAutoClose = `${path}?${params.toString()}`;
|
||||||
|
|
||||||
|
// Open the popout window
|
||||||
|
await BrowserPopupUtils.openPopout(`popup/index.html#${urlWithAutoClose}`);
|
||||||
|
|
||||||
|
// Close the original popup window
|
||||||
|
BrowserApi.closePopup(window);
|
||||||
|
|
||||||
|
return false; // Block navigation - popout will reload
|
||||||
|
}
|
||||||
|
|
||||||
|
return true; // Allow navigation
|
||||||
|
};
|
||||||
|
}
|
||||||
@@ -48,6 +48,7 @@ import { LockComponent, ConfirmKeyConnectorDomainComponent } from "@bitwarden/ke
|
|||||||
import { AccountSwitcherComponent } from "../auth/popup/account-switching/account-switcher.component";
|
import { AccountSwitcherComponent } from "../auth/popup/account-switching/account-switcher.component";
|
||||||
import { AuthExtensionRoute } from "../auth/popup/constants/auth-extension-route.constant";
|
import { AuthExtensionRoute } from "../auth/popup/constants/auth-extension-route.constant";
|
||||||
import { fido2AuthGuard } from "../auth/popup/guards/fido2-auth.guard";
|
import { fido2AuthGuard } from "../auth/popup/guards/fido2-auth.guard";
|
||||||
|
import { platformPopoutGuard } from "../auth/popup/guards/platform-popout.guard";
|
||||||
import { AccountSecurityComponent } from "../auth/popup/settings/account-security.component";
|
import { AccountSecurityComponent } from "../auth/popup/settings/account-security.component";
|
||||||
import { ExtensionDeviceManagementComponent } from "../auth/popup/settings/extension-device-management.component";
|
import { ExtensionDeviceManagementComponent } from "../auth/popup/settings/extension-device-management.component";
|
||||||
import { Fido2Component } from "../autofill/popup/fido2/fido2.component";
|
import { Fido2Component } from "../autofill/popup/fido2/fido2.component";
|
||||||
@@ -414,7 +415,7 @@ const routes: Routes = [
|
|||||||
},
|
},
|
||||||
{
|
{
|
||||||
path: AuthRoute.LoginWithPasskey,
|
path: AuthRoute.LoginWithPasskey,
|
||||||
canActivate: [unauthGuardFn(unauthRouteOverrides)],
|
canActivate: [unauthGuardFn(unauthRouteOverrides), platformPopoutGuard(["linux"])],
|
||||||
data: {
|
data: {
|
||||||
pageIcon: TwoFactorAuthSecurityKeyIcon,
|
pageIcon: TwoFactorAuthSecurityKeyIcon,
|
||||||
pageTitle: {
|
pageTitle: {
|
||||||
|
|||||||
@@ -2,7 +2,7 @@
|
|||||||
// @ts-strict-ignore
|
// @ts-strict-ignore
|
||||||
import { CommonModule } from "@angular/common";
|
import { CommonModule } from "@angular/common";
|
||||||
import { Component, OnInit } from "@angular/core";
|
import { Component, OnInit } from "@angular/core";
|
||||||
import { Router, RouterModule } from "@angular/router";
|
import { ActivatedRoute, Router, RouterModule } from "@angular/router";
|
||||||
import { firstValueFrom } from "rxjs";
|
import { firstValueFrom } from "rxjs";
|
||||||
|
|
||||||
import { JslibModule } from "@bitwarden/angular/jslib.module";
|
import { JslibModule } from "@bitwarden/angular/jslib.module";
|
||||||
@@ -19,6 +19,7 @@ import { ClientType } from "@bitwarden/common/enums";
|
|||||||
import { ErrorResponse } from "@bitwarden/common/models/response/error.response";
|
import { ErrorResponse } from "@bitwarden/common/models/response/error.response";
|
||||||
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
|
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
|
||||||
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
|
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
|
||||||
|
import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
|
||||||
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
|
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
|
||||||
import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service";
|
import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service";
|
||||||
import {
|
import {
|
||||||
@@ -49,6 +50,7 @@ export type State = "assert" | "assertFailed";
|
|||||||
})
|
})
|
||||||
export class LoginViaWebAuthnComponent implements OnInit {
|
export class LoginViaWebAuthnComponent implements OnInit {
|
||||||
protected currentState: State = "assert";
|
protected currentState: State = "assert";
|
||||||
|
private shouldAutoClosePopout = false;
|
||||||
|
|
||||||
protected readonly Icons = {
|
protected readonly Icons = {
|
||||||
TwoFactorAuthSecurityKeyIcon,
|
TwoFactorAuthSecurityKeyIcon,
|
||||||
@@ -70,6 +72,7 @@ export class LoginViaWebAuthnComponent implements OnInit {
|
|||||||
constructor(
|
constructor(
|
||||||
private webAuthnLoginService: WebAuthnLoginServiceAbstraction,
|
private webAuthnLoginService: WebAuthnLoginServiceAbstraction,
|
||||||
private router: Router,
|
private router: Router,
|
||||||
|
private route: ActivatedRoute,
|
||||||
private logService: LogService,
|
private logService: LogService,
|
||||||
private validationService: ValidationService,
|
private validationService: ValidationService,
|
||||||
private i18nService: I18nService,
|
private i18nService: I18nService,
|
||||||
@@ -77,9 +80,14 @@ export class LoginViaWebAuthnComponent implements OnInit {
|
|||||||
private keyService: KeyService,
|
private keyService: KeyService,
|
||||||
private platformUtilsService: PlatformUtilsService,
|
private platformUtilsService: PlatformUtilsService,
|
||||||
private anonLayoutWrapperDataService: AnonLayoutWrapperDataService,
|
private anonLayoutWrapperDataService: AnonLayoutWrapperDataService,
|
||||||
|
private messagingService: MessagingService,
|
||||||
) {}
|
) {}
|
||||||
|
|
||||||
ngOnInit(): void {
|
ngOnInit(): void {
|
||||||
|
// Check if we should auto-close the popout after successful authentication
|
||||||
|
this.shouldAutoClosePopout =
|
||||||
|
this.route.snapshot.queryParamMap.get("autoClosePopout") === "true";
|
||||||
|
|
||||||
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
|
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
|
||||||
// eslint-disable-next-line @typescript-eslint/no-floating-promises
|
// eslint-disable-next-line @typescript-eslint/no-floating-promises
|
||||||
this.authenticate();
|
this.authenticate();
|
||||||
@@ -123,6 +131,17 @@ export class LoginViaWebAuthnComponent implements OnInit {
|
|||||||
await this.loginSuccessHandlerService.run(authResult.userId, null);
|
await this.loginSuccessHandlerService.run(authResult.userId, null);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// If autoClosePopout is enabled and we're in a browser extension,
|
||||||
|
// re-open the regular popup and close this popout window
|
||||||
|
if (
|
||||||
|
this.shouldAutoClosePopout &&
|
||||||
|
this.platformUtilsService.getClientType() === ClientType.Browser
|
||||||
|
) {
|
||||||
|
this.messagingService.send("openPopup");
|
||||||
|
window.close();
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
await this.router.navigate([this.successRoute]);
|
await this.router.navigate([this.successRoute]);
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
if (error instanceof ErrorResponse) {
|
if (error instanceof ErrorResponse) {
|
||||||
|
|||||||
Reference in New Issue
Block a user