Skip to content

Commit 57736fc

Browse files
authored
feat(users): retire legacy users_user.tenant_id (#381) [stack 5/11] (#389)
* feat(users): retire legacy users_user.tenant_id (#381) UserContext.from_user no longer copies a tenant from the user row, UserRead drops tenant_id, and an Alembic migration drops the column and its index. Adds a regression test that membership add/remove/role change/switch apply on the next request without re-login. Claude-Session: https://claude.ai/code/session_01F8RiTBUJQnZmSq56qReZeV * fix(users): ignore the tenant_id cached in pre-upgrade session cookies (review of #381) Cookies signed before the users_user.tenant_id column was retired still carry it inside user_ctx, and the cached path returned it verbatim. The users provider now drops it there; the active tenant comes from memberships only. Claude-Session: https://claude.ai/code/session_01F8RiTBUJQnZmSq56qReZeV
1 parent 30d4ccd commit 57736fc

14 files changed

Lines changed: 169 additions & 13 deletions

File tree

‎docs/framework/multi-tenancy.md‎

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

‎framework/core/simple_module_core/diagnostics/_tenancy.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@
99
1010
Duck-typed on SQLAlchemy ``Table`` objects (core does not depend on
1111
SQLAlchemy). Only tables of models that inherit ``MultiTenantMixin`` count: a
12-
plain ``tenant_id`` column (``users_user``'s legacy one, the ``tenants``
13-
registry's own tables) carries no isolation and no per-tenant key rule.
12+
plain ``tenant_id`` column (the ``tenants`` registry's own tables) carries no
13+
isolation and no per-tenant key rule.
1414
1515
SM025: ``multi_tenant`` is on but no module registered
1616
``app.state.tenant_resolver`` — see :func:`check_tenant_resolver`.
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
"""users_user: drop the legacy tenant_id column
2+
3+
Tenant membership lives in ``tenants_membership`` and the active tenant is
4+
resolved per request, so the copy on the user row was dead weight that could
5+
only go stale. ``User`` stays platform-global. See GH #381.
6+
7+
Revision ID: d4e8a1b6c392
8+
Revises: c3a1d7e45f20
9+
Create Date: 2026-10-01 10:00:00.000000
10+
"""
11+
12+
from collections.abc import Sequence
13+
14+
import sqlalchemy as sa
15+
from alembic import op
16+
17+
# revision identifiers, used by Alembic.
18+
revision: str = "d4e8a1b6c392"
19+
down_revision: str | None = "c3a1d7e45f20"
20+
branch_labels: str | Sequence[str] | None = None
21+
depends_on: str | Sequence[str] | None = None
22+
23+
_INDEX = "ix_users_user_tenant_id"
24+
25+
26+
def upgrade() -> None:
27+
# batch mode: SQLite cannot drop an indexed column in place.
28+
with op.batch_alter_table("users_user") as batch:
29+
batch.drop_index(_INDEX)
30+
batch.drop_column("tenant_id")
31+
32+
33+
def downgrade() -> None:
34+
with op.batch_alter_table("users_user") as batch:
35+
batch.add_column(sa.Column("tenant_id", sa.String(length=50), nullable=True))
36+
batch.create_index(_INDEX, ["tenant_id"], unique=False)

‎modules/auth/auth/contracts/schemas.py‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,17 @@ def from_user(cls, user: User | Any) -> UserContext:
2626
"""Build a UserContext from a users.models.User with eagerly-loaded roles.
2727
2828
Duck-typed to avoid importing users.models at runtime — any object
29-
exposing .id, .email, .full_name, .roles[*].name, .tenant_id works.
29+
exposing .id, .email, .full_name, .roles[*].name works. The user row
30+
carries no tenant: the active tenant is resolved per request from
31+
``tenants_membership``, so ``tenant_id`` stays ``None`` here (only a
32+
generic provider's claim path sets it).
3033
The caller is responsible for eager-loading roles (selectinload).
3134
"""
3235
return cls(
3336
id=str(user.id),
3437
email=user.email,
3538
name=user.full_name or user.email,
3639
roles=[r.name for r in user.roles],
37-
tenant_id=user.tenant_id,
3840
)
3941

4042
def to_session_dict(self) -> dict[str, Any]:

‎modules/auth/tests/test_user_context.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,14 @@ async def test_from_user_basic(self):
1818
email="charlie@example.com",
1919
full_name="Charlie Brown",
2020
roles=[role_a, role_b],
21-
tenant_id="tenant-42",
2221
)
2322
ctx = UserContext.from_user(fake_user)
2423
assert ctx.id == "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"
2524
assert ctx.email == "charlie@example.com"
2625
assert ctx.name == "Charlie Brown"
2726
assert ctx.roles == ["admin", "editor"]
28-
assert ctx.tenant_id == "tenant-42"
27+
# The user row carries no tenant; the resolver supplies it per request.
28+
assert ctx.tenant_id is None
2929

3030
async def test_from_user_name_fallback_to_email(self):
3131
"""When full_name is None, ctx.name falls back to the user's email."""
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
"""A membership change reaches the user's NEXT request, with no re-login (#381, #362).
2+
3+
``users_user.tenant_id`` used to be copied into the session at login, so moving
4+
a user between tenants only took effect after they signed in again. The active
5+
tenant now comes from ``tenants_membership`` on every request; these tests
6+
keep the member's signed session cookie fixed throughout and change membership
7+
underneath it.
8+
"""
9+
10+
from __future__ import annotations
11+
12+
13+
async def test_removal_takes_effect_on_the_next_request(tenant_client):
14+
async with (
15+
tenant_client() as owner,
16+
tenant_client("member", tenant_id=owner.tenant_id) as member,
17+
):
18+
assert (await member.client.get("/api/tenants/current/members")).status_code == 200
19+
20+
removed = await owner.client.delete(f"/api/tenants/current/members/{member.user_id}")
21+
assert removed.status_code == 204
22+
23+
assert (await member.client.get("/api/tenants/current/members")).status_code != 200
24+
assert (await member.client.get("/api/tenants/")).json() == []
25+
26+
27+
async def test_role_change_takes_effect_on_the_next_request(tenant_client):
28+
async with (
29+
tenant_client() as owner,
30+
tenant_client("member", tenant_id=owner.tenant_id) as member,
31+
):
32+
invite = {"email": "new@example.com"}
33+
before = await member.client.post("/api/tenants/current/invitations", json=invite)
34+
assert before.status_code == 403
35+
36+
promoted = await owner.client.patch(
37+
f"/api/tenants/current/members/{member.user_id}", json={"role": "admin"}
38+
)
39+
assert promoted.status_code == 200
40+
after = await member.client.post("/api/tenants/current/invitations", json=invite)
41+
assert after.status_code == 201
42+
43+
await owner.client.patch(
44+
f"/api/tenants/current/members/{member.user_id}", json={"role": "member"}
45+
)
46+
demoted = await member.client.post(
47+
"/api/tenants/current/invitations", json={"email": "other@example.com"}
48+
)
49+
assert demoted.status_code == 403
50+
51+
52+
async def test_joining_and_switching_tenants_needs_no_relogin(tenant_client):
53+
async with tenant_client() as user:
54+
first = user.tenant_id
55+
created = await user.client.post("/api/tenants/", json={"name": "Second", "slug": "second"})
56+
assert created.status_code == 201
57+
second = created.json()["id"]
58+
59+
mine = {t["id"] for t in (await user.client.get("/api/tenants/")).json()}
60+
assert mine == {first, second}
61+
62+
assert (await user.client.post(f"/api/tenants/{first}/switch")).status_code == 204
63+
members = (await user.client.get("/api/tenants/current/members")).json()
64+
assert [m["user_id"] for m in members] == [user.user_id]

‎modules/users/tests/_middleware_support.py‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ async def _default_handler(request: Request):
4949
"email": user.email,
5050
"name": user.name,
5151
"roles": user.roles,
52-
"tenant_id": user.tenant_id,
5352
}
5453
if user is not None
5554
else None
@@ -106,7 +105,6 @@ async def mw_active_user(db_session, _mw_seed_roles):
106105
is_superuser=False,
107106
is_verified=True,
108107
full_name="Middleware Tester",
109-
tenant_id="acme",
110108
)
111109
link = UserRole(user_id=user_id, role_id=ADMIN_ROLE_ID)
112110
db_session.add_all([user, link])
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
"""A pre-#381 session cookie's cached ``tenant_id`` is ignored.
2+
3+
``users_user.tenant_id`` is gone and the active tenant is resolved per request
4+
from memberships, but cookies signed before the upgrade still carry the old
5+
value inside ``user_ctx``. Honouring it would keep acting for a tenant the
6+
user may have left.
7+
"""
8+
9+
from __future__ import annotations
10+
11+
import time
12+
13+
import pytest
14+
from auth.contracts.schemas import UserContext
15+
from simple_module_hosting.session import SESSION_EXPIRES_AT_KEY
16+
from sqlalchemy import select
17+
from starlette.requests import Request
18+
from users.models import User
19+
from users.provider import UsersAuthProvider
20+
21+
22+
async def _admin_id(users_app) -> str:
23+
async with users_app.state.sm.db.session_factory() as session:
24+
stmt = select(User.id).where(User.email == "admin@example.com")
25+
return str((await session.execute(stmt)).scalar_one())
26+
27+
28+
@pytest.mark.anyio
29+
async def test_old_cookie_tenant_id_is_dropped_on_the_cached_path(users_app):
30+
user_id = await _admin_id(users_app)
31+
legacy_ctx = UserContext(
32+
id=user_id, email="admin@example.com", name="Admin", roles=["admin"], tenant_id="acme"
33+
).to_session_dict()
34+
session = {
35+
"user_id": user_id,
36+
"user_ctx": legacy_ctx,
37+
"session_version": 0,
38+
SESSION_EXPIRES_AT_KEY: int(time.time()) + 3600,
39+
}
40+
scope = {
41+
"type": "http",
42+
"method": "GET",
43+
"path": "/",
44+
"headers": [],
45+
"app": users_app,
46+
"session": session,
47+
}
48+
49+
ctx = await UsersAuthProvider().resolve_user(Request(scope))
50+
51+
assert ctx is not None
52+
assert ctx.id == user_id
53+
assert ctx.name == "Admin" # the cached context was used, not a DB reload
54+
assert ctx.tenant_id is None

‎modules/users/tests/test_user_model.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ def test_required_columns(self):
3535
"is_superuser",
3636
"is_verified",
3737
"full_name",
38-
"tenant_id",
3938
"disabled_at",
4039
"last_login_at",
4140
# AuditMixin columns

‎modules/users/tests/test_users_middleware.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,6 @@ async def test_authenticated_request_sets_user_context(db_state, mw_active_user)
8181
assert data["user"]["email"] == "middleware-test@example.com"
8282
assert data["user"]["name"] == "Middleware Tester"
8383
assert data["user"]["roles"] == ["admin"]
84-
assert data["user"]["tenant_id"] == "acme"
8584

8685

8786
# ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)