diff --git a/src/gateway/role-policy.test.ts b/src/gateway/role-policy.test.ts new file mode 100644 index 00000000000..ba371b56bfe --- /dev/null +++ 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); + }); +}); diff --git a/src/gateway/role-policy.ts b/src/gateway/role-policy.ts new file mode 100644 index 00000000000..8366cd1c6c2 --- /dev/null +++ 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"; +} diff --git a/src/gateway/server-methods.ts b/src/gateway/server-methods.ts index d1bc16630af..60a6662102b 100644 --- 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; diff --git a/src/gateway/server.auth.e2e.test.ts b/src/gateway/server.auth.e2e.test.ts index f07900e2abd..be69a77ee85 100644 --- 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[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(); + } } }); diff --git a/src/gateway/server/ws-connection/connect-policy.test.ts b/src/gateway/server/ws-connection/connect-policy.test.ts new file mode 100644 index 00000000000..69fa92e7c4a --- /dev/null +++ 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); + }); +}); diff --git a/src/gateway/server/ws-connection/connect-policy.ts b/src/gateway/server/ws-connection/connect-policy.ts new file mode 100644 index 00000000000..96ec140365c --- /dev/null +++ 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" }; +} diff --git a/src/gateway/server/ws-connection/message-handler.ts b/src/gateway/server/ws-connection/message-handler.ts index aae94280d57..a4675a3c140 100644 --- 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; 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; } }