feat(users): retire legacy users_user.tenant_id (#381) [stack 5/11]

docs/framework/multi-tenancy.md

@@ -178,6 +178,10 @@ for anonymous visitors, on public routes), the tenant header (members only),
 then the session's choice validated against a membership (#363). Without one it falls back to the principal's
 `tenant_id` claim, and for **anonymous** requests only, the configured
 `tenant_header`. An authenticated user can never pick a tenant by header.
+The `users` module's `User` is platform-global and has no `tenant_id`
+column (dropped in #381): a user belongs to tenants only through
+`tenants_membership`, and a membership change applies on their next request,
+no re-login needed.
 Most auth providers set no `tenant_id` claim, so `multi_tenant` with no
 resolver fails every tenant-scoped query closed; the boot reports that as
 `SM025`.

framework/core/simple_module_core/diagnostics/_tenancy.py

@@ -9,8 +9,8 @@
 
 Duck-typed on SQLAlchemy ``Table`` objects (core does not depend on
 SQLAlchemy). Only tables of models that inherit ``MultiTenantMixin`` count: a
-plain ``tenant_id`` column (``users_user``'s legacy one, the ``tenants``
-registry's own tables) carries no isolation and no per-tenant key rule.
+plain ``tenant_id`` column (the ``tenants`` registry's own tables) carries no
+isolation and no per-tenant key rule.
 
 SM025: ``multi_tenant`` is on but no module registered
 ``app.state.tenant_resolver`` — see :func:`check_tenant_resolver`.

host/migrations/versions/d4e8a1b6c392_users_drop_legacy_tenant_id.py

@@ -0,0 +1,36 @@
+"""users_user: drop the legacy tenant_id column
+
+Tenant membership lives in ``tenants_membership`` and the active tenant is
+resolved per request, so the copy on the user row was dead weight that could
+only go stale. ``User`` stays platform-global. See GH #381.
+
+Revision ID: d4e8a1b6c392
+Revises: c3a1d7e45f20
+Create Date: 2026-10-01 10:00:00.000000
+"""
+
+from collections.abc import Sequence
+
+import sqlalchemy as sa
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision: str = "d4e8a1b6c392"
+down_revision: str | None = "c3a1d7e45f20"
+branch_labels: str | Sequence[str] | None = None
+depends_on: str | Sequence[str] | None = None
+
+_INDEX = "ix_users_user_tenant_id"
+
+
+def upgrade() -> None:
+    # batch mode: SQLite cannot drop an indexed column in place.
+    with op.batch_alter_table("users_user") as batch:
+        batch.drop_index(_INDEX)
+        batch.drop_column("tenant_id")
+
+
+def downgrade() -> None:
+    with op.batch_alter_table("users_user") as batch:
+        batch.add_column(sa.Column("tenant_id", sa.String(length=50), nullable=True))
+        batch.create_index(_INDEX, ["tenant_id"], unique=False)

modules/auth/auth/contracts/schemas.py

@@ -26,15 +26,17 @@ def from_user(cls, user: User | Any) -> UserContext:
         """Build a UserContext from a users.models.User with eagerly-loaded roles.
 
         Duck-typed to avoid importing users.models at runtime — any object
-        exposing .id, .email, .full_name, .roles[*].name, .tenant_id works.
+        exposing .id, .email, .full_name, .roles[*].name works. The user row
+        carries no tenant: the active tenant is resolved per request from
+        ``tenants_membership``, so ``tenant_id`` stays ``None`` here (only a
+        generic provider's claim path sets it).
         The caller is responsible for eager-loading roles (selectinload).
         """
         return cls(
             id=str(user.id),
             email=user.email,
             name=user.full_name or user.email,
             roles=[r.name for r in user.roles],
-            tenant_id=user.tenant_id,
         )
 
     def to_session_dict(self) -> dict[str, Any]:

modules/auth/tests/test_user_context.py

@@ -18,14 +18,14 @@ async def test_from_user_basic(self):
             email="charlie@example.com",
             full_name="Charlie Brown",
             roles=[role_a, role_b],
-            tenant_id="tenant-42",
         )
         ctx = UserContext.from_user(fake_user)
         assert ctx.id == "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"
         assert ctx.email == "charlie@example.com"
         assert ctx.name == "Charlie Brown"
         assert ctx.roles == ["admin", "editor"]
-        assert ctx.tenant_id == "tenant-42"
+        # The user row carries no tenant; the resolver supplies it per request.
+        assert ctx.tenant_id is None
 
     async def test_from_user_name_fallback_to_email(self):
         """When full_name is None, ctx.name falls back to the user's email."""

modules/tenants/tests/test_membership_changes.py

@@ -0,0 +1,64 @@
+"""A membership change reaches the user's NEXT request, with no re-login (#381, #362).
+
+``users_user.tenant_id`` used to be copied into the session at login, so moving
+a user between tenants only took effect after they signed in again. The active
+tenant now comes from ``tenants_membership`` on every request; these tests
+keep the member's signed session cookie fixed throughout and change membership
+underneath it.
+"""
+
+from __future__ import annotations
+
+
+async def test_removal_takes_effect_on_the_next_request(tenant_client):
+    async with (
+        tenant_client() as owner,
+        tenant_client("member", tenant_id=owner.tenant_id) as member,
+    ):
+        assert (await member.client.get("/api/tenants/current/members")).status_code == 200
+
+        removed = await owner.client.delete(f"/api/tenants/current/members/{member.user_id}")
+        assert removed.status_code == 204
+
+        assert (await member.client.get("/api/tenants/current/members")).status_code != 200
+        assert (await member.client.get("/api/tenants/")).json() == []
+
+
+async def test_role_change_takes_effect_on_the_next_request(tenant_client):
+    async with (
+        tenant_client() as owner,
+        tenant_client("member", tenant_id=owner.tenant_id) as member,
+    ):
+        invite = {"email": "new@example.com"}
+        before = await member.client.post("/api/tenants/current/invitations", json=invite)
+        assert before.status_code == 403
+
+        promoted = await owner.client.patch(
+            f"/api/tenants/current/members/{member.user_id}", json={"role": "admin"}
+        )
+        assert promoted.status_code == 200
+        after = await member.client.post("/api/tenants/current/invitations", json=invite)
+        assert after.status_code == 201
+
+        await owner.client.patch(
+            f"/api/tenants/current/members/{member.user_id}", json={"role": "member"}
+        )
+        demoted = await member.client.post(
+            "/api/tenants/current/invitations", json={"email": "other@example.com"}
+        )
+        assert demoted.status_code == 403
+
+
+async def test_joining_and_switching_tenants_needs_no_relogin(tenant_client):
+    async with tenant_client() as user:
+        first = user.tenant_id
+        created = await user.client.post("/api/tenants/", json={"name": "Second", "slug": "second"})
+        assert created.status_code == 201
+        second = created.json()["id"]
+
+        mine = {t["id"] for t in (await user.client.get("/api/tenants/")).json()}
+        assert mine == {first, second}
+
+        assert (await user.client.post(f"/api/tenants/{first}/switch")).status_code == 204
+        members = (await user.client.get("/api/tenants/current/members")).json()
+        assert [m["user_id"] for m in members] == [user.user_id]

modules/users/tests/_middleware_support.py

@@ -49,7 +49,6 @@ async def _default_handler(request: Request):
                         "email": user.email,
                         "name": user.name,
                         "roles": user.roles,
-                        "tenant_id": user.tenant_id,
                     }
                     if user is not None
                     else None
@@ -106,7 +105,6 @@ async def mw_active_user(db_session, _mw_seed_roles):
         is_superuser=False,
         is_verified=True,
         full_name="Middleware Tester",
-        tenant_id="acme",
     )
     link = UserRole(user_id=user_id, role_id=ADMIN_ROLE_ID)
     db_session.add_all([user, link])

