fix(gateway): restore localhost Control UI pairing when allowInsecureAuth is set (#22996)
* fix(gateway): allow localhost Control UI without device identity when allowInsecureAuth is set * fix(gateway): pass isLocalClient to evaluateMissingDeviceIdentity * test: add regression tests for localhost Control UI pairing * fix(gateway): require pairing for legacy metadata upgrades * test(gateway): fix legacy metadata e2e ws typing --------- Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
@@ -759,7 +759,7 @@ describe("gateway server auth/connect", () => {
|
||||
});
|
||||
});
|
||||
|
||||
test("rejects control ui without device identity even when insecure auth is enabled", async () => {
|
||||
test("allows localhost control ui without device identity when insecure auth is enabled", async () => {
|
||||
testState.gatewayControlUi = { allowInsecureAuth: true };
|
||||
const { server, ws, prevToken } = await startServerWithClient("secret", {
|
||||
wsHeaders: { origin: "http://127.0.0.1" },
|
||||
@@ -774,14 +774,18 @@ describe("gateway server auth/connect", () => {
|
||||
mode: GATEWAY_CLIENT_MODES.WEBCHAT,
|
||||
},
|
||||
});
|
||||
expect(res.ok).toBe(false);
|
||||
expect(res.error?.message ?? "").toContain("secure context");
|
||||
expect(res.ok).toBe(true);
|
||||
const status = await rpcReq(ws, "status");
|
||||
expect(status.ok).toBe(false);
|
||||
expect(status.error?.message ?? "").toContain("missing scope");
|
||||
const health = await rpcReq(ws, "health");
|
||||
expect(health.ok).toBe(true);
|
||||
ws.close();
|
||||
await server.close();
|
||||
restoreGatewayToken(prevToken);
|
||||
});
|
||||
|
||||
test("rejects control ui password-only auth when insecure auth is enabled", async () => {
|
||||
test("allows control ui password-only auth on localhost when insecure auth is enabled", async () => {
|
||||
testState.gatewayControlUi = { allowInsecureAuth: true };
|
||||
testState.gatewayAuth = { mode: "password", password: "secret" };
|
||||
await withGatewayServer(async ({ port }) => {
|
||||
@@ -793,8 +797,12 @@ describe("gateway server auth/connect", () => {
|
||||
...CONTROL_UI_CLIENT,
|
||||
},
|
||||
});
|
||||
expect(res.ok).toBe(false);
|
||||
expect(res.error?.message ?? "").toContain("secure context");
|
||||
expect(res.ok).toBe(true);
|
||||
const status = await rpcReq(ws, "status");
|
||||
expect(status.ok).toBe(false);
|
||||
expect(status.error?.message ?? "").toContain("missing scope");
|
||||
const health = await rpcReq(ws, "health");
|
||||
expect(health.ok).toBe(true);
|
||||
ws.close();
|
||||
});
|
||||
});
|
||||
@@ -1270,6 +1278,108 @@ describe("gateway server auth/connect", () => {
|
||||
}
|
||||
});
|
||||
|
||||
test("rejects scope escalation from legacy paired metadata", async () => {
|
||||
const { mkdtemp } = await import("node:fs/promises");
|
||||
const { tmpdir } = await import("node:os");
|
||||
const { join } = await import("node:path");
|
||||
const { readJsonFile, resolvePairingPaths } = await import("../infra/pairing-files.js");
|
||||
const { writeJsonAtomic } = await import("../infra/json-files.js");
|
||||
const { buildDeviceAuthPayload } = await import("./device-auth.js");
|
||||
const { loadOrCreateDeviceIdentity, publicKeyRawBase64UrlFromPem, signDevicePayload } =
|
||||
await import("../infra/device-identity.js");
|
||||
const { approveDevicePairing, getPairedDevice, listDevicePairing } =
|
||||
await import("../infra/device-pairing.js");
|
||||
const { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } =
|
||||
await import("../utils/message-channel.js");
|
||||
const { server, ws, port, prevToken } = await startServerWithClient("secret");
|
||||
let ws2: WebSocket | undefined;
|
||||
try {
|
||||
const identityDir = await mkdtemp(join(tmpdir(), "openclaw-device-legacy-"));
|
||||
const identity = loadOrCreateDeviceIdentity(join(identityDir, "device.json"));
|
||||
const client = {
|
||||
id: GATEWAY_CLIENT_NAMES.TEST,
|
||||
version: "1.0.0",
|
||||
platform: "test",
|
||||
mode: GATEWAY_CLIENT_MODES.TEST,
|
||||
};
|
||||
const buildDevice = (scopes: string[]) => {
|
||||
const signedAtMs = Date.now();
|
||||
const payload = buildDeviceAuthPayload({
|
||||
deviceId: identity.deviceId,
|
||||
clientId: client.id,
|
||||
clientMode: client.mode,
|
||||
role: "operator",
|
||||
scopes,
|
||||
signedAtMs,
|
||||
token: "secret",
|
||||
});
|
||||
return {
|
||||
id: identity.deviceId,
|
||||
publicKey: publicKeyRawBase64UrlFromPem(identity.publicKeyPem),
|
||||
signature: signDevicePayload(identity.privateKeyPem, payload),
|
||||
signedAt: signedAtMs,
|
||||
};
|
||||
};
|
||||
|
||||
const initial = await connectReq(ws, {
|
||||
token: "secret",
|
||||
scopes: ["operator.read"],
|
||||
client,
|
||||
device: buildDevice(["operator.read"]),
|
||||
});
|
||||
if (!initial.ok) {
|
||||
const list = await listDevicePairing();
|
||||
const pending = list.pending.at(0);
|
||||
expect(pending?.requestId).toBeDefined();
|
||||
if (pending?.requestId) {
|
||||
await approveDevicePairing(pending.requestId);
|
||||
}
|
||||
}
|
||||
ws.close();
|
||||
|
||||
const { pairedPath } = resolvePairingPaths(undefined, "devices");
|
||||
const paired =
|
||||
(await readJsonFile<Record<string, Record<string, unknown>>>(pairedPath)) ?? {};
|
||||
const legacy = paired[identity.deviceId];
|
||||
expect(legacy).toBeTruthy();
|
||||
if (!legacy) {
|
||||
throw new Error(`Expected paired metadata for deviceId=${identity.deviceId}`);
|
||||
}
|
||||
delete legacy.roles;
|
||||
delete legacy.scopes;
|
||||
await writeJsonAtomic(pairedPath, paired);
|
||||
|
||||
const wsUpgrade = new WebSocket(`ws://127.0.0.1:${port}`);
|
||||
ws2 = wsUpgrade;
|
||||
await new Promise<void>((resolve) => wsUpgrade.once("open", resolve));
|
||||
const upgraded = await connectReq(wsUpgrade, {
|
||||
token: "secret",
|
||||
scopes: ["operator.admin"],
|
||||
client,
|
||||
device: buildDevice(["operator.admin"]),
|
||||
});
|
||||
expect(upgraded.ok).toBe(false);
|
||||
expect(upgraded.error?.message ?? "").toContain("pairing required");
|
||||
wsUpgrade.close();
|
||||
|
||||
const pendingUpgrade = (await listDevicePairing()).pending.find(
|
||||
(entry) => entry.deviceId === identity.deviceId,
|
||||
);
|
||||
expect(pendingUpgrade?.requestId).toBeDefined();
|
||||
expect(pendingUpgrade?.scopes).toContain("operator.admin");
|
||||
const repaired = await getPairedDevice(identity.deviceId);
|
||||
expect(repaired?.role).toBe("operator");
|
||||
expect(repaired?.roles).toBeUndefined();
|
||||
expect(repaired?.scopes).toBeUndefined();
|
||||
expect(repaired?.approvedScopes).not.toContain("operator.admin");
|
||||
} finally {
|
||||
ws.close();
|
||||
ws2?.close();
|
||||
await server.close();
|
||||
restoreGatewayToken(prevToken);
|
||||
}
|
||||
});
|
||||
|
||||
test("rejects revoked device token", async () => {
|
||||
const { revokeDeviceToken } = await import("../infra/device-pairing.js");
|
||||
const { server, ws, port, prevToken } = await startServerWithClient("secret");
|
||||
|
||||
@@ -40,6 +40,7 @@ describe("ws connect policy", () => {
|
||||
sharedAuthOk: true,
|
||||
authOk: true,
|
||||
hasSharedAuth: true,
|
||||
isLocalClient: false,
|
||||
}).kind,
|
||||
).toBe("allow");
|
||||
|
||||
@@ -48,6 +49,7 @@ describe("ws connect policy", () => {
|
||||
controlUiConfig: { allowInsecureAuth: true, dangerouslyDisableDeviceAuth: false },
|
||||
deviceRaw: null,
|
||||
});
|
||||
// Remote Control UI with allowInsecureAuth -> still rejected.
|
||||
expect(
|
||||
evaluateMissingDeviceIdentity({
|
||||
hasDeviceIdentity: false,
|
||||
@@ -57,6 +59,40 @@ describe("ws connect policy", () => {
|
||||
sharedAuthOk: true,
|
||||
authOk: true,
|
||||
hasSharedAuth: true,
|
||||
isLocalClient: false,
|
||||
}).kind,
|
||||
).toBe("reject-control-ui-insecure-auth");
|
||||
|
||||
// Local Control UI with allowInsecureAuth -> allowed.
|
||||
expect(
|
||||
evaluateMissingDeviceIdentity({
|
||||
hasDeviceIdentity: false,
|
||||
role: "operator",
|
||||
isControlUi: true,
|
||||
controlUiAuthPolicy: controlUiStrict,
|
||||
sharedAuthOk: true,
|
||||
authOk: true,
|
||||
hasSharedAuth: true,
|
||||
isLocalClient: true,
|
||||
}).kind,
|
||||
).toBe("allow");
|
||||
|
||||
// Control UI without allowInsecureAuth, even on localhost -> rejected.
|
||||
const controlUiNoInsecure = resolveControlUiAuthPolicy({
|
||||
isControlUi: true,
|
||||
controlUiConfig: { dangerouslyDisableDeviceAuth: false },
|
||||
deviceRaw: null,
|
||||
});
|
||||
expect(
|
||||
evaluateMissingDeviceIdentity({
|
||||
hasDeviceIdentity: false,
|
||||
role: "operator",
|
||||
isControlUi: true,
|
||||
controlUiAuthPolicy: controlUiNoInsecure,
|
||||
sharedAuthOk: true,
|
||||
authOk: true,
|
||||
hasSharedAuth: true,
|
||||
isLocalClient: true,
|
||||
}).kind,
|
||||
).toBe("reject-control-ui-insecure-auth");
|
||||
|
||||
@@ -69,6 +105,7 @@ describe("ws connect policy", () => {
|
||||
sharedAuthOk: true,
|
||||
authOk: true,
|
||||
hasSharedAuth: true,
|
||||
isLocalClient: false,
|
||||
}).kind,
|
||||
).toBe("allow");
|
||||
|
||||
@@ -81,6 +118,7 @@ describe("ws connect policy", () => {
|
||||
sharedAuthOk: false,
|
||||
authOk: false,
|
||||
hasSharedAuth: true,
|
||||
isLocalClient: false,
|
||||
}).kind,
|
||||
).toBe("reject-unauthorized");
|
||||
|
||||
@@ -93,6 +131,7 @@ describe("ws connect policy", () => {
|
||||
sharedAuthOk: true,
|
||||
authOk: true,
|
||||
hasSharedAuth: true,
|
||||
isLocalClient: false,
|
||||
}).kind,
|
||||
).toBe("reject-device-required");
|
||||
});
|
||||
|
||||
@@ -53,12 +53,20 @@ export function evaluateMissingDeviceIdentity(params: {
|
||||
sharedAuthOk: boolean;
|
||||
authOk: boolean;
|
||||
hasSharedAuth: boolean;
|
||||
isLocalClient: boolean;
|
||||
}): MissingDeviceIdentityDecision {
|
||||
if (params.hasDeviceIdentity) {
|
||||
return { kind: "allow" };
|
||||
}
|
||||
if (params.isControlUi && !params.controlUiAuthPolicy.allowBypass) {
|
||||
return { kind: "reject-control-ui-insecure-auth" };
|
||||
// Allow localhost Control UI connections when allowInsecureAuth is configured.
|
||||
// Localhost has no network interception risk, and browser SubtleCrypto
|
||||
// (needed for device identity) is unavailable in insecure HTTP contexts.
|
||||
// Remote connections are still rejected to preserve the MitM protection
|
||||
// that the security fix (#20684) intended.
|
||||
if (!params.controlUiAuthPolicy.allowInsecureAuthConfigured || !params.isLocalClient) {
|
||||
return { kind: "reject-control-ui-insecure-auth" };
|
||||
}
|
||||
}
|
||||
if (roleCanSkipDeviceIdentity(params.role, params.sharedAuthOk)) {
|
||||
return { kind: "allow" };
|
||||
|
||||
@@ -469,6 +469,7 @@ export function attachGatewayWsMessageHandler(params: {
|
||||
sharedAuthOk,
|
||||
authOk,
|
||||
hasSharedAuth,
|
||||
isLocalClient,
|
||||
});
|
||||
if (decision.kind === "allow") {
|
||||
return true;
|
||||
@@ -706,50 +707,50 @@ export function attachGatewayWsMessageHandler(params: {
|
||||
return;
|
||||
}
|
||||
} else {
|
||||
const hasLegacyPairedMetadata =
|
||||
paired.roles === undefined && paired.scopes === undefined;
|
||||
const pairedRoles = Array.isArray(paired.roles)
|
||||
? paired.roles
|
||||
: paired.role
|
||||
? [paired.role]
|
||||
: [];
|
||||
if (!hasLegacyPairedMetadata) {
|
||||
const allowedRoles = new Set(pairedRoles);
|
||||
if (allowedRoles.size === 0) {
|
||||
logUpgradeAudit("role-upgrade", pairedRoles, paired.scopes);
|
||||
const ok = await requirePairing("role-upgrade");
|
||||
if (!ok) {
|
||||
return;
|
||||
}
|
||||
} else if (!allowedRoles.has(role)) {
|
||||
logUpgradeAudit("role-upgrade", pairedRoles, paired.scopes);
|
||||
const ok = await requirePairing("role-upgrade");
|
||||
if (!ok) {
|
||||
return;
|
||||
}
|
||||
const pairedScopes = Array.isArray(paired.scopes)
|
||||
? paired.scopes
|
||||
: Array.isArray(paired.approvedScopes)
|
||||
? paired.approvedScopes
|
||||
: [];
|
||||
const allowedRoles = new Set(pairedRoles);
|
||||
if (allowedRoles.size === 0) {
|
||||
logUpgradeAudit("role-upgrade", pairedRoles, pairedScopes);
|
||||
const ok = await requirePairing("role-upgrade");
|
||||
if (!ok) {
|
||||
return;
|
||||
}
|
||||
} else if (!allowedRoles.has(role)) {
|
||||
logUpgradeAudit("role-upgrade", pairedRoles, pairedScopes);
|
||||
const ok = await requirePairing("role-upgrade");
|
||||
if (!ok) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
const pairedScopes = Array.isArray(paired.scopes) ? paired.scopes : [];
|
||||
if (scopes.length > 0) {
|
||||
if (pairedScopes.length === 0) {
|
||||
if (scopes.length > 0) {
|
||||
if (pairedScopes.length === 0) {
|
||||
logUpgradeAudit("scope-upgrade", pairedRoles, pairedScopes);
|
||||
const ok = await requirePairing("scope-upgrade");
|
||||
if (!ok) {
|
||||
return;
|
||||
}
|
||||
} else {
|
||||
const scopesAllowed = roleScopesAllow({
|
||||
role,
|
||||
requestedScopes: scopes,
|
||||
allowedScopes: pairedScopes,
|
||||
});
|
||||
if (!scopesAllowed) {
|
||||
logUpgradeAudit("scope-upgrade", pairedRoles, pairedScopes);
|
||||
const ok = await requirePairing("scope-upgrade");
|
||||
if (!ok) {
|
||||
return;
|
||||
}
|
||||
} else {
|
||||
const scopesAllowed = roleScopesAllow({
|
||||
role,
|
||||
requestedScopes: scopes,
|
||||
allowedScopes: pairedScopes,
|
||||
});
|
||||
if (!scopesAllowed) {
|
||||
logUpgradeAudit("scope-upgrade", pairedRoles, pairedScopes);
|
||||
const ok = await requirePairing("scope-upgrade");
|
||||
if (!ok) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user