From 1e6db7d23c70b7bd7e1422f09911b3645f0fb2e2 Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Tue, 1 Sep 2026 20:50:45 +0200 Subject: revert(hub): M6 — add_group_member must keep accepting node tokens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit M6 in the third review was a misread. `add_group_member` accepting a node-scoped token is deliberate (commit 0443cf8): the node calls POST /v1/groups/{id}/members/{username} after a CLI `member invite` so the group shows up in the invitee's SPA, authenticating with a node-scoped token. `group.admin_id == caller` is the real guard. An older test (`test_node_scope_blocks_add_member`) asserted the opposite and had been left red on main; the M6 "fix" (commit 6b38704) satisfied that test by switching the dependency to `require_user_scope` — which made `ops.create_invite`'s hub-membership call 403. That exception is swallowed with a log.warning, so an invited user silently never lands in group_members and the group is invisible to them. Reported from live testing (CLI `member invite grenet`, grenet saw nothing). Dependency back to `get_current_user`. The stale test now asserts the intended behaviour: a node token may add a member to its own operator's group (201) but not to a group it does not own (403). Third-review M6 marked WITHDRAWN. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_011pG75yGK3NthNfyjH74omG --- packages/meshbay-hub/src/meshbay_hub/api/groups.py | 14 +++++++------ packages/meshbay-hub/tests/test_node_auth.py | 23 ++++++++++++++++------ 2 files changed, 25 insertions(+), 12 deletions(-) (limited to 'packages') diff --git a/packages/meshbay-hub/src/meshbay_hub/api/groups.py b/packages/meshbay-hub/src/meshbay_hub/api/groups.py index 15c7a5d..07d3ec0 100644 --- a/packages/meshbay-hub/src/meshbay_hub/api/groups.py +++ b/packages/meshbay-hub/src/meshbay_hub/api/groups.py @@ -586,14 +586,16 @@ async def update_group( async def add_group_member( group_id: str, username: str, - current_user: User = Depends(require_user_scope), + current_user: User = Depends(get_current_user), db: AsyncSession = Depends(get_db), ): - # `require_user_scope`, like every other mutating group endpoint: a - # node-scoped daemon token must not manage membership (NS7 — the operator - # manages groups from the browser). This was the one membership endpoint - # still on `get_current_user`, so a node token could add members to its - # operator's own groups. + # `get_current_user`, not `require_user_scope`: the node calls this after a + # CLI `member invite` so the group becomes visible in the invitee's SPA + # (commit 0443cf8). The node authenticates with a node-scoped token, and the + # `group.admin_id == current_user.id` check below is the real guard — a node + # can only touch its own operator's groups, adding an already-registered + # account. (Third-review M6 proposed tightening this to `require_user_scope`; + # that broke the CLI invite flow and was reverted — see the review doc.) group = await db.get(Group, group_id) if not group: raise HTTPException(status_code=404, detail="Group not found") diff --git a/packages/meshbay-hub/tests/test_node_auth.py b/packages/meshbay-hub/tests/test_node_auth.py index 72ce412..a104a72 100644 --- a/packages/meshbay-hub/tests/test_node_auth.py +++ b/packages/meshbay-hub/tests/test_node_auth.py @@ -148,24 +148,35 @@ async def test_node_scope_blocks_group_create(client): @pytest.mark.asyncio -async def test_node_scope_blocks_add_member(client): - sk_node, user_token = await _setup_node_user(client, "op1") +async def test_node_token_may_add_a_member_to_its_own_operators_group(client): + """The node calls this after a CLI `member invite` so the group shows up in + the invitee's SPA (commit 0443cf8). A node-scoped token is accepted here — + the `group.admin_id == caller` check is the guard — but only for a group the + node's operator owns.""" + sk_op, op_token = await _setup_node_user(client, "op1") r = await client.post("/v1/groups", json={ "name": "mygroup", "visibility": "private", "join_policy": "invite", - }, headers={"Authorization": f"Bearer {user_token}"}) - assert r.status_code == 201 + }, headers={"Authorization": f"Bearer {op_token}"}) gid = r.json()["group_id"] _, pk2 = _gen_ed25519() _, px2 = _gen_x25519() await _register(client, "member1", pk2, px2) - r = await _node_auth(client, "op1", sk_node) - node_token = r.json()["access_token"] + node_token = (await _node_auth(client, "op1", sk_op)).json()["access_token"] r = await client.post(f"/v1/groups/{gid}/members/member1", headers={"Authorization": f"Bearer {node_token}"}) + assert r.status_code == 201 + + # …but not to a group it does not own. + sk_other, other_token = await _setup_node_user(client, "op2") + r = await client.post("/v1/groups", json={"name": "theirs", "visibility": "private"}, + headers={"Authorization": f"Bearer {other_token}"}) + other_gid = r.json()["group_id"] + r = await client.post(f"/v1/groups/{other_gid}/members/member1", + headers={"Authorization": f"Bearer {node_token}"}) assert r.status_code == 403 -- cgit v1.2.3