Node revoke follow-ups: /systems node auth ignores approval_status; no UI to reject an approved node #211

New Issue

2026-10-04T18:02:35-04:00

logan commented

2026-10-04 18:02:35 -04:00

Found while fixing #195 (PR #206). Two leftover gaps, both small:

app/internal/auth.py require_service_or_firebase_token grants node read access (/systems) to any bearer that matches a node_keys entry, without checking the node's approval_status. After #206 a successful reject deletes node_keys, so this only matters in the partial-failure window: reject flips status to rejected, then dynsec delete fails (502) and node_keys stays until the admin retries. /upload now checks approval itself; this route doesn't.
Fix shape: after the node_keys match, fetch nodes/{node_id} and require approved. One extra read per node /systems call.
The node page (drb-frontend/app/nodes/[id]/page.tsx) only shows Approve/Reject for pending nodes. An approved node can only be revoked via Delete (which also wipes the node doc) or a direct API call to POST /nodes/{id}/reject. If "revoke but keep the record" is wanted from the UI, it needs a button on approved nodes.

Found while fixing #195 (PR #206). Two leftover gaps, both small: 1. `app/internal/auth.py` `require_service_or_firebase_token` grants node read access (`/systems`) to any bearer that matches a `node_keys` entry, without checking the node's `approval_status`. After #206 a successful reject deletes `node_keys`, so this only matters in the partial-failure window: reject flips status to rejected, then dynsec delete fails (502) and `node_keys` stays until the admin retries. `/upload` now checks approval itself; this route doesn't. Fix shape: after the `node_keys` match, fetch `nodes/{node_id}` and require `approved`. One extra read per node `/systems` call. 2. The node page (`drb-frontend/app/nodes/[id]/page.tsx`) only shows Approve/Reject for **pending** nodes. An approved node can only be revoked via Delete (which also wipes the node doc) or a direct API call to `POST /nodes/{id}/reject`. If "revoke but keep the record" is wanted from the UI, it needs a button on approved nodes.

logan commented

2026-10-04 18:07:28 -04:00

Another follow-up from reviewing PR #206: POST /nodes/{id}/reissue-key does not check approval_status, so running it on a rejected node re-creates its dynsec login and node_keys while it stays rejected (MQTT works again until the next c2-core restart, when reconcile_all deletes the login; /upload still 403s). reissue-key should refuse non-approved nodes.

logan added the area/auth area/mqtt

labels 2026-10-05 02:05:03 -04:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: logan/server-26#211