From b8821ee139b492a47567438922e74f14b1a7af43 Mon Sep 17 00:00:00 2001 From: Wojciech Wojtyniak Date: Fri, 11 Sep 2026 18:06:21 -0700 Subject: [PATCH 1/3] feat(gateway): workspace, invitation and member routes (CHOO-2722) POST /tenants creates a workspace through ClientLifecycleService's existing provisioning seam and makes the caller its owner; a taken slug is a 409. Invitation routes (mint/list/revoke, owner+admin only) and POST /invitations/{token}/accept resolve the token's tenant through the exempt lookup and do their work in a fresh tenant_session, since the request's own session is already bound to the caller's current workspace. Member routes add list/patch-role/remove, refusing to demote or remove a workspace's last owner and revoking a removed member's API keys and owned agents in the same transaction as the membership row going away. UserStore.add_membership is now the one place a TenantMember row is constructed; gateway/auth.py grows get_authenticated_caller and tenant_of_invitation_token so the tenant-lookup exemption stays reachable through that one module rather than through gateway/tenants.py directly. --- core/switch_core/db/stores/agent_store.py | 10 + .../switch_core/db/stores/invitation_store.py | 12 + core/switch_core/db/stores/user_store.py | 48 ++ core/switch_core/gateway/app.py | 3 + core/switch_core/gateway/auth.py | 53 +- core/switch_core/gateway/dependencies.py | 7 + core/switch_core/gateway/schemas.py | 42 + core/switch_core/gateway/tenants.py | 384 +++++++++- core/switch_core/main.py | 3 + .../gateway/test_tenant_api_routes.py | 716 ++++++++++++++++++ 10 files changed, 1276 insertions(+), 2 deletions(-) create mode 100644 core/tests/switch_core/gateway/test_tenant_api_routes.py 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/invitation_store.py b/core/switch_core/db/stores/invitation_store.py index 0e6b73d1e..ac433bad8 100644 --- a/core/switch_core/db/stores/invitation_store.py +++ b/core/switch_core/db/stores/invitation_store.py @@ -164,3 +164,15 @@ async def revoke(self, session: AsyncSession, invitation_id: str) -> Invitation: invitation.revoked_at = datetime.now(UTC) await session.flush() return invitation + + async def consume( + self, session: AsyncSession, invitation: Invitation + ) -> Invitation: + """Record one use of `invitation`. + + Called only after every validity check (expiry, revocation, email, + remaining uses) has already passed; it does not repeat any of them. + """ + invitation.uses_remaining -= 1 + await session.flush() + return invitation diff --git a/core/switch_core/db/stores/user_store.py b/core/switch_core/db/stores/user_store.py index 3506b9741..4071c2dc4 100644 --- a/core/switch_core/db/stores/user_store.py +++ b/core/switch_core/db/stores/user_store.py @@ -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. 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..8d9d4ad52 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/{token}/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: 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..a9a81fc21 100644 --- a/core/switch_core/gateway/schemas.py +++ b/core/switch_core/gateway/schemas.py @@ -673,6 +673,48 @@ 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 + expires_in_hours: int = Field(default=168, gt=0) + uses_remaining: int = Field(default=1, ge=1) + + +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..9609e5231 100644 --- a/core/switch_core/gateway/tenants.py +++ b/core/switch_core/gateway/tenants.py @@ -1,29 +1,149 @@ from __future__ import annotations +import hashlib +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 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, 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 ( + InvitationCreateRequest, + InvitationCreateResponse, + InvitationDetail, + MemberDetail, + MemberUpdateRequest, + SessionUserResponse, + TenantCreateRequest, + TenantMembershipResponse, +) +from switch_core.tenant_context import current_tenant_id 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_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). + """ + 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): # type: ignore[operator] + 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 +165,44 @@ 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. + """ + 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 + + 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() + + 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 +236,227 @@ 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)], +) -> InvitationCreateResponse: + """Mint an invitation to the bound tenant. `owner`/`admin` only.""" + _require_bound_tenant(tenant_id) + if req.role not in TENANT_MEMBER_ROLES: + raise HTTPException(status_code=400, detail=f"Invalid role: {req.role}") + + 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/{token}/accept") +async def accept_invitation( + token: str, + 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). + """ + token_hash = hashlib.sha256(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) + role = existing_role if existing_role is not None else invitation.role + if existing_role is None: + await user_store.add_membership( + session, tenant_id=tenant_id, user_id=caller.id, role=role + ) + await invitation_store.consume(session, invitation) + + 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)], +) -> MemberDetail: + """Change a member's role. `owner`/`admin` only. + + Refuses to demote the last `owner`: 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 membership.role == "owner" and req.role != "owner": + 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)], +) -> dict[str, bool]: + """Remove a member from the bound tenant. `owner`/`admin` only. + + Refuses to remove the last `owner`, same as demoting one. + + 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 the row behind would leave the person + with working, invisible access (`docs/old/multi-tenancy-phase2-tenants.md`, + §3). Concretely: every agent this user owns here is deleted outright — + there is no "disabled" flag on `Agent` to set instead, and an agent's own + bearer key (`agents.api_key_id`) is a `NOT NULL` column with no `ON + DELETE` action, so the key cannot be revoked while the agent still + references it. Deleting the agent first, through `AgentStore.delete` + rather than `ProtocolService.delete_agent`, frees that key to be deleted + with the rest of this user's keys right after — `delete_agent` spans + several sessions and commits of its own and reaches out to the live + Matrix client and collaboration bridges, none of which can be folded into + one transaction, so it is deliberately not used here. What this does not + do is tear down a live Matrix session such an agent already holds; its + next call to Switch fails at authentication instead, since the credential + behind it is gone. + """ + _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" and await user_store.count_owners(session) <= 1: + raise HTTPException(status_code=409, detail="Cannot remove the last owner") + + revoked_agent_ids = [a.id for a in await agent_store.get_by_owner(session, user_id)] + for agent_id in revoked_agent_ids: + await agent_store.delete(session, agent_id) + + 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 agent_id in revoked_agent_ids: + protocol.api_key_cache.invalidate_agent(agent_id) + 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_tenant_api_routes.py b/core/tests/switch_core/gateway/test_tenant_api_routes.py new file mode 100644 index 000000000..54f2a811b --- /dev/null +++ b/core/tests/switch_core/gateway/test_tenant_api_routes.py @@ -0,0 +1,716 @@ +"""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]: + """An agent owned by `owner_id`, plus the `agents`-type key backing it. + + Returns `(agent_id, 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 + 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=f"agent-{uuid.uuid4().hex[:8]}", + 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, 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"] + + +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(f"/invitations/{token}/accept") + + 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_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(f"/invitations/{token}/accept") + 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(f"/invitations/{token}/accept") + + 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(f"/invitations/{token}/accept") + + 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(f"/invitations/{token}/accept") + + 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(f"/invitations/{token}/accept") + + 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(f"/invitations/{token}/accept") + + 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_removing_a_member_deletes_the_agents_they_own_here( + self, session_factory: async_sessionmaker[AsyncSession] + ) -> None: + 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_key_hash = await _make_agent_with_key( + session_factory, tenant_id=TENANT_A, owner_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 == 200, response.text + + async with session_factory() as session: + assert await AgentStore().get(session, agent_id) is None + assert await ApiKeyStore().get_by_hash(session, agent_key_hash) is 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 From f5f073e59cb108a671972c9fafbfcd34232aa69e Mon Sep 17 00:00:00 2001 From: Wojciech Wojtyniak Date: Fri, 11 Sep 2026 18:19:13 -0700 Subject: [PATCH 2/3] fix(gateway): refuse to remove a member who owns agents here (CHOO-2722) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deleting a member's agents alongside their membership went past revoking access into destroying shared infrastructure: an agent sits in rooms with other members and its name is depended on well outside the membership being removed, and unlike a personal key it cannot be reminted. Member removal now refuses with a 409 naming the agents when the target owns any in this tenant, and removes nothing — no membership, no keys — until an admin reassigns or deletes them deliberately. Personal API key revocation and the last-owner refusal are unchanged. --- core/switch_core/gateway/tenants.py | 45 ++++++++++--------- .../gateway/test_tenant_api_routes.py | 39 +++++++++++----- 2 files changed, 53 insertions(+), 31 deletions(-) diff --git a/core/switch_core/gateway/tenants.py b/core/switch_core/gateway/tenants.py index 9609e5231..fbc36c2cd 100644 --- a/core/switch_core/gateway/tenants.py +++ b/core/switch_core/gateway/tenants.py @@ -418,21 +418,21 @@ async def remove_member( 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 the row behind would leave the person - with working, invisible access (`docs/old/multi-tenancy-phase2-tenants.md`, - §3). Concretely: every agent this user owns here is deleted outright — - there is no "disabled" flag on `Agent` to set instead, and an agent's own - bearer key (`agents.api_key_id`) is a `NOT NULL` column with no `ON - DELETE` action, so the key cannot be revoked while the agent still - references it. Deleting the agent first, through `AgentStore.delete` - rather than `ProtocolService.delete_agent`, frees that key to be deleted - with the rest of this user's keys right after — `delete_agent` spans - several sessions and commits of its own and reaches out to the live - Matrix client and collaboration bridges, none of which can be folded into - one transaction, so it is deliberately not used here. What this does not - do is tear down a live Matrix session such an agent already holds; its - next call to Switch fails at authentication instead, since the credential - behind it is gone. + 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 + reassigns or deletes those agents deliberately, with the agent in front of + them, then removes the member. """ _require_bound_tenant(tenant_id) membership = await session.get(TenantMember, (tenant_id, user_id)) @@ -442,9 +442,16 @@ async def remove_member( if membership.role == "owner" and await user_store.count_owners(session) <= 1: raise HTTPException(status_code=409, detail="Cannot remove the last owner") - revoked_agent_ids = [a.id for a in await agent_store.get_by_owner(session, user_id)] - for agent_id in revoked_agent_ids: - await agent_store.delete(session, agent_id) + 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"reassign or 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] @@ -454,8 +461,6 @@ async def remove_member( await session.delete(membership) await session.commit() - for agent_id in revoked_agent_ids: - protocol.api_key_cache.invalidate_agent(agent_id) for key_hash in revoked_key_hashes: protocol.api_key_cache.invalidate(key_hash) diff --git a/core/tests/switch_core/gateway/test_tenant_api_routes.py b/core/tests/switch_core/gateway/test_tenant_api_routes.py index 54f2a811b..7b646042e 100644 --- a/core/tests/switch_core/gateway/test_tenant_api_routes.py +++ b/core/tests/switch_core/gateway/test_tenant_api_routes.py @@ -166,14 +166,15 @@ async def _make_agent_with_key( *, tenant_id: str, owner_id: str, -) -> tuple[str, str]: +) -> tuple[str, str, str]: """An agent owned by `owner_id`, plus the `agents`-type key backing it. - Returns `(agent_id, 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. + 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, @@ -197,7 +198,7 @@ async def _make_agent_with_key( agent = Agent( tenant_id=tenant_id, - name=f"agent-{uuid.uuid4().hex[:8]}", + name=name, description="test agent", agent_type="other", connector_type="claude-code", @@ -208,7 +209,7 @@ async def _make_agent_with_key( ) session.add(agent) await session.commit() - return agent.id, key_hash + return agent.id, name, key_hash class TestCreateTenant: @@ -675,9 +676,16 @@ async def test_removing_a_member_stops_their_api_key_working( async with session_factory() as session: assert await ApiKeyStore().get_by_hash(session, key_hash) is None - async def test_removing_a_member_deletes_the_agents_they_own_here( + 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" @@ -685,18 +693,27 @@ async def test_removing_a_member_deletes_the_agents_they_own_here( target_id = await _make_member( session_factory, name="agent-owner", tenant_id=TENANT_A, role="member" ) - agent_id, agent_key_hash = await _make_agent_with_key( + 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 == 200, response.text + + assert response.status_code == 409, response.text + assert agent_name in response.json()["detail"] async with session_factory() as session: - assert await AgentStore().get(session, agent_id) is None - assert await ApiKeyStore().get_by_hash(session, agent_key_hash) is None + 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] From af22ad1fdd53e14896e68d62e5ae87274c6167f3 Mon Sep 17 00:00:00 2001 From: Petr Bauch Date: Mon, 14 Sep 2026 12:23:46 -0400 Subject: [PATCH 3/3] fix(gateway): make workspace ownership an owner-only decision (CHOO-2722) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An `admin` could take a workspace from its owner in three requests — promote self to `owner`, demote the founder, remove them — and the last-owner guard never fired, because there genuinely were two owners at every step. `authz.owns_tenant` is the narrower bit beside `administers_tenant`, and granting `owner`, changing an owner's role, removing an owner and minting an `owner` invitation now all answer to it. Alongside that: - Accepting an invitation goes through `InvitationStore.consume`'s conditional `UPDATE` (#446) instead of the read-then-write copy this branch carried, so two simultaneous acceptances of a single-use link grant exactly one membership. The cherry-pick had textually merged a second `consume` into the store, shadowing the safe one. - A use is spent only when a membership is actually granted, so a double-clicked shared link no longer burns a seat for nothing. - The token moves from the path to the body: it is a bearer credential, and a URL segment is written to proxy logs, browser history and `Referer`. - `expires_in_hours` is bounded above. `1000000000` passed `gt=0` and then raised `OverflowError` inside `timedelta` — a 500 for a bad request. - The "every route that opens a session is authenticated" guard keyed on `get_system_session`, which routes taking `get_session_factory` walk straight past. It now keys on the absence of `get_current_user`, with an explicit allowlist, so the next door is a failing test rather than a route nobody counted. - The 409 on removing a member who owns agents no longer offers reassignment; no route reassigns an agent's owner in this phase. - Workspace creation cannot be one transaction (the tenant row must be committed before its admin client can be provisioned against it), so the window where the owner membership fails to land now logs an error naming the workspace and the person who should have owned it. Co-Authored-By: Claude Opus 5 --- core/switch_core/authz.py | 13 ++ .../switch_core/db/stores/invitation_store.py | 12 -- core/switch_core/db/stores/user_store.py | 22 ++- core/switch_core/gateway/auth.py | 16 +- core/switch_core/gateway/schemas.py | 12 +- core/switch_core/gateway/tenants.py | 137 ++++++++++++-- .../test_session_requires_authentication.py | 79 ++++++-- .../gateway/test_tenant_api_routes.py | 176 +++++++++++++++++- core/tests/switch_core/test_authz.py | 24 +++ docs/old/multi-tenancy-phase2-tenants.md | 12 +- 10 files changed, 444 insertions(+), 59 deletions(-) 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/invitation_store.py b/core/switch_core/db/stores/invitation_store.py index ac433bad8..0e6b73d1e 100644 --- a/core/switch_core/db/stores/invitation_store.py +++ b/core/switch_core/db/stores/invitation_store.py @@ -164,15 +164,3 @@ async def revoke(self, session: AsyncSession, invitation_id: str) -> Invitation: invitation.revoked_at = datetime.now(UTC) await session.flush() return invitation - - async def consume( - self, session: AsyncSession, invitation: Invitation - ) -> Invitation: - """Record one use of `invitation`. - - Called only after every validity check (expiry, revocation, email, - remaining uses) has already passed; it does not repeat any of them. - """ - invitation.uses_remaining -= 1 - await session.flush() - return invitation diff --git a/core/switch_core/db/stores/user_store.py b/core/switch_core/db/stores/user_store.py index 4071c2dc4..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, @@ -384,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/auth.py b/core/switch_core/gateway/auth.py index 8d9d4ad52..2aff989b4 100644 --- a/core/switch_core/gateway/auth.py +++ b/core/switch_core/gateway/auth.py @@ -182,7 +182,7 @@ async def get_authenticated_caller( ) -> AuthenticatedCaller: """Like `get_authenticated_user_id`, but with the caller's own email too. - Backs `POST /invitations/{token}/accept` (`gateway/tenants.py`), which has + 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, @@ -461,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/schemas.py b/core/switch_core/gateway/schemas.py index a9a81fc21..bc67cfe2e 100644 --- a/core/switch_core/gateway/schemas.py +++ b/core/switch_core/gateway/schemas.py @@ -682,10 +682,20 @@ class InvitationCreateRequest(BaseModel): # 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 - expires_in_hours: int = Field(default=168, gt=0) + # 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 diff --git a/core/switch_core/gateway/tenants.py b/core/switch_core/gateway/tenants.py index fbc36c2cd..45f84901c 100644 --- a/core/switch_core/gateway/tenants.py +++ b/core/switch_core/gateway/tenants.py @@ -1,6 +1,7 @@ from __future__ import annotations import hashlib +import logging import re from datetime import UTC, datetime, timedelta from typing import Annotated @@ -16,13 +17,17 @@ 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.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, @@ -43,6 +48,7 @@ get_user_store, ) from switch_core.gateway.schemas import ( + InvitationAcceptRequest, InvitationCreateRequest, InvitationCreateResponse, InvitationDetail, @@ -54,6 +60,8 @@ ) from switch_core.tenant_context import current_tenant_id +logger = logging.getLogger(__name__) + router = APIRouter() TENANT_MEMBER_ROLES = ("owner", "admin", "member") @@ -119,6 +127,23 @@ def _member_detail(user: User, membership: TenantMember) -> MemberDetail: ) +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`. @@ -126,10 +151,17 @@ def _require_invitation_usable(invitation: Invitation, caller_email: str) -> Non 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): # type: ignore[operator] + if invitation.expires_at < datetime.now(UTC): raise HTTPException(status_code=403, detail="This invitation has expired") if invitation.uses_remaining <= 0: raise HTTPException( @@ -183,6 +215,16 @@ async def create_tenant( 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: @@ -192,11 +234,22 @@ async def create_tenant( status_code=409, detail=f"Slug already taken: {slug}" ) from exc - 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" + 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, ) - await session.commit() + raise return TenantMembershipResponse( id=tenant.id, slug=tenant.slug, name=tenant.name, role="owner" @@ -248,11 +301,19 @@ async def create_invitation( 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.""" + """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( @@ -298,9 +359,9 @@ async def revoke_invitation( return _invitation_detail(invitation) -@router.post("/invitations/{token}/accept") +@router.post("/invitations/accept") async def accept_invitation( - token: str, + req: InvitationAcceptRequest, caller: Annotated[AuthenticatedCaller, Depends(get_authenticated_caller)], session_factory: Annotated[ async_sessionmaker[AsyncSession], Depends(get_session_factory) @@ -320,8 +381,19 @@ async def accept_invitation( 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(token.encode()).hexdigest() + 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") @@ -333,12 +405,19 @@ async def accept_invitation( _require_invitation_usable(invitation, caller.email) existing_role = await user_store.tenant_role(session, tenant_id, caller.id) - role = existing_role if existing_role is not None else invitation.role 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 ) - await invitation_store.consume(session, invitation) + else: + role = existing_role tenant = await session.get(Tenant, tenant_id) assert tenant is not None @@ -374,11 +453,18 @@ async def update_member_role( 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. - Refuses to demote the last `owner`: with no workspace deletion in this - phase, there is no legitimate route to a workspace with none. + 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: @@ -388,7 +474,11 @@ async def update_member_role( 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") @@ -410,10 +500,12 @@ async def remove_member( 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. - Refuses to remove the last `owner`, same as demoting one. + 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 — @@ -431,16 +523,21 @@ async def remove_member( 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 - reassigns or deletes those agents deliberately, with the agent in front of - them, then removes the member. + 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" and await user_store.count_owners(session) <= 1: - raise HTTPException(status_code=409, detail="Cannot remove the last owner") + 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: @@ -449,7 +546,7 @@ async def remove_member( status_code=409, detail=( "Cannot remove: this member owns agents in this tenant — " - f"reassign or delete them first: {names}" + f"delete them first: {names}" ), ) 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 index 7b646042e..f023dd4d2 100644 --- a/core/tests/switch_core/gateway/test_tenant_api_routes.py +++ b/core/tests/switch_core/gateway/test_tenant_api_routes.py @@ -328,6 +328,131 @@ async def test_an_owner_can_mint_an_invitation_to_their_own_tenant( 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( @@ -373,7 +498,7 @@ async def test_accepting_grants_membership_in_the_invited_role( caller_token = _token(invitee_id, "invitee@example.invalid", TENANT_B) async with _client(_app(session_factory), caller_token) as client: - response = await client.post(f"/invitations/{token}/accept") + response = await client.post("/invitations/accept", json={"token": token}) assert response.status_code == 200, response.text body = response.json() @@ -385,6 +510,43 @@ async def test_accepting_grants_membership_in_the_invited_role( 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: @@ -407,13 +569,13 @@ async def test_accepting_twice_fails_the_second_time( async with _client( app, _token(first_invitee, "first-invitee@example.invalid", TENANT_B) ) as client: - first = await client.post(f"/invitations/{token}/accept") + 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(f"/invitations/{token}/accept") + second = await client.post("/invitations/accept", json={"token": token}) assert second.status_code == 403 assert "used" in second.json()["detail"].lower() @@ -444,7 +606,7 @@ async def test_a_revoked_invitation_is_refused( _app(session_factory), _token(invitee_id, "too-late@example.invalid", TENANT_B), ) as client: - response = await client.post(f"/invitations/{token}/accept") + response = await client.post("/invitations/accept", json={"token": token}) assert response.status_code == 403 assert "revoked" in response.json()["detail"].lower() @@ -480,7 +642,7 @@ async def test_an_expired_invitation_is_refused( _app(session_factory), _token(invitee_id, "too-slow@example.invalid", TENANT_B), ) as client: - response = await client.post(f"/invitations/{token}/accept") + response = await client.post("/invitations/accept", json={"token": token}) assert response.status_code == 403 assert "expired" in response.json()["detail"].lower() @@ -511,7 +673,7 @@ async def test_an_email_bound_invitation_refuses_a_different_address( _app(session_factory), _token(wrong_person, "someone-else@example.invalid", TENANT_B), ) as client: - response = await client.post(f"/invitations/{token}/accept") + response = await client.post("/invitations/accept", json={"token": token}) assert response.status_code == 403 assert "email" in response.json()["detail"].lower() @@ -542,7 +704,7 @@ async def test_an_email_bound_invitation_accepts_the_addressed_email( _app(session_factory), _token(right_person, "right-person@example.invalid", TENANT_B), ) as client: - response = await client.post(f"/invitations/{token}/accept") + response = await client.post("/invitations/accept", json={"token": token}) assert response.status_code == 200, response.text 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