refactor(gateway): extract connect and role policy logic

2026-03-30 08:32:00 +00:00 · 2026-02-21 19:47:17 +01:00
parent f97c45c5b5
commit 51149fcaf1
7 changed files with 342 additions and 157 deletions
--- a/src/gateway/role-policy.test.ts
+++ b/src/gateway/role-policy.test.ts
@@ -0,0 +1,28 @@
+import { describe, expect, test } from "vitest";
+import {
+  isRoleAuthorizedForMethod,
+  parseGatewayRole,
+  roleCanSkipDeviceIdentity,
+} from "./role-policy.js";
+
+describe("gateway role policy", () => {
+  test("parses supported roles", () => {
+    expect(parseGatewayRole("operator")).toBe("operator");
+    expect(parseGatewayRole("node")).toBe("node");
+    expect(parseGatewayRole("admin")).toBeNull();
+    expect(parseGatewayRole(undefined)).toBeNull();
+  });
+
+  test("allows device-less bypass only for operator + shared auth", () => {
+    expect(roleCanSkipDeviceIdentity("operator", true)).toBe(true);
+    expect(roleCanSkipDeviceIdentity("operator", false)).toBe(false);
+    expect(roleCanSkipDeviceIdentity("node", true)).toBe(false);
+  });
+
+  test("authorizes roles against node vs operator methods", () => {
+    expect(isRoleAuthorizedForMethod("node", "node.event")).toBe(true);
+    expect(isRoleAuthorizedForMethod("node", "status")).toBe(false);
+    expect(isRoleAuthorizedForMethod("operator", "status")).toBe(true);
+    expect(isRoleAuthorizedForMethod("operator", "node.event")).toBe(false);
+  });
+});
--- a/src/gateway/role-policy.ts
+++ b/src/gateway/role-policy.ts
@@ -0,0 +1,23 @@
+import { isNodeRoleMethod } from "./method-scopes.js";
+
+export const GATEWAY_ROLES = ["operator", "node"] as const;
+
+export type GatewayRole = (typeof GATEWAY_ROLES)[number];
+
+export function parseGatewayRole(roleRaw: unknown): GatewayRole | null {
+  if (roleRaw === "operator" || roleRaw === "node") {
+    return roleRaw;
+  }
+  return null;
+}
+
+export function roleCanSkipDeviceIdentity(role: GatewayRole, sharedAuthOk: boolean): boolean {
+  return role === "operator" && sharedAuthOk;
+}
+
+export function isRoleAuthorizedForMethod(role: GatewayRole, method: string): boolean {
+  if (isNodeRoleMethod(method)) {
+    return role === "node";
+  }
+  return role === "operator";
+}
--- a/src/gateway/server-methods.ts
+++ b/src/gateway/server-methods.ts
@@ -1,11 +1,8 @@
 import { formatControlPlaneActor, resolveControlPlaneActor } from "./control-plane-audit.js";
 import { consumeControlPlaneWriteBudget } from "./control-plane-rate-limit.js";
-import {
-  ADMIN_SCOPE,
-  authorizeOperatorScopesForMethod,
-  isNodeRoleMethod,
-} from "./method-scopes.js";
+import { ADMIN_SCOPE, authorizeOperatorScopesForMethod } from "./method-scopes.js";
 import { ErrorCodes, errorShape } from "./protocol/index.js";
+import { isRoleAuthorizedForMethod, parseGatewayRole } from "./role-policy.js";
 import { agentHandlers } from "./server-methods/agent.js";
 import { agentsHandlers } from "./server-methods/agents.js";
 import { browserHandlers } from "./server-methods/browser.js";
