handle remote-auth methods in multi-step MFA flows (#1154)

Author: wojcik91

.github/workflows/test-web.yml

@@ -0,0 +1,52 @@
+name: Test web
+
+on:
+  push:
+    branches:
+      - main
+      - dev
+      - "release/**"
+      - "stable/**"
+    paths-ignore:
+      - "*.md"
+      - "LICENSE"
+  pull_request:
+    branches:
+      - main
+      - dev
+      - "release/**"
+      - "stable/**"
+    paths-ignore:
+      - "*.md"
+      - "LICENSE"
+
+permissions:
+  contents: read
+
+jobs:
+  test-web:
+    runs-on:
+      - codebuild-defguard-client-runner-${{ github.run_id }}-${{ github.run_attempt }}
+
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v7
+        with:
+          submodules: recursive
+
+      - uses: actions/setup-node@v6
+        with:
+          node-version-file: new-ui/.nvmrc
+
+      - uses: pnpm/action-setup@v6
+        with:
+          version: 11.11
+          run_install: false
+
+      - name: Install Node dependencies for new UI
+        working-directory: ./new-ui
+        run: pnpm install --frozen-lockfile
+
+      - name: Run frontend tests
+        working-directory: ./new-ui
+        run: pnpm test

e2e/README.md

@@ -0,0 +1,63 @@
+# Client E2E tests
+
+The client E2E suite runs the Linux Tauri client through WebDriver. It expects the
+Defguard services to be running separately.
+
+## Requirements
+
+- Linux with WireGuard support.
+- A dedicated, disposable Core instance. The tests create and remove users and devices and change network MFA settings.
+- Core, Edge, and Gateway running and reachable from the test machine.
+- The Gateway connected to the test network and reachable through WireGuard.
+- An administrator account whose login does not require MFA. The password is supplied through `e2e/.env`.
+- A dedicated deployment with exactly one test network named `e2e`. The connection checks use the first visible connect button.
+- Root `defguard-service` running with `/var/run/defguard.socket` available.
+- Rust/Cargo with the Tauri CLI, Node.js, `pnpm`, `just`, `WebKitWebDriver`, and `xvfb-run` available.
+- `tauri-driver` available at `~/.cargo/bin/tauri-driver`, or at the path set by `TAURI_DRIVER`.
+
+## Configure the test environment
+
+Copy the example file and fill in the local deployment values:
+
+```bash
+cp e2e/.env.example e2e/.env
+```
+
+The variables are:
+
+| Variable | Purpose |
+| --- | --- |
+| `CORE_URL` | Core HTTP base URL |
+| `PROXY_URL` | Edge enrollment URL |
+| `CORE_ADMIN_USER` | Core administrator username, default `admin` |
+| `CORE_ADMIN_PASSWORD` | Core administrator password |
+| `TEST_USERNAME` | Optional fixed test username |
+| `GATEWAY_VPN_IP` | Gateway address used for the connectivity check |
+| `NETWORK_ENDPOINT` | Endpoint advertised by the test network |
+| `NETWORK_NAME` | Test network name, `e2e` |
+| `NETWORK_ADDRESS` | Test network address |
+| `NETWORK_PORT` | WireGuard port, default `50051` |
+| `NETWORK_ALLOWED_IPS` | Allowed IP ranges for the test network |
+
+Keep `e2e/.env` local and do not commit it.
+
+## Run the suite
+
+From the client repository root:
+
+```bash
+just e2e-build
+just e2e-provision
+just e2e-test
+```
+
+`just e2e` runs all three steps. Core must already be running before provisioning.
+
+To run one spec directly:
+
+```bash
+cd e2e
+NATIVE_DRIVER="$(command -v WebKitWebDriver)" \
+  xvfb-run -a pnpm exec wdio run ./wdio.conf.ts \
+  --spec=./tests/enrollment.spec.ts
+```

e2e/helpers/coreApi.ts

@@ -67,6 +67,23 @@ export class CoreApi {
 		return response;
 	}
 
+	// Core serves the web UI for any unrouted GET, so an endpoint the deployed core does not
+	// have arrives as 200 text/html instead of a 404.
+	private async requestJson<T>(
+		method: string,
+		apiPath: string,
+		body?: unknown,
+	): Promise<T> {
+		const response = await this.request(method, apiPath, body);
+		const contentType = response.headers.get("content-type") ?? "";
+		if (!contentType.includes("application/json")) {
+			throw new Error(
+				`Core API ${method} ${apiPath} returned ${contentType || "no content type"} instead of JSON - the endpoint is missing from this core version`,
+			);
+		}
+		return (await response.json()) as T;
+	}
+
 	async login(): Promise<void> {
 		const response = await this.request("POST", "/api/v1/auth", {
 			username: process.env.CORE_ADMIN_USER ?? "admin",
@@ -100,82 +117,125 @@ export class CoreApi {
 		await this.request("DELETE", `/api/v1/user/${username}`);
 	}
 
-	async listNetworks(): Promise<
-		Array<{ id: number; location_mfa_mode: LocationMfaMode }>
-	> {
-		const response = await this.request("GET", "/api/v1/network");
-		return (await response.json()) as Array<{
-			id: number;
-			location_mfa_mode: LocationMfaMode;
-		}>;
+	async listNetworks(): Promise<Array<{ id: number; name: string }>> {
+		return this.requestJson<Array<{ id: number; name: string }>>(
+			"GET",
+			"/api/v1/network",
+		);
 	}
 
 	async addUserDevice(name: string, pubkey: string): Promise<AddedUserDevice> {
 		const username = process.env.CORE_ADMIN_USER ?? "admin";
-		const response = await this.request("POST", `/api/v1/device/${username}`, {
+		const data = await this.requestJson<{
+			configs: DeviceConfig[];
+			device: { id: number };
+		}>("POST", `/api/v1/device/${username}`, {
 			name,
 			wireguard_pubkey: pubkey,
 		});
-		const data = (await response.json()) as {
-			configs: DeviceConfig[];
-			device: { id: number };
-		};
 		return { deviceId: data.device.id, configs: data.configs };
 	}
 
 	async deleteDevice(deviceId: number): Promise<void> {
 		await this.request("DELETE", `/api/v1/device/${deviceId}`);
 	}
 
+	private async getNetworkDetails(
+		networkId: number,
+	): Promise<Record<string, unknown>> {
+		return this.requestJson<Record<string, unknown>>(
+			"GET",
+			`/api/v1/network/${networkId}`,
+		);
+	}
+
 	async setLocationMfaMode(
 		networkId: number,
 		mode: LocationMfaMode,
 	): Promise<LocationMfaMode> {
-		const current = (await (
-			await this.request("GET", `/api/v1/network/${networkId}`)
-		).json()) as Record<string, unknown>;
+		const current = await this.getNetworkDetails(networkId);
 		const previous = current.location_mfa_mode as LocationMfaMode;
 		if (previous === mode) {
 			return previous;
 		}
 		const joinList = (value: unknown): string =>
-			Array.isArray(value) ? value.join(",") : ((value as string | null) ?? "");
+			Array.isArray(value)
+				? value.join(",")
+				: typeof value === "string"
+					? value
+					: "";
+		const peerDisconnectThreshold = Number(
+			current.peer_disconnect_threshold ?? 0,
+		);
 		await this.request("PUT", `/api/v1/network/${networkId}`, {
 			name: current.name,
 			address: joinList(current.address),
 			endpoint: current.endpoint,
 			port: current.port,
 			allowed_ips: joinList(current.allowed_ips) || null,
-			dns: (current.dns as string | null) ?? null,
+			dns: typeof current.dns === "string" ? current.dns : null,
 			mtu: current.mtu,
 			fwmark: current.fwmark,
-			allow_all_groups: current.allow_all_groups,
-			allowed_groups: current.allowed_groups ?? [],
+			allow_all_groups: current.allow_all_groups === true,
+			allowed_groups: Array.isArray(current.allowed_groups)
+				? current.allowed_groups
+				: [],
 			keepalive_interval: current.keepalive_interval,
-			peer_disconnect_threshold: Math.max(
-				Number(current.peer_disconnect_threshold ?? 0),
-				MIN_PEER_DISCONNECT_THRESHOLD_WITH_MFA,
-			),
-			acl_enabled: current.acl_enabled,
-			acl_default_allow: current.acl_default_allow,
+			peer_disconnect_threshold:
+				mode === "disabled"
+					? peerDisconnectThreshold
+					: Math.max(
+							peerDisconnectThreshold,
+							MIN_PEER_DISCONNECT_THRESHOLD_WITH_MFA,
+						),
+			acl_enabled: current.acl_enabled === true,
+			acl_default_allow: current.acl_default_allow === true,
 			location_mfa_mode: mode,
-			service_location_mode: current.service_location_mode ?? "disabled",
+			service_location_mode:
+				typeof current.service_location_mode === "string"
+					? current.service_location_mode
+					: "disabled",
 		});
 		return previous;
 	}
 
+	// Core reports `mfa_required` during enrollment when any location on the instance enforces
+	// internal MFA, so a test that expects no MFA has to clear every location, not just its own.
+	async disableAllLocationMfa(): Promise<Map<number, LocationMfaMode>> {
+		const previous = new Map<number, LocationMfaMode>();
+		try {
+			for (const network of await this.listNetworks()) {
+				previous.set(
+					network.id,
+					await this.setLocationMfaMode(network.id, "disabled"),
+				);
+			}
+		} catch (error) {
+			await this.restoreLocationMfaModes(previous).catch(() => undefined);
+			throw error;
+		}
+		return previous;
+	}
+
+	async restoreLocationMfaModes(
+		modes: Map<number, LocationMfaMode>,
+	): Promise<void> {
+		for (const [networkId, mode] of modes) {
+			await this.setLocationMfaMode(networkId, mode);
+		}
+	}
+
 	private async startEnrollment(
 		username: string,
 		ephemeral: boolean,
 	): Promise<EnrollmentFixture> {
-		const response = await this.request(
+		const data = await this.requestJson<{ enrollment_token: string }>(
 			"POST",
 			`/api/v1/user/${username}/start_enrollment`,
 			{
 				send_enrollment_notification: false,
 			},
 		);
-		const data = (await response.json()) as { enrollment_token: string };
 		return {
 			username,
 			enrollmentToken: data.enrollment_token,

e2e/helpers/enrollment.ts

@@ -33,6 +33,16 @@ export const setPassword = async () => {
 	await clickNext();
 };
 
+export const selectTotpIfNeeded = async () => {
+	if (
+		await $("#mfa-choice-step")
+			.isDisplayed()
+			.catch(() => false)
+	) {
+		await clickNext();
+	}
+};
+
 export const configureTotp = async (): Promise<string> => {
 	await expect($("#mfa-configuration-step")).toBeDisplayed();
 	const secretField = $("#mfa-configuration-step .copy-field .track p");

e2e/scripts/provision.mjs

@@ -36,6 +36,7 @@ const NETWORK = {
 	peer_disconnect_threshold: 300,
 	acl_enabled: false,
 	acl_default_allow: false,
+	allowed_ips_from_acl: false,
 	location_mfa_mode: "disabled",
 	service_location_mode: "disabled",
 };

e2e/tests/enrollment.spec.ts

@@ -16,31 +16,43 @@ import {
 	addInstance,
 	configureTotp,
 	finishEnrollment,
+	selectTotpIfNeeded,
 	setPassword,
 } from "../helpers/enrollment.js";
 import { switchToTrayView } from "../helpers/windows.js";
 
 describe("enrollment", () => {
 	let core: CoreApi;
 	let networkId: number;
-	let previousMfaMode: LocationMfaMode;
-	let fixture: EnrollmentFixture;
+	let previousMfaModes: Map<number, LocationMfaMode> | undefined;
+	let fixture: EnrollmentFixture | undefined;
 
 	beforeEach(async () => {
+		previousMfaModes = undefined;
+		fixture = undefined;
 		core = await loggedInCoreApi();
-		networkId = (await core.listNetworks())[0].id;
+		const networkName = process.env.NETWORK_NAME ?? "e2e";
+		const network = (await core.listNetworks()).find(
+			(item) => item.name === networkName,
+		);
+		if (!network) {
+			throw new Error(`Core network "${networkName}" was not found`);
+		}
+		networkId = network.id;
 	});
 
 	afterEach(async () => {
-		await core.setLocationMfaMode(networkId, previousMfaMode);
+		if (previousMfaModes) {
+			await core.restoreLocationMfaModes(previousMfaModes);
+		}
 		await resetInstances();
 		if (fixture?.ephemeral) {
 			await core.deleteUser(fixture.username);
 		}
 	});
 
 	it("enrolls a user without MFA and connects from the full and tray views", async () => {
-		previousMfaMode = await core.setLocationMfaMode(networkId, "disabled");
+		previousMfaModes = await core.disableAllLocationMfa();
 		fixture = await core.createEnrollmentFixture();
 
 		await addInstance(fixture);
@@ -56,11 +68,13 @@ describe("enrollment", () => {
 	});
 
 	it("enrolls a user with TOTP MFA and connects from the full and tray views", async () => {
-		previousMfaMode = await core.setLocationMfaMode(networkId, "internal");
+		previousMfaModes = await core.disableAllLocationMfa();
+		await core.setLocationMfaMode(networkId, "internal");
 		fixture = await core.createEnrollmentFixture();
 
 		await addInstance(fixture);
 		await setPassword();
+		await selectTotpIfNeeded();
 		const secret = await configureTotp();
 		await finishEnrollment();