Skip to content

Commit 1c85eff

Browse files
pgondhi987pgondhi987
authored
Require admin scope for node device token management [AI] (#81067)
* fix: require admin for node device token management * addressing review-skill * addressing review-skill * addressing review-skill * addressing review-skill * addressing claude review * addressing ci * docs: add changelog entry for PR merge
1 parent e2965b5 commit 1c85eff

5 files changed

Lines changed: 563 additions & 18 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ Docs: https://docs.openclaw.ai
66

77
### Fixes
88

9+
- Require admin scope for node device token management [AI]. (#81067) Thanks @pgondhi987.
910
- Restrict chat sender allowlist matching [AI]. (#80898) Thanks @pgondhi987.
1011
- Sessions: redact persisted tool result detail metadata before writing transcripts so diagnostic secrets do not survive tool output redaction. (#80444) Thanks @nimbleenigma.
1112
- Codex migration: make Enter activate the highlighted checkbox row before continuing, so `Skip for now` and bulk-selection rows work even when planned items start preselected.

‎src/gateway/server-methods/devices.test.ts‎

Lines changed: 149 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,40 @@ describe("deviceHandlers", () => {
181181
);
182182
});
183183

184+
it("rejects removing mixed-role devices without admin scope", async () => {
185+
getPairedDeviceMock.mockResolvedValue({
186+
deviceId: "device-1",
187+
role: "operator",
188+
roles: ["operator", "node"],
189+
tokens: {
190+
operator: {
191+
token: "operator-token",
192+
role: "operator",
193+
scopes: ["operator.pairing"],
194+
createdAtMs: 100,
195+
},
196+
node: {
197+
token: "node-token",
198+
role: "node",
199+
scopes: [],
200+
createdAtMs: 100,
201+
revokedAtMs: 200,
202+
},
203+
},
204+
});
205+
const opts = createOptions(
206+
"device.pair.remove",
207+
{ deviceId: "device-1" },
208+
{ client: createClient(["operator.pairing"], "device-1", { isDeviceTokenAuth: true }) },
209+
);
210+
211+
await deviceHandlers["device.pair.remove"](opts);
212+
213+
expect(removePairedDeviceMock).not.toHaveBeenCalled();
214+
expect(opts.context.disconnectClientsForDevice).not.toHaveBeenCalled();
215+
expectRespondedErrorMessage(opts, "device pairing removal denied");
216+
});
217+
184218
it("disconnects active clients after revoking a device token", async () => {
185219
revokeDeviceTokenMock.mockResolvedValue({
186220
ok: true,
@@ -234,6 +268,20 @@ describe("deviceHandlers", () => {
234268
);
235269
});
236270

271+
it("rejects revoking node tokens without admin scope", async () => {
272+
const opts = createOptions(
273+
"device.token.revoke",
274+
{ deviceId: "device-1", role: "node" },
275+
{ client: createClient(["operator.pairing"], "device-1", { isDeviceTokenAuth: true }) },
276+
);
277+
278+
await deviceHandlers["device.token.revoke"](opts);
279+
280+
expect(revokeDeviceTokenMock).not.toHaveBeenCalled();
281+
expect(opts.context.disconnectClientsForDevice).not.toHaveBeenCalled();
282+
expectRespondedErrorMessage(opts, "device token revocation denied");
283+
});
284+
237285
it("treats normalized device ids as self-owned for token revocation", async () => {
238286
revokeDeviceTokenMock.mockResolvedValue({
239287
ok: true,
@@ -336,6 +384,65 @@ describe("deviceHandlers", () => {
336384
);
337385
});
338386

387+
it("allows pairing-scoped device sessions to manage their own operator token", async () => {
388+
rotateDeviceTokenMock.mockResolvedValue({
389+
ok: true,
390+
entry: {
391+
token: "rotated-token",
392+
role: "operator",
393+
scopes: ["operator.pairing"],
394+
createdAtMs: 456,
395+
rotatedAtMs: 789,
396+
},
397+
});
398+
revokeDeviceTokenMock.mockResolvedValue({
399+
ok: true,
400+
entry: { role: "operator", revokedAtMs: 987 },
401+
});
402+
403+
const rotateOpts = createOptions(
404+
"device.token.rotate",
405+
{ deviceId: "device-1", role: "operator", scopes: ["operator.pairing"] },
406+
{ client: createClient(["operator.pairing"], "device-1", { isDeviceTokenAuth: true }) },
407+
);
408+
const revokeOpts = createOptions(
409+
"device.token.revoke",
410+
{ deviceId: "device-1", role: "operator" },
411+
{ client: createClient(["operator.pairing"], "device-1", { isDeviceTokenAuth: true }) },
412+
);
413+
414+
await deviceHandlers["device.token.rotate"](rotateOpts);
415+
await deviceHandlers["device.token.revoke"](revokeOpts);
416+
417+
expect(rotateDeviceTokenMock).toHaveBeenCalledWith({
418+
deviceId: "device-1",
419+
role: "operator",
420+
scopes: ["operator.pairing"],
421+
callerScopes: ["operator.pairing"],
422+
});
423+
expect(revokeDeviceTokenMock).toHaveBeenCalledWith({
424+
deviceId: "device-1",
425+
role: "operator",
426+
callerScopes: ["operator.pairing"],
427+
});
428+
expect(rotateOpts.respond).toHaveBeenCalledWith(
429+
true,
430+
{
431+
deviceId: "device-1",
432+
role: "operator",
433+
token: "rotated-token",
434+
scopes: ["operator.pairing"],
435+
rotatedAtMs: 789,
436+
},
437+
undefined,
438+
);
439+
expect(revokeOpts.respond).toHaveBeenCalledWith(
440+
true,
441+
{ deviceId: "device-1", role: "operator", revokedAtMs: 987 },
442+
undefined,
443+
);
444+
});
445+
339446
it("omits rotated tokens when an admin rotates another device token", async () => {
340447
mockPairedOperatorDevice();
341448
mockRotateOperatorTokenSuccess();
@@ -368,8 +475,27 @@ describe("deviceHandlers", () => {
368475
});
369476

370477
it("rejects rotating a token for a role that was never approved", async () => {
371-
mockPairedOperatorDevice();
372478
rotateDeviceTokenMock.mockResolvedValue({ ok: false, reason: "unknown-device-or-role" });
479+
const opts = createOptions(
480+
"device.token.rotate",
481+
{ deviceId: "device-1", role: "node" },
482+
{ client: createClient(["operator.admin"], "admin-device", { isDeviceTokenAuth: true }) },
483+
);
484+
485+
await deviceHandlers["device.token.rotate"](opts);
486+
487+
expect(rotateDeviceTokenMock).toHaveBeenCalledWith({
488+
deviceId: "device-1",
489+
role: "node",
490+
scopes: undefined,
491+
callerScopes: ["operator.admin"],
492+
});
493+
expect(opts.context.disconnectClientsForDevice).not.toHaveBeenCalled();
494+
expectRespondedErrorMessage(opts, "device token rotation denied");
495+
});
496+
497+
it("rejects rotating node tokens without admin scope", async () => {
498+
mockPairedOperatorDevice();
373499
const opts = createOptions(
374500
"device.token.rotate",
375501
{
@@ -387,12 +513,7 @@ describe("deviceHandlers", () => {
387513

388514
await deviceHandlers["device.token.rotate"](opts);
389515

390-
expect(rotateDeviceTokenMock).toHaveBeenCalledWith({
391-
deviceId: "device-1",
392-
role: "node",
393-
scopes: undefined,
394-
callerScopes: ["operator.pairing"],
395-
});
516+
expect(rotateDeviceTokenMock).not.toHaveBeenCalled();
396517
expect(opts.context.disconnectClientsForDevice).not.toHaveBeenCalled();
397518
expectRespondedErrorMessage(opts, "device token rotation denied");
398519
});
@@ -688,6 +809,27 @@ describe("deviceHandlers", () => {
688809
);
689810
});
690811

812+
it("rejects approving node roles for the caller device without admin scope", async () => {
813+
getPendingDevicePairingMock.mockResolvedValue({
814+
requestId: "req-1",
815+
deviceId: " device-1 ",
816+
publicKey: "pk-1",
817+
role: "node",
818+
roles: ["node"],
819+
ts: 100,
820+
});
821+
const opts = createOptions(
822+
"device.pair.approve",
823+
{ requestId: "req-1" },
824+
{ client: createClient(["operator.pairing"], "device-1", { isDeviceTokenAuth: true }) },
825+
);
826+
827+
await deviceHandlers["device.pair.approve"](opts);
828+
829+
expect(approveDevicePairingMock).not.toHaveBeenCalled();
830+
expectRespondedErrorMessage(opts, "device pairing approval denied");
831+
});
832+
691833
it("rejects rejecting another device from a non-admin device session", async () => {
692834
getPendingDevicePairingMock.mockResolvedValue({
693835
requestId: "req-2",

‎src/gateway/server-methods/devices.ts‎

Lines changed: 113 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {
22
approveDevicePairing,
33
formatDevicePairingForbiddenMessage,
4+
getPairedDevice,
45
getPendingDevicePairing,
56
listDevicePairing,
67
removePairedDevice,
@@ -55,7 +56,11 @@ function logDeviceTokenRotationDenied(params: {
5556
log: { warn: (message: string) => void };
5657
deviceId: string;
5758
role: string;
58-
reason: RotateDeviceTokenDenyReason | "unknown-device-or-role" | "device-ownership-mismatch";
59+
reason:
60+
| RotateDeviceTokenDenyReason
61+
| "unknown-device-or-role"
62+
| "device-ownership-mismatch"
63+
| "role-management-requires-admin";
5964
scope?: string | null;
6065
}) {
6166
const suffix = params.scope ? ` scope=${params.scope}` : "";
@@ -68,7 +73,10 @@ function logDeviceTokenRevocationDenied(params: {
6873
log: { warn: (message: string) => void };
6974
deviceId: string;
7075
role: string;
71-
reason: RevokeDeviceTokenDenyReason | "device-ownership-mismatch";
76+
reason:
77+
| RevokeDeviceTokenDenyReason
78+
| "device-ownership-mismatch"
79+
| "role-management-requires-admin";
7280
scope?: string | null;
7381
}) {
7482
const suffix = params.scope ? ` scope=${params.scope}` : "";
@@ -113,6 +121,56 @@ function shouldReturnRotatedDeviceToken(authz: DeviceManagementAuthz): boolean {
113121
return Boolean(authz.callerDeviceId && authz.callerDeviceId === authz.normalizedTargetDeviceId);
114122
}
115123

124+
function deniesDeviceTokenRoleManagement(
125+
authz: DeviceManagementAuthz,
126+
targetRole: string,
127+
): boolean {
128+
const normalizedTargetRole = targetRole.trim();
129+
if (!normalizedTargetRole || authz.isAdminCaller) {
130+
return false;
131+
}
132+
return normalizedTargetRole !== "operator";
133+
}
134+
135+
function hasNonOperatorDeviceRole(input: { role?: string; roles?: string[] }): boolean {
136+
const roles = new Set<string>();
137+
const role = input.role?.trim();
138+
if (role) {
139+
roles.add(role);
140+
}
141+
for (const entry of input.roles ?? []) {
142+
const normalized = entry.trim();
143+
if (normalized) {
144+
roles.add(normalized);
145+
}
146+
}
147+
return [...roles].some((entry) => entry !== "operator");
148+
}
149+
150+
function hasNonOperatorDeviceTokenRole(
151+
tokens: Record<string, DeviceAuthToken> | undefined,
152+
): boolean {
153+
for (const token of Object.values(tokens ?? {})) {
154+
const normalized = token.role.trim();
155+
if (normalized && normalized !== "operator") {
156+
return true;
157+
}
158+
}
159+
return false;
160+
}
161+
162+
function requestsNonOperatorDeviceRole(pending: { role?: string; roles?: string[] }): boolean {
163+
return hasNonOperatorDeviceRole(pending);
164+
}
165+
166+
function pairedDeviceHasNonOperatorRole(device: {
167+
role?: string;
168+
roles?: string[];
169+
tokens?: Record<string, DeviceAuthToken>;
170+
}): boolean {
171+
return hasNonOperatorDeviceRole(device) || hasNonOperatorDeviceTokenRole(device.tokens);
172+
}
173+
116174
export const deviceHandlers: GatewayRequestHandlers = {
117175
"device.pair.list": async ({ params, respond, client }) => {
118176
if (!validateDevicePairListParams(params)) {
@@ -185,6 +243,17 @@ export const deviceHandlers: GatewayRequestHandlers = {
185243
);
186244
return;
187245
}
246+
if (requestsNonOperatorDeviceRole(pending)) {
247+
context.logGateway.warn(
248+
`device pairing approval denied request=${requestId} reason=role-management-requires-admin`,
249+
);
250+
respond(
251+
false,
252+
undefined,
253+
errorShape(ErrorCodes.INVALID_REQUEST, DEVICE_PAIR_APPROVAL_DENIED_MESSAGE),
254+
);
255+
return;
256+
}
188257
}
189258
const approved = await approveDevicePairing(requestId, { callerScopes: authz.callerScopes });
190259
if (!approved) {
@@ -296,6 +365,20 @@ export const deviceHandlers: GatewayRequestHandlers = {
296365
);
297366
return;
298367
}
368+
if (authz.callerDeviceId && !authz.isAdminCaller) {
369+
const paired = await getPairedDevice(authz.normalizedTargetDeviceId);
370+
if (paired && pairedDeviceHasNonOperatorRole(paired)) {
371+
context.logGateway.warn(
372+
`device pairing removal denied device=${deviceId} reason=role-management-requires-admin`,
373+
);
374+
respond(
375+
false,
376+
undefined,
377+
errorShape(ErrorCodes.INVALID_REQUEST, "device pairing removal denied"),
378+
);
379+
return;
380+
}
381+
}
299382
const removed = await removePairedDevice(deviceId);
300383
if (!removed) {
301384
respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "unknown deviceId"));
@@ -341,6 +424,20 @@ export const deviceHandlers: GatewayRequestHandlers = {
341424
);
342425
return;
343426
}
427+
if (deniesDeviceTokenRoleManagement(authz, role)) {
428+
logDeviceTokenRotationDenied({
429+
log: context.logGateway,
430+
deviceId,
431+
role,
432+
reason: "role-management-requires-admin",
433+
});
434+
respond(
435+
false,
436+
undefined,
437+
errorShape(ErrorCodes.INVALID_REQUEST, DEVICE_TOKEN_ROTATION_DENIED_MESSAGE),
438+
);
439+
return;
440+
}
344441
const rotated = await rotateDeviceToken({
345442
deviceId,
346443
role,
@@ -408,6 +505,20 @@ export const deviceHandlers: GatewayRequestHandlers = {
408505
);
409506
return;
410507
}
508+
if (deniesDeviceTokenRoleManagement(authz, role)) {
509+
logDeviceTokenRevocationDenied({
510+
log: context.logGateway,
511+
deviceId,
512+
role,
513+
reason: "role-management-requires-admin",
514+
});
515+
respond(
516+
false,
517+
undefined,
518+
errorShape(ErrorCodes.INVALID_REQUEST, DEVICE_TOKEN_REVOCATION_DENIED_MESSAGE),
519+
);
520+
return;
521+
}
411522
const revoked = await revokeDeviceToken({ deviceId, role, callerScopes: authz.callerScopes });
412523
if (!revoked.ok) {
413524
logDeviceTokenRevocationDenied({

0 commit comments

Comments
 (0)