diff --git a/core/switch_core/authz.py b/core/switch_core/authz.py index f21eb4947..da6a15087 100644 --- a/core/switch_core/authz.py +++ b/core/switch_core/authz.py @@ -68,6 +68,19 @@ def administers_tenant(*, is_operator: bool, tenant_role: str | None) -> bool: return is_operator or tenant_role in TENANT_ADMIN_ROLES +def owns_tenant(*, is_operator: bool, tenant_role: str | None) -> bool: + """Whether a caller may dispose of the tenant they hold `tenant_role` in. + + Narrower than `administers_tenant` on purpose. An `admin` runs a workspace; + an `owner` decides who else gets to. The two collapse into one bit for + everything inside a workspace — rooms, agents, references — but not for the + ownership set itself: if an admin may grant `owner`, demote an owner, or + remove one, then an admin may take the workspace, and each of those steps + passes a last-owner guard on its own. Gate those three on this. + """ + return is_operator or tenant_role == "owner" + + @runtime_checkable class Authorizable(Protocol): """Structural interface for any entity `can` arbitrates over. diff --git a/core/switch_core/db/stores/agent_store.py b/core/switch_core/db/stores/agent_store.py index af2e65e2b..29d0db7a0 100644 --- a/core/switch_core/db/stores/agent_store.py +++ b/core/switch_core/db/stores/agent_store.py @@ -89,6 +89,16 @@ async def get_all(self, session: AsyncSession) -> list[Agent]: result = await session.execute(select(Agent)) return list(result.scalars().all()) + async def get_by_owner(self, session: AsyncSession, owner_id: str) -> list[Agent]: + """Every agent `owner_id` owns in the bound tenant. + + Backs member removal: an owner's agents authenticate with a key that + carries the same `user_id`, so this is what a removal cascade walks + to find them. + """ + result = await session.execute(select(Agent).where(Agent.owner_id == owner_id)) + return list(result.scalars().all()) + async def get_children( self, session: AsyncSession, parent_agent_ids: list[str] ) -> list[Agent]: diff --git a/core/switch_core/db/stores/user_store.py b/core/switch_core/db/stores/user_store.py index 3506b9741..51b0d2283 100644 --- a/core/switch_core/db/stores/user_store.py +++ b/core/switch_core/db/stores/user_store.py @@ -6,7 +6,7 @@ from sqlalchemy.exc import IntegrityError from sqlalchemy.ext.asyncio import AsyncSession -from switch_core.authz import administers_tenant +from switch_core.authz import administers_tenant, owns_tenant from switch_core.db.models import ( OidcIdentity, TenantMember, @@ -300,6 +300,26 @@ async def get_all(self, session: AsyncSession) -> list[User]: result = await session.execute(select(User)) return list(result.scalars().all()) + async def add_membership( + self, session: AsyncSession, *, tenant_id: str, user_id: str, role: str + ) -> TenantMember: + """Insert a `role` membership for `user_id` in `tenant_id`. + + Distinct from `ensure_membership`, which derives the role from the + caller's global bit and is a no-op when a membership already exists: + this is the explicit write for the two places that grant a *specific* + role by a caller's own action — creating a workspace (the creator + becomes its `owner`) and accepting an invitation (the invited role). + It does not check for an existing row first; a caller that needs that + checks before calling. Kept as the one place `TenantMember` is + constructed (`tests/switch_core/test_seed_admin_membership.py` pins + that), so a route never writes one directly. + """ + membership = TenantMember(tenant_id=tenant_id, user_id=user_id, role=role) + session.add(membership) + await session.flush() + return membership + async def tenant_role( self, session: AsyncSession, tenant_id: str, user_id: str ) -> str | None: @@ -312,6 +332,34 @@ async def tenant_role( membership = await session.get(TenantMember, (tenant_id, user_id)) return membership.role if membership is not None else None + async def list_tenant_members( + self, session: AsyncSession + ) -> list[tuple[User, TenantMember]]: + """Every member of the session's bound tenant, joined with their user row. + + No `WHERE tenant_id = …` of its own: the policy on `tenant_members` is + what narrows this, the same as `InvitationStore.list_for_tenant`. + """ + result = await session.execute( + select(User, TenantMember).join( + TenantMember, TenantMember.user_id == User.id + ) + ) + return [(row[0], row[1]) for row in result.all()] + + async def count_owners(self, session: AsyncSession) -> int: + """How many `owner` memberships exist in the session's bound tenant. + + Backs "a workspace must always have an owner": removing or demoting a + member is refused when they are the one row this counts. + """ + result = await session.execute( + select(func.count()) + .select_from(TenantMember) + .where(TenantMember.role == "owner") + ) + return result.scalar_one() + 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. @@ -336,3 +384,23 @@ async def administers(self, session: AsyncSession, user: User) -> bool: ) role = await self.tenant_role(session, tenant_id, user.id) return administers_tenant(is_operator=user.role == "admin", tenant_role=role) + + async def owns(self, session: AsyncSession, user: User) -> bool: + """Whether `user` may decide who owns the tenant bound to `session`'s + context — the operator bypass, or an `owner` membership in it. + + The same composition as `administers`, over `authz.owns_tenant`; see + there for why the two are separate bits rather than one. + + Raises: + RuntimeError: no tenant is bound, for the same reason + `administers` raises. + """ + tenant_id = current_tenant_id() + if tenant_id is None: + raise RuntimeError( + "owns requires a bound tenant; whether someone owns a " + "workspace is only answerable about a particular one" + ) + role = await self.tenant_role(session, tenant_id, user.id) + return owns_tenant(is_operator=user.role == "admin", tenant_role=role) diff --git a/core/switch_core/gateway/app.py b/core/switch_core/gateway/app.py index 376ca1e27..6f1b46d2f 100644 --- a/core/switch_core/gateway/app.py +++ b/core/switch_core/gateway/app.py @@ -18,6 +18,7 @@ from switch_core.db.stores.api_key_store import ApiKeyStore from switch_core.db.stores.collaboration_bridge_store import CollaborationBridgeStore from switch_core.db.stores.external_user_store import ExternalUserStore +from switch_core.db.stores.invitation_store import InvitationStore from switch_core.db.stores.room_group_store import RoomGroupStore from switch_core.db.stores.room_store import RoomStore from switch_core.db.stores.server_connector_store import ServerConnectorStore @@ -59,6 +60,7 @@ def create_gateway_app( user_store: UserStore, external_user_store: ExternalUserStore, api_key_store: ApiKeyStore, + invitation_store: InvitationStore, template_store: TemplateStore, resource_service: ResourceService, protocol: ProtocolService, @@ -79,6 +81,7 @@ def create_gateway_app( user_store=user_store, external_user_store=external_user_store, api_key_store=api_key_store, + invitation_store=invitation_store, template_store=template_store, resource_service=resource_service, protocol=protocol, diff --git a/core/switch_core/gateway/auth.py b/core/switch_core/gateway/auth.py index 546da4767..2aff989b4 100644 --- a/core/switch_core/gateway/auth.py +++ b/core/switch_core/gateway/auth.py @@ -3,6 +3,7 @@ import datetime import logging from collections.abc import AsyncIterator +from dataclasses import dataclass from typing import Annotated import bcrypt @@ -16,7 +17,7 @@ from switch_core.db.session_scope import tenant_session from switch_core.db.stores.room_store import RoomStore from switch_core.db.stores.user_store import UserStore -from switch_core.db.tenant_lookup import tenants_of_user +from switch_core.db.tenant_lookup import tenant_of_invitation, tenants_of_user from switch_core.gateway.dependencies import ( get_config, get_session, @@ -162,6 +163,56 @@ async def get_authenticated_user_id( return payload["sub"] # type: ignore[no-any-return] +@dataclass(frozen=True) +class AuthenticatedCaller: + """An authenticated caller with no tenant bound — id and email, read from + `users`, which carries neither a tenant nor a policy.""" + + id: str + email: str + + +async def get_authenticated_caller( + request: Request, + session_factory: Annotated[ + async_sessionmaker[AsyncSession], Depends(get_session_factory) + ], + user_store: Annotated[UserStore, Depends(get_user_store)], + config: Annotated[SwitchConfig, Depends(get_config)], +) -> AuthenticatedCaller: + """Like `get_authenticated_user_id`, but with the caller's own email too. + + Backs `POST /invitations/accept` (`gateway/tenants.py`), which has + to check an email-bound invitation against the caller's *current* address + before any tenant is bound — the same "no tenant chosen yet" shape as + `get_authenticated_user_id`, plus one more column of the same global, + unscoped `users` row. Reads the row fresh rather than trusting the JWT's + own `email` claim, which is a snapshot from login and can go stale. + """ + payload = await _authenticate(request, session_factory, user_store, config) + async with session_factory() as system_session: + user = await user_store.get(system_session, payload["sub"]) + if user is None: + raise HTTPException(status_code=401, detail="User not found") + return AuthenticatedCaller(id=user.id, email=user.email) + + +async def tenant_of_invitation_token( + session_factory: async_sessionmaker[AsyncSession], token_hash: str +) -> str | None: + """Which tenant an invitation token belongs to, or `None` if it names none. + + A thin wrapper around `db.tenant_lookup.tenant_of_invitation`, kept here + rather than called directly from `gateway/tenants.py`: the exemption from + row-level security is meant to be reachable through this one module, + never through a store or lookup any endpoint could reach for itself (see + `db/tenant_lookup.py` and `test_tenant_exemption_allowlist.py`) — the same + reason `_resolve_tenant_id` and `list_tenant_memberships` live here rather + than being called from the gateway directly. + """ + return await tenant_of_invitation(session_factory, token_hash) + + async def is_tenant_member( session_factory: async_sessionmaker[AsyncSession], user_id: str, tenant_id: str ) -> bool: @@ -410,6 +461,20 @@ async def get_tenant_is_admin( return await user_store.administers(session, user) +async def get_tenant_is_owner( + 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 decide who owns the tenant this request is bound to. + + Strictly narrower than ``get_tenant_is_admin`` — see + ``authz.owns_tenant``. A route that changes the ownership set needs both: + ``require_tenant_admin`` to get in, this to make the call. + """ + return await user_store.owns(session, user) + + async def require_tenant_admin( user: Annotated[User, Depends(get_current_user)], is_admin: Annotated[bool, Depends(get_tenant_is_admin)], diff --git a/core/switch_core/gateway/dependencies.py b/core/switch_core/gateway/dependencies.py index 8a07e7e62..abc1e3782 100644 --- a/core/switch_core/gateway/dependencies.py +++ b/core/switch_core/gateway/dependencies.py @@ -20,6 +20,7 @@ from switch_core.db.stores.api_key_store import ApiKeyStore from switch_core.db.stores.collaboration_bridge_store import CollaborationBridgeStore from switch_core.db.stores.external_user_store import ExternalUserStore +from switch_core.db.stores.invitation_store import InvitationStore from switch_core.db.stores.room_group_store import RoomGroupStore from switch_core.db.stores.room_store import RoomStore from switch_core.db.stores.server_connector_store import ServerConnectorStore @@ -47,6 +48,7 @@ def init_dependencies( user_store: UserStore, external_user_store: ExternalUserStore, api_key_store: ApiKeyStore, + invitation_store: InvitationStore, template_store: TemplateStore, resource_service: ResourceService, protocol: ProtocolService, @@ -66,6 +68,7 @@ def init_dependencies( _state["user_store"] = user_store _state["external_user_store"] = external_user_store _state["api_key_store"] = api_key_store + _state["invitation_store"] = invitation_store _state["template_store"] = template_store _state["resource_service"] = resource_service _state["protocol"] = protocol @@ -187,6 +190,10 @@ def get_api_key_store() -> ApiKeyStore: return _state["api_key_store"] # type: ignore[no-any-return] +def get_invitation_store() -> InvitationStore: + return _state["invitation_store"] # type: ignore[no-any-return] + + def get_connector_lifecycle() -> ServerSideConnectorLifecycleService: return _state["connector_lifecycle"] # type: ignore[no-any-return] diff --git a/core/switch_core/gateway/schemas.py b/core/switch_core/gateway/schemas.py index cd0d40794..bc67cfe2e 100644 --- a/core/switch_core/gateway/schemas.py +++ b/core/switch_core/gateway/schemas.py @@ -673,6 +673,58 @@ class TenantMembershipResponse(BaseModel): role: str +class TenantCreateRequest(BaseModel): + name: str + + +class InvitationCreateRequest(BaseModel): + role: str = "member" + # None mints a shareable link; set, the invitation is addressed to one + # email and accepting it with any other is refused. + email: str | None = None + # Bounded above as well as below: `timedelta` raises `OverflowError` past + # roughly 2.4e9 hours, so an unbounded value turns a bad request into a 500. + # A year is well beyond any legitimate invitation's life. + expires_in_hours: int = Field(default=168, gt=0, le=8760) + uses_remaining: int = Field(default=1, ge=1) + + +class InvitationAcceptRequest(BaseModel): + # In the body rather than the path: a URL travels through proxy logs, + # browser history and `Referer` headers, and this one is a bearer + # credential. + token: str + + +class InvitationDetail(BaseModel): + id: str + role: str + email: str | None + expires_at: str + uses_remaining: int + revoked_at: str | None + created_by: str + created_at: str + + +class InvitationCreateResponse(InvitationDetail): + # The plaintext token. Present only here — see + # `InvitationStore.create`, which is the one call that can hand it back. + token: str + + +class MemberDetail(BaseModel): + user_id: str + name: str + email: str + role: str + created_at: str + + +class MemberUpdateRequest(BaseModel): + role: str + + class AuthConfigResponse(BaseModel): # Read unauthenticated by the login page to decide which login methods to # show. `oidc_provider_label` is the button text (e.g. "Okta"). diff --git a/core/switch_core/gateway/tenants.py b/core/switch_core/gateway/tenants.py index 00d7d49c2..45f84901c 100644 --- a/core/switch_core/gateway/tenants.py +++ b/core/switch_core/gateway/tenants.py @@ -1,29 +1,181 @@ from __future__ import annotations +import hashlib +import logging +import re +from datetime import UTC, datetime, timedelta from typing import Annotated from fastapi import APIRouter, Depends, HTTPException, Response +from sqlalchemy.exc import IntegrityError from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker +from switch_core.bridges.agent.protocol.service import ProtocolService +from switch_core.clients.client_lifecycle_service import ClientLifecycleService from switch_core.config import SwitchConfig +from switch_core.db.models import Invitation, Tenant, TenantMember, User +from switch_core.db.session_scope import tenant_session +from switch_core.db.stores.agent_store import AgentStore +from switch_core.db.stores.api_key_store import ApiKeyStore +from switch_core.db.stores.invitation_store import ( + InvitationNotUsableError, + InvitationStore, +) from switch_core.db.stores.user_store import UserStore from switch_core.gateway.auth import ( + AuthenticatedCaller, + get_authenticated_caller, get_authenticated_user_id, + get_current_user, + get_tenant_is_owner, is_tenant_member, list_tenant_memberships, + require_tenant_admin, set_session_cookie, + tenant_of_invitation_token, ) from switch_core.gateway.auth_routes import _session_response from switch_core.gateway.dependencies import ( + get_agent_store, + get_api_key_store, + get_client_lifecycle, get_config, + get_invitation_store, + get_protocol, + get_session, get_session_factory, get_system_session, get_user_store, ) -from switch_core.gateway.schemas import SessionUserResponse, TenantMembershipResponse +from switch_core.gateway.schemas import ( + InvitationAcceptRequest, + InvitationCreateRequest, + InvitationCreateResponse, + InvitationDetail, + MemberDetail, + MemberUpdateRequest, + SessionUserResponse, + TenantCreateRequest, + TenantMembershipResponse, +) +from switch_core.tenant_context import current_tenant_id + +logger = logging.getLogger(__name__) router = APIRouter() +TENANT_MEMBER_ROLES = ("owner", "admin", "member") + +_SLUG_INVALID_CHARS = re.compile(r"[^a-z0-9]+") + + +def _derive_slug(name: str) -> str: + """A URL-safe slug from a workspace name. + + A taken slug is a 409 (`create_tenant` below), never a silently + suffixed alternative — the design is explicit that a caller must be told + rather than handed a workspace under a name it did not ask for. + """ + slug = _SLUG_INVALID_CHARS.sub("-", name.strip().lower()).strip("-") + if not slug: + raise HTTPException( + status_code=400, + detail="Name must contain at least one letter or digit", + ) + return slug + + +def _require_bound_tenant(tenant_id: str) -> None: + """Raise 403 unless `tenant_id` names the tenant this request is bound to. + + Every write below this point uses the bound session, which writes into + the caller's *bound* tenant regardless of what a path segment says — + `TenantScoped.tenant_id` defaults to `require_tenant_id()`, not to + anything an endpoint parses. Without this check, a member of workspace A + naming workspace B in the path would not fail; it would silently act on A + instead, which is the one thing the error-handling philosophy this + codebase follows refuses to do. + """ + if current_tenant_id() != tenant_id: + raise HTTPException(status_code=403, detail="Not authorized for this tenant") + + +def _invitation_fields(invitation: Invitation) -> dict[str, object]: + return { + "id": invitation.id, + "role": invitation.role, + "email": invitation.email, + "expires_at": str(invitation.expires_at), + "uses_remaining": invitation.uses_remaining, + "revoked_at": str(invitation.revoked_at) if invitation.revoked_at else None, + "created_by": invitation.created_by, + "created_at": str(invitation.created_at), + } + + +def _invitation_detail(invitation: Invitation) -> InvitationDetail: + return InvitationDetail(**_invitation_fields(invitation)) + + +def _member_detail(user: User, membership: TenantMember) -> MemberDetail: + return MemberDetail( + user_id=user.id, + name=user.name, + email=user.email, + role=membership.role, + created_at=str(membership.created_at), + ) + + +def _require_owner(is_owner: bool, action: str) -> None: + """Raise 403 unless the caller owns the bound tenant. + + The three actions this guards — granting `owner`, changing an owner's + role, removing an owner — are the ones that move a workspace's ownership + set, and none of them is reachable by a last-owner guard: an admin who + promotes themselves first leaves two owners standing at every subsequent + step, so each individual request looks safe while the sequence takes the + workspace. `require_tenant_admin` still gates getting this far; this is + the narrower bit on top (`authz.owns_tenant`). + """ + if not is_owner: + raise HTTPException( + status_code=403, detail=f"Only a workspace owner may {action}" + ) + + +def _require_invitation_usable(invitation: Invitation, caller_email: str) -> None: + """Raise 403 unless `invitation` may still be accepted by `caller_email`. + + Checked in this order, but every branch is independent: revocation, + expiry, remaining uses and an addressed email are four separate ways an + invitation stops working, none of them optional + (`docs/old/multi-tenancy-phase2-tenants.md`, §5). + + This is for the message, not for the decision. Three of the four gates are + re-checked inside `InvitationStore.consume`'s own `UPDATE`, which is what + actually settles a race between two acceptances; read here, they can only + say why an invitation that is already unusable is unusable, in words the + invitee can act on. The fourth — the addressed email — is the one gate + only this function applies, because the store has no idea who is asking. + """ + if invitation.revoked_at is not None: + raise HTTPException(status_code=403, detail="This invitation has been revoked") + if invitation.expires_at < datetime.now(UTC): + raise HTTPException(status_code=403, detail="This invitation has expired") + if invitation.uses_remaining <= 0: + raise HTTPException( + status_code=403, detail="This invitation has already been used" + ) + if ( + invitation.email is not None + and invitation.email.lower() != caller_email.lower() + ): + raise HTTPException( + status_code=403, + detail="This invitation is addressed to a different email", + ) + @router.get("/tenants") async def list_tenants( @@ -45,6 +197,65 @@ async def list_tenants( return await list_tenant_memberships(session_factory, user_store, user_id) +@router.post("/tenants", status_code=201) +async def create_tenant( + req: TenantCreateRequest, + user_id: Annotated[str, Depends(get_authenticated_user_id)], + session_factory: Annotated[ + async_sessionmaker[AsyncSession], Depends(get_session_factory) + ], + user_store: Annotated[UserStore, Depends(get_user_store)], + client_lifecycle: Annotated[ClientLifecycleService, Depends(get_client_lifecycle)], +) -> TenantMembershipResponse: + """Create a workspace. The caller becomes its `owner`. + + Provisioning goes through `ClientLifecycleService.create_tenant` — "the + one seam a tenant comes into existence through" — rather than inserting a + `Tenant` row here, so a workspace created through this route gets the same + admin client every other tenant does. The membership row is not part of + that call (it provisions the tenant, not any particular person's place in + it), so it is written here, in its own session bound to the new tenant. + + Two transactions, therefore, and the gap between them is real: the tenant + is committed before the membership is attempted, so a failure in the + second leaves a workspace nobody belongs to and a slug nobody can reuse. + It is not folded into one because the tenant row has to be committed + before `ensure_system_client` can provision against it, and that call is + inside the seam. What the gap gets instead is a log line naming the + workspace and the person who should have owned it, because the alternative + — a 500 with the orphan unrecorded — is the silent degradation this + codebase refuses. An operator repairs it by inserting the membership. + """ + slug = _derive_slug(req.name) + try: + tenant = await client_lifecycle.create_tenant(req.name, slug) + except IntegrityError as exc: + raise HTTPException( + status_code=409, detail=f"Slug already taken: {slug}" + ) from exc + + try: + async with tenant_session(session_factory, tenant.id) as session: + await user_store.add_membership( + session, tenant_id=tenant.id, user_id=user_id, role="owner" + ) + await session.commit() + except Exception: + logger.error( + "Workspace %s (slug %s) was created but its owner membership for " + "user %s was not written: it now has no members and its slug is " + "taken. Insert the membership to repair it.", + tenant.id, + slug, + user_id, + ) + raise + + return TenantMembershipResponse( + id=tenant.id, slug=tenant.slug, name=tenant.name, role="owner" + ) + + @router.post("/tenants/{tenant_id}/switch") async def switch_tenant( tenant_id: str, @@ -78,3 +289,276 @@ async def switch_tenant( response, user, config.jwt_secret_key, config.gateway_cookie_secure, tenant_id ) return _session_response(user) + + +# ── Invitations ─────────────────────────────────────────────────────────────── + + +@router.post("/tenants/{tenant_id}/invitations", status_code=201) +async def create_invitation( + tenant_id: str, + req: InvitationCreateRequest, + session: Annotated[AsyncSession, Depends(get_session)], + invitation_store: Annotated[InvitationStore, Depends(get_invitation_store)], + user: Annotated[User, Depends(require_tenant_admin)], + is_owner: Annotated[bool, Depends(get_tenant_is_owner)], +) -> InvitationCreateResponse: + """Mint an invitation to the bound tenant. `owner`/`admin` only. + + An `owner` invitation is owner-only: minting one is granting ownership + with a step of indirection, so it answers to the same gate the direct + grant does (`_require_owner`). + """ + _require_bound_tenant(tenant_id) + if req.role not in TENANT_MEMBER_ROLES: + raise HTTPException(status_code=400, detail=f"Invalid role: {req.role}") + if req.role == "owner": + _require_owner(is_owner, "invite another owner") + + expires_at = datetime.now(UTC) + timedelta(hours=req.expires_in_hours) + invitation, token = await invitation_store.create( + session, + role=req.role, + email=req.email, + expires_at=expires_at, + uses_remaining=req.uses_remaining, + created_by=user.id, + ) + await session.commit() + return InvitationCreateResponse(token=token, **_invitation_fields(invitation)) + + +@router.get("/tenants/{tenant_id}/invitations") +async def list_invitations( + tenant_id: str, + session: Annotated[AsyncSession, Depends(get_session)], + invitation_store: Annotated[InvitationStore, Depends(get_invitation_store)], + _user: Annotated[User, Depends(require_tenant_admin)], +) -> list[InvitationDetail]: + """Every invitation of the bound tenant. `owner`/`admin` only.""" + _require_bound_tenant(tenant_id) + invitations = await invitation_store.list_for_tenant(session) + return [_invitation_detail(i) for i in invitations] + + +@router.delete("/tenants/{tenant_id}/invitations/{invitation_id}") +async def revoke_invitation( + tenant_id: str, + invitation_id: str, + session: Annotated[AsyncSession, Depends(get_session)], + invitation_store: Annotated[InvitationStore, Depends(get_invitation_store)], + _user: Annotated[User, Depends(require_tenant_admin)], +) -> InvitationDetail: + """Revoke an invitation of the bound tenant. `owner`/`admin` only.""" + _require_bound_tenant(tenant_id) + try: + invitation = await invitation_store.revoke(session, invitation_id) + except ValueError as exc: + raise HTTPException(status_code=404, detail=str(exc)) from exc + await session.commit() + return _invitation_detail(invitation) + + +@router.post("/invitations/accept") +async def accept_invitation( + req: InvitationAcceptRequest, + caller: Annotated[AuthenticatedCaller, Depends(get_authenticated_caller)], + session_factory: Annotated[ + async_sessionmaker[AsyncSession], Depends(get_session_factory) + ], + user_store: Annotated[UserStore, Depends(get_user_store)], + invitation_store: Annotated[InvitationStore, Depends(get_invitation_store)], +) -> TenantMembershipResponse: + """Accept an invitation, joining its tenant. + + Authenticated with `get_authenticated_caller`, not `get_current_user`: + the caller's own session may be bound to a different tenant than the + invitation names, or to none at all, and inserting a membership for a + tenant other than the one a session is bound to is refused by the + policy — rebinding an already-open session raises + (`TenantBindingDriftError`, `db/tenant_session.py`). So this handler + resolves the token's tenant through the exempt lookup and does all of its + own work inside a fresh `tenant_session` bound to exactly that tenant, + rather than touching the request's session at all + (`docs/old/multi-tenancy-phase2-tenants.md`, §5). + + The token arrives in the body, not the path: it is a bearer credential, + and a path segment is written to proxy and access logs, kept in browser + history, and sent onward in `Referer`. + + A use is spent only when a membership is actually granted. Accepting an + invitation you already hold is a no-op that returns your existing role — + a double-clicked shared link must not cost the invite a slot — and the + `consume` that does spend one is a conditional `UPDATE` that arbitrates + between simultaneous acceptances, so a single-use link grants exactly one + membership however many people race for it. + """ + token_hash = hashlib.sha256(req.token.encode()).hexdigest() + tenant_id = await tenant_of_invitation_token(session_factory, token_hash) + if tenant_id is None: + raise HTTPException(status_code=404, detail="Invitation not found") + + async with tenant_session(session_factory, tenant_id) as session: + invitation = await invitation_store.get_by_token_hash(session, token_hash) + if invitation is None: + raise HTTPException(status_code=404, detail="Invitation not found") + _require_invitation_usable(invitation, caller.email) + + existing_role = await user_store.tenant_role(session, tenant_id, caller.id) + if existing_role is None: + try: + await invitation_store.consume(session, invitation.id) + except InvitationNotUsableError as exc: + raise HTTPException( + status_code=403, detail="This invitation has already been used" + ) from exc + role = invitation.role + await user_store.add_membership( + session, tenant_id=tenant_id, user_id=caller.id, role=role + ) + else: + role = existing_role + + tenant = await session.get(Tenant, tenant_id) + assert tenant is not None + await session.commit() + + return TenantMembershipResponse( + id=tenant.id, slug=tenant.slug, name=tenant.name, role=role + ) + + +# ── Members ─────────────────────────────────────────────────────────────────── + + +@router.get("/tenants/{tenant_id}/members") +async def list_members( + tenant_id: str, + session: Annotated[AsyncSession, Depends(get_session)], + user_store: Annotated[UserStore, Depends(get_user_store)], + _user: Annotated[User, Depends(get_current_user)], +) -> list[MemberDetail]: + """Every member of the bound tenant. Any member may list their own + workspace's roster; changing or removing one is admin-gated below.""" + _require_bound_tenant(tenant_id) + members = await user_store.list_tenant_members(session) + return [_member_detail(user, membership) for user, membership in members] + + +@router.patch("/tenants/{tenant_id}/members/{user_id}") +async def update_member_role( + tenant_id: str, + user_id: str, + req: MemberUpdateRequest, + session: Annotated[AsyncSession, Depends(get_session)], + user_store: Annotated[UserStore, Depends(get_user_store)], + _admin: Annotated[User, Depends(require_tenant_admin)], + is_owner: Annotated[bool, Depends(get_tenant_is_owner)], +) -> MemberDetail: + """Change a member's role. `owner`/`admin` only. + + Both ends of the ownership set are owner-only: an admin may promote a + member to admin, but not to `owner`, and may not touch an existing + owner's role at all. See `_require_owner` for what an admin who could do + either would be able to do in three requests. + + Refuses to demote the last `owner` on top of that: with no workspace + deletion in this phase, there is no legitimate route to a workspace with + none. + """ + _require_bound_tenant(tenant_id) + if req.role not in TENANT_MEMBER_ROLES: + raise HTTPException(status_code=400, detail=f"Invalid role: {req.role}") + + membership = await session.get(TenantMember, (tenant_id, user_id)) + if membership is None: + raise HTTPException(status_code=404, detail="Not a member of this tenant") + + if req.role == "owner" and membership.role != "owner": + _require_owner(is_owner, "grant ownership") + + if membership.role == "owner" and req.role != "owner": + _require_owner(is_owner, "change an owner's role") + if await user_store.count_owners(session) <= 1: + raise HTTPException(status_code=409, detail="Cannot demote the last owner") + + membership.role = req.role + await session.commit() + + user = await user_store.get(session, user_id) + assert user is not None + return _member_detail(user, membership) + + +@router.delete("/tenants/{tenant_id}/members/{user_id}") +async def remove_member( + tenant_id: str, + user_id: str, + session: Annotated[AsyncSession, Depends(get_session)], + user_store: Annotated[UserStore, Depends(get_user_store)], + agent_store: Annotated[AgentStore, Depends(get_agent_store)], + api_key_store: Annotated[ApiKeyStore, Depends(get_api_key_store)], + protocol: Annotated[ProtocolService, Depends(get_protocol)], + _admin: Annotated[User, Depends(require_tenant_admin)], + is_owner: Annotated[bool, Depends(get_tenant_is_owner)], +) -> dict[str, bool]: + """Remove a member from the bound tenant. `owner`/`admin` only. + + Removing an *owner* is owner-only, the same as demoting one and for the + same reason (`_require_owner`), and refuses on the last owner besides. + + Removal also revokes what the member's own credentials in this tenant let + them do here, in the same transaction as the membership row going away — + a bearer credential resolves its tenant from its own row and never + consults membership, so leaving it behind would leave the person with + working, invisible access (`docs/old/multi-tenancy-phase2-tenants.md`, + §3). Concretely, this deletes every personal API key the member holds + here. + + It does **not** touch any agent the member owns — it refuses instead, 409, + naming them. An agent is not that member's private property to lose along + with their membership: it sits in rooms with other people, it may be the + only copy of a working configuration, and its name is something other + members already depend on. A personal key is trivially recoverable — mint + another — but deleting an agent is not, and a member removal is routine + enough, including by mistake, that it must not be the thing that makes an + irreversible call about shared infrastructure. An admin who hits this + deletes those agents deliberately, with the agent in front of them, then + removes the member. The 409 says exactly that and nothing more: there is + no route that reassigns an agent's owner in this phase, so offering + reassignment as the way out would send an admin looking for a button that + does not exist. + """ + _require_bound_tenant(tenant_id) + membership = await session.get(TenantMember, (tenant_id, user_id)) + if membership is None: + raise HTTPException(status_code=404, detail="Not a member of this tenant") + + if membership.role == "owner": + _require_owner(is_owner, "remove an owner") + if await user_store.count_owners(session) <= 1: + raise HTTPException(status_code=409, detail="Cannot remove the last owner") + + owned_agents = await agent_store.get_by_owner(session, user_id) + if owned_agents: + names = ", ".join(sorted(agent.name for agent in owned_agents)) + raise HTTPException( + status_code=409, + detail=( + "Cannot remove: this member owns agents in this tenant — " + f"delete them first: {names}" + ), + ) + + keys = await api_key_store.get_by_user(session, user_id) + revoked_key_hashes = [key.key_hash for key in keys] + for key in keys: + await api_key_store.delete(session, key.id) + + await session.delete(membership) + await session.commit() + + for key_hash in revoked_key_hashes: + protocol.api_key_cache.invalidate(key_hash) + + return {"ok": True} diff --git a/core/switch_core/main.py b/core/switch_core/main.py index 4750e9f67..321eafc47 100644 --- a/core/switch_core/main.py +++ b/core/switch_core/main.py @@ -97,6 +97,7 @@ from switch_core.db.stores.collaboration_bridge_store import CollaborationBridgeStore from switch_core.db.stores.document_store import DocumentStore from switch_core.db.stores.external_user_store import ExternalUserStore +from switch_core.db.stores.invitation_store import InvitationStore from switch_core.db.stores.media_store import MediaStore from switch_core.db.stores.message_store import MessageStore from switch_core.db.stores.package_store import PackageStore @@ -281,6 +282,7 @@ async def run(config: SwitchConfig) -> None: bridge_message_map_store = BridgeMessageMapStore() user_store = UserStore() api_key_store = ApiKeyStore() + invitation_store = InvitationStore() tenant_store = TenantStore() reference_store = ReferenceStore() reference_type_store = ReferenceTypeStore() @@ -489,6 +491,7 @@ async def run(config: SwitchConfig) -> None: user_store=user_store, external_user_store=external_user_store, api_key_store=api_key_store, + invitation_store=invitation_store, template_store=template_store, resource_service=resource_service, protocol=protocol, diff --git a/core/tests/switch_core/gateway/test_session_requires_authentication.py b/core/tests/switch_core/gateway/test_session_requires_authentication.py index 31895eb62..d362550c4 100644 --- a/core/tests/switch_core/gateway/test_session_requires_authentication.py +++ b/core/tests/switch_core/gateway/test_session_requires_authentication.py @@ -31,6 +31,7 @@ import switch_core.gateway as gateway_package from switch_core.gateway.auth import ( + get_authenticated_caller, get_authenticated_user_id, get_current_user, require_admin, @@ -50,15 +51,39 @@ ("POST", "/tenants/{tenant_id}/switch"), } -# The only routes that may authenticate a caller without resolving a tenant at -# all. `get_authenticated_user_id` verifies the cookie and stops there, so it -# is a reachable way past the membership check `get_current_user` performs — -# every case of `_resolve_tenant_id`, including the two 403s. These two need -# it because a caller with several memberships and no selection cannot reach -# `get_current_user` by construction; nothing else has that excuse. -_ROUTES_AUTHENTICATED_WITHOUT_RESOLVING_A_TENANT = { +# Every route that does not resolve a tenant, for any reason. `get_current_user` +# is the only dependency that performs the membership check in +# `_resolve_tenant_id`, so a route without it authorizes against no workspace — +# whether it skipped the check deliberately (`get_authenticated_user_id`, +# `get_authenticated_caller`) or simply has no caller yet (login, the OIDC +# handshake). +# +# The predicate is the absence of `get_current_user` rather than the presence of +# any particular alternative, because the alternatives keep arriving: keying on +# `get_system_session` missed the routes that take `get_session_factory` and open +# their own, and keying on `get_authenticated_user_id` missed +# `get_authenticated_caller`. This way the next door is a failing test rather +# than a route nobody counted. `_ROUTES_WITH_NO_TENANT_BOUND` is a strict subset. +_ROUTES_THAT_NEVER_BIND_A_TENANT = { + # No caller yet: the sign-in surface and what it hands back. + ("GET", "/auth/config"), + ("GET", "/auth/oidc/login"), + ("GET", "/auth/oidc/callback"), + ("POST", "/auth/login"), + ("POST", "/auth/logout"), + # A deployment-wide constant — the agent types this build knows about. + # Nothing tenant-specific to scope it to. + ("GET", "/known-types"), + # A caller with several memberships and no selection cannot reach + # `get_current_user` by construction, and these are the routes such a + # caller needs: list the workspaces, make one, pick one. ("GET", "/tenants"), + ("POST", "/tenants"), ("POST", "/tenants/{tenant_id}/switch"), + # Accepting an invitation binds the tenant the *token* names, which is + # neither the one on the caller's session nor one they belong to yet. It + # opens its own `tenant_session` around that id — see `accept_invitation`. + ("POST", "/invitations/accept"), } @@ -139,16 +164,38 @@ def test_the_routes_with_no_tenant_bound_are_exactly_these_three() -> None: } == _ROUTES_WITH_NO_TENANT_BOUND -def test_the_routes_skipping_tenant_resolution_are_exactly_these_two() -> None: - """`get_authenticated_user_id` is the newest way past the tenant check, and - the one with the least around it: no membership read, no scoped session, - nothing a policy would refuse. Pinned by the same reasoning - `test_tenant_exemption_allowlist` gives for the `SECURITY DEFINER` - lookups — what matters is that it is reachable, not that today's two - callers happen to be the right ones.""" +def test_the_routes_that_never_bind_a_tenant_are_exactly_these() -> None: + """Pinned by the same reasoning `test_tenant_exemption_allowlist` gives for + the `SECURITY DEFINER` lookups — what matters is that skipping the tenant + check is *reachable*, not that today's callers happen to be the right ones. + A route that lands here has no workspace to authorize against and has to + say, in its own docstring, what it does instead.""" assert { - route.key for route in ROUTES if get_authenticated_user_id in route.calls - } == _ROUTES_AUTHENTICATED_WITHOUT_RESOLVING_A_TENANT + route.key for route in ROUTES if get_current_user not in route.calls + } == _ROUTES_THAT_NEVER_BIND_A_TENANT + + +def test_the_routes_with_no_tenant_bound_do_not_bind_a_tenant() -> None: + """The two exemptions agree: anything allowed to open an unscoped session + is, necessarily, a route that resolves no tenant.""" + assert _ROUTES_WITH_NO_TENANT_BOUND <= _ROUTES_THAT_NEVER_BIND_A_TENANT + + +def test_skipping_tenant_resolution_is_only_reached_by_a_known_dependency() -> None: + """`get_authenticated_user_id` and `get_authenticated_caller` verify the + cookie and stop there — no membership read, no scoped session, nothing a + policy would refuse. They are the only authenticated way into the set + above; a route that invents a third is caught here rather than inheriting + the exemption quietly.""" + assert not [ + str(route) + for route in ROUTES + if ( + get_authenticated_user_id in route.calls + or get_authenticated_caller in route.calls + ) + and route.key not in _ROUTES_THAT_NEVER_BIND_A_TENANT + ] def test_no_route_both_resolves_a_tenant_and_skips_resolving_one() -> None: diff --git a/core/tests/switch_core/gateway/test_tenant_api_routes.py b/core/tests/switch_core/gateway/test_tenant_api_routes.py new file mode 100644 index 000000000..f023dd4d2 --- /dev/null +++ b/core/tests/switch_core/gateway/test_tenant_api_routes.py @@ -0,0 +1,895 @@ +"""The route half of CHOO-2722: `gateway/tenants.py`'s workspace, invitation +and member routes. + +Builds route-scoped apps the same way `test_tenant_resolution.py` does — +`switch_core.gateway.dependencies`' functions overridden individually rather +than through `init_dependencies` — so nothing here leaks into another test and +nothing but `switch_core.gateway.tenants` itself is exercised for real. + +`_FakeClientLifecycle.create_tenant` mirrors +`ClientLifecycleService.create_tenant`'s DB half (insert the tenant row inside +a `tenant_session` bound to its own id) without the Matrix admin-client +provisioning, which needs a running homeserver these tests have no business +depending on. The unique-slug behaviour under test comes from the same +`tenants.slug` column either way. +""" + +from __future__ import annotations + +import uuid +from datetime import UTC, datetime, timedelta +from types import SimpleNamespace + +import httpx +from fastapi import FastAPI +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker + +from switch_core.db.models import ( + Agent, + ApiKey, + Client, + Invitation, + Tenant, + TenantMember, + User, +) +from switch_core.db.session_scope import tenant_session +from switch_core.db.stores.agent_store import AgentStore +from switch_core.db.stores.api_key_store import ApiKeyStore +from switch_core.db.stores.invitation_store import InvitationStore +from switch_core.db.stores.user_store import UserStore +from switch_core.gateway import dependencies as gw_deps +from switch_core.gateway.auth import create_jwt +from switch_core.gateway.tenants import router as tenants_router + +_SECRET = "unit-test-jwt-key-unit-test-jwt-key-unit-test" # gitleaks:allow +TENANT_A = "tenant-api-routes-a" +TENANT_B = "tenant-api-routes-b" + + +class _FakeClientLifecycle: + def __init__(self, session_factory: async_sessionmaker[AsyncSession]) -> None: + self._session_factory = session_factory + + async def create_tenant(self, name: str, slug: str) -> Tenant: + tenant = Tenant(id=str(uuid.uuid4()), name=name, slug=slug) + async with tenant_session(self._session_factory, tenant.id) as session: + session.add(tenant) + await session.commit() + return tenant + + +def _fake_protocol() -> SimpleNamespace: + return SimpleNamespace( + api_key_cache=SimpleNamespace( + invalidate=lambda key_hash: None, + invalidate_agent=lambda agent_id: None, + ) + ) + + +def _app( + session_factory: async_sessionmaker[AsyncSession], + *, + client_lifecycle: object | None = None, +) -> FastAPI: + async def _session_dep(): + async with session_factory() as session: + yield session + + app = FastAPI() + app.include_router(tenants_router) + app.dependency_overrides[gw_deps.get_session] = _session_dep + app.dependency_overrides[gw_deps.get_system_session] = _session_dep + app.dependency_overrides[gw_deps.get_session_factory] = lambda: session_factory + app.dependency_overrides[gw_deps.get_user_store] = lambda: UserStore() + app.dependency_overrides[gw_deps.get_agent_store] = lambda: AgentStore() + app.dependency_overrides[gw_deps.get_api_key_store] = lambda: ApiKeyStore() + app.dependency_overrides[gw_deps.get_invitation_store] = lambda: InvitationStore() + app.dependency_overrides[gw_deps.get_protocol] = lambda: _fake_protocol() + app.dependency_overrides[gw_deps.get_client_lifecycle] = lambda: ( + client_lifecycle or _FakeClientLifecycle(session_factory) + ) + app.dependency_overrides[gw_deps.get_config] = lambda: SimpleNamespace( + jwt_secret_key=_SECRET, + gateway_cookie_secure=False, + gateway_tenant_choice_enabled=False, + ) + return app + + +def _client(app: FastAPI, token: str) -> httpx.AsyncClient: + return httpx.AsyncClient( + transport=httpx.ASGITransport(app=app), + base_url="http://test", + cookies={"switch_auth": token}, + ) + + +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 _make_member( + session_factory: async_sessionmaker[AsyncSession], + *, + name: str, + tenant_id: str, + role: str, + email: str | None = None, +) -> str: + async with session_factory() as session: + user = User(name=name, email=email or f"{name}@example.invalid", role="user") + session.add(user) + await session.flush() + session.add(TenantMember(tenant_id=tenant_id, user_id=user.id, role=role)) + await session.commit() + return user.id + + +def _token(user_id: str, email: str, tenant_id: str) -> str: + return create_jwt(user_id, email, "user", _SECRET, tenant_id) + + +async def _make_api_key( + session_factory: async_sessionmaker[AsyncSession], + *, + tenant_id: str, + user_id: str, + key_type: str = "registration", +) -> str: + """A personal (non-agent) API key for `user_id` in `tenant_id`. Returns + its `key_hash`.""" + key_hash = uuid.uuid4().hex + async with session_factory() as session: + session.add( + ApiKey( + tenant_id=tenant_id, + user_id=user_id, + key_hash=key_hash, + encrypted_key="unused-in-tests", + label="test-key", + type=key_type, + ) + ) + await session.commit() + return key_hash + + +async def _make_agent_with_key( + session_factory: async_sessionmaker[AsyncSession], + *, + tenant_id: str, + owner_id: str, +) -> tuple[str, str, str]: + """An agent owned by `owner_id`, plus the `agents`-type key backing it. + + Returns `(agent_id, agent_name, key_hash)`. The agent's own client and + api_key rows are constructed first — `agents.client_id` and + `agents.api_key_id` are both foreign keys, not free-form strings. + """ + key_hash = uuid.uuid4().hex + name = f"agent-{uuid.uuid4().hex[:8]}" + async with session_factory() as session: + client = Client( + tenant_id=tenant_id, + matrix_user_id=f"@bot-{uuid.uuid4().hex[:8]}:test", + display_name="test-agent-bot", + type="agent", + ) + session.add(client) + await session.flush() + + api_key = ApiKey( + tenant_id=tenant_id, + user_id=owner_id, + key_hash=key_hash, + encrypted_key="unused-in-tests", + label="agent-key", + type="agent", + ) + session.add(api_key) + await session.flush() + + agent = Agent( + tenant_id=tenant_id, + name=name, + description="test agent", + agent_type="other", + connector_type="claude-code", + integration_profile={}, + client_id=client.id, + api_key_id=api_key.id, + owner_id=owner_id, + ) + session.add(agent) + await session.commit() + return agent.id, name, key_hash + + +class TestCreateTenant: + async def test_creating_a_tenant_makes_the_caller_its_owner( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + await _make_tenant(session_factory, TENANT_A) + user_id = await _make_member( + session_factory, name="founder", tenant_id=TENANT_A, role="member" + ) + token = _token(user_id, "founder@example.invalid", TENANT_A) + + async with _client(_app(session_factory), token) as client: + response = await client.post("/tenants", json={"name": "Acme Corp"}) + + assert response.status_code == 201, response.text + body = response.json() + assert body["slug"] == "acme-corp" + assert body["role"] == "owner" + + async with session_factory() as session: + membership = await session.get(TenantMember, (body["id"], user_id)) + assert membership is not None + assert membership.role == "owner" + + async def test_a_taken_slug_is_409_not_a_silent_suffix( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + await _make_tenant(session_factory, TENANT_A) + user_id = await _make_member( + session_factory, name="second-founder", tenant_id=TENANT_A, role="member" + ) + token = _token(user_id, "second-founder@example.invalid", TENANT_A) + + async with _client(_app(session_factory), token) as client: + first = await client.post("/tenants", json={"name": "Widgets Inc"}) + assert first.status_code == 201, first.text + second = await client.post("/tenants", json={"name": "Widgets Inc"}) + + assert second.status_code == 409 + + async def test_a_name_with_no_slug_characters_is_400( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + await _make_tenant(session_factory, TENANT_A) + user_id = await _make_member( + session_factory, name="emoji-fan", tenant_id=TENANT_A, role="member" + ) + token = _token(user_id, "emoji-fan@example.invalid", TENANT_A) + + async with _client(_app(session_factory), token) as client: + response = await client.post("/tenants", json={"name": "!!!"}) + + assert response.status_code == 400 + + +class TestInvitationAuthorisation: + """A member of workspace A cannot mint an invitation to workspace B — + each request is bound to exactly one tenant, and the path segment cannot + override that.""" + + async def test_a_plain_member_cannot_mint_an_invitation( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + await _make_tenant(session_factory, TENANT_A) + user_id = await _make_member( + session_factory, name="rank-and-file", tenant_id=TENANT_A, role="member" + ) + token = _token(user_id, "rank-and-file@example.invalid", TENANT_A) + + async with _client(_app(session_factory), token) as client: + response = await client.post( + f"/tenants/{TENANT_A}/invitations", json={"role": "member"} + ) + + assert response.status_code == 403 + + async def test_an_admin_of_a_cannot_mint_an_invitation_naming_b( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + await _make_tenant(session_factory, TENANT_A) + await _make_tenant(session_factory, TENANT_B) + user_id = await _make_member( + session_factory, name="a-admin", tenant_id=TENANT_A, role="admin" + ) + token = _token(user_id, "a-admin@example.invalid", TENANT_A) + + async with _client(_app(session_factory), token) as client: + response = await client.post( + f"/tenants/{TENANT_B}/invitations", json={"role": "member"} + ) + + assert response.status_code == 403 + + async with tenant_session(session_factory, TENANT_B) as scoped: + assert await InvitationStore().list_for_tenant(scoped) == [] + + async def test_an_owner_can_mint_an_invitation_to_their_own_tenant( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + await _make_tenant(session_factory, TENANT_A) + user_id = await _make_member( + session_factory, name="a-owner", tenant_id=TENANT_A, role="owner" + ) + token = _token(user_id, "a-owner@example.invalid", TENANT_A) + + async with _client(_app(session_factory), token) as client: + response = await client.post( + f"/tenants/{TENANT_A}/invitations", + json={"role": "member", "uses_remaining": 3}, + ) + + assert response.status_code == 201, response.text + body = response.json() + assert body["role"] == "member" + assert body["uses_remaining"] == 3 + assert "token" in body and body["token"] + + async def test_an_absurd_expiry_is_refused_rather_than_crashing( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + """`timedelta` raises `OverflowError` well before `int` runs out, so an + expiry bounded only from below turns a bad request into a 500. The + bound belongs on the schema, where it is a 422 with a field name.""" + await _make_tenant(session_factory, TENANT_A) + user_id = await _make_member( + session_factory, name="time-lord", tenant_id=TENANT_A, role="owner" + ) + token = _token(user_id, "time-lord@example.invalid", TENANT_A) + + async with _client(_app(session_factory), token) as client: + response = await client.post( + f"/tenants/{TENANT_A}/invitations", + json={"role": "member", "expires_in_hours": 1000000000}, + ) + + assert response.status_code == 422, response.text + + +class TestOwnershipIsOwnerOnly: + """Who owns a workspace is decided by its owners, not by its admins. + + An `admin` runs the workspace; only an `owner` moves the ownership set. + The distinction exists because a last-owner guard cannot see a sequence: + each request below is individually safe — two owners stand at every + step — and together they hand the workspace to someone the founder never + promoted. + """ + + async def test_an_admin_cannot_take_the_workspace_from_its_owner( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + await _make_tenant(session_factory, TENANT_A) + founder_id = await _make_member( + session_factory, name="founder-owner", tenant_id=TENANT_A, role="owner" + ) + admin_id = await _make_member( + session_factory, name="ambitious-admin", tenant_id=TENANT_A, role="admin" + ) + token = _token(admin_id, "ambitious-admin@example.invalid", TENANT_A) + + async with _client(_app(session_factory), token) as client: + promote_self = await client.patch( + f"/tenants/{TENANT_A}/members/{admin_id}", json={"role": "owner"} + ) + demote_founder = await client.patch( + f"/tenants/{TENANT_A}/members/{founder_id}", json={"role": "member"} + ) + remove_founder = await client.delete( + f"/tenants/{TENANT_A}/members/{founder_id}" + ) + + assert promote_self.status_code == 403, promote_self.text + assert demote_founder.status_code == 403, demote_founder.text + assert remove_founder.status_code == 403, remove_founder.text + + async with session_factory() as session: + founder = await session.get(TenantMember, (TENANT_A, founder_id)) + admin = await session.get(TenantMember, (TENANT_A, admin_id)) + assert founder is not None and founder.role == "owner" + assert admin is not None and admin.role == "admin" + + async def test_an_admin_cannot_mint_an_owner_invitation( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + """Otherwise the escalation above is the same three steps with a link + in the middle: invite an accomplice as owner, then let them do it.""" + await _make_tenant(session_factory, TENANT_A) + await _make_member( + session_factory, name="quiet-owner", tenant_id=TENANT_A, role="owner" + ) + admin_id = await _make_member( + session_factory, name="inviting-admin", tenant_id=TENANT_A, role="admin" + ) + token = _token(admin_id, "inviting-admin@example.invalid", TENANT_A) + + async with _client(_app(session_factory), token) as client: + refused = await client.post( + f"/tenants/{TENANT_A}/invitations", json={"role": "owner"} + ) + allowed = await client.post( + f"/tenants/{TENANT_A}/invitations", json={"role": "admin"} + ) + + assert refused.status_code == 403, refused.text + assert allowed.status_code == 201, allowed.text + + async with tenant_session(session_factory, TENANT_A) as scoped: + roles = [i.role for i in await InvitationStore().list_for_tenant(scoped)] + assert roles == ["admin"] + + async def test_an_owner_can_do_all_of_it( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + """The guard is narrower authority, not a locked door: everything the + admin was refused above succeeds for an owner.""" + await _make_tenant(session_factory, TENANT_A) + owner_id = await _make_member( + session_factory, name="real-owner", tenant_id=TENANT_A, role="owner" + ) + member_id = await _make_member( + session_factory, name="promoted", tenant_id=TENANT_A, role="member" + ) + token = _token(owner_id, "real-owner@example.invalid", TENANT_A) + + async with _client(_app(session_factory), token) as client: + promote = await client.patch( + f"/tenants/{TENANT_A}/members/{member_id}", json={"role": "owner"} + ) + demote = await client.patch( + f"/tenants/{TENANT_A}/members/{member_id}", json={"role": "member"} + ) + invite = await client.post( + f"/tenants/{TENANT_A}/invitations", json={"role": "owner"} + ) + remove = await client.delete(f"/tenants/{TENANT_A}/members/{member_id}") + + assert promote.status_code == 200, promote.text + assert promote.json()["role"] == "owner" + assert demote.status_code == 200, demote.text + assert invite.status_code == 201, invite.text + assert remove.status_code == 200, remove.text + + +class TestInvitationLifecycle: + async def _mint( + self, + session_factory: async_sessionmaker[AsyncSession], + *, + tenant_id: str, + admin_id: str, + role: str = "member", + email: str | None = None, + uses_remaining: int = 1, + expires_in_hours: int = 168, + ) -> tuple[str, str]: + """Mint directly through the store (not the route) so a lifecycle + test does not depend on `TestInvitationAuthorisation` passing first. + Returns `(invitation_id, token)`.""" + async with tenant_session(session_factory, tenant_id) as session: + invitation, token = await InvitationStore().create( + session, + role=role, + email=email, + expires_at=datetime.now(UTC) + timedelta(hours=expires_in_hours), + uses_remaining=uses_remaining, + created_by=admin_id, + ) + await session.commit() + return invitation.id, token + + async def test_accepting_grants_membership_in_the_invited_role( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + await _make_tenant(session_factory, TENANT_A) + admin_id = await _make_member( + session_factory, name="inviter", tenant_id=TENANT_A, role="owner" + ) + _id, token = await self._mint( + session_factory, tenant_id=TENANT_A, admin_id=admin_id, role="admin" + ) + await _make_tenant(session_factory, TENANT_B) + invitee_id = await _make_member( + session_factory, name="invitee", tenant_id=TENANT_B, role="member" + ) + caller_token = _token(invitee_id, "invitee@example.invalid", TENANT_B) + + async with _client(_app(session_factory), caller_token) as client: + response = await client.post("/invitations/accept", json={"token": token}) + + assert response.status_code == 200, response.text + body = response.json() + assert body["id"] == TENANT_A + assert body["role"] == "admin" + + async with session_factory() as session: + membership = await session.get(TenantMember, (TENANT_A, invitee_id)) + assert membership is not None + assert membership.role == "admin" + + async def test_re_accepting_an_invitation_you_already_hold_costs_nothing( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + """A shared link is double-clicked, or reloaded, or opened on a phone + as well. The second acceptance grants nothing new, so it must spend + nothing either — otherwise one careless refresh burns a seat that was + meant for somebody else, and the invitation dies with nobody's + membership to show for it. + """ + await _make_tenant(session_factory, TENANT_A) + admin_id = await _make_member( + session_factory, name="link-sharer", tenant_id=TENANT_A, role="owner" + ) + invitation_id, token = await self._mint( + session_factory, tenant_id=TENANT_A, admin_id=admin_id, uses_remaining=2 + ) + await _make_tenant(session_factory, TENANT_B) + invitee_id = await _make_member( + session_factory, name="double-clicker", tenant_id=TENANT_B, role="member" + ) + + app = _app(session_factory) + async with _client( + app, _token(invitee_id, "double-clicker@example.invalid", TENANT_B) + ) as client: + first = await client.post("/invitations/accept", json={"token": token}) + second = await client.post("/invitations/accept", json={"token": token}) + + assert first.status_code == 200, first.text + assert second.status_code == 200, second.text + assert second.json()["role"] == first.json()["role"] + + async with tenant_session(session_factory, TENANT_A) as session: + invitation = await session.get(Invitation, invitation_id) + assert invitation is not None + assert invitation.uses_remaining == 1 + + async def test_accepting_twice_fails_the_second_time( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + await _make_tenant(session_factory, TENANT_A) + admin_id = await _make_member( + session_factory, name="inviter2", tenant_id=TENANT_A, role="owner" + ) + _id, token = await self._mint( + session_factory, tenant_id=TENANT_A, admin_id=admin_id, uses_remaining=1 + ) + await _make_tenant(session_factory, TENANT_B) + first_invitee = await _make_member( + session_factory, name="first-invitee", tenant_id=TENANT_B, role="member" + ) + second_invitee = await _make_member( + session_factory, name="second-invitee", tenant_id=TENANT_B, role="member" + ) + + app = _app(session_factory) + async with _client( + app, _token(first_invitee, "first-invitee@example.invalid", TENANT_B) + ) as client: + first = await client.post("/invitations/accept", json={"token": token}) + assert first.status_code == 200, first.text + + async with _client( + app, _token(second_invitee, "second-invitee@example.invalid", TENANT_B) + ) as client: + second = await client.post("/invitations/accept", json={"token": token}) + + assert second.status_code == 403 + assert "used" in second.json()["detail"].lower() + + async with session_factory() as session: + assert await session.get(TenantMember, (TENANT_A, second_invitee)) is None + + async def test_a_revoked_invitation_is_refused( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + await _make_tenant(session_factory, TENANT_A) + admin_id = await _make_member( + session_factory, name="revoker", tenant_id=TENANT_A, role="owner" + ) + invitation_id, token = await self._mint( + session_factory, tenant_id=TENANT_A, admin_id=admin_id + ) + async with tenant_session(session_factory, TENANT_A) as session: + await InvitationStore().revoke(session, invitation_id) + await session.commit() + + await _make_tenant(session_factory, TENANT_B) + invitee_id = await _make_member( + session_factory, name="too-late", tenant_id=TENANT_B, role="member" + ) + + async with _client( + _app(session_factory), + _token(invitee_id, "too-late@example.invalid", TENANT_B), + ) as client: + response = await client.post("/invitations/accept", json={"token": token}) + + assert response.status_code == 403 + assert "revoked" in response.json()["detail"].lower() + + async def test_an_expired_invitation_is_refused( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + await _make_tenant(session_factory, TENANT_A) + admin_id = await _make_member( + session_factory, name="expiry-setter", tenant_id=TENANT_A, role="owner" + ) + _id, token = await self._mint( + session_factory, + tenant_id=TENANT_A, + admin_id=admin_id, + expires_in_hours=1, + ) + # Force it into the past directly — the store only accepts a future + # `expires_at` implicitly by convention, not by constraint, so this is + # the straightforward way to get an already-expired row. + async with session_factory() as session: + result = await session.execute(select(Invitation)) + invitation = result.scalars().one() + invitation.expires_at = datetime.now(UTC) - timedelta(hours=1) # type: ignore[assignment] + await session.commit() + + await _make_tenant(session_factory, TENANT_B) + invitee_id = await _make_member( + session_factory, name="too-slow", tenant_id=TENANT_B, role="member" + ) + + async with _client( + _app(session_factory), + _token(invitee_id, "too-slow@example.invalid", TENANT_B), + ) as client: + response = await client.post("/invitations/accept", json={"token": token}) + + assert response.status_code == 403 + assert "expired" in response.json()["detail"].lower() + + async def test_an_email_bound_invitation_refuses_a_different_address( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + await _make_tenant(session_factory, TENANT_A) + admin_id = await _make_member( + session_factory, name="picky-inviter", tenant_id=TENANT_A, role="owner" + ) + _id, token = await self._mint( + session_factory, + tenant_id=TENANT_A, + admin_id=admin_id, + email="expected@example.invalid", + ) + await _make_tenant(session_factory, TENANT_B) + wrong_person = await _make_member( + session_factory, + name="wrong-person", + tenant_id=TENANT_B, + role="member", + email="someone-else@example.invalid", + ) + + async with _client( + _app(session_factory), + _token(wrong_person, "someone-else@example.invalid", TENANT_B), + ) as client: + response = await client.post("/invitations/accept", json={"token": token}) + + assert response.status_code == 403 + assert "email" in response.json()["detail"].lower() + + async def test_an_email_bound_invitation_accepts_the_addressed_email( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + await _make_tenant(session_factory, TENANT_A) + admin_id = await _make_member( + session_factory, name="picky-inviter2", tenant_id=TENANT_A, role="owner" + ) + _id, token = await self._mint( + session_factory, + tenant_id=TENANT_A, + admin_id=admin_id, + email="right-person@example.invalid", + ) + await _make_tenant(session_factory, TENANT_B) + right_person = await _make_member( + session_factory, + name="right-person", + tenant_id=TENANT_B, + role="member", + email="right-person@example.invalid", + ) + + async with _client( + _app(session_factory), + _token(right_person, "right-person@example.invalid", TENANT_B), + ) as client: + response = await client.post("/invitations/accept", json={"token": token}) + + assert response.status_code == 200, response.text + + +class TestMemberRoutes: + async def test_listing_shows_every_member_and_their_role( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + await _make_tenant(session_factory, TENANT_A) + owner_id = await _make_member( + session_factory, name="list-owner", tenant_id=TENANT_A, role="owner" + ) + await _make_member( + session_factory, name="list-member", tenant_id=TENANT_A, role="member" + ) + token = _token(owner_id, "list-owner@example.invalid", TENANT_A) + + async with _client(_app(session_factory), token) as client: + response = await client.get(f"/tenants/{TENANT_A}/members") + + assert response.status_code == 200, response.text + roles = {row["name"]: row["role"] for row in response.json()} + assert roles == {"list-owner": "owner", "list-member": "member"} + + async def test_the_last_owner_cannot_be_demoted( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + await _make_tenant(session_factory, TENANT_A) + owner_id = await _make_member( + session_factory, name="sole-owner", tenant_id=TENANT_A, role="owner" + ) + token = _token(owner_id, "sole-owner@example.invalid", TENANT_A) + + async with _client(_app(session_factory), token) as client: + response = await client.patch( + f"/tenants/{TENANT_A}/members/{owner_id}", json={"role": "member"} + ) + + assert response.status_code == 409 + + async with session_factory() as session: + membership = await session.get(TenantMember, (TENANT_A, owner_id)) + assert membership is not None + assert membership.role == "owner" + + async def test_an_owner_can_be_demoted_when_another_owner_remains( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + await _make_tenant(session_factory, TENANT_A) + owner_id = await _make_member( + session_factory, name="owner-one", tenant_id=TENANT_A, role="owner" + ) + other_owner_id = await _make_member( + session_factory, name="owner-two", tenant_id=TENANT_A, role="owner" + ) + token = _token(owner_id, "owner-one@example.invalid", TENANT_A) + + async with _client(_app(session_factory), token) as client: + response = await client.patch( + f"/tenants/{TENANT_A}/members/{other_owner_id}", + json={"role": "member"}, + ) + + assert response.status_code == 200, response.text + assert response.json()["role"] == "member" + + async def test_the_last_owner_cannot_be_removed( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + await _make_tenant(session_factory, TENANT_A) + owner_id = await _make_member( + session_factory, name="undeletable-owner", tenant_id=TENANT_A, role="owner" + ) + token = _token(owner_id, "undeletable-owner@example.invalid", TENANT_A) + + async with _client(_app(session_factory), token) as client: + response = await client.delete(f"/tenants/{TENANT_A}/members/{owner_id}") + + assert response.status_code == 409 + + async with session_factory() as session: + assert await session.get(TenantMember, (TENANT_A, owner_id)) is not None + + async def test_removing_a_member_deletes_their_membership( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + await _make_tenant(session_factory, TENANT_A) + owner_id = await _make_member( + session_factory, name="remover", tenant_id=TENANT_A, role="owner" + ) + target_id = await _make_member( + session_factory, name="removed", tenant_id=TENANT_A, role="member" + ) + token = _token(owner_id, "remover@example.invalid", TENANT_A) + + async with _client(_app(session_factory), token) as client: + response = await client.delete(f"/tenants/{TENANT_A}/members/{target_id}") + + assert response.status_code == 200, response.text + + async with session_factory() as session: + assert await session.get(TenantMember, (TENANT_A, target_id)) is None + + async def test_removing_a_member_stops_their_api_key_working( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + """The bite test: after removal, the credential the member minted in + this tenant no longer resolves to anything — not merely hidden, gone. + A live bearer-auth attempt with the same hash would fail at the very + first lookup `ApiKeyStore.get_by_hash` performs. + """ + await _make_tenant(session_factory, TENANT_A) + owner_id = await _make_member( + session_factory, name="key-remover", tenant_id=TENANT_A, role="owner" + ) + target_id = await _make_member( + session_factory, name="key-holder", tenant_id=TENANT_A, role="member" + ) + key_hash = await _make_api_key( + session_factory, tenant_id=TENANT_A, user_id=target_id + ) + token = _token(owner_id, "key-remover@example.invalid", TENANT_A) + + async with session_factory() as session: + assert await ApiKeyStore().get_by_hash(session, key_hash) is not None + + async with _client(_app(session_factory), token) as client: + response = await client.delete(f"/tenants/{TENANT_A}/members/{target_id}") + assert response.status_code == 200, response.text + + async with session_factory() as session: + assert await ApiKeyStore().get_by_hash(session, key_hash) is None + + async def test_a_member_who_owns_an_agent_here_cannot_be_removed( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + """An agent is not the member's private property to lose along with + their membership — it sits in rooms with other people, and deleting it + is not recoverable the way reminting a key is. Removal must refuse, + naming the agent, and this must not be a partial write: not the + membership, not the member's unrelated personal key, not the agent's + own key may be touched when the route 409s. + """ + await _make_tenant(session_factory, TENANT_A) + owner_id = await _make_member( + session_factory, name="agent-remover", tenant_id=TENANT_A, role="owner" + ) + target_id = await _make_member( + session_factory, name="agent-owner", tenant_id=TENANT_A, role="member" + ) + agent_id, agent_name, agent_key_hash = await _make_agent_with_key( + session_factory, tenant_id=TENANT_A, owner_id=target_id + ) + personal_key_hash = await _make_api_key( + session_factory, tenant_id=TENANT_A, user_id=target_id + ) + token = _token(owner_id, "agent-remover@example.invalid", TENANT_A) + + async with _client(_app(session_factory), token) as client: + response = await client.delete(f"/tenants/{TENANT_A}/members/{target_id}") + + assert response.status_code == 409, response.text + assert agent_name in response.json()["detail"] + + async with session_factory() as session: + assert await session.get(TenantMember, (TENANT_A, target_id)) is not None + assert await AgentStore().get(session, agent_id) is not None + assert await ApiKeyStore().get_by_hash(session, agent_key_hash) is not None + assert ( + await ApiKeyStore().get_by_hash(session, personal_key_hash) is not None + ) + + async def test_a_plain_member_cannot_remove_anyone( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + await _make_tenant(session_factory, TENANT_A) + member_id = await _make_member( + session_factory, name="powerless", tenant_id=TENANT_A, role="member" + ) + target_id = await _make_member( + session_factory, name="untouchable", tenant_id=TENANT_A, role="member" + ) + token = _token(member_id, "powerless@example.invalid", TENANT_A) + + async with _client(_app(session_factory), token) as client: + response = await client.delete(f"/tenants/{TENANT_A}/members/{target_id}") + + assert response.status_code == 403 diff --git a/core/tests/switch_core/test_authz.py b/core/tests/switch_core/test_authz.py index ecf8d1182..4e2214287 100644 --- a/core/tests/switch_core/test_authz.py +++ b/core/tests/switch_core/test_authz.py @@ -9,6 +9,7 @@ administers_tenant, can, can_manage, + owns_tenant, require, require_manage, validate_visibility_pair, @@ -127,6 +128,29 @@ def test_no_membership_does_not_administer(self) -> None: assert not administers_tenant(is_operator=False, tenant_role=None) +class TestOwnsTenant: + """`owns_tenant` is the narrower bit: who may move the ownership set, + which is not everyone who may administer the workspace.""" + + def test_operator_owns_regardless_of_tenant_role(self) -> None: + assert owns_tenant(is_operator=True, tenant_role=None) + assert owns_tenant(is_operator=True, tenant_role="member") + + def test_owner_membership_owns(self) -> None: + assert owns_tenant(is_operator=False, tenant_role="owner") + + def test_admin_membership_administers_but_does_not_own(self) -> None: + """The whole point of there being two functions: an admin who also + owned could take the workspace from its owner one safe-looking + request at a time.""" + assert administers_tenant(is_operator=False, tenant_role="admin") + assert not owns_tenant(is_operator=False, tenant_role="admin") + + def test_plain_member_and_no_membership_do_not_own(self) -> None: + assert not owns_tenant(is_operator=False, tenant_role="member") + assert not owns_tenant(is_operator=False, tenant_role=None) + + class TestValidateVisibilityPair: @pytest.mark.parametrize( "read,write", diff --git a/docs/old/multi-tenancy-phase2-tenants.md b/docs/old/multi-tenancy-phase2-tenants.md index 931048675..6928ea199 100644 --- a/docs/old/multi-tenancy-phase2-tenants.md +++ b/docs/old/multi-tenancy-phase2-tenants.md @@ -188,7 +188,9 @@ this phase must prove it has removed. - `POST /tenants/{id}/switch` — verify membership, re-mint the cookie. - `POST /tenants/{id}/invitations`, `GET`, `DELETE …/{token_id}` — mint, list, revoke. `owner` and `admin` only. -- `POST /invitations/{token}/accept` — accept. +- `POST /invitations/accept` — accept. The token goes in the body, not the + path: a URL segment ends up in proxy logs, browser history and `Referer`, + and this one is a bearer credential. - `GET /tenants/{id}/members`, `PATCH …/{user_id}`, `DELETE …/{user_id}` — list, change a role, remove. The original design omitted these and then assumed the role-change route existed. @@ -197,6 +199,14 @@ this phase must prove it has removed. is refused; deletion of a workspace is not in this phase, so there is no legitimate path to an ownerless one. +**Who owns a workspace is decided by its owners, not by its admins.** Granting +`owner`, changing an owner's role, removing an owner, and minting an `owner` +invitation are all owner-only, over and above the `owner`/`admin` gate on the +route. The last-owner rule above cannot substitute for this, because it cannot +see a sequence: an admin who promotes themselves first leaves two owners +standing at every subsequent step, so each individual request passes while the +three together take the workspace. + **Invitations are a table, and a link that grants membership is a credential.** `id`, `tenant_id`, `role`, `email` (null for a link), `expires_at`, `uses_remaining`, `revoked_at`, `created_by`, and a hash of the token rather