Skip to content

Commit ca88e9f

Browse files
Merge pull request #336 from ipnet-mesh/feat/routes-ownership
feat(routes): ownership-based edit/delete permissions
2 parents 20aaf46 + 2ecca11 commit ca88e9f

11 files changed

Lines changed: 729 additions & 38 deletions

File tree

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
"""add routes.created_by
2+
3+
Revision ID: da69304d8106
4+
Revises: d307ee761a34
5+
Create Date: 2026-07-24 10:00:00.000000+00:00
6+
7+
Adds a nullable ``created_by`` column to ``routes``, storing the OIDC
8+
subject identifier (``user_id``) of the operator or admin who created
9+
the route. This enables ownership-based write permissions: operators
10+
can only modify routes they created, while admins can modify any route
11+
and take ownership on edit.
12+
13+
Existing routes get ``NULL`` (admin-only modification), preserving
14+
backwards compatibility with no data backfill.
15+
"""
16+
17+
from typing import Sequence, Union
18+
19+
import sqlalchemy as sa
20+
from alembic import op
21+
22+
# revision identifiers, used by Alembic.
23+
revision: str = "da69304d8106"
24+
down_revision: Union[str, None] = "d307ee761a34"
25+
branch_labels: Union[str, Sequence[str], None] = None
26+
depends_on: Union[str, Sequence[str], None] = None
27+
28+
29+
def upgrade() -> None:
30+
with op.batch_alter_table("routes", schema=None) as batch_op:
31+
batch_op.add_column(sa.Column("created_by", sa.String(255), nullable=True))
32+
33+
34+
def downgrade() -> None:
35+
with op.batch_alter_table("routes", schema=None) as batch_op:
36+
batch_op.drop_column("created_by")

docs/auth.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ User roles are read from the OIDC token's `roles` claim (configurable via `OIDC_
2121
| Role | Config Variable | Default | Description |
2222
|------|----------------|---------|-------------|
2323
| Admin | `OIDC_ROLE_ADMIN` | `admin` | Full write access to all API endpoints through the proxy |
24-
| Operator | `OIDC_ROLE_OPERATOR` | `operator` | Manage nodes, node tags, adoptions, and routes (create/edit/delete, scoped to the operator visibility tier) |
24+
| Operator | `OIDC_ROLE_OPERATOR` | `operator` | Manage nodes, node tags, adoptions, and routes (create; edit/delete only routes they created) |
2525
| Member | `OIDC_ROLE_MEMBER` | `member` | Read-only access (no endpoint assignments) |
2626

2727
The role names are configurable to match your IdP's role naming convention. For example, if your IdP uses `superuser` instead of `admin`, set `OIDC_ROLE_ADMIN=superuser`.

docs/routes.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,9 @@ The collector runs a background thread that re-evaluates every enabled route on
4444

4545
Routes carry the same role-based visibility levels as channels — `community`, `member`, `operator`, `admin`. A user only sees routes whose visibility is at or below their role's maximum level. Seeded routes default to `community` (visible to everyone); set a higher level to restrict a route to operators/admins only. Visibility is enforced on both the list and detail endpoints, so a hidden route's existence is not leaked.
4646

47-
Both operators and admins can create, edit, and delete routes. A user may never scope a route above their own role (e.g. an operator cannot create an `admin`-visibility route) — this is enforced on the write endpoints and prevents a user from creating a route they could then never see or modify. Operators can only edit/delete routes whose visibility is at or below the operator tier; attempting to modify a higher-visibility route returns `404`.
47+
Both operators and admins can create routes. A user may never scope a route above their own role (e.g. an operator cannot create an `admin`-visibility route) — this is enforced on the write endpoints and prevents a user from creating a route they could then never see or modify.
48+
49+
**Ownership-based editing:** Each route records the user who created it (`created_by`). Operators can edit and delete only the routes they created. Admins can edit and delete any route; they claim ownership of legacy (unowned) routes — those with a `NULL` `created_by` — when they edit them, but do not displace an existing creator. Routes created before ownership tracking was introduced have a `NULL` `created_by` and are admin-only. The creator's display name is shown on each route card when available. Attempting to modify a route above the caller's visibility tier returns `404`; modifying a visible route the caller does not own returns `403`.
4850

4951
## Defining routes
5052

