Skip to content

Commit 09ea979

Browse files
authored
fix(auth): propagate invite role to users.role + recognise FO-1b admin taxonomy (#253)
Multi-layer mismatch was silently 403'ing freshly-bootstrapped founders on every admin-gated route. Trace: 1. INSERT_USER / SELECT_USER_BY_USERNAME didn't touch the users.role column. New rows got the DB default ('user'); reads dropped the column entirely, so /auth/me always reported "role": "user" even when the row was correct. 2. ensure_user (the claim helper) didn't forward the invite role to store.create_user. Even with the storage layer fixed, the founder bootstrap path stamped role='user' for every claimer. 3. require_admin matched only the legacy role string 'admin'. The FO-1b invite taxonomy uses 'enterprise_admin' / 'l2_admin' / 'user' — so a correctly-rolled enterprise_admin user still 403'd on every admin route. Fix: thread role through INSERT_USER + SELECT_USER_BY_USERNAME + store.create_user + ensure_user + claim_invite_route. Centralise the admin-equivalence check in auth.is_admin_role() and use it in require_admin plus the five existing `role == "admin"` sites in crosstalk_routes + activity_routes so the legacy and new taxonomies stay in sync. New test: test_claim_persists_invite_role — invites a user with role=enterprise_admin, claims it, asserts /auth/me reports the correct role. Prevents the regression that produced this bug.
1 parent d858a25 commit 09ea979

8 files changed

Lines changed: 81 additions & 19 deletions

File tree

server/backend/src/cq_server/activity_routes.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
from pydantic import BaseModel
3838

3939
from .activity import EVENT_TYPES
40-
from .auth import get_current_user, scope_filter
40+
from .auth import get_current_user, is_admin_role, scope_filter
4141
from .deps import get_store
4242
from .store._sqlite import SqliteStore
4343

@@ -170,7 +170,7 @@ async def list_activity(
170170
# - admin: pass through whatever they sent (including None for
171171
# "all personas in this Enterprise")
172172
# - non-admin: pin to their own username regardless of input
173-
if role == "admin":
173+
if is_admin_role(role):
174174
effective_persona = persona
175175
else:
176176
effective_persona = username

server/backend/src/cq_server/auth.py

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,27 @@ async def get_current_user(
381381
return caller.username
382382

383383

384+
# Roles that grant admin-equivalent privilege.
385+
# - ``admin`` — legacy global admin (pre-FO-1b)
386+
# - ``enterprise_admin`` — Enterprise-scoped admin (FO-1b invite taxonomy)
387+
# - ``l2_admin`` — single-L2-scoped admin (FO-1b invite taxonomy)
388+
# All three pass ``require_admin`` until per-Enterprise / per-L2 scoping
389+
# lands. Without this set the founder bootstrap path produces a
390+
# role=enterprise_admin user who then 403s on every admin route, since
391+
# require_admin only matched the legacy string ``admin``.
392+
_ADMIN_ROLES: frozenset[str] = frozenset({"admin", "enterprise_admin", "l2_admin"})
393+
394+
395+
def is_admin_role(role: str | None) -> bool:
396+
"""Return True when ``role`` grants admin-equivalent privilege.
397+
398+
Centralises the legacy/new role mapping so callers in
399+
crosstalk_routes / activity_routes / etc. don't drift apart from
400+
``require_admin``.
401+
"""
402+
return role in _ADMIN_ROLES if role is not None else False
403+
404+
384405
async def require_admin(
385406
request: Request,
386407
username: str = Depends(get_current_user),
@@ -391,12 +412,14 @@ async def require_admin(
391412
Returns the username on success; 401 on missing/invalid JWT (raised
392413
by the chained ``get_current_user`` dep), 403 when the caller is
393414
authenticated but not an admin. Admin-ness is global in v1 — there
394-
is no per-Enterprise scoping yet (see plan doc, Lane D).
415+
is no per-Enterprise scoping yet (see plan doc, Lane D). Accepts
416+
the legacy ``admin`` role plus the FO-1b invite roles
417+
``enterprise_admin`` / ``l2_admin`` via ``_ADMIN_ROLES``.
395418
"""
396419
user = await store.get_user(username)
397420
if user is None:
398421
raise HTTPException(status_code=401, detail="User not found")
399-
if user.get("role") != "admin":
422+
if not is_admin_role(user.get("role")):
400423
raise HTTPException(status_code=403, detail="Admin role required")
401424
return username
402425

server/backend/src/cq_server/crosstalk_routes.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@
5858
from pydantic import BaseModel, Field
5959

6060
from .activity_logger import log_activity
61-
from .auth import get_current_user, scope_filter
61+
from .auth import get_current_user, is_admin_role, scope_filter
6262
from .deps import get_store
6363
from .store._sqlite import SqliteStore
6464

@@ -388,7 +388,7 @@ async def reply_on_thread(
388388
raise HTTPException(status_code=404, detail="Thread not found")
389389
if thread["status"] != "open":
390390
raise HTTPException(status_code=409, detail="Thread is closed")
391-
if username not in thread["participants"] and role != "admin":
391+
if username not in thread["participants"] and not is_admin_role(role):
392392
raise HTTPException(status_code=403, detail="Not a participant")
393393

394394
# Pick the recipient — for two-party threads, it's the other participant.
@@ -459,7 +459,7 @@ async def list_threads(
459459
username=username,
460460
tenant_enterprise=read_ent,
461461
tenant_group=read_grp,
462-
is_admin=(role == "admin"),
462+
is_admin=is_admin_role(role),
463463
limit=limit,
464464
)
465465
items = [ThreadSummary(**r) for r in rows]
@@ -480,7 +480,7 @@ async def get_thread(
480480
thread = await store.get_crosstalk_thread(thread_id=thread_id, tenant_enterprise=read_ent, tenant_group=read_grp)
481481
if thread is None:
482482
raise HTTPException(status_code=404, detail="Thread not found")
483-
if username not in thread["participants"] and role != "admin":
483+
if username not in thread["participants"] and not is_admin_role(role):
484484
raise HTTPException(status_code=403, detail="Not a participant")
485485

486486
msgs = await store.list_crosstalk_messages(
@@ -510,7 +510,7 @@ async def close_thread(
510510
thread = await store.get_crosstalk_thread(thread_id=thread_id, tenant_enterprise=read_ent, tenant_group=read_grp)
511511
if thread is None:
512512
raise HTTPException(status_code=404, detail="Thread not found")
513-
if username not in thread["participants"] and role != "admin":
513+
if username not in thread["participants"] and not is_admin_role(role):
514514
raise HTTPException(status_code=403, detail="Not a participant")
515515

516516
won = await store.close_crosstalk_thread(

server/backend/src/cq_server/invite_routes.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,7 @@ async def claim_invite_route(
393393
username=canonical_username,
394394
password=request.password,
395395
email=metadata.email,
396+
role=metadata.role,
396397
)
397398

398399
outcome = claim_invite(store, token=token, claiming_user_id=user_id)

server/backend/src/cq_server/invites.py

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -413,13 +413,19 @@ async def ensure_user(
413413
username: str,
414414
password: str,
415415
email: str,
416+
role: str = "user",
416417
) -> int:
417418
"""Create-or-fetch the user record for an invite-claimer.
418419
419420
If a user with this ``username`` already exists, the existing row's
420-
id is returned (no password rotation, no email mutation — that's
421-
out of scope for FO-1b). Otherwise we hash the password and insert,
422-
then return the new user's id.
421+
id is returned (no password rotation, no email mutation, no role
422+
rotation — that's out of scope for FO-1b). Otherwise we hash the
423+
password, insert with the supplied role, and return the new user's id.
424+
425+
The ``role`` argument comes from the invite (FO-1b's role taxonomy:
426+
``enterprise_admin`` / ``l2_admin`` / ``user``). Without this wiring
427+
the founder bootstrap path produced a ``role='user'`` admin who then
428+
got 403'd from every admin-gated endpoint.
423429
424430
The ``email`` argument is currently unused by the create path
425431
(FO-1a's ``users.email`` column is additive but ``create_user``
@@ -431,7 +437,7 @@ async def ensure_user(
431437
existing = await store.get_user(username)
432438
if existing is not None:
433439
return int(existing["id"])
434-
await store.create_user(username, hash_password(password))
440+
await store.create_user(username, hash_password(password), role=role)
435441
fresh = await store.get_user(username)
436442
if fresh is None: # pragma: no cover — race that breaks the world
437443
raise RuntimeError("user creation succeeded but lookup returned None")

server/backend/src/cq_server/store/_queries.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -225,11 +225,12 @@ def select_list_units(
225225
# --- users ------------------------------------------------------------------
226226

227227
INSERT_USER: TextClause = text(
228-
"INSERT INTO users (username, password_hash, created_at) VALUES (:username, :password_hash, :created_at)"
228+
"INSERT INTO users (username, password_hash, role, created_at) "
229+
"VALUES (:username, :password_hash, :role, :created_at)"
229230
)
230231

231232
SELECT_USER_BY_USERNAME: TextClause = text(
232-
"SELECT id, username, password_hash, created_at FROM users WHERE username = :username"
233+
"SELECT id, username, password_hash, role, created_at FROM users WHERE username = :username"
233234
)
234235

235236
# --- api_keys ---------------------------------------------------------------

server/backend/src/cq_server/store/_sqlite.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -191,8 +191,8 @@ async def create_api_key(
191191
expires_at=expires_at,
192192
)
193193

194-
async def create_user(self, username: str, password_hash: str) -> None:
195-
await self._run_sync(self._create_user_sync, username, password_hash)
194+
async def create_user(self, username: str, password_hash: str, role: str = "user") -> None:
195+
await self._run_sync(self._create_user_sync, username, password_hash, role)
196196

197197
# --- WebAuthn / passkey credentials (FO-1a, #191) ---------------------
198198

@@ -1580,15 +1580,20 @@ def _create_api_key_sync(
15801580
"revoked_at": None,
15811581
}
15821582

1583-
def _create_user_sync(self, username: str, password_hash: str) -> None:
1583+
def _create_user_sync(self, username: str, password_hash: str, role: str = "user") -> None:
15841584
from ._queries import INSERT_USER
15851585

15861586
created_at = datetime.now(UTC).isoformat()
15871587
try:
15881588
with self._engine.begin() as conn:
15891589
conn.execute(
15901590
INSERT_USER,
1591-
{"username": username, "password_hash": password_hash, "created_at": created_at},
1591+
{
1592+
"username": username,
1593+
"password_hash": password_hash,
1594+
"role": role,
1595+
"created_at": created_at,
1596+
},
15921597
)
15931598
except IntegrityError as e:
15941599
if e.orig is not None:

server/backend/tests/test_invites.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -449,6 +449,32 @@ def test_get_metadata_then_claim(
449449
assert "cq_session" in claim_resp.cookies
450450
assert claim_resp.cookies["cq_session"] == claim_body["token"]
451451

452+
def test_claim_persists_invite_role(
453+
self,
454+
client: TestClient,
455+
mock_sender: MockEmailSender,
456+
) -> None:
457+
# The invite role (enterprise_admin / l2_admin / user) must round-trip
458+
# to users.role — otherwise the founder bootstrap path produces a
459+
# role=user admin who 403s on every admin-gated route.
460+
client.post(
461+
"/api/v1/admin/invites",
462+
json={"email": "founder@example.com", "role": "enterprise_admin"},
463+
headers=_admin_headers(client),
464+
)
465+
token = mock_sender.sent[0].jwt
466+
467+
claim_resp = client.post(
468+
f"/api/v1/invites/{token}/claim",
469+
json={"password": "password123"},
470+
)
471+
assert claim_resp.status_code == 200, claim_resp.text
472+
473+
# /auth/me must reflect the enterprise_admin role.
474+
me_resp = client.get("/api/v1/auth/me")
475+
assert me_resp.status_code == 200, me_resp.text
476+
assert me_resp.json()["role"] == "enterprise_admin"
477+
452478
def test_double_claim_returns_409(
453479
self,
454480
client: TestClient,

0 commit comments

Comments
 (0)