modules/users/tests/test_legacy_cached_tenant.py

@@ -0,0 +1,54 @@
+"""A pre-#381 session cookie's cached ``tenant_id`` is ignored.
+
+``users_user.tenant_id`` is gone and the active tenant is resolved per request
+from memberships, but cookies signed before the upgrade still carry the old
+value inside ``user_ctx``. Honouring it would keep acting for a tenant the
+user may have left.
+"""
+
+from __future__ import annotations
+
+import time
+
+import pytest
+from auth.contracts.schemas import UserContext
+from simple_module_hosting.session import SESSION_EXPIRES_AT_KEY
+from sqlalchemy import select
+from starlette.requests import Request
+from users.models import User
+from users.provider import UsersAuthProvider
+
+
+async def _admin_id(users_app) -> str:
+    async with users_app.state.sm.db.session_factory() as session:
+        stmt = select(User.id).where(User.email == "admin@example.com")
+        return str((await session.execute(stmt)).scalar_one())
+
+
+@pytest.mark.anyio
+async def test_old_cookie_tenant_id_is_dropped_on_the_cached_path(users_app):
+    user_id = await _admin_id(users_app)
+    legacy_ctx = UserContext(
+        id=user_id, email="admin@example.com", name="Admin", roles=["admin"], tenant_id="acme"
+    ).to_session_dict()
+    session = {
+        "user_id": user_id,
+        "user_ctx": legacy_ctx,
+        "session_version": 0,
+        SESSION_EXPIRES_AT_KEY: int(time.time()) + 3600,
+    }
+    scope = {
+        "type": "http",
+        "method": "GET",
+        "path": "/",
+        "headers": [],
+        "app": users_app,
+        "session": session,
+    }
+
+    ctx = await UsersAuthProvider().resolve_user(Request(scope))
+
+    assert ctx is not None
+    assert ctx.id == user_id
+    assert ctx.name == "Admin"  # the cached context was used, not a DB reload
+    assert ctx.tenant_id is None

