diff --git a/core/switch_core/authz.py b/core/switch_core/authz.py index 9c65903f8..f21eb4947 100644 --- a/core/switch_core/authz.py +++ b/core/switch_core/authz.py @@ -8,8 +8,14 @@ - The subject of every decision is a **user** (`Principal`). Agent-initiated requests resolve to the agent's owner (see the protocol service's `_resolve_acting_identity`); an agent inherits exactly its owner's - permissions. - - `User.role == "admin"` is a global bypass. + permissions in the tenant the request is bound to — never more than the + owner holds there. + - `Principal.is_admin` means "may administer the tenant bound to this + request" — the deployment operator bypass (`User.role == "admin"`, global, + never granted by anything self-service) OR an `owner`/`admin` membership + row for this user in the bound tenant (`tenant_members.role`, granted per + tenant, e.g. by creating it). `administers_tenant` below computes it; a + caller with a database session should use that rather than re-deriving it. - Owned entities carry a nullable owner plus independent `read_visibility` and `write_visibility` ("public" | "private"). @@ -38,12 +44,30 @@ class Principal: `id` may be ``None`` for an agent with no owner; such a principal owns nothing and is limited to public access. + + `is_admin` is tenant-scoped, not global — see `administers_tenant`. """ id: str | None is_admin: bool +# `tenant_members.role` is a checked string, not an enum type (`db/models.py`). +TENANT_ADMIN_ROLES = ("owner", "admin") + + +def administers_tenant(*, is_operator: bool, tenant_role: str | None) -> bool: + """Whether a caller may administer the tenant they hold `tenant_role` in. + + True for a deployment operator (the global bypass), regardless of + membership — an operator administers every tenant. Otherwise true only if + the membership role itself is `owner` or `admin`; a plain `member`, or no + membership row at all, is false. This is the one function that should ever + turn a role into an admin bit — see `Principal.is_admin`. + """ + return is_operator or tenant_role in TENANT_ADMIN_ROLES + + @runtime_checkable class Authorizable(Protocol): """Structural interface for any entity `can` arbitrates over. diff --git a/core/switch_core/bridges/agent/protocol/service.py b/core/switch_core/bridges/agent/protocol/service.py index cac0e1859..14d87b6c9 100644 --- a/core/switch_core/bridges/agent/protocol/service.py +++ b/core/switch_core/bridges/agent/protocol/service.py @@ -2308,13 +2308,9 @@ async def create_moderation_room( ) async with self.session_factory() as session: - agent = await self.agent_store.get(session, agent_id) - if agent is None: - raise ValueError(f"Unknown agent: {agent_id}") - owner_is_admin = False - if agent.owner_id is not None: - owner = await session.get(User, agent.owner_id) - owner_is_admin = owner is not None and owner.role == "admin" + agent, _owner_id, owner_is_admin = await self._resolve_acting_identity( + session, agent_id + ) group_id = await self._resolve_group_name(session, group_name) if (reference_ids or package_ids) and agent.owner_id is None: raise ValueError( @@ -2463,6 +2459,13 @@ async def _resolve_acting_identity( Returns ``(agent, owner_id, owner_is_admin)``. Used by moderation methods that perform resource-access checks on the agent's behalf. + + ``owner_is_admin`` is the owner's tenant-scoped administrative bit + (``UserStore.administers``), not the global operator flag alone: the + agent must not gain more than its owner holds in the tenant this + request is bound to. `session` is already scoped there by the time an + agent-bridge request reaches this method, so the read is the owner's + membership in the same tenant the agent itself belongs to. """ agent = await self.agent_store.get(session, agent_id) if agent is None: @@ -2470,7 +2473,9 @@ async def _resolve_acting_identity( owner_is_admin = False if agent.owner_id is not None: owner = await session.get(User, agent.owner_id) - owner_is_admin = owner is not None and owner.role == "admin" + owner_is_admin = owner is not None and await self.user_store.administers( + session, owner + ) return agent, agent.owner_id, owner_is_admin async def _require_room_action( diff --git a/core/switch_core/db/stores/user_store.py b/core/switch_core/db/stores/user_store.py index a0feea59f..3506b9741 100644 --- a/core/switch_core/db/stores/user_store.py +++ b/core/switch_core/db/stores/user_store.py @@ -6,12 +6,14 @@ from sqlalchemy.exc import IntegrityError from sqlalchemy.ext.asyncio import AsyncSession +from switch_core.authz import administers_tenant from switch_core.db.models import ( OidcIdentity, TenantMember, User, require_tenant_id, ) +from switch_core.tenant_context import current_tenant_id logger = logging.getLogger(__name__) @@ -297,3 +299,40 @@ async def _link_identity( async def get_all(self, session: AsyncSession) -> list[User]: result = await session.execute(select(User)) return list(result.scalars().all()) + + async def tenant_role( + self, session: AsyncSession, tenant_id: str, user_id: str + ) -> str | None: + """`user_id`'s membership role in `tenant_id`, or None if not a member. + + `TenantMember` is addressed by its whole primary key, so this is a + plain `session.get` rather than a query — same shape as + `ensure_membership`'s write. + """ + membership = await session.get(TenantMember, (tenant_id, user_id)) + return membership.role if membership is not None else None + + async def administers(self, session: AsyncSession, user: User) -> bool: + """Whether `user` may administer the tenant bound to `session`'s + context — the operator bypass, or an owner/admin membership in it. + + See `authz.administers_tenant`, which this composes with a read of the + one membership row that can answer "in *this* tenant". + + Raises: + RuntimeError: no tenant is bound. The question has no + tenant-independent answer: half of it is a membership row that + cannot be read without one. Answering on the operator bit + alone would quietly demote a workspace owner to a plain + member, and the symptom — a 403 on their own workspace — says + nothing about why. + """ + tenant_id = current_tenant_id() + if tenant_id is None: + raise RuntimeError( + "administers requires a bound tenant; whether someone may " + "administer a workspace is only answerable about a particular " + "one" + ) + role = await self.tenant_role(session, tenant_id, user.id) + return administers_tenant(is_operator=user.role == "admin", tenant_role=role) diff --git a/core/switch_core/gateway/agents.py b/core/switch_core/gateway/agents.py index e5316c2e5..a2033b826 100644 --- a/core/switch_core/gateway/agents.py +++ b/core/switch_core/gateway/agents.py @@ -26,7 +26,7 @@ from switch_core.db.stores.agent_store import AgentStore from switch_core.db.stores.room_store import RoomStore from switch_core.db.stores.user_store import UserStore -from switch_core.gateway.auth import get_current_user +from switch_core.gateway.auth import get_current_user, get_tenant_is_admin from switch_core.gateway.dependencies import ( get_agent_store, get_protocol, @@ -74,12 +74,16 @@ async def delete_agent_by_name( agent_store: Annotated[AgentStore, Depends(get_agent_store)], protocol: Annotated[ProtocolService, Depends(get_protocol)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> dict[str, bool]: agent = await agent_store.get_by_name(session, agent_name) if agent is None: raise HTTPException(status_code=404, detail=f"Agent not found: {agent_name}") try: - require_manage(Principal(user.id, user.role == "admin"), agent.owner_id) + require_manage( + Principal(user.id, is_admin), + agent.owner_id, + ) except PermissionError: raise HTTPException( status_code=403, @@ -101,12 +105,16 @@ async def delete_agent( agent_store: Annotated[AgentStore, Depends(get_agent_store)], protocol: Annotated[ProtocolService, Depends(get_protocol)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> dict[str, bool]: agent = await agent_store.get(session, agent_id) if agent is None: raise HTTPException(status_code=404, detail=f"Agent not found: {agent_id}") try: - require_manage(Principal(user.id, user.role == "admin"), agent.owner_id) + require_manage( + Principal(user.id, is_admin), + agent.owner_id, + ) except PermissionError: raise HTTPException( status_code=403, @@ -192,6 +200,7 @@ async def register_known_subagents( protocol: Annotated[ProtocolService, Depends(get_protocol)], session: Annotated[AsyncSession, Depends(get_session)], agent_store: Annotated[AgentStore, Depends(get_agent_store)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> RegisterKnownSubagentsResponse: """Register many Claude Code subagents under one parent agent (session-authed). @@ -216,7 +225,10 @@ async def register_known_subagents( status_code=404, detail=f"Parent agent not found: {req.parent_agent_id}" ) try: - require_manage(Principal(user.id, user.role == "admin"), parent.owner_id) + require_manage( + Principal(user.id, is_admin), + parent.owner_id, + ) except PermissionError as exc: raise HTTPException( status_code=403, @@ -299,6 +311,7 @@ async def update_agent_options( session: Annotated[AsyncSession, Depends(get_session)], agent_store: Annotated[AgentStore, Depends(get_agent_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> AgentSummary: """Replace a known-agent's options. @@ -317,7 +330,10 @@ async def update_agent_options( raise HTTPException(status_code=404, detail=f"Agent not found: {agent_id}") try: - require_manage(Principal(user.id, user.role == "admin"), agent.owner_id) + require_manage( + Principal(user.id, is_admin), + agent.owner_id, + ) except PermissionError: raise HTTPException( status_code=403, @@ -351,6 +367,7 @@ async def update_agent_icon( session: Annotated[AsyncSession, Depends(get_session)], agent_store: Annotated[AgentStore, Depends(get_agent_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> AgentSummary: """Set, change, or clear an agent's icon (CHOO-2171). @@ -370,7 +387,10 @@ async def update_agent_icon( raise HTTPException(status_code=404, detail=f"Agent not found: {agent_id}") try: - require_manage(Principal(user.id, user.role == "admin"), agent.owner_id) + require_manage( + Principal(user.id, is_admin), + agent.owner_id, + ) except PermissionError: raise HTTPException( status_code=403, @@ -404,6 +424,7 @@ async def update_agent_display_name( session: Annotated[AsyncSession, Depends(get_session)], agent_store: Annotated[AgentStore, Depends(get_agent_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> AgentSummary: """Set, change, or clear an agent's human display name. @@ -420,7 +441,10 @@ async def update_agent_display_name( raise HTTPException(status_code=404, detail=f"Agent not found: {agent_id}") try: - require_manage(Principal(user.id, user.role == "admin"), agent.owner_id) + require_manage( + Principal(user.id, is_admin), + agent.owner_id, + ) except PermissionError: raise HTTPException( status_code=403, @@ -498,6 +522,7 @@ async def update_addressing_policy( user_store: Annotated[UserStore, Depends(get_user_store)], protocol: Annotated[ProtocolService, Depends(get_protocol)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> AgentDetail: """Set or clear an agent's scoped addressing policy (CHOO-1585). @@ -509,7 +534,10 @@ async def update_addressing_policy( raise HTTPException(status_code=404, detail=f"Agent not found: {agent_id}") try: - require_manage(Principal(user.id, user.role == "admin"), agent.owner_id) + require_manage( + Principal(user.id, is_admin), + agent.owner_id, + ) except PermissionError: raise HTTPException( status_code=403, diff --git a/core/switch_core/gateway/auth.py b/core/switch_core/gateway/auth.py index 61e14a694..2b9e7ccea 100644 --- a/core/switch_core/gateway/auth.py +++ b/core/switch_core/gateway/auth.py @@ -234,17 +234,60 @@ async def get_current_user( async def require_admin( user: Annotated[User, Depends(get_current_user)], ) -> User: + """Raise 403 unless `user` is a deployment operator. + + ``User.role == "admin"`` is deliberately global and deliberately not + self-service: it names the person who runs the server, not a role any + tenant can grant. Gate deployment-wide actions on this — creating or + listing every user in the deployment — never a single tenant's resources. + Those use ``require_tenant_admin``. + """ if user.role != "admin": raise HTTPException(status_code=403, detail="Admin access required") return user +async def get_tenant_is_admin( + session: Annotated[AsyncSession, Depends(get_session)], + user: Annotated[User, Depends(get_current_user)], + user_store: Annotated[UserStore, Depends(get_user_store)], +) -> bool: + """Whether `user` may administer the tenant this request is bound to. + + This is the boolean every ``Principal.is_admin`` in a gateway request + should be built from — see ``UserStore.administers`` for what it actually + checks. + + Also callable directly (not just as a FastAPI dependency) from any + handler or service function that already holds a bound `session`, the + caller's `User`, and a `UserStore`. + """ + return await user_store.administers(session, user) + + +async def require_tenant_admin( + user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], +) -> User: + """Raise 403 unless `user` may administer the tenant this request is bound to. + + Unlike ``require_admin``, this is granted by ``tenant_members.role`` as + well as the operator bit — use it for routes that manage one tenant's + resources (a collaboration bridge, a room) rather than the deployment + itself. + """ + if not is_admin: + raise HTTPException(status_code=403, detail="Tenant admin access required") + return user + + async def require_room_access( session: AsyncSession, room_store: RoomStore, room_id: str, user: User, action: Action, + is_admin: bool, ) -> Room: """Load a room (404 if missing) and authorize `action` for `user`, raising HTTP 403 if denied. @@ -253,12 +296,21 @@ async def require_room_access( id (attaching references, linking rooms, …) so they cannot operate on a room the caller lacks access to. Mirrors the protocol layer's ``_require_room_action``. + + `is_admin` comes from ``get_tenant_is_admin``; it is a parameter rather + than a read of its own so that a route cannot end up authorizing against + a different admin bit than the one its own dependencies resolved. + + This is the widest of the tenant-admin gates: `authz.can` short-circuits + on the admin bit, so a workspace owner or admin passes it for every room + in their workspace, private ones included. See the phase 2 design note on + what that grant covers. """ room = await room_store.get(session, room_id) if room is None: raise HTTPException(status_code=404, detail="Room not found") try: - require(Principal(user.id, user.role == "admin"), action, room) + require(Principal(user.id, is_admin), action, room) except PermissionError as e: raise HTTPException(status_code=403, detail=str(e)) from e return room diff --git a/core/switch_core/gateway/collaborations.py b/core/switch_core/gateway/collaborations.py index 243d5066c..902a5c974 100644 --- a/core/switch_core/gateway/collaborations.py +++ b/core/switch_core/gateway/collaborations.py @@ -21,7 +21,11 @@ from switch_core.db.stores.external_user_store import ExternalUserStore from switch_core.db.stores.room_store import RoomStore from switch_core.db.stores.user_store import UserStore -from switch_core.gateway.auth import get_current_user, require_admin +from switch_core.gateway.auth import ( + get_current_user, + get_tenant_is_admin, + require_tenant_admin, +) from switch_core.gateway.dependencies import ( get_bridge_store, get_collab_lifecycle, @@ -164,7 +168,7 @@ async def create_bridge( # Admin-only: a bridge is an unowned, workspace-wide integration holding # platform secrets, so there is no owner to scope to (unlike connectors, # whose authz is owner-or-admin) — registering one is an admin action. - _user: Annotated[User, Depends(require_admin)], + _user: Annotated[User, Depends(require_tenant_admin)], ) -> BridgeDetail: try: bridge = await collab_lifecycle.register( @@ -202,7 +206,7 @@ async def set_default_bridge( collab_lifecycle: Annotated[ CollaborationBridgeLifecycleService, Depends(get_collab_lifecycle) ], - _user: Annotated[User, Depends(require_admin)], + _user: Annotated[User, Depends(require_tenant_admin)], ) -> BridgeDetail: """Nominate a bridge as the instance default, demoting the previous one.""" try: @@ -253,7 +257,7 @@ async def update_bridge( ], # Admin-only for the same reason as registering one: a bridge is an unowned, # workspace-wide integration, so there is no owner to scope mutation to. - _user: Annotated[User, Depends(require_admin)], + _user: Annotated[User, Depends(require_tenant_admin)], ) -> BridgeDetail: bridge = await bridge_store.get(session, bridge_id) if bridge is None: @@ -529,6 +533,7 @@ async def claim_bridge_identity( CollaborationBridgeLifecycleService, Depends(get_collab_lifecycle) ], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> ExternalUserSummary: """Claim a platform identity for a Switch user (CHOO-2137). @@ -548,7 +553,7 @@ async def claim_bridge_identity( raise HTTPException(status_code=404, detail="Bridge not found") target_user_id = payload.user_id or user.id - if target_user_id != user.id and user.role != "admin": + if target_user_id != user.id and not is_admin: raise HTTPException( status_code=403, detail="Only an admin may claim a messaging identity for another user", @@ -635,6 +640,7 @@ async def release_bridge_identity( external_user_store: Annotated[ExternalUserStore, Depends(get_external_user_store)], user_store: Annotated[UserStore, Depends(get_user_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], user_id: str | None = None, ) -> ExternalUserSummary: """Drop a claim on a platform account — your own, or someone else's if you @@ -644,7 +650,7 @@ async def release_bridge_identity( raise HTTPException(status_code=404, detail="Identity not found") target_user_id = user_id or user.id - if target_user_id != user.id and user.role != "admin": + if target_user_id != user.id and not is_admin: raise HTTPException( status_code=403, detail="Only an admin may release another user's messaging identity", @@ -676,7 +682,7 @@ async def delete_bridge( ], # Admin-only: deleting a bridge cascades into deleting every room on it, so # this is the most destructive operation on the router. - _user: Annotated[User, Depends(require_admin)], + _user: Annotated[User, Depends(require_tenant_admin)], ) -> dict[str, bool]: bridge = await bridge_store.get(session, bridge_id) if bridge is None: diff --git a/core/switch_core/gateway/connectors.py b/core/switch_core/gateway/connectors.py index 94b186edf..8dd425657 100644 --- a/core/switch_core/gateway/connectors.py +++ b/core/switch_core/gateway/connectors.py @@ -12,7 +12,7 @@ from switch_core.db.models import User from switch_core.db.stores.api_key_store import ApiKeyStore from switch_core.db.stores.server_connector_store import ServerConnectorStore -from switch_core.gateway.auth import get_current_user +from switch_core.gateway.auth import get_current_user, get_tenant_is_admin from switch_core.gateway.dependencies import ( get_api_key_store, get_connector_lifecycle, @@ -131,16 +131,18 @@ async def delete_connector( ServerSideConnectorLifecycleService, Depends(get_connector_lifecycle) ], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> dict[str, bool]: record = await connector_store.get(session, connector_id) if record is None: raise HTTPException(status_code=404, detail="Connector not found") reg_key = await api_key_store.get(session, record.api_key_id) - if (reg_key is None or reg_key.user_id != user.id) and user.role != "admin": - raise HTTPException( - status_code=403, detail="Not authorized to delete this connector" - ) + if reg_key is None or reg_key.user_id != user.id: + if not is_admin: + raise HTTPException( + status_code=403, detail="Not authorized to delete this connector" + ) try: await connector_lifecycle.remove(connector_id) diff --git a/core/switch_core/gateway/documents.py b/core/switch_core/gateway/documents.py index 8a6301244..93a7dd362 100644 --- a/core/switch_core/gateway/documents.py +++ b/core/switch_core/gateway/documents.py @@ -10,7 +10,11 @@ from switch_core.db.stores.agent_store import AgentStore from switch_core.db.stores.room_store import RoomStore from switch_core.db.stores.user_store import UserStore -from switch_core.gateway.auth import get_current_user, require_room_access +from switch_core.gateway.auth import ( + get_current_user, + get_tenant_is_admin, + require_room_access, +) from switch_core.gateway.dependencies import ( get_agent_store, get_resource_service, @@ -200,10 +204,11 @@ async def get_document( user_store: Annotated[UserStore, Depends(get_user_store)], agent_store: Annotated[AgentStore, Depends(get_agent_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> DocumentDetail: try: doc = await resource_service.get_document_for_user( - session, document_id, user.id, is_admin=user.role == "admin" + session, document_id, user.id, is_admin=is_admin ) except ValueError as e: raise HTTPException(status_code=404, detail=str(e)) from e @@ -221,13 +226,14 @@ async def patch_document( user_store: Annotated[UserStore, Depends(get_user_store)], agent_store: Annotated[AgentStore, Depends(get_agent_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> DocumentDetail: try: doc = await resource_service.update_document( session, document_id, user_id=user.id, - is_admin=user.role == "admin", + is_admin=is_admin, name=req.name, description=req.description, instructions=req.instructions, @@ -249,6 +255,7 @@ async def delete_document( session: Annotated[AsyncSession, Depends(get_session)], resource_service: Annotated[ResourceService, Depends(get_resource_service)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> DocumentDeleteResponse: affected_packages = await resource_service.list_packages_for_document( session, document_id @@ -258,7 +265,7 @@ async def delete_document( session, document_id, user_id=user.id, - is_admin=user.role == "admin", + is_admin=is_admin, ) except ValueError as e: raise HTTPException(status_code=404, detail=str(e)) from e @@ -278,10 +285,11 @@ async def list_rooms_for_document( session: Annotated[AsyncSession, Depends(get_session)], resource_service: Annotated[ResourceService, Depends(get_resource_service)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> list[ResourceRoom]: try: await resource_service.get_document_for_user( - session, document_id, user.id, is_admin=user.role == "admin" + session, document_id, user.id, is_admin=is_admin ) except ValueError as e: raise HTTPException(status_code=404, detail=str(e)) from e @@ -300,8 +308,9 @@ async def list_room_documents( agent_store: Annotated[AgentStore, Depends(get_agent_store)], room_store: Annotated[RoomStore, Depends(get_room_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> list[DocumentSummary]: - await require_room_access(session, room_store, room_id, user, "read") + await require_room_access(session, room_store, room_id, user, "read", is_admin) docs = await resource_service.list_room_documents(session, room_id) return await _enrich_summaries( session, docs, resource_service, user_store, agent_store @@ -318,15 +327,16 @@ async def attach_document_to_room( agent_store: Annotated[AgentStore, Depends(get_agent_store)], room_store: Annotated[RoomStore, Depends(get_room_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> DocumentDetail: - await require_room_access(session, room_store, room_id, user, "write") + await require_room_access(session, room_store, room_id, user, "write", is_admin) try: await resource_service.attach_document_to_room( session, room_id, document_id, user_id=user.id, - is_admin=user.role == "admin", + is_admin=is_admin, ) except ValueError as e: raise HTTPException(status_code=404, detail=str(e)) from e @@ -334,7 +344,7 @@ async def attach_document_to_room( raise HTTPException(status_code=403, detail=str(e)) from e await session.commit() doc = await resource_service.get_document_for_user( - session, document_id, user.id, is_admin=user.role == "admin" + session, document_id, user.id, is_admin=is_admin ) return await _enrich_detail(session, doc, resource_service, user_store, agent_store) @@ -347,10 +357,11 @@ async def detach_document_from_room( resource_service: Annotated[ResourceService, Depends(get_resource_service)], room_store: Annotated[RoomStore, Depends(get_room_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> None: """For globally-owned docs: detach from the room. For room-scoped docs: hard-delete them entirely (since they live only in this room).""" - await require_room_access(session, room_store, room_id, user, "write") + await require_room_access(session, room_store, room_id, user, "write", is_admin) doc = await resource_service.get_room_scoped_document_or_none( session, room_id, document_id ) @@ -373,11 +384,12 @@ async def get_room_document( agent_store: Annotated[AgentStore, Depends(get_agent_store)], room_store: Annotated[RoomStore, Depends(get_room_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> DocumentDetail: """Fetch a document by id within the context of a room. Accepts both room-scoped documents (read-only) and globally-attached documents. Room read access is required and, once granted, implies access to its docs.""" - await require_room_access(session, room_store, room_id, user, "read") + await require_room_access(session, room_store, room_id, user, "read", is_admin) docs = await resource_service.list_room_documents(session, room_id) doc = next((d for d in docs if d.id == document_id), None) if doc is None: diff --git a/core/switch_core/gateway/packages.py b/core/switch_core/gateway/packages.py index 0ae22d56b..4521f6d71 100644 --- a/core/switch_core/gateway/packages.py +++ b/core/switch_core/gateway/packages.py @@ -10,7 +10,11 @@ from switch_core.db.stores.agent_store import AgentStore from switch_core.db.stores.room_store import RoomStore from switch_core.db.stores.user_store import UserStore -from switch_core.gateway.auth import get_current_user, require_room_access +from switch_core.gateway.auth import ( + get_current_user, + get_tenant_is_admin, + require_room_access, +) from switch_core.gateway.dependencies import ( get_agent_store, get_resource_service, @@ -140,10 +144,14 @@ async def get_package( resource_service: Annotated[ResourceService, Depends(get_resource_service)], user_store: Annotated[UserStore, Depends(get_user_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> PackageDetail: try: pkg = await resource_service.get_package_for_user( - session, package_id, user.id, is_admin=user.role == "admin" + session, + package_id, + user.id, + is_admin=is_admin, ) except ValueError as e: raise HTTPException(status_code=404, detail=str(e)) from e @@ -160,13 +168,14 @@ async def patch_package( resource_service: Annotated[ResourceService, Depends(get_resource_service)], user_store: Annotated[UserStore, Depends(get_user_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> PackageDetail: try: pkg = await resource_service.update_package( session, package_id, user_id=user.id, - is_admin=user.role == "admin", + is_admin=is_admin, name=req.name, description=req.description, instructions=req.instructions, @@ -187,13 +196,14 @@ async def delete_package( session: Annotated[AsyncSession, Depends(get_session)], resource_service: Annotated[ResourceService, Depends(get_resource_service)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> PackageDeleteResponse: try: detached = await resource_service.delete_package( session, package_id, user_id=user.id, - is_admin=user.role == "admin", + is_admin=is_admin, ) except ValueError as e: raise HTTPException(status_code=404, detail=str(e)) from e @@ -209,10 +219,14 @@ async def list_rooms_for_package( session: Annotated[AsyncSession, Depends(get_session)], resource_service: Annotated[ResourceService, Depends(get_resource_service)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> list[ResourceRoom]: try: await resource_service.get_package_for_user( - session, package_id, user.id, is_admin=user.role == "admin" + session, + package_id, + user.id, + is_admin=is_admin, ) except ValueError as e: raise HTTPException(status_code=404, detail=str(e)) from e @@ -232,10 +246,14 @@ async def list_package_references( resource_service: Annotated[ResourceService, Depends(get_resource_service)], user_store: Annotated[UserStore, Depends(get_user_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> list[ReferenceDetail]: try: await resource_service.get_package_for_user( - session, package_id, user.id, is_admin=user.role == "admin" + session, + package_id, + user.id, + is_admin=is_admin, ) except ValueError as e: raise HTTPException(status_code=404, detail=str(e)) from e @@ -252,6 +270,7 @@ async def add_reference_to_package( session: Annotated[AsyncSession, Depends(get_session)], resource_service: Annotated[ResourceService, Depends(get_resource_service)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> None: try: await resource_service.add_reference_to_package( @@ -259,7 +278,7 @@ async def add_reference_to_package( package_id, reference_id, user_id=user.id, - is_admin=user.role == "admin", + is_admin=is_admin, ) except ValueError as e: raise HTTPException(status_code=404, detail=str(e)) from e @@ -275,6 +294,7 @@ async def remove_reference_from_package( session: Annotated[AsyncSession, Depends(get_session)], resource_service: Annotated[ResourceService, Depends(get_resource_service)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> PackageMemberRemoveResponse: try: affected = await resource_service.remove_reference_from_package( @@ -282,7 +302,7 @@ async def remove_reference_from_package( package_id, reference_id, user_id=user.id, - is_admin=user.role == "admin", + is_admin=is_admin, ) except ValueError as e: raise HTTPException(status_code=404, detail=str(e)) from e @@ -305,10 +325,14 @@ async def list_package_documents( user_store: Annotated[UserStore, Depends(get_user_store)], agent_store: Annotated[AgentStore, Depends(get_agent_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> list[DocumentSummary]: try: await resource_service.get_package_for_user( - session, package_id, user.id, is_admin=user.role == "admin" + session, + package_id, + user.id, + is_admin=is_admin, ) except ValueError as e: raise HTTPException(status_code=404, detail=str(e)) from e @@ -327,6 +351,7 @@ async def add_document_to_package( session: Annotated[AsyncSession, Depends(get_session)], resource_service: Annotated[ResourceService, Depends(get_resource_service)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> None: try: await resource_service.add_document_to_package( @@ -334,7 +359,7 @@ async def add_document_to_package( package_id, document_id, user_id=user.id, - is_admin=user.role == "admin", + is_admin=is_admin, ) except ValueError as e: raise HTTPException(status_code=404, detail=str(e)) from e @@ -350,6 +375,7 @@ async def remove_document_from_package( session: Annotated[AsyncSession, Depends(get_session)], resource_service: Annotated[ResourceService, Depends(get_resource_service)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> PackageMemberRemoveResponse: try: affected = await resource_service.remove_document_from_package( @@ -357,7 +383,7 @@ async def remove_document_from_package( package_id, document_id, user_id=user.id, - is_admin=user.role == "admin", + is_admin=is_admin, ) except ValueError as e: raise HTTPException(status_code=404, detail=str(e)) from e @@ -383,8 +409,9 @@ async def list_room_packages( room_store: Annotated[RoomStore, Depends(get_room_store)], user_store: Annotated[UserStore, Depends(get_user_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> list[PackageDetail]: - await require_room_access(session, room_store, room_id, user, "read") + await require_room_access(session, room_store, room_id, user, "read", is_admin) pkgs = await resource_service.list_room_packages(session, room_id) return await _enrich(session, pkgs, resource_service, user_store) @@ -398,15 +425,16 @@ async def attach_package_to_room( room_store: Annotated[RoomStore, Depends(get_room_store)], user_store: Annotated[UserStore, Depends(get_user_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> PackageDetail: - await require_room_access(session, room_store, room_id, user, "write") + await require_room_access(session, room_store, room_id, user, "write", is_admin) try: await resource_service.attach_package_to_room( session, room_id, package_id, user_id=user.id, - is_admin=user.role == "admin", + is_admin=is_admin, ) except ValueError as e: raise HTTPException(status_code=404, detail=str(e)) from e @@ -414,7 +442,10 @@ async def attach_package_to_room( raise HTTPException(status_code=403, detail=str(e)) from e await session.commit() pkg = await resource_service.get_package_for_user( - session, package_id, user.id, is_admin=user.role == "admin" + session, + package_id, + user.id, + is_admin=is_admin, ) return await _enrich_one(session, pkg, resource_service, user_store) @@ -427,7 +458,8 @@ async def detach_package_from_room( resource_service: Annotated[ResourceService, Depends(get_resource_service)], room_store: Annotated[RoomStore, Depends(get_room_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> None: - await require_room_access(session, room_store, room_id, user, "write") + await require_room_access(session, room_store, room_id, user, "write", is_admin) await resource_service.detach_package_from_room(session, room_id, package_id) await session.commit() diff --git a/core/switch_core/gateway/references.py b/core/switch_core/gateway/references.py index 9b5e0979a..c78b6e7d8 100644 --- a/core/switch_core/gateway/references.py +++ b/core/switch_core/gateway/references.py @@ -15,7 +15,11 @@ from switch_core.db.models import Reference, User from switch_core.db.stores.room_store import RoomStore from switch_core.db.stores.user_store import UserStore -from switch_core.gateway.auth import get_current_user, require_room_access +from switch_core.gateway.auth import ( + get_current_user, + get_tenant_is_admin, + require_room_access, +) from switch_core.gateway.dependencies import ( get_resource_service, get_room_store, @@ -165,9 +169,10 @@ async def list_reference_types( resource_service: Annotated[ResourceService, Depends(get_resource_service)], user_store: Annotated[UserStore, Depends(get_user_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> list[ReferenceTypeInfo]: views = await resource_service.list_reference_types_for_principal( - session, user_id=user.id, is_admin=user.role == "admin" + session, user_id=user.id, is_admin=is_admin ) owners = await _owner_names( session, user_store, {v.owner_id for v in views if v.owner_id is not None} @@ -184,9 +189,10 @@ async def list_owned_reference_types( resource_service: Annotated[ResourceService, Depends(get_resource_service)], user_store: Annotated[UserStore, Depends(get_user_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> list[ReferenceTypeDetail]: rows = await resource_service.list_owned_reference_types( - session, user_id=user.id, is_admin=user.role == "admin" + session, user_id=user.id, is_admin=is_admin ) owners = await _owner_names(session, user_store, {e.row.owner_id for e in rows}) return [_type_to_detail(e, owners.get(e.row.owner_id)) for e in rows] @@ -229,13 +235,14 @@ async def patch_reference_type( resource_service: Annotated[ResourceService, Depends(get_resource_service)], user_store: Annotated[UserStore, Depends(get_user_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> ReferenceTypeDetail: try: row = await resource_service.update_reference_type( session, type, user_id=user.id, - is_admin=user.role == "admin", + is_admin=is_admin, display_name=req.display_name, instructions=req.instructions, value_hint=req.value_hint, @@ -260,10 +267,14 @@ async def delete_reference_type( session: Annotated[AsyncSession, Depends(get_session)], resource_service: Annotated[ResourceService, Depends(get_resource_service)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> ReferenceTypeDeleteResponse: try: await resource_service.delete_reference_type( - session, type, user_id=user.id, is_admin=user.role == "admin" + session, + type, + user_id=user.id, + is_admin=is_admin, ) except ReferenceTypeInUseError as e: raise HTTPException(status_code=409, detail=str(e)) from e @@ -293,12 +304,13 @@ async def create_reference( resource_service: Annotated[ResourceService, Depends(get_resource_service)], user_store: Annotated[UserStore, Depends(get_user_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> ReferenceDetail: try: ref = await resource_service.create_reference( session, owner_id=user.id, - is_admin=user.role == "admin", + is_admin=is_admin, read_visibility=req.read_visibility, write_visibility=req.write_visibility, type=req.type, @@ -320,10 +332,14 @@ async def get_reference( resource_service: Annotated[ResourceService, Depends(get_resource_service)], user_store: Annotated[UserStore, Depends(get_user_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> ReferenceDetail: try: ref = await resource_service.get_reference_for_user( - session, reference_id, user.id, is_admin=user.role == "admin" + session, + reference_id, + user.id, + is_admin=is_admin, ) except ValueError as e: raise HTTPException(status_code=404, detail=str(e)) from e @@ -340,13 +356,14 @@ async def patch_reference( resource_service: Annotated[ResourceService, Depends(get_resource_service)], user_store: Annotated[UserStore, Depends(get_user_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> ReferenceDetail: try: ref = await resource_service.update_reference( session, reference_id, user_id=user.id, - is_admin=user.role == "admin", + is_admin=is_admin, name=req.name, description=req.description, instructions=req.instructions, @@ -368,6 +385,7 @@ async def delete_reference( session: Annotated[AsyncSession, Depends(get_session)], resource_service: Annotated[ResourceService, Depends(get_resource_service)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> ReferenceDeleteResponse: affected_packages = await resource_service.list_packages_for_reference( session, reference_id @@ -377,7 +395,7 @@ async def delete_reference( session, reference_id, user_id=user.id, - is_admin=user.role == "admin", + is_admin=is_admin, ) except ValueError as e: raise HTTPException(status_code=404, detail=str(e)) from e @@ -397,10 +415,14 @@ async def list_rooms_for_reference( session: Annotated[AsyncSession, Depends(get_session)], resource_service: Annotated[ResourceService, Depends(get_resource_service)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> list[ResourceRoom]: try: await resource_service.get_reference_for_user( - session, reference_id, user.id, is_admin=user.role == "admin" + session, + reference_id, + user.id, + is_admin=is_admin, ) except ValueError as e: raise HTTPException(status_code=404, detail=str(e)) from e @@ -418,8 +440,9 @@ async def list_room_references( room_store: Annotated[RoomStore, Depends(get_room_store)], user_store: Annotated[UserStore, Depends(get_user_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> list[ReferenceDetail]: - await require_room_access(session, room_store, room_id, user, "read") + await require_room_access(session, room_store, room_id, user, "read", is_admin) refs = await resource_service.list_room_references(session, room_id) return await _enrich(session, refs, resource_service, user_store) @@ -433,15 +456,16 @@ async def attach_reference_to_room( room_store: Annotated[RoomStore, Depends(get_room_store)], user_store: Annotated[UserStore, Depends(get_user_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> ReferenceDetail: - await require_room_access(session, room_store, room_id, user, "write") + await require_room_access(session, room_store, room_id, user, "write", is_admin) try: await resource_service.attach_reference_to_room( session, room_id, reference_id, user_id=user.id, - is_admin=user.role == "admin", + is_admin=is_admin, ) except ValueError as e: raise HTTPException(status_code=404, detail=str(e)) from e @@ -449,7 +473,7 @@ async def attach_reference_to_room( raise HTTPException(status_code=403, detail=str(e)) from e await session.commit() ref = await resource_service.get_reference_for_user( - session, reference_id, user.id, is_admin=user.role == "admin" + session, reference_id, user.id, is_admin=is_admin ) return await _enrich_one(session, ref, resource_service, user_store) @@ -462,7 +486,8 @@ async def detach_reference_from_room( resource_service: Annotated[ResourceService, Depends(get_resource_service)], room_store: Annotated[RoomStore, Depends(get_room_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> None: - await require_room_access(session, room_store, room_id, user, "write") + await require_room_access(session, room_store, room_id, user, "write", is_admin) await resource_service.detach_reference_from_room(session, room_id, reference_id) await session.commit() diff --git a/core/switch_core/gateway/room_groups.py b/core/switch_core/gateway/room_groups.py index 70a3ca729..dca17f78d 100644 --- a/core/switch_core/gateway/room_groups.py +++ b/core/switch_core/gateway/room_groups.py @@ -9,7 +9,7 @@ from switch_core.db.models import RoomGroup, User from switch_core.db.stores.room_group_store import RoomGroupStore from switch_core.db.stores.room_store import RoomStore -from switch_core.gateway.auth import get_current_user +from switch_core.gateway.auth import get_current_user, get_tenant_is_admin from switch_core.gateway.dependencies import ( get_room_group_store, get_room_store, @@ -109,13 +109,14 @@ async def assign_rooms_to_group( room_group_store: Annotated[RoomGroupStore, Depends(get_room_group_store)], room_store: Annotated[RoomStore, Depends(get_room_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> RoomGroupAssignResponse: """Bulk-assign rooms into a group. Requires write access to each room; unknown room ids are skipped.""" if await room_group_store.get(session, group_id) is None: raise HTTPException(status_code=404, detail="Room group not found") - principal = Principal(user.id, user.role == "admin") + principal = Principal(user.id, is_admin) forbidden: list[str] = [] allowed: list[str] = [] for room_id in req.room_ids: diff --git a/core/switch_core/gateway/room_links.py b/core/switch_core/gateway/room_links.py index e219ee7fb..3c2e18a14 100644 --- a/core/switch_core/gateway/room_links.py +++ b/core/switch_core/gateway/room_links.py @@ -8,7 +8,11 @@ from switch_core.bridges.resource.service import ResourceService from switch_core.db.models import User from switch_core.db.stores.room_store import RoomStore -from switch_core.gateway.auth import get_current_user, require_room_access +from switch_core.gateway.auth import ( + get_current_user, + get_tenant_is_admin, + require_room_access, +) from switch_core.gateway.dependencies import ( get_resource_service, get_room_store, @@ -41,8 +45,9 @@ async def list_linked_rooms( resource_service: Annotated[ResourceService, Depends(get_resource_service)], room_store: Annotated[RoomStore, Depends(get_room_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> list[LinkedRoomDetail]: - await require_room_access(session, room_store, room_id, user, "read") + await require_room_access(session, room_store, room_id, user, "read", is_admin) rows = await resource_service.list_linked_rooms_for_room(session, room_id) return [LinkedRoomDetail(**row) for row in rows] @@ -54,8 +59,9 @@ async def list_inbound_linked_rooms( resource_service: Annotated[ResourceService, Depends(get_resource_service)], room_store: Annotated[RoomStore, Depends(get_room_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> list[InboundLinkedRoomDetail]: - await require_room_access(session, room_store, room_id, user, "read") + await require_room_access(session, room_store, room_id, user, "read", is_admin) rows = await resource_service.list_inbound_linked_rooms(session, room_id) return [InboundLinkedRoomDetail(**row) for row in rows] @@ -68,8 +74,9 @@ async def create_linked_room( resource_service: Annotated[ResourceService, Depends(get_resource_service)], room_store: Annotated[RoomStore, Depends(get_room_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> LinkedRoomDetail: - await require_room_access(session, room_store, room_id, user, "write") + await require_room_access(session, room_store, room_id, user, "write", is_admin) try: row = await resource_service.attach_linked_room( session, @@ -93,8 +100,9 @@ async def delete_linked_room( resource_service: Annotated[ResourceService, Depends(get_resource_service)], room_store: Annotated[RoomStore, Depends(get_room_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> Response: - await require_room_access(session, room_store, room_id, user, "write") + await require_room_access(session, room_store, room_id, user, "write", is_admin) removed = await resource_service.detach_linked_room( session, source_room_id=room_id, target_room_id=target_room_id ) diff --git a/core/switch_core/gateway/rooms.py b/core/switch_core/gateway/rooms.py index 061b6faeb..752326e05 100644 --- a/core/switch_core/gateway/rooms.py +++ b/core/switch_core/gateway/rooms.py @@ -19,7 +19,7 @@ from switch_core.db.stores.room_group_store import RoomGroupStore from switch_core.db.stores.room_store import RoomStore from switch_core.db.stores.user_store import UserStore -from switch_core.gateway.auth import get_current_user +from switch_core.gateway.auth import get_current_user, get_tenant_is_admin from switch_core.gateway.dependencies import ( get_bridge_store, get_collab_lifecycle, @@ -65,6 +65,7 @@ async def _require_room( room_id: str, user: User, action: Action, + is_admin: bool, ) -> Room: """Load a room (404 if missing) and authorize `action` for `user`, raising HTTP 403 if denied.""" @@ -72,7 +73,11 @@ async def _require_room( if room is None: raise HTTPException(status_code=404, detail="Room not found") try: - require(Principal(user.id, user.role == "admin"), action, room) + require( + Principal(user.id, is_admin), + action, + room, + ) except PermissionError as e: raise HTTPException(status_code=403, detail=str(e)) from e return room @@ -238,13 +243,14 @@ async def list_rooms( external_user_store: Annotated[ExternalUserStore, Depends(get_external_user_store)], user_store: Annotated[UserStore, Depends(get_user_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], search: Annotated[str | None, Query()] = None, include_archived: Annotated[bool, Query()] = False, ) -> list[RoomSummary]: rooms = await room_store.list_readable( session, user.id, - is_admin=user.role == "admin", + is_admin=is_admin, include_archived=include_archived, ) @@ -371,8 +377,10 @@ async def create_room( @router.post("/from-yaml", status_code=201) async def create_room_from_yaml( request: Request, + session: Annotated[AsyncSession, Depends(get_session)], rooms_yaml: Annotated[RoomYamlService, Depends(get_room_yaml_service)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> ProvisionResult: """Provision a single room and its attachments from a YAML spec. @@ -399,9 +407,7 @@ async def create_room_from_yaml( text = (await request.body()).decode("utf-8") inputs = None spec = rooms_yaml.parse(text, inputs=inputs) - return await rooms_yaml.provision( - spec, user_id=user.id, is_admin=user.role == "admin" - ) + return await rooms_yaml.provision(spec, user_id=user.id, is_admin=is_admin) except ValueError as e: raise HTTPException(status_code=400, detail=str(e)) from e except PermissionError as e: @@ -417,6 +423,7 @@ async def export_room_yaml( room_store: Annotated[RoomStore, Depends(get_room_store)], rooms_yaml: Annotated[RoomYamlService, Depends(get_room_yaml_service)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], agents: Annotated[bool, Query()] = True, users: Annotated[bool, Query()] = True, references: Annotated[bool, Query()] = True, @@ -425,7 +432,7 @@ async def export_room_yaml( ) -> Response: """Export a room to YAML in the same surface ``/rooms/from-yaml`` accepts. Each section can be dropped via its boolean toggle (default included).""" - await _require_room(session, room_store, room_id, user, "read") + await _require_room(session, room_store, room_id, user, "read", is_admin) yaml_text = await rooms_yaml.export( room_id, agents=agents, @@ -446,8 +453,9 @@ async def get_room( external_user_store: Annotated[ExternalUserStore, Depends(get_external_user_store)], protocol: Annotated[ProtocolService, Depends(get_protocol)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> RoomDetail: - room = await _require_room(session, room_store, room_id, user, "read") + room = await _require_room(session, room_store, room_id, user, "read", is_admin) return await _build_room_detail( session, room, room_store, bridge_store, external_user_store, protocol ) @@ -464,8 +472,9 @@ async def patch_room( external_user_store: Annotated[ExternalUserStore, Depends(get_external_user_store)], protocol: Annotated[ProtocolService, Depends(get_protocol)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> RoomDetail: - await _require_room(session, room_store, room_id, user, "write") + await _require_room(session, room_store, room_id, user, "write", is_admin) try: await room_service.update_room( room_id, @@ -494,8 +503,9 @@ async def list_room_roles( room_store: Annotated[RoomStore, Depends(get_room_store)], protocol: Annotated[ProtocolService, Depends(get_protocol)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> list[RoomRoleDetail]: - await _require_room(session, room_store, room_id, user, "read") + await _require_room(session, room_store, room_id, user, "read", is_admin) return await _list_room_role_details(session, room_id, protocol) @@ -507,8 +517,9 @@ async def create_room_role( room_store: Annotated[RoomStore, Depends(get_room_store)], protocol: Annotated[ProtocolService, Depends(get_protocol)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> list[RoomRoleDetail]: - await _require_room(session, room_store, room_id, user, "write") + await _require_room(session, room_store, room_id, user, "write", is_admin) try: await protocol.room_role_store.define_role( session, room_id, req.name, req.instructions, req.exclusive @@ -528,8 +539,9 @@ async def patch_room_role( room_store: Annotated[RoomStore, Depends(get_room_store)], protocol: Annotated[ProtocolService, Depends(get_protocol)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> list[RoomRoleDetail]: - await _require_room(session, room_store, room_id, user, "write") + await _require_room(session, room_store, room_id, user, "write", is_admin) try: await protocol.room_role_store.edit_role( session, room_id, name, req.instructions, req.exclusive @@ -548,8 +560,9 @@ async def delete_room_role( room_store: Annotated[RoomStore, Depends(get_room_store)], protocol: Annotated[ProtocolService, Depends(get_protocol)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> list[RoomRoleDetail]: - await _require_room(session, room_store, room_id, user, "write") + await _require_room(session, room_store, room_id, user, "write", is_admin) try: await protocol.room_role_store.delete_role(session, room_id, name) except ValueError as e: @@ -568,9 +581,10 @@ async def put_room_group( external_user_store: Annotated[ExternalUserStore, Depends(get_external_user_store)], protocol: Annotated[ProtocolService, Depends(get_protocol)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> RoomDetail: """Assign the room to a group, or make it standalone (`group_id=null`).""" - await _require_room(session, room_store, room_id, user, "write") + await _require_room(session, room_store, room_id, user, "write", is_admin) try: await room_store.set_group(session, room_id, req.group_id) except ValueError as e: @@ -598,8 +612,9 @@ async def put_protection( external_user_store: Annotated[ExternalUserStore, Depends(get_external_user_store)], protocol: Annotated[ProtocolService, Depends(get_protocol)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> RoomDetail: - await _require_room(session, room_store, room_id, user, "write") + await _require_room(session, room_store, room_id, user, "write", is_admin) try: await room_service.update_protection_config(room_id, req.protection_config) except ValueError: @@ -624,8 +639,9 @@ async def put_observe( external_user_store: Annotated[ExternalUserStore, Depends(get_external_user_store)], protocol: Annotated[ProtocolService, Depends(get_protocol)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> RoomDetail: - await _require_room(session, room_store, room_id, user, "write") + await _require_room(session, room_store, room_id, user, "write", is_admin) try: await room_service.update_observe_config(room_id, req.observe_config) except ValueError: @@ -650,8 +666,9 @@ async def post_room_agents( external_user_store: Annotated[ExternalUserStore, Depends(get_external_user_store)], protocol: Annotated[ProtocolService, Depends(get_protocol)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> RoomDetail: - await _require_room(session, room_store, room_id, user, "write") + await _require_room(session, room_store, room_id, user, "write", is_admin) try: await room_service.add_agents_to_room( room_id, @@ -682,8 +699,9 @@ async def patch_room_agent( external_user_store: Annotated[ExternalUserStore, Depends(get_external_user_store)], protocol: Annotated[ProtocolService, Depends(get_protocol)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> RoomDetail: - await _require_room(session, room_store, room_id, user, "write") + await _require_room(session, room_store, room_id, user, "write", is_admin) try: await room_service.set_join_event_listeners( room_id, {agent_id: req.receives_join_events} @@ -709,8 +727,9 @@ async def delete_room_agent( external_user_store: Annotated[ExternalUserStore, Depends(get_external_user_store)], protocol: Annotated[ProtocolService, Depends(get_protocol)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> RoomDetail: - await _require_room(session, room_store, room_id, user, "write") + await _require_room(session, room_store, room_id, user, "write", is_admin) try: await room_service.remove_agents_from_room(room_id, [agent_id]) except ValueError as e: @@ -735,8 +754,9 @@ async def post_room_users( external_user_store: Annotated[ExternalUserStore, Depends(get_external_user_store)], protocol: Annotated[ProtocolService, Depends(get_protocol)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> RoomDetail: - await _require_room(session, room_store, room_id, user, "write") + await _require_room(session, room_store, room_id, user, "write", is_admin) try: await room_service.add_users_to_room(room_id, req.user_names) except ValueError as e: @@ -760,8 +780,9 @@ async def _set_archived( external_user_store: ExternalUserStore, protocol: ProtocolService, user: User, + is_admin: bool, ) -> RoomDetail: - await _require_room(session, room_store, room_id, user, "write") + await _require_room(session, room_store, room_id, user, "write", is_admin) try: await room_service.set_room_archived(room_id, archived) except ValueError: @@ -784,8 +805,9 @@ async def archive_room( external_user_store: Annotated[ExternalUserStore, Depends(get_external_user_store)], protocol: Annotated[ProtocolService, Depends(get_protocol)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> RoomDetail: - """Archive a room: hide it from the default active list. Reversible and + """: hide it from the default active list. Reversible and metadata-only — the Matrix room, members, and bridge channel are intact.""" return await _set_archived( room_id, @@ -797,6 +819,7 @@ async def archive_room( external_user_store, protocol, user, + is_admin, ) @@ -810,8 +833,9 @@ async def unarchive_room( external_user_store: Annotated[ExternalUserStore, Depends(get_external_user_store)], protocol: Annotated[ProtocolService, Depends(get_protocol)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> RoomDetail: - """Unarchive a room: restore it to the active list.""" + """: restore it to the active list.""" return await _set_archived( room_id, False, @@ -822,6 +846,7 @@ async def unarchive_room( external_user_store, protocol, user, + is_admin, ) @@ -832,8 +857,9 @@ async def delete_room( room_service: Annotated[RoomService, Depends(get_room_service)], room_store: Annotated[RoomStore, Depends(get_room_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> dict[str, bool]: - await _require_room(session, room_store, room_id, user, "delete") + await _require_room(session, room_store, room_id, user, "delete", is_admin) try: await room_service.delete_room(room_id) except ValueError: @@ -848,8 +874,9 @@ async def bulk_delete_rooms( room_service: Annotated[RoomService, Depends(get_room_service)], room_store: Annotated[RoomStore, Depends(get_room_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> BulkDeleteResponse: - principal = Principal(user.id, user.role == "admin") + principal = Principal(user.id, is_admin) deleted = 0 for room_id in req.room_ids: room = await room_store.get(session, room_id) @@ -873,11 +900,12 @@ async def bulk_archive_rooms( room_service: Annotated[RoomService, Depends(get_room_service)], room_store: Annotated[RoomStore, Depends(get_room_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> BulkArchiveResponse: """Archive or unarchive many rooms at once. Each room is authorized individually with the same `write` check as the single archive endpoint; rooms the caller cannot write (or that no longer exist) are skipped.""" - principal = Principal(user.id, user.role == "admin") + principal = Principal(user.id, is_admin) updated = 0 for room_id in req.room_ids: room = await room_store.get(session, room_id) diff --git a/core/switch_core/gateway/templates.py b/core/switch_core/gateway/templates.py index 43ac1abd5..44f9034b4 100644 --- a/core/switch_core/gateway/templates.py +++ b/core/switch_core/gateway/templates.py @@ -29,7 +29,7 @@ TemplateStore, ) from switch_core.db.stores.user_store import UserStore -from switch_core.gateway.auth import get_current_user +from switch_core.gateway.auth import get_current_user, get_tenant_is_admin from switch_core.gateway.dependencies import ( get_config, get_session, @@ -155,7 +155,11 @@ def _require_within_size_limit(content: str, config: SwitchConfig) -> None: async def _load_for_management( - session: AsyncSession, store: TemplateStore, template_id: str, user: User + session: AsyncSession, + store: TemplateStore, + template_id: str, + user: User, + is_admin: bool, ) -> Template: """Fetch a template the caller is allowed to change, or fail saying why.""" template = await store.get(session, template_id) @@ -164,7 +168,10 @@ async def _load_for_management( status_code=404, detail=f"Template not found: {template_id}" ) try: - require_manage(Principal(user.id, user.role == "admin"), template.owner_id) + require_manage( + Principal(user.id, is_admin), + template.owner_id, + ) except PermissionError as e: raise HTTPException(status_code=403, detail=str(e)) from e return template @@ -303,8 +310,9 @@ async def patch_template( user_store: Annotated[UserStore, Depends(get_user_store)], config: Annotated[SwitchConfig, Depends(get_config)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> TemplateDetail: - await _load_for_management(session, template_store, template_id, user) + await _load_for_management(session, template_store, template_id, user, is_admin) if req.content is not None: _require_within_size_limit(req.content, config) _require_storable(req.content) @@ -333,8 +341,9 @@ async def delete_template( session: Annotated[AsyncSession, Depends(get_session)], template_store: Annotated[TemplateStore, Depends(get_template_store)], user: Annotated[User, Depends(get_current_user)], + is_admin: Annotated[bool, Depends(get_tenant_is_admin)], ) -> TemplateDeleteResponse: - await _load_for_management(session, template_store, template_id, user) + await _load_for_management(session, template_store, template_id, user, is_admin) try: await template_store.delete(session, template_id) except ValueError as e: diff --git a/core/switch_core/migrations/versions/8ef6d4038ecc_operator_owns_tenant_zero.py b/core/switch_core/migrations/versions/8ef6d4038ecc_operator_owns_tenant_zero.py new file mode 100644 index 000000000..ac4150c07 --- /dev/null +++ b/core/switch_core/migrations/versions/8ef6d4038ecc_operator_owns_tenant_zero.py @@ -0,0 +1,93 @@ +"""operator/workspace-admin role split: the seeded administrator becomes an owner of tenant zero + +Revision ID: 8ef6d4038ecc +Revises: a3f61c02d5be +Create Date: 2026-09-11 00:00:00.000000 + +Phase 2 of multi-tenancy (`docs/old/multi-tenancy-phase2-tenants.md`, §2) +starts reading `tenant_members.role` instead of leaving it inert: +`authz.Principal.is_admin` now grants tenant-scoped administrative power to a +caller whose membership role in the bound tenant is `owner`/`admin`, not only +to the deployment operator bit (`users.role == "admin"`, still global, still +an unconditional bypass, still never self-service). + +That bypass is exactly why this migration is not a lockout fix. An operator +administers every tenant regardless of what `tenant_members` says about them, +before this change and after it — nothing here can strand an operator out of +their own deployment, because nothing here is what lets them in. What this +migration fixes is a *label*: a deployment whose operator's own membership +row still says `member` would show that operator as a guest of the tenant +they administer, in any future member listing or ownership check that reads +`tenant_members` instead of going through `authz`. Correcting it now, once, +means that surface can be built later without also auditing every deployment +that upgraded before it existed. + +Two ways an operator's row can already disagree with `owner`: + +- The Phase 1 schema migration (`8b276792ee30`) backfilled `owner` for every + user whose `role` was `admin` at the time it ran, and `member` for everyone + else. It cannot see a promotion that happened after. +- `UserStore.ensure_membership` writes a membership exactly once, at account + creation, deriving its role from `users.role` at that moment. Promoting an + account to `admin` afterwards — a direct database edit, since there is no + gateway endpoint for it (`multi-tenancy-phase2-tenants.md` §8) — never + revisits the row that promotion left behind. + +So every user with `users.role = 'admin'` gets an `owner` row in tenant zero: +updated in place if their row there is `member`, inserted if they have no row +there at all. The `INSERT` covers a state the two paths above should not be +able to produce today (`ensure_membership` runs inside `UserStore.create`, +which never leaves an account without one) but which nothing enforces at the +database level — cheaper to close here than to assume. + +Nothing else moves. A non-operator's row is left exactly as it is, matching +this phase's stated invariant that anyone with a single membership and no +operator bit sees no behavioural change from this release. Only tenant zero +is touched: it is the only tenant that can exist in a deployment old enough +to need this migration at all. + +Idempotent: re-running it updates nothing the first run did not already fix, +and inserts nothing that already exists. +""" + +from collections.abc import Sequence + +from alembic import op + +revision: str = "8ef6d4038ecc" +down_revision: str | None = "a3f61c02d5be" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + +TENANT_ZERO_ID = "00000000-0000-0000-0000-000000000000" + + +def upgrade() -> None: + op.execute( + f""" + UPDATE tenant_members + SET role = 'owner' + WHERE tenant_id = '{TENANT_ZERO_ID}' + AND role NOT IN ('owner', 'admin') + AND user_id IN (SELECT id FROM users WHERE role = 'admin') + """ + ) + op.execute( + f""" + INSERT INTO tenant_members (tenant_id, user_id, role) + SELECT '{TENANT_ZERO_ID}', u.id, 'owner' + FROM users u + WHERE u.role = 'admin' + AND NOT EXISTS ( + SELECT 1 FROM tenant_members tm + WHERE tm.tenant_id = '{TENANT_ZERO_ID}' AND tm.user_id = u.id + ) + """ + ) + + +def downgrade() -> None: + # Not reversible: the rows this corrected were indistinguishable from an + # ordinary `owner` row the moment it ran, and rows it inserted have no + # prior state to restore — there was no row there before. + pass diff --git a/core/tests/switch_core/bridges/agent/protocol/test_agent_tenant_scoped_admin.py b/core/tests/switch_core/bridges/agent/protocol/test_agent_tenant_scoped_admin.py new file mode 100644 index 000000000..87b40b674 --- /dev/null +++ b/core/tests/switch_core/bridges/agent/protocol/test_agent_tenant_scoped_admin.py @@ -0,0 +1,211 @@ +"""An agent inherits exactly its owner's tenant-scoped power, never more +(CHOO-2726 role split, phase 2 design §2). + +``_resolve_acting_identity`` used to compute ``owner_is_admin`` from +``owner.role == "admin"`` alone — the same global bit every other admin check +in the codebase used before this split. Once a person can hold ``owner`` in +one tenant and merely ``member`` (or nothing) in another, that global read is +wrong for an agent the same way it was wrong for a human acting directly: an +agent must not administer a tenant its owner does not administer, and must +administer one its owner does, purely because of the owner's role in *that* +tenant rather than any global bit. + +``test_agent_administers_the_tenant_its_owner_owns`` is the one that actually +distinguishes old from new: the owner here holds no global admin bit at all, +so the pre-split code (`owner.role == "admin"`) denies it regardless of the +`tenant_members` row, and only reading that row grants it. The other two +tests hold under both the old and new code — they pin down that the fix does +not *overreach* (a workspace owner's agent must not gain cross-tenant power, +and a global operator's agent must not lose its bypass) rather than proving a +new grant. +""" + +from __future__ import annotations + +from types import SimpleNamespace + +import pytest +from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker + +from switch_core.bridges.agent.protocol.service import ProtocolService +from switch_core.bridges.agent.protocol.types import ( + IntegrationProfile, + TaskProtocolConfig, +) +from switch_core.db.models import ( + TENANT_ZERO_ID, + Client, + Room, + Tenant, + TenantMember, + User, +) +from switch_core.db.stores.agent_store import AgentStore +from switch_core.db.stores.api_key_store import ApiKeyStore +from switch_core.db.stores.room_store import RoomStore +from switch_core.db.stores.user_store import UserStore +from switch_core.tenant_context import tenant_scope + +TENANT_B = "tenant-b" + +_PROFILE = IntegrationProfile( + connection_model="session_passive", + message_exchange=True, + pre_invocation_mediation=[], + post_invocation_mediation=[], + event_reporting=[], + task_protocol=TaskProtocolConfig(can_delegate=False, can_accept=False), +) + + +class _FakeClientLifecycle: + def __init__(self, session_factory: async_sessionmaker[AsyncSession]) -> None: + self._session_factory = session_factory + + async def create_client(self, *, client_type: str, display_name: str) -> Client: + async with self._session_factory() as session: + client = Client( + matrix_user_id=f"@{display_name}:test", + display_name=display_name, + type=client_type, + ) + session.add(client) + await session.commit() + return client + + def start_client(self, client: Client) -> None: + pass + + +class _NoBridges: + def bridges_for_tenant(self, tenant_id: str) -> list[object]: + return [] + + +def _service(session_factory: async_sessionmaker[AsyncSession]) -> ProtocolService: + svc = object.__new__(ProtocolService) + svc.session_factory = session_factory # type: ignore[attr-defined] + svc.agent_store = AgentStore() # type: ignore[attr-defined] + svc.api_key_store = ApiKeyStore() # type: ignore[attr-defined] + svc.room_store = RoomStore() # type: ignore[attr-defined] + svc.user_store = UserStore() # type: ignore[attr-defined] + svc.client_lifecycle = _FakeClientLifecycle(session_factory) # type: ignore[attr-defined] + svc.collab_lifecycle = _NoBridges() # type: ignore[attr-defined] + svc.config = SimpleNamespace(jwt_secret_key="test-secret") # type: ignore[attr-defined] + return svc + + +async def _make_user( + session_factory: async_sessionmaker[AsyncSession], name: str, *, role: str = "user" +) -> str: + async with session_factory() as session: + user = User(name=name, email=f"{name}@test", role=role, password_hash="x") + session.add(user) + await session.commit() + return user.id + + +async def _make_tenant( + session_factory: async_sessionmaker[AsyncSession], tenant_id: str +) -> None: + async with session_factory() as session: + session.add(Tenant(id=tenant_id, slug=tenant_id, name=tenant_id)) + await session.commit() + + +async def _membership( + session_factory: async_sessionmaker[AsyncSession], + tenant_id: str, + user_id: str, + role: str, +) -> None: + async with session_factory() as session: + session.add(TenantMember(tenant_id=tenant_id, user_id=user_id, role=role)) + await session.commit() + + +async def _register(svc: ProtocolService, name: str, owner_id: str) -> str: + result = await svc.register_agent( + name=name, + description=f"{name} desc", + connector_type="test", + integration_profile=_PROFILE, + owner_id=owner_id, + ) + return result.agent_id + + +async def _private_room( + session_factory: async_sessionmaker[AsyncSession], tenant_id: str, owner_id: str +) -> str: + async with session_factory() as session: + room = Room( + tenant_id=tenant_id, + matrix_room_id=f"!{tenant_id}-private:test", + name="private", + description="", + owner_id=owner_id, + read_visibility="private", + write_visibility="private", + ) + session.add(room) + await session.commit() + return room.id + + +class TestAgentInheritsOwnerTenantScopedAdmin: + async def test_agent_administers_the_tenant_its_owner_owns( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + """The owner holds no global admin bit — only `owner` in the bound + tenant. Only reading `tenant_members` (not `users.role`) can grant + this, so this is the one test that fails against the pre-split code. + """ + svc = _service(session_factory) + owner = await _make_user(session_factory, "workspace-owner") + await _membership(session_factory, TENANT_ZERO_ID, owner, "owner") + someone_else = await _make_user(session_factory, "tenant-zero-room-owner") + agent_id = await _register(svc, "owner-agent", owner) + room_id = await _private_room(session_factory, TENANT_ZERO_ID, someone_else) + + async with session_factory() as session: + room = await svc._require_room_action(session, agent_id, room_id, "write") + assert room.id == room_id + + async def test_agent_cannot_administer_a_tenant_where_owner_is_only_a_member( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + """Same owner, same agent: `owner` in tenant zero grants nothing in + tenant B, where that owner only holds `member`.""" + svc = _service(session_factory) + await _make_tenant(session_factory, TENANT_B) + owner = await _make_user(session_factory, "cross-tenant-owner") + await _membership(session_factory, TENANT_ZERO_ID, owner, "owner") + await _membership(session_factory, TENANT_B, owner, "member") + someone_else = await _make_user(session_factory, "tenant-b-room-owner") + agent_id = await _register(svc, "cross-tenant-agent", owner) + room_id = await _private_room(session_factory, TENANT_B, someone_else) + + async with session_factory() as session: + with tenant_scope(TENANT_B), pytest.raises(PermissionError): + await svc._require_room_action(session, agent_id, room_id, "write") + + async def test_operator_owned_agent_administers_every_tenant( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + """The deployment-operator bypass is unconditional and still applies + through an agent, in a tenant the operator holds no membership in at + all.""" + svc = _service(session_factory) + await _make_tenant(session_factory, TENANT_B) + operator = await _make_user(session_factory, "operator", role="admin") + someone_else = await _make_user(session_factory, "tenant-b-room-owner-2") + agent_id = await _register(svc, "operator-agent", operator) + room_id = await _private_room(session_factory, TENANT_B, someone_else) + + async with session_factory() as session: + with tenant_scope(TENANT_B): + room = await svc._require_room_action( + session, agent_id, room_id, "write" + ) + assert room.id == room_id diff --git a/core/tests/switch_core/bridges/agent/protocol/test_list_all_references.py b/core/tests/switch_core/bridges/agent/protocol/test_list_all_references.py index 592a1a080..28a9aca9e 100644 --- a/core/tests/switch_core/bridges/agent/protocol/test_list_all_references.py +++ b/core/tests/switch_core/bridges/agent/protocol/test_list_all_references.py @@ -26,6 +26,7 @@ from switch_core.db.stores.reference_store import ReferenceStore from switch_core.db.stores.reference_type_store import ReferenceTypeStore from switch_core.db.stores.room_link_store import RoomLinkStore +from switch_core.db.stores.user_store import UserStore T_OLD = datetime(2024, 1, 1, 12, 0, tzinfo=UTC) @@ -50,6 +51,7 @@ def _build_service( svc.connections = ConnectionRegistry() svc.session_factory = session_factory # type: ignore[assignment] svc.agent_store = _FakeAgentStore(owners) # type: ignore[assignment] + svc.user_store = UserStore() # type: ignore[assignment] svc.resource_service = ResourceService( reference_store=ReferenceStore(), reference_type_store=ReferenceTypeStore(), diff --git a/core/tests/switch_core/bridges/agent/protocol/test_reference_type_parity.py b/core/tests/switch_core/bridges/agent/protocol/test_reference_type_parity.py index c79dd480e..b0656af7e 100644 --- a/core/tests/switch_core/bridges/agent/protocol/test_reference_type_parity.py +++ b/core/tests/switch_core/bridges/agent/protocol/test_reference_type_parity.py @@ -5,10 +5,10 @@ not the other is a drift bug that no other test would catch. `ProtocolService.__init__` takes 17 required collaborators, so the service is -built with `object.__new__` and the three attributes this method touches, per +built with `object.__new__` and the four attributes this method touches, per `test_agent_detail_mcp_tools.py`. `_resolve_acting_identity` loads the owner -with `session.get(User, ...)` on the live session rather than through -`user_store`, so no `user_store` is needed here. +with `session.get(User, ...)` on the live session, then reads +`user_store.administers` for the owner's tenant-scoped admin bit. """ from __future__ import annotations @@ -52,6 +52,7 @@ def _protocol_service( svc.session_factory = session_factory # type: ignore[attr-defined] svc.agent_store = AgentStore() # type: ignore[attr-defined] svc.resource_service = resource_service # type: ignore[attr-defined] + svc.user_store = _USER_STORE # type: ignore[attr-defined] return svc @@ -140,7 +141,8 @@ async def _seed( async def _slugs_from_route( session: AsyncSession, service: ResourceService, user: User ) -> set[str]: - listed = await route_list_types(session, service, _USER_STORE, user) + is_admin = await _USER_STORE.administers(session, user) + listed = await route_list_types(session, service, _USER_STORE, user, is_admin) return {info.type for info in listed} diff --git a/core/tests/switch_core/bridges/agent/protocol/test_room_roster_authz.py b/core/tests/switch_core/bridges/agent/protocol/test_room_roster_authz.py index d25f710ee..3ef918b1f 100644 --- a/core/tests/switch_core/bridges/agent/protocol/test_room_roster_authz.py +++ b/core/tests/switch_core/bridges/agent/protocol/test_room_roster_authz.py @@ -24,6 +24,7 @@ from switch_core.db.stores.agent_store import AgentStore from switch_core.db.stores.api_key_store import ApiKeyStore from switch_core.db.stores.room_store import RoomStore +from switch_core.db.stores.user_store import UserStore _PROFILE = IntegrationProfile( connection_model="session_passive", @@ -65,6 +66,7 @@ def _service(session_factory: async_sessionmaker[AsyncSession]) -> ProtocolServi svc.agent_store = AgentStore() # type: ignore[attr-defined] svc.api_key_store = ApiKeyStore() # type: ignore[attr-defined] svc.room_store = RoomStore() # type: ignore[attr-defined] + svc.user_store = UserStore() # type: ignore[attr-defined] svc.client_lifecycle = _FakeClientLifecycle(session_factory) # type: ignore[attr-defined] svc.collab_lifecycle = _NoBridges() # type: ignore[attr-defined] svc.config = SimpleNamespace(jwt_secret_key="test-secret") # type: ignore[attr-defined] diff --git a/core/tests/switch_core/db/test_tenant_admin_role_split.py b/core/tests/switch_core/db/test_tenant_admin_role_split.py new file mode 100644 index 000000000..9f87d889f --- /dev/null +++ b/core/tests/switch_core/db/test_tenant_admin_role_split.py @@ -0,0 +1,266 @@ +"""The operator/workspace-admin role split (CHOO-2726, phase 2 design §2). + +Before this change, `authz.Principal.is_admin` came from `users.role == +"admin"` alone — a global bit that `tenant_members.role` never fed into. So a +workspace's `owner` membership row granted nothing: every admin-gated route +checked the global bit, never the caller's role in the tenant the request is +bound to. + +These tests pin the behavioural claims that split makes true: + +- a workspace `owner` can administer resources in *their own* tenant; +- the same person has no such power in a tenant they only hold a `member` + row in — `UserStore.administers` must read the *bound* tenant's row, not + any row belonging to the caller; +- a deployment operator (`users.role == "admin"`) keeps its unconditional + bypass regardless of what `tenant_members` says, in every tenant, with or + without a membership row at all; +- a plain `member` with no operator bit gains nothing; +- and with no tenant bound at all the question is refused rather than + answered on the operator bit alone, for operators too. + +Each is proven both at the pure-decision level (`authz.administers_tenant`, +covered in `test_authz.py`) and here, against a real Postgres session and a +real `tenant_members` row, through `UserStore.administers` — the one place +that turns "who is this" plus "what tenant is bound" into that bit — and +through `authz.can` on a real `Room`, so the wiring from a membership row to +an actual resource decision is exercised end to end, not just the boolean. +""" + +from __future__ import annotations + +import pytest +from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker + +from switch_core.authz import Principal, can +from switch_core.db.models import TENANT_ZERO_ID, Room, Tenant, TenantMember, User +from switch_core.db.stores.user_store import UserStore +from switch_core.tenant_context import no_tenant, tenant_scope + +TENANT_B = "tenant-b" + + +async def _make_user(session: AsyncSession, name: str, *, role: str = "user") -> User: + user = User( + name=name, email=f"{name}@example.invalid", role=role, password_hash="x" + ) + session.add(user) + await session.flush() + return user + + +async def _make_tenant(session: AsyncSession, tenant_id: str) -> Tenant: + tenant = Tenant(id=tenant_id, slug=tenant_id, name=tenant_id) + session.add(tenant) + await session.flush() + return tenant + + +async def _membership( + session: AsyncSession, tenant_id: str, user_id: str, role: str +) -> None: + session.add(TenantMember(tenant_id=tenant_id, user_id=user_id, role=role)) + await session.flush() + + +class TestUserStoreAdministers: + async def test_owner_administers_their_own_tenant( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + store = UserStore() + async with session_factory() as session: + owner = await _make_user(session, "owner-of-zero") + await _membership(session, TENANT_ZERO_ID, owner.id, "owner") + await session.commit() + + # Ambient tenant is TENANT_ZERO_ID (see conftest.session_factory). + assert await store.administers(session, owner) is True + + async def test_owner_has_no_power_in_a_tenant_they_only_belong_to_as_member( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + # Two sessions, one per tenant: `TenantCheckedSession` stamps a + # session's transaction with whichever tenant is bound when it opens + # and refuses to let it drift mid-transaction + # (`db/tenant_session.py`), the same guard that would catch a request + # rebinding a session it did not open scoped. A real cross-tenant + # comparison goes through two sessions for the same reason. + store = UserStore() + async with session_factory() as session: + tenant_b = await _make_tenant(session, TENANT_B) + owner = await _make_user(session, "cross-tenant-owner") + await _membership(session, TENANT_ZERO_ID, owner.id, "owner") + await _membership(session, tenant_b.id, owner.id, "member") + await session.commit() + + assert await store.administers(session, owner) is True + + with tenant_scope(tenant_b.id): + async with session_factory() as session_b: + assert await store.administers(session_b, owner) is False + + async def test_admin_membership_role_also_administers( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + store = UserStore() + async with session_factory() as session: + admin_member = await _make_user(session, "admin-of-zero") + await _membership(session, TENANT_ZERO_ID, admin_member.id, "admin") + await session.commit() + + assert await store.administers(session, admin_member) is True + + async def test_operator_bypasses_regardless_of_tenant_membership( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + store = UserStore() + async with session_factory() as session: + tenant_b = await _make_tenant(session, TENANT_B) + operator = await _make_user(session, "operator", role="admin") + # A plain member in tenant zero... + await _membership(session, TENANT_ZERO_ID, operator.id, "member") + await session.commit() + + assert await store.administers(session, operator) is True + + # ...and no membership row at all in tenant B — a fresh session bound + # there, per the note above. + with tenant_scope(tenant_b.id): + async with session_factory() as session_b: + assert await store.administers(session_b, operator) is True + + async def test_plain_member_gains_nothing( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + store = UserStore() + async with session_factory() as session: + member = await _make_user(session, "plain-member") + await _membership(session, TENANT_ZERO_ID, member.id, "member") + await session.commit() + + assert await store.administers(session, member) is False + + async def test_no_membership_row_in_the_bound_tenant_gains_nothing( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + store = UserStore() + async with session_factory() as session: + stray = await _make_user(session, "no-membership") + await session.commit() + + assert await store.administers(session, stray) is False + + async def test_unbound_is_refused_rather_than_answered_on_the_operator_bit( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + # Answering here would mean answering a question that was not asked: + # "may this person administer" has no deployment-wide form. The + # tempting fallback — the operator bit alone — silently strips a + # workspace owner of their role, and the 403 that follows names no + # cause. Same call as the one #442 made for the bridge identities. + store = UserStore() + async with session_factory() as session: + owner = await _make_user(session, "unbound-owner") + await _membership(session, TENANT_ZERO_ID, owner.id, "owner") + await session.commit() + + with no_tenant(): + with pytest.raises(RuntimeError, match="requires a bound tenant"): + await store.administers(session, owner) + + async def test_unbound_is_refused_for_an_operator_too( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + # The operator bypass is the one answer that *would* have been right + # without a tenant, which is exactly why it must not be given: a + # caller that reaches here unbound has a bug, and returning True for + # the most privileged accounts is the worst moment to hide one. + store = UserStore() + async with session_factory() as session: + operator = await _make_user(session, "unbound-operator", role="admin") + await session.commit() + + with no_tenant(): + with pytest.raises(RuntimeError, match="requires a bound tenant"): + await store.administers(session, operator) + + +class TestRoomAuthzThroughTenantRole: + """The same claims, one layer up: a real `authz.can` decision on a real + `Room` neither party personally owns, built from `UserStore.administers` + the way `gateway/rooms.py` and `gateway/auth.py` build it.""" + + async def test_workspace_owner_can_delete_a_room_they_do_not_personally_own( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + store = UserStore() + async with session_factory() as session: + owner = await _make_user(session, "room-admin-owner") + someone_else = await _make_user(session, "room-creator") + await _membership(session, TENANT_ZERO_ID, owner.id, "owner") + room = Room( + tenant_id=TENANT_ZERO_ID, + matrix_room_id="!private-room:test", + name="private room", + description="desc", + owner_id=someone_else.id, + read_visibility="private", + write_visibility="private", + ) + session.add(room) + await session.commit() + + principal = Principal(owner.id, await store.administers(session, owner)) + assert can(principal, "delete", room) + + async def test_workspace_owner_cannot_delete_the_same_room_from_another_tenant( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + store = UserStore() + async with session_factory() as session: + tenant_b = await _make_tenant(session, TENANT_B) + owner = await _make_user(session, "room-admin-owner-2") + someone_else = await _make_user(session, "room-creator-2") + await _membership(session, TENANT_ZERO_ID, owner.id, "owner") + await _membership(session, tenant_b.id, owner.id, "member") + room = Room( + tenant_id=TENANT_ZERO_ID, + matrix_room_id="!private-room-2:test", + name="private room", + description="desc", + owner_id=someone_else.id, + read_visibility="private", + write_visibility="private", + ) + session.add(room) + await session.commit() + + with tenant_scope(tenant_b.id): + async with session_factory() as session_b: + principal = Principal( + owner.id, await store.administers(session_b, owner) + ) + assert not can(principal, "delete", room) + + async def test_plain_member_cannot_delete_a_room_they_do_not_own( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + store = UserStore() + async with session_factory() as session: + member = await _make_user(session, "room-member") + someone_else = await _make_user(session, "room-creator-3") + await _membership(session, TENANT_ZERO_ID, member.id, "member") + room = Room( + tenant_id=TENANT_ZERO_ID, + matrix_room_id="!private-room-3:test", + name="private room", + description="desc", + owner_id=someone_else.id, + read_visibility="private", + write_visibility="private", + ) + session.add(room) + await session.commit() + + principal = Principal(member.id, await store.administers(session, member)) + assert not can(principal, "delete", room) diff --git a/core/tests/switch_core/gateway/agent_route_harness.py b/core/tests/switch_core/gateway/agent_route_harness.py index f39af1c7a..697437dbf 100644 --- a/core/tests/switch_core/gateway/agent_route_harness.py +++ b/core/tests/switch_core/gateway/agent_route_harness.py @@ -9,6 +9,19 @@ from sqlalchemy.ext.asyncio import AsyncSession from switch_core.db.models import Agent, ApiKey, Client, User +from switch_core.db.stores.user_store import UserStore + +_USER_STORE = UserStore() + + +async def is_admin(session: AsyncSession, user: User) -> bool: + """The bit `get_tenant_is_admin` hands a route, resolved the same way. + + A route takes it as an argument now, so a test calling the coroutine + directly has to supply one. Reading it through the store rather than + passing a literal keeps a `role="admin"` on the user row meaningful. + """ + return await _USER_STORE.administers(session, user) async def add_user(session: AsyncSession, *, name: str, role: str = "user") -> User: diff --git a/core/tests/switch_core/gateway/test_agent_display_name_route.py b/core/tests/switch_core/gateway/test_agent_display_name_route.py index d461993d2..474159bcc 100644 --- a/core/tests/switch_core/gateway/test_agent_display_name_route.py +++ b/core/tests/switch_core/gateway/test_agent_display_name_route.py @@ -15,7 +15,11 @@ from switch_core.db.stores.agent_store import AgentStore from switch_core.gateway.agents import update_agent_display_name from switch_core.gateway.schemas import UpdateAgentDisplayNameRequest -from tests.switch_core.gateway.agent_route_harness import add_agent, add_user +from tests.switch_core.gateway.agent_route_harness import ( + add_agent, + add_user, + is_admin, +) _AGENT_STORE = AgentStore() @@ -37,6 +41,7 @@ async def test_owner_can_set_a_display_name( session, _AGENT_STORE, owner, + await is_admin(session, owner), ) assert summary.display_name == _NAME @@ -59,6 +64,7 @@ async def test_surrounding_whitespace_is_trimmed( session, _AGENT_STORE, owner, + await is_admin(session, owner), ) assert summary.display_name == _NAME @@ -78,6 +84,7 @@ async def test_owner_can_change_an_existing_display_name( session, _AGENT_STORE, owner, + await is_admin(session, owner), ) assert summary.display_name == _OTHER_NAME @@ -100,6 +107,7 @@ async def test_null_or_blank_clears_it( session, _AGENT_STORE, owner, + await is_admin(session, owner), ) assert summary.display_name is None @@ -122,6 +130,7 @@ async def test_non_owner_is_refused( session, _AGENT_STORE, other, + await is_admin(session, other), ) assert exc.value.status_code == 403 @@ -143,6 +152,7 @@ async def test_admin_can_set_one_on_an_agent_they_do_not_own( session, _AGENT_STORE, admin, + await is_admin(session, admin), ) assert summary.display_name == _NAME @@ -160,6 +170,7 @@ async def test_unknown_agent_is_404( session, _AGENT_STORE, owner, + await is_admin(session, owner), ) assert exc.value.status_code == 404 @@ -189,6 +200,7 @@ async def test_unsafe_names_are_400_not_stored( session, _AGENT_STORE, owner, + await is_admin(session, owner), ) assert exc.value.status_code == 400 diff --git a/core/tests/switch_core/gateway/test_agent_icon_route.py b/core/tests/switch_core/gateway/test_agent_icon_route.py index 673541a56..4cc002405 100644 --- a/core/tests/switch_core/gateway/test_agent_icon_route.py +++ b/core/tests/switch_core/gateway/test_agent_icon_route.py @@ -15,7 +15,11 @@ from switch_core.db.stores.agent_store import AgentStore from switch_core.gateway.agents import update_agent_icon from switch_core.gateway.schemas import UpdateAgentIconRequest -from tests.switch_core.gateway.agent_route_harness import add_agent, add_user +from tests.switch_core.gateway.agent_route_harness import ( + add_agent, + add_user, + is_admin, +) _AGENT_STORE = AgentStore() @@ -37,6 +41,7 @@ async def test_owner_can_set_an_icon( session, _AGENT_STORE, owner, + await is_admin(session, owner), ) assert summary.icon_url == _ICON @@ -59,6 +64,7 @@ async def test_owner_can_change_an_existing_icon( session, _AGENT_STORE, owner, + await is_admin(session, owner), ) assert summary.icon_url == _OTHER_ICON @@ -80,6 +86,7 @@ async def test_null_clears_the_icon( session, _AGENT_STORE, owner, + await is_admin(session, owner), ) assert summary.icon_url is None @@ -102,6 +109,7 @@ async def test_blank_string_clears_rather_than_storing_empty( session, _AGENT_STORE, owner, + await is_admin(session, owner), ) assert summary.icon_url is None @@ -121,6 +129,7 @@ async def test_non_owner_is_refused( session, _AGENT_STORE, other, + await is_admin(session, other), ) assert exc.value.status_code == 403 @@ -142,6 +151,7 @@ async def test_admin_can_set_an_icon_on_an_agent_they_do_not_own( session, _AGENT_STORE, admin, + await is_admin(session, admin), ) assert summary.icon_url == _ICON @@ -159,6 +169,7 @@ async def test_unknown_agent_is_404( session, _AGENT_STORE, owner, + await is_admin(session, owner), ) assert exc.value.status_code == 404 @@ -192,6 +203,7 @@ async def test_unsafe_url_is_rejected_and_nothing_is_stored( session, _AGENT_STORE, owner, + await is_admin(session, owner), ) assert exc.value.status_code == 400 @@ -215,6 +227,7 @@ async def test_a_rejected_change_leaves_the_previous_icon_intact( session, _AGENT_STORE, owner, + await is_admin(session, owner), ) stored = await _AGENT_STORE.get(session, agent.id) diff --git a/core/tests/switch_core/gateway/test_collaborations_authz.py b/core/tests/switch_core/gateway/test_collaborations_authz.py index f71fb8186..aeeeb8b53 100644 --- a/core/tests/switch_core/gateway/test_collaborations_authz.py +++ b/core/tests/switch_core/gateway/test_collaborations_authz.py @@ -10,9 +10,10 @@ These tests lock in three properties: 1. `/collab` is gone — not in the auth-bypass list, and its modules deleted. -2. Every collaboration write route on the gateway requires an admin (the - route dependency is `require_admin`), and every route requires at least - authentication. +2. Every collaboration write route on the gateway requires a tenant admin (the + route dependency is `require_tenant_admin` — a collaboration bridge is + tenant-scoped, so this is workspace administration, not the deployment + operator bit), and every route requires at least authentication. 3. The new set-default behaviour is correct against real Postgres. """ @@ -31,7 +32,7 @@ from switch_core.db.models import Client, CollaborationBridge, User from switch_core.db.stores.collaboration_bridge_store import CollaborationBridgeStore from switch_core.db.stores.room_store import RoomStore -from switch_core.gateway.auth import get_current_user, require_admin +from switch_core.gateway.auth import get_current_user, require_tenant_admin from switch_core.gateway.collaborations import ( _require_directory_account, claim_bridge_identity, @@ -91,9 +92,10 @@ def test_collab_api_module_is_gone() -> None: def test_bridge_write_routes_require_admin() -> None: - """EVERY state-changing route must be gated on require_admin — a bridge is an - unowned, workspace-wide integration holding platform secrets, so there is no - owner to scope mutation to. + """EVERY state-changing route must be gated on require_tenant_admin — a + bridge is an unowned, tenant-scoped integration holding platform secrets, + so there is no per-resource owner to scope mutation to, but it still + belongs to one workspace rather than the deployment as a whole. Derived from the router rather than a hand-listed set: a new write route that forgets the gate fails here instead of shipping open. @@ -103,10 +105,10 @@ def test_bridge_write_routes_require_admin() -> None: for route in router.routes if route.methods & WRITE_METHODS and route.path not in _OWNER_SCOPED_PATHS - and require_admin not in _dependency_calls(route.dependant) + and require_tenant_admin not in _dependency_calls(route.dependant) ) assert not unguarded, ( - f"collaboration write routes missing require_admin: {unguarded}" + f"collaboration write routes missing require_tenant_admin: {unguarded}" ) @@ -118,7 +120,7 @@ def test_owner_scoped_exemptions_name_real_routes() -> None: class TestIdentityRoutesAreSelfOrAdmin: - """The check the require_admin exemption trades away moved into the + """The check the require_tenant_admin exemption trades away moved into the handler; it did not disappear.""" async def test_claiming_for_another_user_is_refused(self) -> None: @@ -136,6 +138,7 @@ async def test_claiming_for_another_user_is_refused(self) -> None: user_store=_StubGet(None), # type: ignore[arg-type] collab_lifecycle=object(), # type: ignore[arg-type] user=_user(id="me", role="user"), + is_admin=False, ) assert excinfo.value.status_code == 403 @@ -151,6 +154,7 @@ async def test_releasing_another_users_claim_is_refused(self) -> None: external_user_store=_StubGet(theirs), # type: ignore[arg-type] user_store=_StubGet(None), # type: ignore[arg-type] user=_user(id="me", role="user"), + is_admin=False, user_id="someone-else", ) assert excinfo.value.status_code == 403 @@ -236,6 +240,9 @@ def __init__(self, value: object) -> None: async def get(self, *_args: object, **_kwargs: object) -> object: return self._value + async def administers(self, *_args: object, **_kwargs: object) -> bool: + return False + def _user(*, id: str, role: str) -> SimpleNamespace: return SimpleNamespace(id=id, role=role, name=id) @@ -253,10 +260,10 @@ def test_known_bridge_write_routes_are_present() -> None: def test_every_collaboration_route_requires_authentication() -> None: """No route may fall back to the old unauthenticated behaviour: each must - depend on get_current_user directly or via require_admin.""" + depend on get_current_user directly or via require_tenant_admin.""" for route in router.routes: calls = _dependency_calls(route.dependant) - assert get_current_user in calls or require_admin in calls, ( + assert get_current_user in calls or require_tenant_admin in calls, ( f"{sorted(route.methods)} {route.path} is not authenticated" ) diff --git a/core/tests/switch_core/gateway/test_connectors.py b/core/tests/switch_core/gateway/test_connectors.py index 39f95eff1..a1b19b58d 100644 --- a/core/tests/switch_core/gateway/test_connectors.py +++ b/core/tests/switch_core/gateway/test_connectors.py @@ -7,8 +7,16 @@ from switch_core.db.models import ApiKey, ServerConnector, User from switch_core.db.stores.api_key_store import ApiKeyStore from switch_core.db.stores.server_connector_store import ServerConnectorStore +from switch_core.db.stores.user_store import UserStore from switch_core.gateway.connectors import delete_connector +_USER_STORE = UserStore() + + +async def _is_admin(session: AsyncSession, user: User) -> bool: + """The bit `get_tenant_is_admin` hands the route, resolved the same way.""" + return await _USER_STORE.administers(session, user) + class _StubLifecycle: """Records remove() calls so we can assert whether deletion happened.""" @@ -78,6 +86,7 @@ async def test_non_owner_non_admin_is_forbidden( api_key_store=ApiKeyStore(), connector_lifecycle=lifecycle, # type: ignore[arg-type] user=other, + is_admin=await _is_admin(session, other), ) assert exc.value.status_code == 403 @@ -99,6 +108,7 @@ async def test_owner_can_delete( api_key_store=ApiKeyStore(), connector_lifecycle=lifecycle, # type: ignore[arg-type] user=owner, + is_admin=await _is_admin(session, owner), ) assert result == {"ok": True} @@ -121,6 +131,7 @@ async def test_admin_can_delete_others_connector( api_key_store=ApiKeyStore(), connector_lifecycle=lifecycle, # type: ignore[arg-type] user=admin, + is_admin=await _is_admin(session, admin), ) assert result == {"ok": True} @@ -142,6 +153,7 @@ async def test_missing_connector_is_not_found( api_key_store=ApiKeyStore(), connector_lifecycle=lifecycle, # type: ignore[arg-type] user=user, + is_admin=await _is_admin(session, user), ) assert exc.value.status_code == 404 diff --git a/core/tests/switch_core/gateway/test_reference_types_routes.py b/core/tests/switch_core/gateway/test_reference_types_routes.py index a792f7c60..58f81fb54 100644 --- a/core/tests/switch_core/gateway/test_reference_types_routes.py +++ b/core/tests/switch_core/gateway/test_reference_types_routes.py @@ -20,7 +20,7 @@ from switch_core.db.stores.reference_type_store import ReferenceTypeStore from switch_core.db.stores.room_link_store import RoomLinkStore from switch_core.db.stores.user_store import UserStore -from switch_core.gateway.auth import get_current_user +from switch_core.gateway.auth import get_current_user, get_tenant_is_admin from switch_core.gateway.dependencies import ( get_config, get_resource_service, @@ -49,6 +49,11 @@ _REFERENCE_STORE = ReferenceStore() +async def _is_admin(session: AsyncSession, user: User) -> bool: + """The bit `get_tenant_is_admin` hands the route, resolved the same way.""" + return await _USER_STORE.administers(session, user) + + def _resource_service( session_factory: async_sessionmaker[AsyncSession], ) -> ResourceService: @@ -142,7 +147,13 @@ def _app() -> FastAPI: # stubbed. It is never called: a request with no cookie is refused # before it is touched. app.dependency_overrides[get_session_factory] = lambda: None - app.dependency_overrides[get_user_store] = lambda: None + app.dependency_overrides[get_user_store] = lambda: UserStore() + # Authorizing reads the tenant-admin bit before the route reaches the + # resource service, and this app never runs `get_current_user`'s + # `tenant_scope`, so the real dependency would refuse rather than answer + # (`UserStore.administers`). These tests are about routing and body + # validation, so the bit is supplied directly. + app.dependency_overrides[get_tenant_is_admin] = lambda: False app.dependency_overrides[get_config] = lambda: None app.dependency_overrides[get_resource_service] = lambda: None return app @@ -214,7 +225,9 @@ async def test_create_then_list_and_own( assert created.shadowed_by_builtin is False assert created.value_schema["properties"]["urls"] - listed = await list_reference_types(session, svc, _USER_STORE, alice) + listed = await list_reference_types( + session, svc, _USER_STORE, alice, await _is_admin(session, alice) + ) by_slug = {t.type: t for t in listed} assert "notion" in by_slug assert by_slug["notion"].owner_name == "alice" @@ -223,7 +236,9 @@ async def test_create_then_list_and_own( assert by_slug["github"].owner_name is None assert by_slug["github"].value_hint - owned = await list_owned_reference_types(session, svc, _USER_STORE, alice) + owned = await list_owned_reference_types( + session, svc, _USER_STORE, alice, await _is_admin(session, alice) + ) assert [t.type for t in owned] == ["notion"] async def test_a_private_type_is_invisible_to_another_user( @@ -237,11 +252,16 @@ async def test_a_private_type_is_invisible_to_another_user( _create_request("notion"), session, svc, _USER_STORE, alice ) - listed = await list_reference_types(session, svc, _USER_STORE, bob) + listed = await list_reference_types( + session, svc, _USER_STORE, bob, await _is_admin(session, bob) + ) assert "notion" not in {t.type for t in listed} assert ( - await list_owned_reference_types(session, svc, _USER_STORE, bob) == [] + await list_owned_reference_types( + session, svc, _USER_STORE, bob, await _is_admin(session, bob) + ) + == [] ) async def test_invalid_slug_is_400( @@ -309,6 +329,7 @@ async def test_patch_updates_and_a_non_owner_is_403( svc, _USER_STORE, alice, + await _is_admin(session, alice), ) assert updated.display_name == "Notion Workspace" @@ -320,6 +341,7 @@ async def test_patch_updates_and_a_non_owner_is_403( svc, _USER_STORE, bob, + await _is_admin(session, bob), ) assert exc.value.status_code == 403 @@ -338,6 +360,7 @@ async def test_patch_of_an_unknown_slug_is_404( svc, _USER_STORE, alice, + await _is_admin(session, alice), ) assert exc.value.status_code == 404 @@ -352,11 +375,16 @@ async def test_delete_removes_an_unused_type( _create_request("notion"), session, svc, _USER_STORE, alice ) - response = await delete_reference_type("notion", session, svc, alice) + response = await delete_reference_type( + "notion", session, svc, alice, await _is_admin(session, alice) + ) assert response.deleted_type == "notion" assert ( - await list_owned_reference_types(session, svc, _USER_STORE, alice) == [] + await list_owned_reference_types( + session, svc, _USER_STORE, alice, await _is_admin(session, alice) + ) + == [] ) async def test_delete_of_a_type_in_use_is_409( @@ -383,10 +411,13 @@ async def test_delete_of_a_type_in_use_is_409( svc, _USER_STORE, alice, + await _is_admin(session, alice), ) with pytest.raises(HTTPException) as exc: - await delete_reference_type("notion", session, svc, alice) + await delete_reference_type( + "notion", session, svc, alice, await _is_admin(session, alice) + ) assert exc.value.status_code == 409 assert "cannot be deleted" in exc.value.detail @@ -421,11 +452,19 @@ async def test_a_resolvable_slug_carries_its_display_name( svc, _USER_STORE, alice, + await _is_admin(session, alice), ) assert created.type_display_name == "Notion" - fetched = await get_reference(created.id, session, svc, _USER_STORE, alice) + fetched = await get_reference( + created.id, + session, + svc, + _USER_STORE, + alice, + await _is_admin(session, alice), + ) assert fetched.type_display_name == "Notion" async def test_an_unresolvable_slug_maps_to_none( diff --git a/core/tests/switch_core/gateway/test_room_resource_read_authz.py b/core/tests/switch_core/gateway/test_room_resource_read_authz.py index fd9dbce9b..d1d32fb20 100644 --- a/core/tests/switch_core/gateway/test_room_resource_read_authz.py +++ b/core/tests/switch_core/gateway/test_room_resource_read_authz.py @@ -36,6 +36,11 @@ _AGENT_STORE = AgentStore() +async def _is_admin(session: AsyncSession, user: User) -> bool: + """The bit `get_tenant_is_admin` hands the route, resolved the same way.""" + return await _USER_STORE.administers(session, user) + + def _svc(session_factory: async_sessionmaker[AsyncSession]) -> ResourceService: return ResourceService( reference_store=ReferenceStore(), @@ -88,6 +93,7 @@ async def test_non_owner_cannot_read_room_document( _AGENT_STORE, _ROOM_STORE, other, + await _is_admin(session, other), ) assert exc.value.status_code == 403 @@ -109,6 +115,7 @@ async def test_non_owner_cannot_list_room_documents( _AGENT_STORE, _ROOM_STORE, other, + await _is_admin(session, other), ) assert exc.value.status_code == 403 @@ -123,7 +130,13 @@ async def test_non_owner_cannot_detach_room_document( with pytest.raises(HTTPException) as exc: await detach_document_from_room( - room.id, "any-doc", session, svc, _ROOM_STORE, other + room.id, + "any-doc", + session, + svc, + _ROOM_STORE, + other, + await _is_admin(session, other), ) assert exc.value.status_code == 403 @@ -138,7 +151,13 @@ async def test_non_owner_cannot_detach_reference( with pytest.raises(HTTPException) as exc: await detach_reference_from_room( - room.id, "any-ref", session, svc, _ROOM_STORE, other + room.id, + "any-ref", + session, + svc, + _ROOM_STORE, + other, + await _is_admin(session, other), ) assert exc.value.status_code == 403 @@ -153,7 +172,13 @@ async def test_non_owner_cannot_detach_package( with pytest.raises(HTTPException) as exc: await detach_package_from_room( - room.id, "any-pkg", session, svc, _ROOM_STORE, other + room.id, + "any-pkg", + session, + svc, + _ROOM_STORE, + other, + await _is_admin(session, other), ) assert exc.value.status_code == 403 @@ -176,6 +201,7 @@ async def test_owner_read_passes_authz( _AGENT_STORE, _ROOM_STORE, owner, + await _is_admin(session, owner), ) == [] ) @@ -189,5 +215,6 @@ async def test_owner_read_passes_authz( _AGENT_STORE, _ROOM_STORE, owner, + await _is_admin(session, owner), ) assert exc.value.status_code == 404 diff --git a/core/tests/switch_core/gateway/test_room_write_authz.py b/core/tests/switch_core/gateway/test_room_write_authz.py index 1b352078d..0cd97809a 100644 --- a/core/tests/switch_core/gateway/test_room_write_authz.py +++ b/core/tests/switch_core/gateway/test_room_write_authz.py @@ -31,6 +31,16 @@ _USER_STORE = UserStore() +async def _is_admin(session: AsyncSession, user: User) -> bool: + """The bit `get_tenant_is_admin` hands the route, resolved the same way. + + Passing a literal here would test the handler against an admin bit no + dependency could have produced; reading it through the store keeps the + role on the user row meaningful. + """ + return await _USER_STORE.administers(session, user) + + def _resource_service( session_factory: async_sessionmaker[AsyncSession], ) -> ResourceService: @@ -103,7 +113,14 @@ async def test_non_owner_cannot_attach_to_private_room( with pytest.raises(HTTPException) as exc: await attach_reference_to_room( - room.id, ref.id, session, svc, _ROOM_STORE, _USER_STORE, other + room.id, + ref.id, + session, + svc, + _ROOM_STORE, + _USER_STORE, + other, + await _is_admin(session, other), ) assert exc.value.status_code == 403 @@ -119,7 +136,14 @@ async def test_owner_can_attach( svc = _resource_service(session_factory) detail = await attach_reference_to_room( - room.id, ref.id, session, svc, _ROOM_STORE, _USER_STORE, owner + room.id, + ref.id, + session, + svc, + _ROOM_STORE, + _USER_STORE, + owner, + await _is_admin(session, owner), ) assert detail.id == ref.id @@ -137,7 +161,14 @@ async def test_admin_can_attach_to_room_they_dont_own( svc = _resource_service(session_factory) detail = await attach_reference_to_room( - room.id, ref.id, session, svc, _ROOM_STORE, _USER_STORE, admin + room.id, + ref.id, + session, + svc, + _ROOM_STORE, + _USER_STORE, + admin, + await _is_admin(session, admin), ) assert detail.id == ref.id @@ -161,7 +192,14 @@ async def test_public_write_room_allows_non_owner( svc = _resource_service(session_factory) detail = await attach_reference_to_room( - room.id, ref.id, session, svc, _ROOM_STORE, _USER_STORE, other + room.id, + ref.id, + session, + svc, + _ROOM_STORE, + _USER_STORE, + other, + await _is_admin(session, other), ) assert detail.id == ref.id @@ -176,7 +214,14 @@ async def test_missing_room_is_404( with pytest.raises(HTTPException) as exc: await attach_reference_to_room( - "missing-room", ref.id, session, svc, _ROOM_STORE, _USER_STORE, user + "missing-room", + ref.id, + session, + svc, + _ROOM_STORE, + _USER_STORE, + user, + await _is_admin(session, user), ) assert exc.value.status_code == 404 @@ -196,7 +241,13 @@ async def test_non_owner_cannot_link( with pytest.raises(HTTPException) as exc: await create_linked_room( - source.id, req, session, svc, _ROOM_STORE, other + source.id, + req, + session, + svc, + _ROOM_STORE, + other, + await _is_admin(session, other), ) assert exc.value.status_code == 403 @@ -213,7 +264,13 @@ async def test_owner_can_link( req = LinkedRoomCreateRequest(target_room_id=target.id, label="rel") detail = await create_linked_room( - source.id, req, session, svc, _ROOM_STORE, owner + source.id, + req, + session, + svc, + _ROOM_STORE, + owner, + await _is_admin(session, owner), ) assert detail.target_room_id == target.id @@ -237,7 +294,13 @@ async def test_non_owner_cannot_delete_link( with pytest.raises(HTTPException) as exc: await delete_linked_room( - source.id, target.id, session, svc, _ROOM_STORE, other + source.id, + target.id, + session, + svc, + _ROOM_STORE, + other, + await _is_admin(session, other), ) assert exc.value.status_code == 403 @@ -257,7 +320,13 @@ async def test_owner_can_delete_link( ) resp = await delete_linked_room( - source.id, target.id, session, svc, _ROOM_STORE, owner + source.id, + target.id, + session, + svc, + _ROOM_STORE, + owner, + await _is_admin(session, owner), ) assert resp.status_code == 204 diff --git a/core/tests/switch_core/gateway/test_template_routes.py b/core/tests/switch_core/gateway/test_template_routes.py index 490c37de6..4b42d6330 100644 --- a/core/tests/switch_core/gateway/test_template_routes.py +++ b/core/tests/switch_core/gateway/test_template_routes.py @@ -31,6 +31,12 @@ _TEMPLATE_STORE = TemplateStore() _USER_STORE = UserStore() + +async def _is_admin(session: AsyncSession, user: object) -> bool: + """The bit `get_tenant_is_admin` hands the route, resolved the same way.""" + return await _USER_STORE.administers(session, user) # type: ignore[arg-type] + + # Awkward on purpose: CRLF, a tab, trailing spaces, unicode, no final newline. _AWKWARD_DOCUMENT = ( "params:\r\n" @@ -290,6 +296,7 @@ async def test_deleting_someone_elses_template_is_refused( session, _TEMPLATE_STORE, bob, + await _is_admin(session, bob), ) assert exc.value.status_code == 403 assert await _TEMPLATE_STORE.get(session, created.id) is not None # type: ignore[attr-defined] @@ -312,6 +319,7 @@ async def test_editing_someone_elses_template_is_refused( _USER_STORE, _config(), bob, + await _is_admin(session, bob), ) assert exc.value.status_code == 403 @@ -349,6 +357,7 @@ async def test_an_owner_may_delete_their_own( session, _TEMPLATE_STORE, alice, + await _is_admin(session, alice), ) assert result.deleted_id == created.id # type: ignore[attr-defined] assert await _TEMPLATE_STORE.get(session, created.id) is None # type: ignore[attr-defined] @@ -367,6 +376,7 @@ async def test_an_admin_may_delete_anyones( session, _TEMPLATE_STORE, admin, + await _is_admin(session, admin), ) assert await _TEMPLATE_STORE.get(session, created.id) is None # type: ignore[attr-defined] @@ -387,6 +397,7 @@ async def test_an_admin_may_edit_anyones( _USER_STORE, _config(), admin, + await _is_admin(session, admin), ) assert updated.description == "tidied up by an admin" # Still Alice's — an admin edit is not a transfer of ownership. @@ -400,7 +411,13 @@ async def test_deleting_a_missing_template_is_a_404( await session.commit() with pytest.raises(HTTPException) as exc: - await delete_template("nope", session, _TEMPLATE_STORE, alice) + await delete_template( + "nope", + session, + _TEMPLATE_STORE, + alice, + await _is_admin(session, alice), + ) assert exc.value.status_code == 404 @@ -421,6 +438,7 @@ async def test_replacing_content_bumps_the_version( _USER_STORE, _config(), alice, + await _is_admin(session, alice), ) assert updated.version == 2 assert updated.content == "room:\n name: changed\n" @@ -441,6 +459,7 @@ async def test_a_metadata_edit_leaves_the_version_alone( _USER_STORE, _config(), alice, + await _is_admin(session, alice), ) assert updated.version == 1 assert updated.description == "clearer" @@ -462,6 +481,7 @@ async def test_an_oversize_replacement_is_refused( _USER_STORE, _config(template_max_bytes=100), alice, + await _is_admin(session, alice), ) assert exc.value.status_code == 413 stored = await _TEMPLATE_STORE.get(session, created.id) # type: ignore[attr-defined] @@ -485,5 +505,6 @@ async def test_renaming_onto_a_name_you_already_use_is_a_conflict( _USER_STORE, _config(), alice, + await _is_admin(session, alice), ) assert exc.value.status_code == 409 diff --git a/core/tests/switch_core/test_authz.py b/core/tests/switch_core/test_authz.py index 61c297fc2..ecf8d1182 100644 --- a/core/tests/switch_core/test_authz.py +++ b/core/tests/switch_core/test_authz.py @@ -6,6 +6,7 @@ from switch_core.authz import ( Principal, + administers_tenant, can, can_manage, require, @@ -106,6 +107,26 @@ def test_require_manage_raises_when_denied(self) -> None: require_manage(ALICE, "u-alice") # owner — no raise +class TestAdministersTenant: + """`administers_tenant` is the one function allowed to turn a role into + the admin bit (`authz.py` module docstring) — the operator bypass and the + per-tenant `owner`/`admin` membership role, combined.""" + + def test_operator_administers_regardless_of_tenant_role(self) -> None: + assert administers_tenant(is_operator=True, tenant_role=None) + assert administers_tenant(is_operator=True, tenant_role="member") + + def test_owner_or_admin_membership_administers(self) -> None: + assert administers_tenant(is_operator=False, tenant_role="owner") + assert administers_tenant(is_operator=False, tenant_role="admin") + + def test_plain_member_does_not_administer(self) -> None: + assert not administers_tenant(is_operator=False, tenant_role="member") + + def test_no_membership_does_not_administer(self) -> None: + assert not administers_tenant(is_operator=False, tenant_role=None) + + class TestValidateVisibilityPair: @pytest.mark.parametrize( "read,write", diff --git a/core/tests/switch_core/test_migration_operator_owns_tenant_zero.py b/core/tests/switch_core/test_migration_operator_owns_tenant_zero.py new file mode 100644 index 000000000..a9dfd8577 --- /dev/null +++ b/core/tests/switch_core/test_migration_operator_owns_tenant_zero.py @@ -0,0 +1,256 @@ +"""The operator/workspace-admin role split migration moves real data correctly. + +`test_migration_parity.py` replays the chain into an *empty* database, so it +can never catch a bug in what `8ef6d4038ecc` does to an existing deployment's +rows — a promoted-then-forgotten operator whose `tenant_members` row still +says `member` would pass every other test and only show up against real data. +This seeds tenant zero the way a deployment with that exact staleness would +look, stops the chain one revision short of the migration under test, runs +it, and inspects what came out. +""" + +from __future__ import annotations + +from pathlib import Path +from typing import Any + +from alembic.config import Config +from alembic.runtime.environment import EnvironmentContext +from alembic.script import ScriptDirectory +from sqlalchemy import Connection, text +from sqlalchemy.ext.asyncio import create_async_engine + +_CORE = Path(__file__).resolve().parents[2] +_MIGRATION_DB = "migration_operator_owns_tenant_zero" + +_PRE_MIGRATION_REVISION = "a3f61c02d5be" +_MIGRATION_UNDER_TEST = "8ef6d4038ecc" +_TENANT_ZERO_ID = "00000000-0000-0000-0000-000000000000" + + +def _script_directory(config: Config) -> ScriptDirectory: + config.set_main_option("script_location", str(_CORE / "switch_core" / "migrations")) + return ScriptDirectory.from_config(config) + + +def _upgrade_to(revision: str) -> Any: + def upgrade(connection: Connection) -> None: + config = Config(str(_CORE / "alembic.ini")) + script = _script_directory(config) + + def do_upgrade(current_revision: str, context: Any) -> Any: + return script._upgrade_revs(revision, current_revision) + + with EnvironmentContext(config, script, fn=do_upgrade) as environment: + environment.configure(connection=connection) + with environment.begin_transaction(): + environment.run_migrations() + + return upgrade + + +async def _fixture_engine(postgres_url: str) -> Any: + admin = create_async_engine(postgres_url, isolation_level="AUTOCOMMIT") + async with admin.connect() as connection: + await connection.execute(text(f'DROP DATABASE IF EXISTS "{_MIGRATION_DB}"')) + await connection.execute(text(f'CREATE DATABASE "{_MIGRATION_DB}"')) + await admin.dispose() + base, _, _ = postgres_url.rpartition("/") + return create_async_engine(f"{base}/{_MIGRATION_DB}") + + +async def _drop_fixture(postgres_url: str) -> None: + admin = create_async_engine(postgres_url, isolation_level="AUTOCOMMIT") + async with admin.connect() as connection: + await connection.execute(text(f'DROP DATABASE IF EXISTS "{_MIGRATION_DB}"')) + await admin.dispose() + + +async def test_a_promoted_operators_stale_member_row_becomes_owner( + postgres_url: str, +) -> None: + """The gap the migration exists for: `users.role` was promoted to + `admin` by direct SQL after the `tenant_members` row already existed as + `member` — nothing before this migration ever revisited it.""" + engine = await _fixture_engine(postgres_url) + try: + async with engine.begin() as connection: + await connection.run_sync(_upgrade_to(_PRE_MIGRATION_REVISION)) + + async with engine.begin() as connection: + await connection.execute( + text( + """ + INSERT INTO users (id, name, email, role, password_hash, metadata) + VALUES ('promoted-op', 'Promoted', 'promoted@example.com', + 'admin', 'bcrypt-hash', NULL) + """ + ) + ) + await connection.execute( + text( + f""" + INSERT INTO tenant_members (tenant_id, user_id, role) + VALUES ('{_TENANT_ZERO_ID}', 'promoted-op', 'member') + """ + ) + ) + + async with engine.begin() as connection: + await connection.run_sync(_upgrade_to(_MIGRATION_UNDER_TEST)) + + async with engine.connect() as connection: + role = ( + await connection.execute( + text( + "SELECT role FROM tenant_members " + "WHERE tenant_id = :t AND user_id = 'promoted-op'" + ), + {"t": _TENANT_ZERO_ID}, + ) + ).scalar_one() + finally: + await engine.dispose() + await _drop_fixture(postgres_url) + + assert role == "owner" + + +async def test_an_operator_with_no_membership_row_at_all_gets_one( + postgres_url: str, +) -> None: + """`UserStore.ensure_membership` should make this unreachable, but + nothing at the database level enforces it — the migration closes the gap + rather than assuming application code always ran first.""" + engine = await _fixture_engine(postgres_url) + try: + async with engine.begin() as connection: + await connection.run_sync(_upgrade_to(_PRE_MIGRATION_REVISION)) + + async with engine.begin() as connection: + await connection.execute( + text( + """ + INSERT INTO users (id, name, email, role, password_hash, metadata) + VALUES ('orphan-op', 'Orphan', 'orphan@example.com', + 'admin', 'bcrypt-hash', NULL) + """ + ) + ) + + async with engine.begin() as connection: + await connection.run_sync(_upgrade_to(_MIGRATION_UNDER_TEST)) + + async with engine.connect() as connection: + role = ( + await connection.execute( + text( + "SELECT role FROM tenant_members " + "WHERE tenant_id = :t AND user_id = 'orphan-op'" + ), + {"t": _TENANT_ZERO_ID}, + ) + ).scalar_one() + finally: + await engine.dispose() + await _drop_fixture(postgres_url) + + assert role == "owner" + + +async def test_a_plain_members_row_is_left_exactly_as_it_is( + postgres_url: str, +) -> None: + """Nothing else moves: a non-operator's membership row is not this + migration's business, whatever role it happens to hold.""" + engine = await _fixture_engine(postgres_url) + try: + async with engine.begin() as connection: + await connection.run_sync(_upgrade_to(_PRE_MIGRATION_REVISION)) + + async with engine.begin() as connection: + await connection.execute( + text( + """ + INSERT INTO users (id, name, email, role, password_hash, metadata) + VALUES ('plain-member', 'Plain', 'plain@example.com', + 'user', 'bcrypt-hash', NULL) + """ + ) + ) + await connection.execute( + text( + f""" + INSERT INTO tenant_members (tenant_id, user_id, role) + VALUES ('{_TENANT_ZERO_ID}', 'plain-member', 'member') + """ + ) + ) + + async with engine.begin() as connection: + await connection.run_sync(_upgrade_to(_MIGRATION_UNDER_TEST)) + + async with engine.connect() as connection: + role = ( + await connection.execute( + text( + "SELECT role FROM tenant_members " + "WHERE tenant_id = :t AND user_id = 'plain-member'" + ), + {"t": _TENANT_ZERO_ID}, + ) + ).scalar_one() + finally: + await engine.dispose() + await _drop_fixture(postgres_url) + + assert role == "member" + + +async def test_running_it_twice_is_a_no_op_the_second_time( + postgres_url: str, +) -> None: + engine = await _fixture_engine(postgres_url) + try: + async with engine.begin() as connection: + await connection.run_sync(_upgrade_to(_PRE_MIGRATION_REVISION)) + + async with engine.begin() as connection: + await connection.execute( + text( + """ + INSERT INTO users (id, name, email, role, password_hash, metadata) + VALUES ('idempotent-op', 'Idempotent', 'idempotent@example.com', + 'admin', 'bcrypt-hash', NULL) + """ + ) + ) + await connection.execute( + text( + f""" + INSERT INTO tenant_members (tenant_id, user_id, role) + VALUES ('{_TENANT_ZERO_ID}', 'idempotent-op', 'member') + """ + ) + ) + + async with engine.begin() as connection: + await connection.run_sync(_upgrade_to(_MIGRATION_UNDER_TEST)) + async with engine.begin() as connection: + await connection.run_sync(_upgrade_to(_MIGRATION_UNDER_TEST)) + + async with engine.connect() as connection: + rows = ( + await connection.execute( + text( + "SELECT role FROM tenant_members " + "WHERE tenant_id = :t AND user_id = 'idempotent-op'" + ), + {"t": _TENANT_ZERO_ID}, + ) + ).all() + finally: + await engine.dispose() + await _drop_fixture(postgres_url) + + assert [r.role for r in rows] == ["owner"] diff --git a/core/tests/switch_core/test_rooms_yaml.py b/core/tests/switch_core/test_rooms_yaml.py index 064c5f461..8aaa0d3b2 100644 --- a/core/tests/switch_core/test_rooms_yaml.py +++ b/core/tests/switch_core/test_rooms_yaml.py @@ -1056,6 +1056,9 @@ async def test_endpoint_json_body(env): svc = _svc(env) user_id = env["user_id"] + # Not an administrator of anything: this exercises body parsing, and the + # caller owns the room it creates. + is_admin = False user = User(name="alice", email="alice@example.com", role="member") # Poke the id to match the seeded user so provision works. object.__setattr__(user, "id", user_id) @@ -1071,12 +1074,14 @@ async def test_endpoint_json_body(env): request.headers = {"content-type": "application/json"} request.body.return_value = body - result = await create_room_from_yaml(request, svc, user) + async with env["session_factory"]() as session: + result = await create_room_from_yaml(request, session, svc, user, is_admin) assert result.room_name == "carol local-deploy" # Non-string yaml value → 400. bad_body = json.dumps({"yaml": 123}).encode() request.body.return_value = bad_body - with pytest.raises(HTTPException) as exc_info: - await create_room_from_yaml(request, svc, user) + async with env["session_factory"]() as session: + with pytest.raises(HTTPException) as exc_info: + await create_room_from_yaml(request, session, svc, user, is_admin) assert exc_info.value.status_code == 400 diff --git a/docs/old/multi-tenancy-phase2-tenants.md b/docs/old/multi-tenancy-phase2-tenants.md index ebc636b93..931048675 100644 --- a/docs/old/multi-tenancy-phase2-tenants.md +++ b/docs/old/multi-tenancy-phase2-tenants.md @@ -70,6 +70,37 @@ This is a bigger change than the other three tickets put together, and it is the reason they cannot ship without it. It should be its own ticket, sequenced first. **This is the decision I want confirmed before anything is built.** +### What a workspace admin actually gets, which is more than a list of routes + +The tempting way to describe this grant is to enumerate the admin-gated routes +and classify each one. That description is incomplete, and incomplete in the +direction that matters. + +`authz.can()` and `can_manage()` both short-circuit on `Principal.is_admin` +*before* they look at ownership or visibility. The moment `tenant_members.role` +feeds that bit, an `owner` or `admin` of a workspace holds read, write and +delete on **every owned resource in it** — rooms, references, documents, +packages — and management of every agent in it, whoever owns them. No route +opts into this; it falls out of the one short-circuit. The widest path is +`require_room_access`, which is not an explicitly admin-gated route at all. + +**The least obvious case, and the one to decide on purpose: a private room +between a colleague and their agent.** A workspace admin can read it, write in +it, and delete it. "Workspace administrators can read private rooms in their +workspace" is a sentence someone should agree to rather than discover. + +This is invisible in tenant zero today. After the backfill migration the only +`owner`/`admin` rows belong to deployment operators, who held all of it through +the global bypass already. It becomes real the first time a person is invited +into a workspace as an admin — that is, with §5. + +**Chosen: accept it for this phase, and write it down here rather than leave it +implicit.** The alternative is a second, narrower bit — "administers the +workspace's configuration" as distinct from "may act on its contents" — which +is a real distinction and a real amount of work, and belongs to whichever phase +takes on resource-level sharing. What must not happen is shipping the wide +grant while the design describes the narrow one. + ## 3. How a request picks its tenant A request must act in exactly one tenant — Phase 1 depends on that and nothing