From d3d8e316bd819d3c7e34253aeb7eccb2510f5f48 Mon Sep 17 00:00:00 2001 From: Jacob Tomlinson Date: Thu, 26 Mar 2026 10:36:44 -0700 Subject: [PATCH] gateway: require pairing for backend scope upgrades (#55286) --- ...silent-scope-upgrade-reconnect.poc.test.ts | 62 +++++++++++++++++++ .../handshake-auth-helpers.test.ts | 40 ------------ .../ws-connection/handshake-auth-helpers.ts | 26 -------- .../server/ws-connection/message-handler.ts | 21 ++----- 4 files changed, 68 insertions(+), 81 deletions(-) diff --git a/src/gateway/server.silent-scope-upgrade-reconnect.poc.test.ts b/src/gateway/server.silent-scope-upgrade-reconnect.poc.test.ts index c7bb095c98d..7bbe27c35ac 100644 --- a/src/gateway/server.silent-scope-upgrade-reconnect.poc.test.ts +++ b/src/gateway/server.silent-scope-upgrade-reconnect.poc.test.ts @@ -84,6 +84,68 @@ describe("gateway silent scope-upgrade reconnect", () => { } }); + test("does not let backend reconnect bypass the paired scope baseline", async () => { + const started = await startServerWithClient("secret"); + const paired = await issueOperatorToken({ + name: "backend-scope-upgrade-reconnect-poc", + approvedScopes: ["operator.read"], + clientId: GATEWAY_CLIENT_NAMES.GATEWAY_CLIENT, + clientMode: GATEWAY_CLIENT_MODES.BACKEND, + }); + + let watcherWs: WebSocket | undefined; + let backendReconnectWs: WebSocket | undefined; + + try { + watcherWs = await openTrackedWs(started.port); + await connectOk(watcherWs, { scopes: ["operator.admin"] }); + const requestedEvent = onceMessage( + watcherWs, + (obj) => obj.type === "event" && obj.event === "device.pair.requested", + ); + + backendReconnectWs = await openTrackedWs(started.port); + const reconnectAttempt = await connectReq(backendReconnectWs, { + token: "secret", + deviceIdentityPath: paired.identityPath, + client: { + id: GATEWAY_CLIENT_NAMES.GATEWAY_CLIENT, + version: "1.0.0", + platform: "node", + mode: GATEWAY_CLIENT_MODES.BACKEND, + }, + role: "operator", + scopes: ["operator.admin"], + }); + expect(reconnectAttempt.ok).toBe(false); + expect(reconnectAttempt.error?.message).toBe("pairing required"); + + const pending = await devicePairingModule.listDevicePairing(); + expect(pending.pending).toHaveLength(1); + expect( + (reconnectAttempt.error?.details as { requestId?: unknown; code?: string })?.requestId, + ).toBe(pending.pending[0]?.requestId); + + const requested = (await requestedEvent) as { + payload?: { requestId?: string; deviceId?: string; scopes?: string[] }; + }; + expect(requested.payload?.requestId).toBe(pending.pending[0]?.requestId); + expect(requested.payload?.deviceId).toBe(paired.deviceId); + expect(requested.payload?.scopes).toEqual(["operator.admin"]); + + const afterAttempt = await getPairedDevice(paired.deviceId); + expect(afterAttempt?.approvedScopes).toEqual(["operator.read"]); + expect(afterAttempt?.tokens?.operator?.scopes).toEqual(["operator.read"]); + expect(afterAttempt?.tokens?.operator?.token).toBe(paired.token); + } finally { + watcherWs?.close(); + backendReconnectWs?.close(); + started.ws.close(); + await started.server.close(); + started.envSnapshot.restore(); + } + }); + test("accepts local silent reconnect when pairing was concurrently approved", async () => { const started = await startServerWithClient("secret"); const loaded = loadDeviceIdentity("silent-reconnect-race"); diff --git a/src/gateway/server/ws-connection/handshake-auth-helpers.test.ts b/src/gateway/server/ws-connection/handshake-auth-helpers.test.ts index cc064e35631..d8b18a89a95 100644 --- a/src/gateway/server/ws-connection/handshake-auth-helpers.test.ts +++ b/src/gateway/server/ws-connection/handshake-auth-helpers.test.ts @@ -1,13 +1,10 @@ import { describe, expect, it } from "vitest"; import type { AuthRateLimiter } from "../../auth-rate-limit.js"; -import { GATEWAY_CLIENT_IDS, GATEWAY_CLIENT_MODES } from "../../protocol/client-info.js"; -import type { ConnectParams } from "../../protocol/index.js"; import { BROWSER_ORIGIN_LOOPBACK_RATE_LIMIT_IP, resolveHandshakeBrowserSecurityContext, resolveUnauthorizedHandshakeContext, shouldAllowSilentLocalPairing, - shouldSkipBackendSelfPairing, } from "./handshake-auth-helpers.js"; function createRateLimiter(): AuthRateLimiter { @@ -88,41 +85,4 @@ describe("handshake auth helpers", () => { }), ).toBe(false); }); - - it("skips backend self-pairing for local trusted backend clients", () => { - const connectParams = { - client: { - id: GATEWAY_CLIENT_IDS.GATEWAY_CLIENT, - mode: GATEWAY_CLIENT_MODES.BACKEND, - }, - } as ConnectParams; - - expect( - shouldSkipBackendSelfPairing({ - connectParams, - isLocalClient: true, - hasBrowserOriginHeader: false, - sharedAuthOk: true, - authMethod: "token", - }), - ).toBe(true); - expect( - shouldSkipBackendSelfPairing({ - connectParams, - isLocalClient: true, - hasBrowserOriginHeader: false, - sharedAuthOk: false, - authMethod: "device-token", - }), - ).toBe(true); - expect( - shouldSkipBackendSelfPairing({ - connectParams, - isLocalClient: false, - hasBrowserOriginHeader: false, - sharedAuthOk: true, - authMethod: "token", - }), - ).toBe(false); - }); }); diff --git a/src/gateway/server/ws-connection/handshake-auth-helpers.ts b/src/gateway/server/ws-connection/handshake-auth-helpers.ts index 20dba4ca2a0..72793fb325d 100644 --- a/src/gateway/server/ws-connection/handshake-auth-helpers.ts +++ b/src/gateway/server/ws-connection/handshake-auth-helpers.ts @@ -3,7 +3,6 @@ import type { AuthRateLimiter } from "../../auth-rate-limit.js"; import type { GatewayAuthResult } from "../../auth.js"; import { buildDeviceAuthPayload, buildDeviceAuthPayloadV3 } from "../../device-auth.js"; import { isLoopbackAddress } from "../../net.js"; -import { GATEWAY_CLIENT_IDS, GATEWAY_CLIENT_MODES } from "../../protocol/client-info.js"; import type { ConnectParams } from "../../protocol/index.js"; import type { AuthProvidedKind } from "./auth-messages.js"; @@ -60,31 +59,6 @@ export function shouldAllowSilentLocalPairing(params: { ); } -export function shouldSkipBackendSelfPairing(params: { - connectParams: ConnectParams; - isLocalClient: boolean; - hasBrowserOriginHeader: boolean; - sharedAuthOk: boolean; - authMethod: GatewayAuthResult["method"]; -}): boolean { - const isGatewayBackendClient = - params.connectParams.client.id === GATEWAY_CLIENT_IDS.GATEWAY_CLIENT && - params.connectParams.client.mode === GATEWAY_CLIENT_MODES.BACKEND; - if (!isGatewayBackendClient) { - return false; - } - const usesSharedSecretAuth = params.authMethod === "token" || params.authMethod === "password"; - const usesDeviceTokenAuth = params.authMethod === "device-token"; - // `authMethod === "device-token"` only reaches this helper after the caller - // has already accepted auth (`authOk === true`), so a separate - // `deviceTokenAuthOk` flag would be redundant here. - return ( - params.isLocalClient && - !params.hasBrowserOriginHeader && - ((params.sharedAuthOk && usesSharedSecretAuth) || usesDeviceTokenAuth) - ); -} - function resolveSignatureToken(connectParams: ConnectParams): string | null { return ( connectParams.auth?.token ?? diff --git a/src/gateway/server/ws-connection/message-handler.ts b/src/gateway/server/ws-connection/message-handler.ts index faba448f26b..9db2f5ee90d 100644 --- a/src/gateway/server/ws-connection/message-handler.ts +++ b/src/gateway/server/ws-connection/message-handler.ts @@ -91,7 +91,6 @@ import { resolveHandshakeBrowserSecurityContext, resolveUnauthorizedHandshakeContext, shouldAllowSilentLocalPairing, - shouldSkipBackendSelfPairing, } from "./handshake-auth-helpers.js"; import { isUnauthorizedRoleError, UnauthorizedFloodGuard } from "./unauthorized-flood-guard.js"; @@ -686,20 +685,12 @@ export function attachGatewayWsMessageHandler(params: { authOk, authMethod, }); - const skipPairing = - shouldSkipBackendSelfPairing({ - connectParams, - isLocalClient, - hasBrowserOriginHeader, - sharedAuthOk, - authMethod, - }) || - shouldSkipControlUiPairing( - controlUiAuthPolicy, - role, - trustedProxyAuthOk, - resolvedAuth.mode, - ); + const skipPairing = shouldSkipControlUiPairing( + controlUiAuthPolicy, + role, + trustedProxyAuthOk, + resolvedAuth.mode, + ); if (device && devicePublicKey && !skipPairing) { const formatAuditList = (items: string[] | undefined): string => { if (!items || items.length === 0) {