@@ -53,4 +55,4 @@ Routes are keyed by their `from`/`to` endpoint labels and upserted by that pair.
5355
- **Seed YAML** — add a `routes.yaml` to your `SEED_HOME` and run the seed process. See [seeding.md → Routes](seeding.md#routes) for the format and rules (path nodes must already exist in the database; the `(from, to)` pair must be unique).
5456
- **API**`POST /api/v1/routes` (operator or admin) creates a route, with a `/preview` endpoint that dry-runs matching against an unsaved configuration so you can tune thresholds before committing. See `SCHEMAS.md` for the request/response shapes.
5557

56-
The `/routes` page renders the live status card, the per-day history strip, recent matching transmissions (with observer attribution), and — for operators and admins — inline edit/delete controls.
58+
The `/routes` page renders the live status card, the per-day history strip, recent matching transmissions (with observer attribution), the route owner's name, and inline edit/delete controls gated per-route by ownership (operators see controls only on their own routes; admins see them on all routes).

e2e/tests/routes-operator.spec.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,16 @@ test.describe.serial("routes (operator)", () => {
1212
await page.goto("/routes");
1313

1414
// Operators see the seeded community route and the add button.
15-
await expect(
16-
page.locator(
17-
'[data-testid="route-card"][data-route-label="Alpha Site \u2192 Bravo Site"]',
18-
),
19-
).toBeVisible();
15+
const seededCard = page.locator(
16+
'[data-testid="route-card"][data-route-label="Alpha Site \u2192 Bravo Site"]',
17+
);
18+
await expect(seededCard).toBeVisible();
2019
await expect(page.getByTestId("add-route")).toBeVisible();
2120

21+
// The seeded route has NULL created_by — operator must NOT see edit/delete.
22+
await expect(seededCard.getByTestId("edit-route")).toHaveCount(0);
23+
await expect(seededCard.getByTestId("delete-route")).toHaveCount(0);
24+
2225
// The visibility dropdown must NOT offer the admin tier to an operator.
2326
await page.getByTestId("add-route").click();
2427
const modal = page.locator('[data-testid="route-modal"]');
@@ -45,7 +48,7 @@ test.describe.serial("routes (operator)", () => {
4548
);
4649
await expect(card).toBeVisible();
4750

48-
// Operator can edit the route they created.
51+
// Operator owns the route they just created — edit/delete should be present.
4952
await card.getByTestId("edit-route").click();
5053
await expect(modal).toBeVisible();
5154
await expect(page.getByTestId("route-from")).toHaveValue("Op From");

e2e/tests/routes.spec.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,14 @@ test.describe.serial("routes (admin)", () => {
1111
}) => {
1212
await page.goto("/routes");
1313

14-
await expect(
15-
page.locator('[data-testid="route-card"][data-route-label="Alpha Site \u2192 Bravo Site"]'),
16-
).toBeVisible();
14+
const seededCard = page.locator(
15+
'[data-testid="route-card"][data-route-label="Alpha Site \u2192 Bravo Site"]',
16+
);
17+
await expect(seededCard).toBeVisible();
18+
19+
// Admin sees edit/delete on all routes, including legacy (NULL created_by) ones.
20+
await expect(seededCard.getByTestId("edit-route")).toBeVisible();
21+
await expect(seededCard.getByTestId("delete-route")).toBeVisible();
1722

1823
await page.getByTestId("add-route").click();
1924
const modal = page.locator('[data-testid="route-modal"]');

src/meshcore_hub/api/routes/routes.py

Lines changed: 98 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
"""Route health monitoring API routes."""
22

3+
import logging
34
from datetime import datetime, timedelta, timezone
45
from typing import Any
56

67
from fastapi import APIRouter, HTTPException, Request
78
from sqlalchemy import select
89

9-
from meshcore_hub.api.auth import RequireOperatorOrAdmin, RequireRead
10+
from meshcore_hub.api.auth import RequireOperatorOrAdmin, RequireRead, X_USER_ID_HEADER
1011
from meshcore_hub.api.cache import cached, sorted_query_string
1112
from meshcore_hub.api.cache_invalidation import invalidate_routes
1213
from meshcore_hub.api.channel_visibility import (
@@ -15,6 +16,7 @@
1516
resolve_user_role,
1617
)
1718
from meshcore_hub.api.dependencies import DbSession
19+
from meshcore_hub.api.profile_utils import get_or_create_profile
1820
from meshcore_hub.collector.routes import (
1921
compute_persisted_quality_avg,
2022
derive_expected_hash,
@@ -34,6 +36,7 @@
3436
from meshcore_hub.common.models.route_observer import RouteObserver
3537
from meshcore_hub.common.models.route_recent_match import RouteRecentMatch
3638
from meshcore_hub.common.models.route_result import RouteResult
39+
from meshcore_hub.common.models.user_profile import UserProfile
3740
from meshcore_hub.common.schemas.routes import (
3841
ContributingObserver,
3942
RecentMatchPath,
@@ -44,6 +47,7 @@
4447
RouteList,
4548
RouteNodeRead,
4649
RouteObserverRead,
50+
RouteOwner,
4751
RoutePreviewRequest,
4852
RoutePreviewResponse,
4953
RouteRead,
@@ -52,6 +56,7 @@
5256
)
5357

5458
router = APIRouter()
59+
logger = logging.getLogger(__name__)
5560

5661
# Sentinel distinguishing "use the precomputed value" from "explicitly None"
5762
# (the latter is what create_route passes to preserve the "brand-new route
@@ -88,15 +93,28 @@ def _assert_visibility_within_role(request: Request, visibility: str) -> None:
8893

8994

9095
def _assert_route_modifiable(request: Request, route: Route) -> None:
91-
"""Reject modifying a route above the caller's visibility tier.
96+
"""Reject modifying a route the caller is not allowed to touch.
9297
93-
Returns 404 (mirroring the GET detail behaviour) so the existence of a
94-
higher-visibility route is not leaked to lower-privileged callers.
98+
Layer 1 — visibility: a route above the caller's tier yields **404**
99+
so its existence is not leaked (mirrors GET detail behaviour).
100+
101+
Layer 2 — ownership: operators may only modify routes *they* created.
102+
A visible-but-unowned route yields **403** (the caller can see it in
103+
the list, so a transparent rejection is better UX). ``created_by``
104+
is ``None`` for legacy routes (pre-ownership-tracking) and is treated
105+
as admin-only. Admins bypass the ownership check entirely.
95106
"""
96107
if VISIBILITY_LEVELS.get(route.visibility, 0) > _caller_max_visibility_level(
97108
request
98109
):
99110
raise HTTPException(status_code=404, detail="Route not found")
111+
if resolve_user_role(request) != "admin":
112+
caller_id = request.headers.get(X_USER_ID_HEADER, "")
113+
if route.created_by is None or route.created_by != caller_id:
114+
raise HTTPException(
115+
status_code=403,
116+
detail="You can only modify routes you created",
117+
)
100118

101119

102120
def _route_node_to_read(rn: RouteNode) -> RouteNodeRead:
@@ -130,13 +148,58 @@ def _result_to_summary(result: RouteResult | None) -> RouteResultSummary | None:
130148
)
131149

132150

133-
def _route_to_read(route: Route, *, quality_avg: Any = _UNSET) -> RouteRead:
151+
def _profile_to_owner(profile: UserProfile) -> RouteOwner:
152+
"""Convert a UserProfile to the lightweight RouteOwner display schema."""
153+
return RouteOwner(
154+
user_id=profile.user_id,
155+
name=profile.name,
156+
callsign=profile.callsign,
157+
profile_id=profile.id,
158+
)
159+
160+
161+
def _resolve_owner(session: DbSession, created_by: str | None) -> UserProfile | None:
162+
"""Resolve a single ``created_by`` user_id to a UserProfile."""
163+
if not created_by:
164+
return None
165+
return session.execute(
166+
select(UserProfile).where(UserProfile.user_id == created_by)
167+
).scalar_one_or_none()
168+
169+
170+
def _resolve_owners_batch(
171+
session: DbSession, routes: list[Route]
172+
) -> dict[str, UserProfile]:
173+
"""Batch-resolve creator profiles for a list of routes (avoids N+1)."""
174+
owner_ids = {r.created_by for r in routes if r.created_by}
175+
if not owner_ids:
176+
return {}
177+
return {
178+
p.user_id: p
179+
for p in session.execute(
180+
select(UserProfile).where(UserProfile.user_id.in_(owner_ids))
181+
)
182+
.scalars()
183+
.all()
184+
}
185+
186+
187+
def _route_to_read(
188+
route: Route,
189+
*,
190+
quality_avg: Any = _UNSET,
191+
owner: UserProfile | None = None,
192+
) -> RouteRead:
134193
"""Serialize a Route to its list-level read schema.
135194
136195
``quality_avg`` defaults to the precomputed value persisted on
137196
``route.route_result.quality_avg`` (written by the background
138197
evaluator). Callers may pass an explicit value (e.g. ``None`` on
139198
create responses) to override.
199+
200+
``owner`` is the resolved UserProfile for ``route.created_by``, if
201+
any. Callers should pass it in to avoid per-row queries in list
202+
contexts (use ``_resolve_owners_batch``).
140203
"""
141204
if quality_avg is _UNSET:
142205
quality_avg = route.route_result.quality_avg if route.route_result else None
@@ -158,6 +221,8 @@ def _route_to_read(route: Route, *, quality_avg: Any = _UNSET) -> RouteRead:
158221
route_observers=[_route_observer_to_read(ro) for ro in route.route_observers],
159222
route_result=_result_to_summary(route.route_result),
160223
quality_avg=quality_avg,
224+
created_by=route.created_by,
225+
owner=_profile_to_owner(owner) if owner else None,
161226
created_at=route.created_at,
162227
updated_at=route.updated_at,
163228
)
@@ -257,22 +322,27 @@ def list_routes(
257322
max_level = get_max_visibility_level(role)
258323

259324
routes = session.execute(select(Route).order_by(Route.from_label)).scalars().all()
325+
visible = [r for r in routes if VISIBILITY_LEVELS.get(r.visibility, 0) <= max_level]
326+
owners_by_id = _resolve_owners_batch(session, visible)
260327
filtered = [
261-
_route_to_read(r)
262-
for r in routes
263-
if VISIBILITY_LEVELS.get(r.visibility, 0) <= max_level
328+
_route_to_read(
329+
r, owner=owners_by_id.get(r.created_by) if r.created_by else None
330+
)
331+
for r in visible
264332
]
265333
return RouteList(items=filtered, total=len(filtered))
266334

267335

268336
@router.post("", response_model=RouteRead, status_code=201)
269337
def create_route(
270-
__: RequireOperatorOrAdmin,
338+
caller: RequireOperatorOrAdmin,
271339
session: DbSession,
272340
body: RouteCreate,
273341
request: Request,
274342
) -> RouteRead:
275343
"""Create a new route (operator or admin)."""
344+
user_id, _ = caller
345+
get_or_create_profile(session, user_id, request)
276346
_assert_visibility_within_role(request, body.visibility)
277347
existing = session.execute(
278348
select(Route).where(
@@ -309,6 +379,7 @@ def create_route(
309379
max_path_length=body.max_path_length,
310380
enabled=body.enabled,
311381
reversible=body.reversible,
382+
created_by=user_id,
312383
)
313384
session.add(route)
314385
session.flush()
@@ -318,7 +389,8 @@ def create_route(
318389
session.refresh(route)
319390
_reevaluate_route(session, route)
320391
invalidate_routes(request)
321-
return _route_to_read(route, quality_avg=None)
392+
owner = _resolve_owner(session, route.created_by)
393+
return _route_to_read(route, quality_avg=None, owner=owner)
322394

323395

324396
@router.get("/{route_id}", response_model=RouteDetail)
@@ -374,7 +446,7 @@ def get_route(
374446
for oid, cnt in contributing.items()
375447
]
376448

377-
read = _route_to_read(route)
449+
read = _route_to_read(route, owner=_resolve_owner(session, route.created_by))
378450
return RouteDetail(
379451
**read.model_dump(),
380452
contributing_observers=contributors,
@@ -566,20 +638,31 @@ def get_route_history(
566638

567639
@router.put("/{route_id}", response_model=RouteRead)
568640
def update_route(
569-
__: RequireOperatorOrAdmin,
641+
caller: RequireOperatorOrAdmin,
570642
session: DbSession,
571643
route_id: str,
572644
body: RouteUpdate,
573645
request: Request,
574646
) -> RouteRead:
575-
"""Update a route (operator or admin)."""
647+
"""Update a route (operator or admin).
648+
649+
Operators may only modify routes they created; admins can modify any
650+
route. Admins claim ownership of legacy (unowned) routes on edit but
651+
do not displace an existing creator.
652+
"""
653+
user_id, _ = caller
576654
route = session.execute(
577655
select(Route).where(Route.id == route_id)
578656
).scalar_one_or_none()
579657
if not route:
580658
raise HTTPException(status_code=404, detail="Route not found")
581659
_assert_route_modifiable(request, route)
582660

661+
# Admin claims ownership of legacy (unowned) routes on edit
662+
if resolve_user_role(request) == "admin" and route.created_by is None:
663+
route.created_by = user_id
664+
logger.info("Admin %s claimed ownership of legacy route %s", user_id, route.id)
665+
583666
if body.from_label is not None or body.to_label is not None:
584667
new_from = body.from_label if body.from_label is not None else route.from_label
585668
new_to = body.to_label if body.to_label is not None else route.to_label
@@ -636,7 +719,8 @@ def update_route(
636719
session.refresh(route)
637720
_reevaluate_route(session, route)
638721
invalidate_routes(request)
639-
return _route_to_read(route)
722+
owner = _resolve_owner(session, route.created_by)
723+
return _route_to_read(route, owner=owner)
640724

641725

642726
@router.delete("/{route_id}", status_code=204)

src/meshcore_hub/common/models/route.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,10 @@ class Route(Base, UUIDMixin, TimestampMixin):
103103
server_default="true",
104104
nullable=False,
105105
)
106+
created_by: Mapped[Optional[str]] = mapped_column(
107+
String(255),
108+
nullable=True,
109+
)
106110

107111
route_nodes: Mapped[list["RouteNode"]] = relationship(
108112
"RouteNode",

0 commit comments

Comments
 (0)