@@ -42,19 +39,17 @@ function authorizeGatewayMethod(method: string, client: GatewayRequestOptions["c
  if (method === "health") {
    return null;
  }
-  const role = client.connect.role ?? "operator";
+  const roleRaw = client.connect.role ?? "operator";
+  const role = parseGatewayRole(roleRaw);
+  if (!role) {
+    return errorShape(ErrorCodes.INVALID_REQUEST, `unauthorized role: ${roleRaw}`);
+  }
  const scopes = client.connect.scopes ?? [];
-  if (isNodeRoleMethod(method)) {
-    if (role === "node") {
-      return null;
-    }
+  if (!isRoleAuthorizedForMethod(role, method)) {
    return errorShape(ErrorCodes.INVALID_REQUEST, `unauthorized role: ${role}`);
  }
  if (role === "node") {
-    return errorShape(ErrorCodes.INVALID_REQUEST, `unauthorized role: ${role}`);
-  }
-  if (role !== "operator") {
-    return errorShape(ErrorCodes.INVALID_REQUEST, `unauthorized role: ${role}`);
+    return null;
  }
  if (scopes.includes(ADMIN_SCOPE)) {
    return null;
--- a/src/gateway/server.auth.e2e.test.ts
+++ b/src/gateway/server.auth.e2e.test.ts
@@ -85,6 +85,13 @@ const CONTROL_UI_CLIENT = {
  mode: GATEWAY_CLIENT_MODES.WEBCHAT,
 };

+const NODE_CLIENT = {
+  id: GATEWAY_CLIENT_NAMES.NODE_HOST,
+  version: "1.0.0",
+  platform: "test",
+  mode: GATEWAY_CLIENT_MODES.NODE,
+};
+
 async function expectHelloOkServerVersion(port: number, expectedVersion: string) {
  const ws = await openWs(port);
  try {
@@ -359,29 +366,56 @@ describe("gateway server auth/connect", () => {
      await expectMissingScopeAfterConnect(port, { scopes: [] });
    });

-    test("ignores requested scopes when device identity is omitted", async () => {
-      await expectMissingScopeAfterConnect(port, { device: null });
-    });
-
-    test("rejects node role when device identity is omitted", async () => {
-      const ws = await openWs(port);
+    test("device-less auth matrix", async () => {
      const token = resolveGatewayTokenOrEnv();
-      try {
-        const res = await connectReq(ws, {
-          role: "node",
-          token,
-          device: null,
-          client: {
-            id: GATEWAY_CLIENT_NAMES.NODE_HOST,
-            version: "1.0.0",
-            platform: "test",
-            mode: GATEWAY_CLIENT_MODES.NODE,
-          },
-        });
-        expect(res.ok).toBe(false);
-        expect(res.error?.message ?? "").toContain("device identity required");
-      } finally {
-        ws.close();
+      const matrix: Array<{
+        name: string;
+        opts: Parameters<typeof connectReq>[1];
+        expectConnectOk: boolean;
+        expectConnectError?: string;
+        expectStatusError?: string;
+      }> = [
+        {
+          name: "operator + valid shared token => connected with zero scopes",
+          opts: { role: "operator", token, device: null },
+          expectConnectOk: true,
+          expectStatusError: "missing scope",
+        },
+        {
+          name: "node + valid shared token => rejected without device",
+          opts: { role: "node", token, device: null, client: NODE_CLIENT },
+          expectConnectOk: false,
+          expectConnectError: "device identity required",
+        },
+        {
+          name: "operator + invalid shared token => unauthorized",
+          opts: { role: "operator", token: "wrong", device: null },
+          expectConnectOk: false,
+          expectConnectError: "unauthorized",
+        },
+      ];
+
+      for (const scenario of matrix) {
+        const ws = await openWs(port);
+        try {
+          const res = await connectReq(ws, scenario.opts);
+          expect(res.ok, scenario.name).toBe(scenario.expectConnectOk);
+          if (!scenario.expectConnectOk) {
+            expect(res.error?.message ?? "", scenario.name).toContain(
+              String(scenario.expectConnectError ?? ""),
+            );
+            continue;
+          }
+          if (scenario.expectStatusError) {
+            const status = await rpcReq(ws, "status");
+            expect(status.ok, scenario.name).toBe(false);
+            expect(status.error?.message ?? "", scenario.name).toContain(
+              scenario.expectStatusError,
+            );
+          }
+        } finally {
+          ws.close();
+        }
      }
    });

--- a/src/gateway/server/ws-connection/connect-policy.test.ts
+++ b/src/gateway/server/ws-connection/connect-policy.test.ts
@@ -0,0 +1,115 @@
+import { describe, expect, test } from "vitest";
+import {
+  evaluateMissingDeviceIdentity,
+  resolveControlUiAuthPolicy,
+  shouldSkipControlUiPairing,
+} from "./connect-policy.js";
+
+describe("ws connect policy", () => {
+  test("resolves control-ui auth policy", () => {
+    const bypass = resolveControlUiAuthPolicy({
+      isControlUi: true,
+      controlUiConfig: { dangerouslyDisableDeviceAuth: true },
+      deviceRaw: { id: "dev-1", publicKey: "pk", signature: "sig", signedAt: Date.now() },
+    });
+    expect(bypass.allowBypass).toBe(true);
+    expect(bypass.device).toBeNull();
+
+    const regular = resolveControlUiAuthPolicy({
+      isControlUi: false,
+      controlUiConfig: { dangerouslyDisableDeviceAuth: true },
+      deviceRaw: { id: "dev-2", publicKey: "pk", signature: "sig", signedAt: Date.now() },
+    });
+    expect(regular.allowBypass).toBe(false);
+    expect(regular.device?.id).toBe("dev-2");
+  });
+
+  test("evaluates missing-device decisions", () => {
+    const policy = resolveControlUiAuthPolicy({
+      isControlUi: false,
+      controlUiConfig: undefined,
+      deviceRaw: null,
+    });
+
+    expect(
+      evaluateMissingDeviceIdentity({
+        hasDeviceIdentity: true,
+        role: "node",
+        isControlUi: false,
+        controlUiAuthPolicy: policy,
+        sharedAuthOk: true,
+        authOk: true,
+        hasSharedAuth: true,
+      }).kind,
+    ).toBe("allow");
+
+    const controlUiStrict = resolveControlUiAuthPolicy({
+      isControlUi: true,
+      controlUiConfig: { allowInsecureAuth: true, dangerouslyDisableDeviceAuth: false },
+      deviceRaw: null,
+    });
+    expect(
+      evaluateMissingDeviceIdentity({
+        hasDeviceIdentity: false,
+        role: "operator",
+        isControlUi: true,
+        controlUiAuthPolicy: controlUiStrict,
+        sharedAuthOk: true,
+        authOk: true,
+        hasSharedAuth: true,
+      }).kind,
+    ).toBe("reject-control-ui-insecure-auth");
+
+    expect(
+      evaluateMissingDeviceIdentity({
+        hasDeviceIdentity: false,
+        role: "operator",
+        isControlUi: false,
+        controlUiAuthPolicy: policy,
+        sharedAuthOk: true,
+        authOk: true,
+        hasSharedAuth: true,
+      }).kind,
+    ).toBe("allow");
+
+    expect(
+      evaluateMissingDeviceIdentity({
+        hasDeviceIdentity: false,
+        role: "operator",
+        isControlUi: false,
+        controlUiAuthPolicy: policy,
+        sharedAuthOk: false,
+        authOk: false,
+        hasSharedAuth: true,
+      }).kind,
+    ).toBe("reject-unauthorized");
+
+    expect(
+      evaluateMissingDeviceIdentity({
+        hasDeviceIdentity: false,
+        role: "node",
+        isControlUi: false,
+        controlUiAuthPolicy: policy,
+        sharedAuthOk: true,
+        authOk: true,
+        hasSharedAuth: true,
+      }).kind,
+    ).toBe("reject-device-required");
+  });
+
+  test("pairing bypass requires control-ui bypass + shared auth", () => {
+    const bypass = resolveControlUiAuthPolicy({
+      isControlUi: true,
+      controlUiConfig: { dangerouslyDisableDeviceAuth: true },
+      deviceRaw: null,
+    });
+    const strict = resolveControlUiAuthPolicy({
+      isControlUi: true,
+      controlUiConfig: undefined,
+      deviceRaw: null,
+    });
+    expect(shouldSkipControlUiPairing(bypass, true)).toBe(true);
+    expect(shouldSkipControlUiPairing(bypass, false)).toBe(false);
+    expect(shouldSkipControlUiPairing(strict, true)).toBe(false);
+  });
+});
--- a/src/gateway/server/ws-connection/connect-policy.ts
+++ b/src/gateway/server/ws-connection/connect-policy.ts
@@ -0,0 +1,70 @@
+import type { ConnectParams } from "../../protocol/index.js";
+import type { GatewayRole } from "../../role-policy.js";
+import { roleCanSkipDeviceIdentity } from "../../role-policy.js";
+
+export type ControlUiAuthPolicy = {
+  allowInsecureAuthConfigured: boolean;
+  dangerouslyDisableDeviceAuth: boolean;
+  allowBypass: boolean;
+  device: ConnectParams["device"] | null | undefined;
+};
+
+export function resolveControlUiAuthPolicy(params: {
+  isControlUi: boolean;
+  controlUiConfig:
+    | {
+        allowInsecureAuth?: boolean;
+        dangerouslyDisableDeviceAuth?: boolean;
+      }
+    | undefined;
+  deviceRaw: ConnectParams["device"] | null | undefined;
+}): ControlUiAuthPolicy {
+  const allowInsecureAuthConfigured =
+    params.isControlUi && params.controlUiConfig?.allowInsecureAuth === true;
+  const dangerouslyDisableDeviceAuth =
+    params.isControlUi && params.controlUiConfig?.dangerouslyDisableDeviceAuth === true;
+  return {
+    allowInsecureAuthConfigured,
+    dangerouslyDisableDeviceAuth,
+    // `allowInsecureAuth` must not bypass secure-context/device-auth requirements.
+    allowBypass: dangerouslyDisableDeviceAuth,
+    device: dangerouslyDisableDeviceAuth ? null : params.deviceRaw,
+  };
+}
+
+export function shouldSkipControlUiPairing(
+  policy: ControlUiAuthPolicy,
+  sharedAuthOk: boolean,
+): boolean {
+  return policy.allowBypass && sharedAuthOk;
+}
+
+export type MissingDeviceIdentityDecision =
+  | { kind: "allow" }
+  | { kind: "reject-control-ui-insecure-auth" }
+  | { kind: "reject-unauthorized" }
+  | { kind: "reject-device-required" };
+
+export function evaluateMissingDeviceIdentity(params: {
+  hasDeviceIdentity: boolean;
+  role: GatewayRole;
+  isControlUi: boolean;
+  controlUiAuthPolicy: ControlUiAuthPolicy;
+  sharedAuthOk: boolean;
+  authOk: boolean;
+  hasSharedAuth: boolean;
+}): MissingDeviceIdentityDecision {
+  if (params.hasDeviceIdentity) {
+    return { kind: "allow" };
+  }
+  if (params.isControlUi && !params.controlUiAuthPolicy.allowBypass) {
+    return { kind: "reject-control-ui-insecure-auth" };
+  }
+  if (roleCanSkipDeviceIdentity(params.role, params.sharedAuthOk)) {
+    return { kind: "allow" };
+  }
+  if (!params.authOk && params.hasSharedAuth) {
+    return { kind: "reject-unauthorized" };
+  }
+  return { kind: "reject-device-required" };
+}
--- a/src/gateway/server/ws-connection/message-handler.ts
+++ b/src/gateway/server/ws-connection/message-handler.ts
@@ -56,6 +56,7 @@ import {
  validateConnectParams,
  validateRequestFrame,
 } from "../../protocol/index.js";
+import { parseGatewayRole } from "../../role-policy.js";
 import { MAX_BUFFERED_BYTES, MAX_PAYLOAD_BYTES, TICK_INTERVAL_MS } from "../../server-constants.js";
 import { handleGatewayRequest } from "../../server-methods.js";
 import type { GatewayRequestContext, GatewayRequestHandlers } from "../../server-methods/types.js";
@@ -71,45 +72,16 @@ import {
 } from "../health-state.js";
 import type { GatewayWsClient } from "../ws-types.js";
 import { formatGatewayAuthFailureMessage, type AuthProvidedKind } from "./auth-messages.js";
+import {
+  evaluateMissingDeviceIdentity,
+  resolveControlUiAuthPolicy,
+  shouldSkipControlUiPairing,
+} from "./connect-policy.js";

 type SubsystemLogger = ReturnType<typeof createSubsystemLogger>;

 const DEVICE_SIGNATURE_SKEW_MS = 10 * 60 * 1000;

-type ControlUiAuthPolicy = {
-  allowInsecureAuthConfigured: boolean;
-  dangerouslyDisableDeviceAuth: boolean;
-  allowBypass: boolean;
-  device: ConnectParams["device"] | null | undefined;
-};
-
-function resolveControlUiAuthPolicy(params: {
-  isControlUi: boolean;
-  controlUiConfig:
-    | {
-        allowInsecureAuth?: boolean;
-        dangerouslyDisableDeviceAuth?: boolean;
-      }
-    | undefined;
-  deviceRaw: ConnectParams["device"] | null | undefined;
-}): ControlUiAuthPolicy {
-  const allowInsecureAuthConfigured =
-    params.isControlUi && params.controlUiConfig?.allowInsecureAuth === true;
-  const dangerouslyDisableDeviceAuth =
-    params.isControlUi && params.controlUiConfig?.dangerouslyDisableDeviceAuth === true;
-  return {
-    allowInsecureAuthConfigured,
-    dangerouslyDisableDeviceAuth,
-    // `allowInsecureAuth` must not bypass secure-context/device-auth requirements.
-    allowBypass: dangerouslyDisableDeviceAuth,
-    device: dangerouslyDisableDeviceAuth ? null : params.deviceRaw,
-  };
-}
-
-function shouldSkipControlUiPairing(policy: ControlUiAuthPolicy, sharedAuthOk: boolean): boolean {
-  return policy.allowBypass && sharedAuthOk;
-}
-
 export function attachGatewayWsMessageHandler(params: {
  socket: WebSocket;
  upgradeReq: IncomingMessage;
@@ -339,7 +311,7 @@ export function attachGatewayWsMessageHandler(params: {
        }

        const roleRaw = connectParams.role ?? "operator";
-        const role = roleRaw === "operator" || roleRaw === "node" ? roleRaw : null;
+        const role = parseGatewayRole(roleRaw);
        if (!role) {
          markHandshakeFailure("invalid-role", {
            role: roleRaw,
@@ -486,13 +458,23 @@ export function attachGatewayWsMessageHandler(params: {
          }
        };
        const handleMissingDeviceIdentity = (): boolean => {
-          if (device) {
+          if (!device) {
+            clearUnboundScopes();
+          }
+          const decision = evaluateMissingDeviceIdentity({
+            hasDeviceIdentity: Boolean(device),
+            role,
+            isControlUi,
+            controlUiAuthPolicy,
+            sharedAuthOk,
+            authOk,
+            hasSharedAuth,
+          });
+          if (decision.kind === "allow") {
            return true;
          }
-          clearUnboundScopes();
-          const canSkipDevice = role === "operator" && sharedAuthOk;

-          if (isControlUi && !controlUiAuthPolicy.allowBypass) {
+          if (decision.kind === "reject-control-ui-insecure-auth") {
            const errorMessage =
              "control ui requires device identity (use HTTPS or localhost secure context)";
            markHandshakeFailure("control-ui-insecure-auth", {
@@ -503,29 +485,24 @@ export function attachGatewayWsMessageHandler(params: {
            return false;
          }

-          // Allow shared-secret authenticated connections (e.g., control-ui) to skip device identity.
-          if (!canSkipDevice) {
-            if (!authOk && hasSharedAuth) {
-              rejectUnauthorized(authResult);
-              return false;
-            }
-            markHandshakeFailure("device-required");
-            sendHandshakeErrorResponse(ErrorCodes.NOT_PAIRED, "device identity required");
-            close(1008, "device identity required");
+          if (decision.kind === "reject-unauthorized") {
+            rejectUnauthorized(authResult);
            return false;
          }

-          return true;
+          markHandshakeFailure("device-required");
+          sendHandshakeErrorResponse(ErrorCodes.NOT_PAIRED, "device identity required");
+          close(1008, "device identity required");
+          return false;
        };
        if (!handleMissingDeviceIdentity()) {
          return;
        }
        if (device) {
-          const derivedId = deriveDeviceIdFromPublicKey(device.publicKey);
-          if (!derivedId || derivedId !== device.id) {
+          const rejectDeviceAuthInvalid = (reason: string, message: string) => {
            setHandshakeState("failed");
            setCloseCause("device-auth-invalid", {
-              reason: "device-id-mismatch",
+              reason,
              client: connectParams.client.id,
              deviceId: device.id,
            });
@@ -533,9 +510,13 @@ export function attachGatewayWsMessageHandler(params: {
              type: "res",
              id: frame.id,
              ok: false,
-              error: errorShape(ErrorCodes.INVALID_REQUEST, "device identity mismatch"),
+              error: errorShape(ErrorCodes.INVALID_REQUEST, message),
            });
-            close(1008, "device identity mismatch");
+            close(1008, message);
+          };
+          const derivedId = deriveDeviceIdFromPublicKey(device.publicKey);
+          if (!derivedId || derivedId !== device.id) {
+            rejectDeviceAuthInvalid("device-id-mismatch", "device identity mismatch");
            return;
          }
          const signedAt = device.signedAt;
@@ -543,53 +524,17 @@ export function attachGatewayWsMessageHandler(params: {
            typeof signedAt !== "number" ||
            Math.abs(Date.now() - signedAt) > DEVICE_SIGNATURE_SKEW_MS
          ) {
-            setHandshakeState("failed");
-            setCloseCause("device-auth-invalid", {
-              reason: "device-signature-stale",
-              client: connectParams.client.id,
-              deviceId: device.id,
-            });
-            send({
-              type: "res",
-              id: frame.id,
-              ok: false,
-              error: errorShape(ErrorCodes.INVALID_REQUEST, "device signature expired"),
-            });
-            close(1008, "device signature expired");
+            rejectDeviceAuthInvalid("device-signature-stale", "device signature expired");
            return;
          }
          const nonceRequired = !isLocalClient;
          const providedNonce = typeof device.nonce === "string" ? device.nonce.trim() : "";
          if (nonceRequired && !providedNonce) {
-            setHandshakeState("failed");
-            setCloseCause("device-auth-invalid", {
-              reason: "device-nonce-missing",
-              client: connectParams.client.id,
-              deviceId: device.id,
-            });
-            send({
-              type: "res",
-              id: frame.id,
-              ok: false,
-              error: errorShape(ErrorCodes.INVALID_REQUEST, "device nonce required"),
-            });
-            close(1008, "device nonce required");
+            rejectDeviceAuthInvalid("device-nonce-missing", "device nonce required");
            return;
          }
          if (providedNonce && providedNonce !== connectNonce) {
-            setHandshakeState("failed");
-            setCloseCause("device-auth-invalid", {
-              reason: "device-nonce-mismatch",
-              client: connectParams.client.id,
-              deviceId: device.id,
-            });
-            send({
-              type: "res",
-              id: frame.id,
-              ok: false,
-              error: errorShape(ErrorCodes.INVALID_REQUEST, "device nonce mismatch"),
-            });
-            close(1008, "device nonce mismatch");
+            rejectDeviceAuthInvalid("device-nonce-mismatch", "device nonce mismatch");
            return;
          }
          const payload = buildDeviceAuthPayload({
@@ -603,21 +548,8 @@ export function attachGatewayWsMessageHandler(params: {
            nonce: providedNonce || undefined,
            version: providedNonce ? "v2" : "v1",
          });
-          const rejectDeviceSignatureInvalid = () => {
-            setHandshakeState("failed");
-            setCloseCause("device-auth-invalid", {
-              reason: "device-signature",
-              client: connectParams.client.id,
-              deviceId: device.id,
-            });
-            send({
-              type: "res",
-              id: frame.id,
-              ok: false,
-              error: errorShape(ErrorCodes.INVALID_REQUEST, "device signature invalid"),
-            });
-            close(1008, "device signature invalid");
-          };
+          const rejectDeviceSignatureInvalid = () =>
+            rejectDeviceAuthInvalid("device-signature", "device signature invalid");
          const signatureOk = verifyDeviceSignature(device.publicKey, payload, device.signature);
          const allowLegacy = !nonceRequired && !providedNonce;
          if (!signatureOk && allowLegacy) {
@@ -643,19 +575,7 @@ export function attachGatewayWsMessageHandler(params: {
          }
          devicePublicKey = normalizeDevicePublicKeyBase64Url(device.publicKey);
          if (!devicePublicKey) {
-            setHandshakeState("failed");
-            setCloseCause("device-auth-invalid", {
-              reason: "device-public-key",
-              client: connectParams.client.id,
-              deviceId: device.id,
-            });
-            send({
-              type: "res",
-              id: frame.id,
-              ok: false,
-              error: errorShape(ErrorCodes.INVALID_REQUEST, "device public key invalid"),
-            });
-            close(1008, "device public key invalid");
+            rejectDeviceAuthInvalid("device-public-key", "device public key invalid");
            return;
          }
        }