diff --git a/drb-c2-core/app/routers/users.py b/drb-c2-core/app/routers/users.py index 0cbcbcd..318a259 100644 --- a/drb-c2-core/app/routers/users.py +++ b/drb-c2-core/app/routers/users.py @@ -6,6 +6,7 @@ from pydantic import BaseModel from firebase_admin import auth as firebase_auth from app.internal.auth import require_admin_token from app.internal import firestore as fstore +from app.internal.tenancy import FOUNDING_ORG_ID from app.internal import audit router = APIRouter(prefix="/admin/users", tags=["users"]) @@ -22,9 +23,13 @@ class UserCreate(BaseModel): role: str = "viewer" display_name: Optional[str] = None owned_node_ids: list[str] = [] + # Org the new user joins as a member. Defaults to the creating admin's own + # org — without one, firestore.rules lets the user read nothing at all. + org_id: Optional[str] = None class UserUpdate(BaseModel): + org_id: Optional[str] = None # attach an org-less user; defaults to the admin's org role: Optional[str] = None owned_node_ids: Optional[list[str]] = None display_name: Optional[str] = None @@ -34,6 +39,32 @@ class UserUpdate(BaseModel): # Helpers # --------------------------------------------------------------------------- +async def _resolve_org(requested: Optional[str], decoded: dict) -> str: + """The org a user created/edited here belongs to: the one asked for, else + the acting admin's own. Every read the frontend makes is gated on the + org_id claim (firestore.rules inOrg()), so a user without one sees no + incidents, calls or nodes — which is how admin-created viewers came out + before this existed.""" + # A platform admin needn't have an org claim (isPlatformAdmin reads every + # org), so fall back to the founding org every pre-tenancy node, call and + # incident was stamped with (app/internal/tenancy.py). + org_id = requested or decoded.get("org_id") or FOUNDING_ORG_ID + if not await fstore.doc_get("organizations", org_id): + raise HTTPException(400, f"Organization '{org_id}' does not exist.") + return org_id + + +async def _write_membership(uid: str, email: Optional[str], org_id: str) -> None: + """Same org_members shape as POST /auth/signup (routers/links.py).""" + await fstore.doc_set("org_members", uid, { + "uid": uid, + "org_id": org_id, + "org_role": "member", + "email": email, + "added_at": datetime.now(timezone.utc).isoformat(), + }, merge=False) + + def _ms_to_iso(ms: Optional[int]) -> Optional[str]: if ms is None: return None @@ -101,6 +132,7 @@ async def create_user(body: UserCreate, decoded: dict = Depends(require_admin_to raise HTTPException(400, f"Invalid role. Must be one of: {', '.join(sorted(VALID_ROLES))}") if body.role == "operator" and not body.owned_node_ids: raise HTTPException(400, "Operator role requires at least one owned node.") + org_id = await _resolve_org(body.org_id, decoded) try: fb_user: firebase_auth.UserRecord = await asyncio.to_thread( @@ -115,10 +147,11 @@ async def create_user(body: UserCreate, decoded: dict = Depends(require_admin_to raise HTTPException(400, f"Failed to create user: {e}") # Set custom claims - claims: dict = {"role": body.role, "owned_node_ids": body.owned_node_ids} + claims: dict = {"role": body.role, "owned_node_ids": body.owned_node_ids, "org_id": org_id, "org_role": "member"} if body.role == "admin": claims["admin"] = True await asyncio.to_thread(firebase_auth.set_custom_user_claims, fb_user.uid, claims) + await _write_membership(fb_user.uid, body.email, org_id) # Write Firestore profile now = datetime.now(timezone.utc).isoformat() @@ -145,7 +178,7 @@ async def create_user(body: UserCreate, decoded: dict = Depends(require_admin_to action="user.create", target_uid=fb_user.uid, target_email=body.email, - details={"role": body.role, "owned_node_ids": body.owned_node_ids}, + details={"role": body.role, "owned_node_ids": body.owned_node_ids, "org_id": org_id}, ) return {**_format_user(fb_user), "invite_link": invite_link} @@ -197,7 +230,20 @@ async def update_user(uid: str, body: UserUpdate, decoded: dict = Depends(requir else: new_claims.pop("admin", None) + # Heal users created before POST /users set an org (they could read + # nothing): any edit attaches them to the requested/admin's org. An + # existing org is never silently moved. + attached_org: Optional[str] = None + if not existing_claims.get("org_id"): + attached_org = await _resolve_org(body.org_id, decoded) + new_claims["org_id"] = attached_org + new_claims["org_role"] = "member" + elif body.org_id and body.org_id != existing_claims["org_id"]: + raise HTTPException(400, "User already belongs to another org; moving orgs isn't supported here.") + await asyncio.to_thread(firebase_auth.set_custom_user_claims, uid, new_claims) + if attached_org: + await _write_membership(uid, fb_user.email, attached_org) if body.display_name is not None: await asyncio.to_thread(firebase_auth.update_user, uid, display_name=body.display_name) @@ -218,6 +264,7 @@ async def update_user(uid: str, body: UserUpdate, decoded: dict = Depends(requir "new_role": new_role, "old_nodes": current_nodes, "new_nodes": new_nodes, + **({"attached_org_id": attached_org} if attached_org else {}), }, ) diff --git a/drb-c2-core/tests/test_users_org.py b/drb-c2-core/tests/test_users_org.py new file mode 100644 index 0000000..7861e85 --- /dev/null +++ b/drb-c2-core/tests/test_users_org.py @@ -0,0 +1,85 @@ +""" +Admin-created users must land in an org. firestore.rules gates every read on +the org_id claim, so a viewer created via POST /admin/users without one saw no +incidents or calls at all (reported 2026-09-27). +""" +from types import SimpleNamespace +from unittest.mock import AsyncMock, patch + +from fastapi.testclient import TestClient + +from app.main import app +from app.internal.auth import require_admin_token +from app.routers import users + +client = TestClient(app) +ADMIN = {"uid": "admin-1", "email": "a@x", "role": "admin", "org_id": "org-A"} + + +def _as(decoded): + app.dependency_overrides[require_admin_token] = lambda: decoded + + +def teardown_function(): + app.dependency_overrides.pop(require_admin_token, None) + + +def _fb(**kw): + base = dict(uid="u1", email="v@x", display_name="", custom_claims={}, disabled=False, + email_verified=False, user_metadata=SimpleNamespace(creation_timestamp=0, last_sign_in_timestamp=None)) + return SimpleNamespace(**{**base, **kw}) + + +def _run(method, path, body, fb_user, orgs=("org-A", "founding")): + fa = users.firebase_auth + with patch.object(fa, "create_user", return_value=fb_user, create=True), \ + patch.object(fa, "get_user", return_value=fb_user, create=True), \ + patch.object(fa, "set_custom_user_claims", create=True) as set_claims, \ + patch.object(fa, "generate_password_reset_link", return_value="link", create=True), \ + patch.object(users.fstore, "doc_get", AsyncMock(side_effect=lambda c, i: {"org_id": i} if c == "organizations" and i in orgs else None)), \ + patch.object(users.fstore, "doc_set", AsyncMock()) as doc_set, \ + patch.object(users.audit, "write_audit", AsyncMock()): + resp = getattr(client, method)(path, json=body) + members = [c for c in doc_set.await_args_list if c.args[0] == "org_members"] + return resp, set_claims, members + + +def test_created_viewer_joins_the_admins_org_as_member(): + _as(ADMIN) + resp, set_claims, members = _run("post", "/admin/users", {"email": "v@x", "role": "viewer"}, _fb()) + assert resp.status_code == 200, resp.text + claims = set_claims.call_args.args[1] + assert (claims["org_id"], claims["org_role"], claims["role"]) == ("org-A", "member", "viewer") + assert members and members[0].args[2]["org_id"] == "org-A" + + +def test_admin_without_an_org_claim_defaults_to_founding(): + _as({k: v for k, v in ADMIN.items() if k != "org_id"}) + resp, set_claims, _ = _run("post", "/admin/users", {"email": "v@x", "role": "viewer"}, _fb()) + assert resp.status_code == 200, resp.text + assert set_claims.call_args.args[1]["org_id"] == "founding" + + +def test_unknown_org_is_rejected(): + _as(ADMIN) + resp, set_claims, _ = _run("post", "/admin/users", {"email": "v@x", "role": "viewer", "org_id": "nope"}, _fb()) + assert resp.status_code == 400 + set_claims.assert_not_called() + + +def test_editing_an_orgless_user_heals_them(): + _as(ADMIN) + resp, set_claims, members = _run("patch", "/admin/users/u1", {"role": "viewer"}, _fb(custom_claims={"role": "viewer"})) + assert resp.status_code == 200, resp.text + assert set_claims.call_args.args[1]["org_id"] == "org-A" + assert members + + +def test_editing_never_silently_moves_an_existing_org(): + _as(ADMIN) + fb = _fb(custom_claims={"role": "viewer", "org_id": "org-B", "org_role": "member"}) + resp, set_claims, members = _run("patch", "/admin/users/u1", {"role": "viewer"}, fb) + assert resp.status_code == 200 + assert set_claims.call_args.args[1]["org_id"] == "org-B" + assert not members + assert _run("patch", "/admin/users/u1", {"org_id": "org-A"}, fb)[0].status_code == 400