refactor(#3003): harden intersection policy, improve tests and docs

jonpspri · jonpspri · commit 6ab105ffa950 · 2026-03-03T11:30:03.000Z
- Extract _narrow_by_jwt_teams() shared helper to prevent logic drift between async and sync paths - Change empty-intersection semantics to fail-closed (return [] instead of falling back to full DB teams) - Fix cache-poisoning regression test to use narrowing payload so it actually detects regressions - Add direct unit tests for _narrow_by_jwt_teams edge cases - Add cache TTL staleness comments in token scoping middleware - Document teams:[] "no restriction" semantics with code comments - Update multitenancy.md, rbac.md, and securing.md with session token intersection tables, cache isolation behavior, and staleness notes Closes #3003 Signed-off-by: Jonathan Springer <jps@s390x.com>
diff --git a/docs/docs/architecture/multitenancy.md b/docs/docs/architecture/multitenancy.md
@@ -319,13 +319,18 @@ Token scoping is the mechanism that determines which resources a user can **see*
 **Key functions** in `mcpgateway/auth.py`:
 
 - `normalize_token_teams()` — The **single source of truth** for interpreting JWT team claims into a canonical form. Used directly by API tokens and legacy tokens.
-- `resolve_session_teams()` / `resolve_session_teams_sync()` — The **single policy point** for session-token team resolution. Teams are always resolved from the database first so that revoked memberships take effect immediately. If the JWT carries a `teams` claim, the result is narrowed to the intersection of DB teams and JWT teams — letting callers scope a session to a subset of their memberships without risking stale grants. If the intersection is empty (e.g. all JWT-claimed teams have been revoked), the full DB team list is returned as a fallback so the user is not locked out.
+- `resolve_session_teams()` / `resolve_session_teams_sync()` — The **single policy point** for session-token team resolution. Teams are always resolved from the database first so that revoked memberships take effect immediately. If the JWT carries a non-empty `teams` claim, the result is narrowed to the intersection of DB teams and JWT teams — letting callers scope a session to a subset of their memberships without risking stale grants. If the intersection is empty (e.g. all JWT-claimed teams have been revoked), an empty list is returned, which demotes the session to public-only scope — the user can still access public resources and their own private resources, but loses access to all team-scoped resources. An explicit `teams: []` (empty list) in the JWT is treated as "no restriction requested" and returns the full DB membership — this intentionally differs from `normalize_token_teams` where `[] → public-only`.
+- `_narrow_by_jwt_teams()` — Shared intersection logic used by both `resolve_session_teams` and `resolve_session_teams_sync` to prevent logic drift.
 - API tokens and legacy tokens always use `normalize_token_teams()` directly.
 
 ### Secure-First Defaults
 
 The token scoping system follows a **secure-first design principle**: when in doubt, access is restricted.
 
+#### API Tokens and Legacy Tokens
+
+These use `normalize_token_teams()` directly — the JWT `teams` claim is the sole authority:
+
 | JWT `teams` State | `is_admin` | Result | Access Level |
 |-------------------|------------|--------|--------------|
 | Key MISSING | any | `[]` | PUBLIC-ONLY (secure default) |
@@ -334,8 +339,24 @@ The token scoping system follows a **secure-first design principle**: when in do
 | Key `[]` (empty) | any | `[]` | PUBLIC-ONLY |
 | Key `["t1", "t2"]` | any | `["t1", "t2"]` | TEAM-SCOPED |
 
+#### Session Tokens
+
+Session tokens use `resolve_session_teams()` — the **database** is the authority, and the JWT `teams` claim only narrows (never broadens) the result:
+
+| JWT `teams` State | DB Teams | Result | Access Level |
+|-------------------|----------|--------|--------------|
+| Key MISSING | `["t1", "t2"]` | `["t1", "t2"]` | Full DB membership |
+| Key `null` | `["t1", "t2"]` | `["t1", "t2"]` | Full DB membership |
+| Key `[]` (empty) | `["t1", "t2"]` | `["t1", "t2"]` | Full DB membership (no restriction requested) |
+| Key `["t1"]` | `["t1", "t2"]` | `["t1"]` | Narrowed to intersection |
+| Key `["revoked"]` | `["t1", "t2"]` | `[]` | Empty intersection — public-only (fail-closed) |
+| any | `None` (admin) | `None` | ADMIN BYPASS (DB authority) |
+
 !!! warning "Critical Security Behavior"
-    A **missing** `teams` key always results in public-only access, even for admin users. Admin bypass requires **explicit** `teams: null` combined with `is_admin: true`.
+    A **missing** `teams` key always results in public-only access for API/legacy tokens, even for admin users. Admin bypass requires **explicit** `teams: null` combined with `is_admin: true`.
+
+!!! note "Session Token Membership Staleness"
+    Session tokens skip the `_check_team_membership` re-validation in the token scoping middleware because `resolve_session_teams()` already resolved membership from the database. Membership staleness is bounded by the `auth_cache` TTL. The cache stores the full DB membership (not the per-session narrowed intersection) so that multiple sessions for the same user narrow independently.
 
 ### Multi-Team Token Behavior
 
@@ -349,7 +370,7 @@ This means the first team in the token's teams array has special significance fo
 
 ### Return Value Semantics
 
-The `normalize_token_teams()` function returns:
+The `normalize_token_teams()` and `resolve_session_teams()` functions return:
 
 | Return Value | Meaning | Query Behavior |
 |--------------|---------|----------------|
diff --git a/docs/docs/manage/rbac.md b/docs/docs/manage/rbac.md
@@ -138,15 +138,17 @@ Protected entities:
 
 ## Token Scoping Model
 
-Token scoping controls what resources a token can access based on the `teams` claim in the JWT payload. The `normalize_token_teams()` function is the **single source of truth** for interpreting JWT team claims into a canonical form. For session tokens, `resolve_session_teams()` is the **single policy point** — it resolves teams from the database first (so revoked memberships take effect immediately), then narrows the result to the intersection with any JWT-embedded `teams` claim (so callers can scope a session to a subset of their memberships). If the intersection is empty (e.g. all JWT-claimed teams have been revoked), the full DB team list is returned as a fallback.
+Token scoping controls what resources a token can access based on the `teams` claim in the JWT payload. The `normalize_token_teams()` function is the **single source of truth** for interpreting JWT team claims into a canonical form. For session tokens, `resolve_session_teams()` is the **single policy point** — it resolves teams from the database first (so revoked memberships take effect immediately), then narrows the result to the intersection with any JWT-embedded `teams` claim (so callers can scope a session to a subset of their memberships). If the intersection is empty (e.g. all JWT-claimed teams have been revoked), an empty list is returned, demoting the session to public-only scope — the user can still access public resources and their own private resources, but loses access to all team-scoped resources. An explicit `teams: []` in a session JWT is treated as "no restriction requested" and returns the full DB membership.
 
 ### Token Scoping Contract
 
 The `teams` claim in JWT tokens determines resource visibility. The system follows a **secure-first design**: when in doubt, access is denied.
 
+**API tokens and legacy tokens** — JWT `teams` claim is the sole authority:
+
 ```
 ┌─────────────────────────────────────────────────────────────────────────────┐
-│                        Token Teams Claim Handling                           │
+│                   API / Legacy Token Teams Claim Handling                    │
 ├─────────────────────────────────────────────────────────────────────────────┤
 │  JWT Claim State          │  is_admin: true       │  is_admin: false        │
 ├───────────────────────────┼───────────────────────┼─────────────────────────┤
@@ -158,6 +160,24 @@ The `teams` claim in JWT tokens determines resource visibility. The system follo
 └───────────────────────────┴───────────────────────┴─────────────────────────┘
 ```
 
+**Session tokens** — DB is the authority; JWT `teams` only narrows (never broadens):
+
+```
+┌──────────────────────────────────────────────────────────────────────────────────┐
+│                     Session Token Teams Resolution                               │
+├──────────────────────────────────────────────────────────────────────────────────┤
+│  JWT Claim State       │  DB Teams          │  Result           │ Access Level   │
+├────────────────────────┼────────────────────┼───────────────────┼────────────────┤
+│  Missing / null / []   │  ["t1", "t2"]      │  ["t1", "t2"]     │ Full DB scope  │
+│  ["t1"]                │  ["t1", "t2"]      │  ["t1"]           │ Narrowed       │
+│  ["revoked"]           │  ["t1", "t2"]      │  []               │ Public-only    │
+│  any                   │  None (admin)      │  None             │ Admin bypass   │
+└────────────────────────┴────────────────────┴───────────────────┴────────────────┘
+```
+
+!!! note "Session Token Staleness"
+    Session tokens skip `_check_team_membership` re-validation in the token scoping middleware because `resolve_session_teams()` already resolved membership from the database. Membership staleness is bounded by the `auth_cache` TTL. The cache stores the full DB membership (not the per-session narrowed intersection) so that multiple sessions for the same user narrow independently.
+
 !!! warning "Admin Bypass Requirements"
     Admin bypass (unrestricted access) requires **BOTH** conditions:
 
@@ -194,7 +214,7 @@ Key points:
    - Verify JWT/API token.
    - Resolve `token_use`.
 2. **Normalize/resolve teams**
-   - `token_use=session` → resolve from DB (`_resolve_teams_from_db()`).
+   - `token_use=session` → resolve from DB and optionally narrow by JWT claim (`resolve_session_teams()`).
    - `token_use!=session` → normalize JWT `teams` (`normalize_token_teams()`).
 3. **Layer 1: Token scoping**
    - Filter accessible resources by `token_teams`.
diff --git a/docs/docs/manage/securing.md b/docs/docs/manage/securing.md
@@ -235,7 +235,7 @@ Tokens can be scoped to specific teams using the `teams` JWT claim:
 **Security Default**: Non-admin tokens without explicit team scope default to public-only access (principle of least privilege).
 
 !!! note "Session Tokens vs API Tokens"
-    For `token_use: "session"` (Admin UI login), teams are resolved server-side from DB/cache on each request.
+    For `token_use: "session"` (Admin UI login), teams are resolved server-side from DB/cache on each request via `resolve_session_teams()`. If the JWT carries a non-empty `teams` claim, the result is narrowed to the intersection of DB teams and JWT teams, allowing callers to scope a session to a subset of their memberships.
     For `token_use: "api"` or legacy tokens, teams are interpreted from the JWT `teams` claim using `normalize_token_teams()`.
 
 #### Server-Scoped Tokens
diff --git a/mcpgateway/auth.py b/mcpgateway/auth.py
@@ -257,6 +257,48 @@ def _resolve_teams_from_db_sync(email: str, is_admin: bool) -> Optional[List[str
     return team_ids
 
 
+def _narrow_by_jwt_teams(payload: Dict[str, Any], db_teams: Optional[List[str]]) -> Optional[List[str]]:
+    """Apply JWT intersection policy to DB-resolved teams.
+
+    Shared implementation used by both :func:`resolve_session_teams` and
+    :func:`resolve_session_teams_sync` to prevent logic drift.
+
+    If *db_teams* is ``None`` (admin bypass), returns ``None`` immediately.
+    If the JWT ``teams`` claim is a non-empty list, returns the intersection
+    of *db_teams* and the JWT teams.  If the intersection is empty (e.g.
+    all JWT-claimed teams have been revoked), returns ``[]`` so that
+    downstream enforcement denies the request rather than silently
+    broadening scope.
+
+    Args:
+        payload: The decoded JWT payload dict.
+        db_teams: Teams resolved from the database, or ``None`` for admin bypass.
+
+    Returns:
+        None (admin bypass), [] (public-only / empty intersection), or list of team ID strings.
+    """
+    if db_teams is None:
+        return None
+
+    jwt_teams = payload.get("teams")
+    if isinstance(jwt_teams, list) and jwt_teams:
+        # Non-empty JWT teams → intersect with DB teams.  An empty
+        # intersection (all JWT teams revoked) returns [], which gives
+        # public-only access and lets downstream enforcement deny the
+        # request (fail-closed).
+        jwt_team_set = set(normalize_token_teams({"teams": jwt_teams}) or [])
+        return [t for t in db_teams if t in jwt_team_set]
+
+    # JWT teams absent, null, or empty list → no narrowing requested.
+    # An explicit ``teams: []`` means "don't restrict by team" (i.e. use
+    # the full DB membership), which intentionally differs from
+    # ``normalize_token_teams`` where ``[] → public-only``.  The
+    # distinction exists because session tokens always start from DB-
+    # resolved teams — an empty JWT claim simply means the caller did not
+    # request a subset.
+    return db_teams
+
+
 def resolve_session_teams_sync(payload: Dict[str, Any], email: str, is_admin: bool) -> Optional[List[str]]:
     """Synchronous companion to :func:`resolve_session_teams`.
 
@@ -272,20 +314,7 @@ def resolve_session_teams_sync(payload: Dict[str, Any], email: str, is_admin: bo
         None (admin bypass), [] (public-only), or list of team ID strings.
     """
     db_teams = _resolve_teams_from_db_sync(email, is_admin)
-
-    # Admin bypass — no narrowing possible
-    if db_teams is None:
-        return None
-
-    # Narrow to intersection when the JWT carries an explicit teams claim
-    jwt_teams = payload.get("teams")
-    if isinstance(jwt_teams, list) and jwt_teams:
-        jwt_team_set = set(normalize_token_teams({"teams": jwt_teams}) or [])
-        narrowed = [t for t in db_teams if t in jwt_team_set]
-        if narrowed:
-            return narrowed
-
-    return db_teams
+    return _narrow_by_jwt_teams(payload, db_teams)
 
 
 async def _resolve_teams_from_db(email: str, user_info) -> Optional[List[str]]:
@@ -359,8 +388,9 @@ async def resolve_session_teams(
            (e.g. via a batched query).
         2. If DB returns ``None`` (admin bypass), return ``None``.
         3. If the JWT ``teams`` claim is a non-empty list, intersect with
-           DB teams.  If the intersection is non-empty, return it;
-           otherwise fall back to the full DB result.
+           DB teams.  If the intersection is empty (all JWT-claimed teams
+           revoked), return ``[]`` so downstream enforcement denies the
+           request.
         4. Otherwise return the full DB result.
 
     Args:
@@ -379,19 +409,7 @@ async def resolve_session_teams(
     else:
         db_teams = await _resolve_teams_from_db(email, user_info)
 
-    # Admin bypass — no narrowing possible
-    if db_teams is None:
-        return None
-
-    # Narrow to intersection when the JWT carries an explicit teams claim
-    jwt_teams = payload.get("teams")
-    if isinstance(jwt_teams, list) and jwt_teams:
-        jwt_team_set = set(normalize_token_teams({"teams": jwt_teams}) or [])
-        narrowed = [t for t in db_teams if t in jwt_team_set]
-        if narrowed:
-            return narrowed
-
-    return db_teams
+    return _narrow_by_jwt_teams(payload, db_teams)
 
 
 def normalize_token_teams(payload: Dict[str, Any]) -> Optional[List[str]]:
diff --git a/mcpgateway/middleware/token_scoping.py b/mcpgateway/middleware/token_scoping.py
@@ -1203,8 +1203,10 @@ async def __call__(self, request: Request, call_next):
                     # come from JWT claims and may be stale.  Session tokens skip this
                     # because resolve_session_teams() already resolved membership from
                     # the DB; re-checking the raw JWT claim here would conflict with
-                    # the intersection-fallback semantics (stale JWT teams would cause
-                    # a 403 even though the user has valid DB teams).
+                    # the intersection semantics (stale JWT teams would cause a 403
+                    # even though the user has valid DB teams).
+                    # NOTE: session-token membership staleness is bounded by the
+                    # auth_cache TTL (see _resolve_teams_from_db).
                     if token_use != "session" and not self._check_team_membership(payload, db=db):  # nosec B105 - Not a password; token_use is a JWT claim type
                         logger.warning("Token rejected: User no longer member of associated team(s)")
                         raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="Token is invalid: User is no longer a member of the associated team")
@@ -1220,8 +1222,11 @@ async def __call__(self, request: Request, call_next):
                     finally:
                         db.close()
             else:
-                # Public-only token: no team membership check needed, but still check resource ownership
-                if not self._check_team_membership(payload):
+                # Public-only token (or session token with empty intersection):
+                # skip _check_team_membership for session tokens — the empty
+                # intersection already means no team-scoped access.
+                # Membership staleness bounded by auth_cache TTL.
+                if token_use != "session" and not self._check_team_membership(payload):  # nosec B105 - Not a password; token_use is a JWT claim type
                     logger.warning("Token rejected: User no longer member of associated team(s)")
                     raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="Token is invalid: User is no longer a member of the associated team")
 
diff --git a/tests/unit/mcpgateway/middleware/test_token_scoping.py b/tests/unit/mcpgateway/middleware/test_token_scoping.py
@@ -726,12 +726,13 @@ async def test_session_token_calls_resolve_session_teams(self, middleware, mock_
 
     @pytest.mark.asyncio
     async def test_session_token_skips_membership_check_on_stale_jwt_teams(self, middleware, mock_request):
-        """Session tokens skip _check_team_membership so stale JWT teams don't cause a spurious 403."""
+        """Session tokens skip _check_team_membership; stale JWT teams produce empty intersection (public-only)."""
         mock_request.url.path = "/servers"
         mock_request.method = "GET"
         mock_request.headers = {"Authorization": "Bearer session_token"}
 
         # JWT claims stale team "revoked-team"; DB only has "db-team"
+        # Intersection is empty → resolve_session_teams returns []
         session_payload = {
             "sub": "user@example.com",
             "token_use": "session",
@@ -740,14 +741,15 @@ async def test_session_token_skips_membership_check_on_stale_jwt_teams(self, mid
         }
 
         with patch.object(middleware, "_extract_token_scopes", return_value=session_payload):
-            # resolve_session_teams already handles intersection & fallback
+            # resolve_session_teams returns [] (empty intersection)
             with patch("mcpgateway.auth._resolve_teams_from_db", return_value=["db-team"]) as mock_resolve:
                 with patch.object(middleware, "_check_team_membership", return_value=False) as mock_membership:
                     with patch.object(middleware, "_check_resource_team_ownership", return_value=True):
                         call_next = AsyncMock(return_value="success")
 
                         result = await middleware(mock_request, call_next)
 
+                        # Request proceeds with public-only scope (token_teams=[])
                         assert result == "success"
                         call_next.assert_called_once()
                         mock_resolve.assert_called_once()
diff --git a/tests/unit/mcpgateway/test_auth.py b/tests/unit/mcpgateway/test_auth.py