modules/users/tests/test_user_model.py

@@ -35,7 +35,6 @@ def test_required_columns(self):
             "is_superuser",
             "is_verified",
             "full_name",
-            "tenant_id",
             "disabled_at",
             "last_login_at",
             # AuditMixin columns

modules/users/tests/test_users_middleware.py

@@ -81,7 +81,6 @@ async def test_authenticated_request_sets_user_context(db_state, mw_active_user)
     assert data["user"]["email"] == "middleware-test@example.com"
     assert data["user"]["name"] == "Middleware Tester"
     assert data["user"]["roles"] == ["admin"]
-    assert data["user"]["tenant_id"] == "acme"
 
 
 # ---------------------------------------------------------------------------

modules/users/users/contracts/schemas.py

@@ -47,7 +47,6 @@ class UserRead(CreateUpdateDictModel, SQLModel):
     is_verified: bool = False
     is_external: bool = False
     full_name: str | None = None
-    tenant_id: str | None = None
     disabled_at: datetime | None = None
     last_login_at: datetime | None = None
 

modules/users/users/models/user.py

@@ -56,7 +56,6 @@ class User(Base, AuditMixin, table=True):  # ty: ignore[unsupported-base]
     )
 
     full_name: str | None = Field(default=None, max_length=255)
-    tenant_id: str | None = Field(default=None, max_length=50, index=True)
     disabled_at: datetime | None = Field(
         default=None,
         sa_type=DateTime(timezone=True),

modules/users/users/provider.py

@@ -129,6 +129,9 @@ async def resolve_user(self, request: Request) -> UserContext | None:
             # sign out a browser this process has never seen — the alternative
             # is a button that only logs out the person pressing it.
             if await self._version_still_current(request.scope, user_uuid, session):
+                # Cookies minted before #381 still cache the retired user-row
+                # tenant; the active tenant now comes from memberships alone.
+                cached.tenant_id = None
                 return cached
             _forget(session)
             return None

tests/loadtest/seed.py

@@ -110,7 +110,6 @@ async def main() -> None:
                     "is_superuser": False,
                     "is_verified": i % 3 != 0,
                     "full_name": fake.name(),
-                    "tenant_id": None,
                     "disabled_at": NOW if disabled else None,
                     "last_login_at": NOW - timedelta(days=i % 90) if i % 4 else None,
                     "created_at": NOW - timedelta(days=i % 365),