Aug 20 - Session tenant binding

app/__init__.py

@@ -18,8 +18,25 @@ login_manager = LoginManager()
 migrate       = Migrate()
 mail          = Mail()
 csrf          = CSRFProtect()    # initialized here; .init_app() called in create_app()
+def _rate_limit_key():
+    """MT-21: scope rate-limit buckets per tenant as well as per client IP.
+
+    With a bare remote-address key, two tenants behind the same NAT egress
+    share every route's counter, so one tenant's traffic can lock another out
+    of (for example) /auth/login. Falls back to the plain address in
+    single-tenant mode and on tenant-exempt paths, leaving today's buckets
+    unchanged there.
+    """
+    from flask import g, has_request_context
+    addr = get_remote_address()
+    if not has_request_context():
+        return addr
+    tenant = getattr(g, 'tenant', None)
+    return f't{tenant.id}|{addr}' if tenant is not None else addr
+
+
 limiter       = Limiter(
-    key_func       = get_remote_address,
+    key_func       = _rate_limit_key,
     default_limits = [],          # no global limit — applied per-route only
     # Use Redis when REDIS_URL is set in the environment (production multi-worker).
     # Falls back to in-process memory for local development (single-worker only;

app/api/decorators.py

@@ -62,6 +62,20 @@ def jwt_required(f):
         if payload is None:
             return api_error('Access token is invalid or expired', 401)
 
+        # MT-21: a token signed for another tenant verifies fine here (shared
+        # SECRET_KEY), so check the tenant claim before 'sub' is resolved
+        # against the bound database. No-op in single-tenant mode.
+        from app.tenancy.session_binding import current_tenant_id
+        tenant_id = current_tenant_id()
+        if tenant_id is not None:
+            token_tid = payload.get('tid')
+            if token_tid != tenant_id:
+                logger.warning(
+                    'API tenant mismatch | token_tid=%s resolved=%s endpoint=%s',
+                    token_tid, tenant_id, request.endpoint,
+                )
+                return api_error('Access token is not valid for this workspace', 401)
+
         user_id = int(payload.get('sub', 0))
         user    = db.session.get(User, user_id)
 

app/api/jwt_utils.py

@@ -10,6 +10,7 @@ claims needed to identify the caller:
     {
         "sub":  "42",          # user.id as string
         "role": "inspector",   # user.role
+        "tid":  3,             # issuing tenant id (multi-tenant mode only)
         "iat":  1710000000,    # issued-at  (UTC epoch)
         "exp":  1710003600,    # expiry     (UTC epoch, 60 min later)
     }
@@ -33,6 +34,12 @@ def _secret():
     return current_app.config['SECRET_KEY']
 
 
+def _current_tenant_id():
+    """Resolved tenant id, or None in single-tenant / unbound contexts."""
+    from app.tenancy.session_binding import current_tenant_id
+    return current_tenant_id()
+
+
 def generate_access_token(user, lifetime_minutes: int = ACCESS_TOKEN_LIFETIME_MINUTES) -> str:
     """
     Create and sign a new access token for the given user.
@@ -54,6 +61,14 @@ def generate_access_token(user, lifetime_minutes: int = ACCESS_TOKEN_LIFETIME_MI
         'iat':  now,
         'exp':  now + timedelta(minutes=lifetime_minutes),
     }
+    # MT-21: bind the token to the issuing tenant. Every tenant is signed with
+    # the same SECRET_KEY, so without this claim a token minted at one tenant
+    # host verifies at another and 'sub' resolves against whichever database
+    # the middleware bound. Omitted in single-tenant mode so token shape is
+    # unchanged there.
+    tid = _current_tenant_id()
+    if tid is not None:
+        payload['tid'] = tid
     return jwt.encode(payload, _secret(), algorithm='HS256')
 
 

app/models/user.py

@@ -25,7 +25,15 @@ ROLE_LABELS = {
 @login_manager.user_loader
 def load_user(user_id):
     from app import db
-    return db.session.get(User, int(user_id))
+    from app.tenancy.session_binding import parse_user_id
+    # MT-21: the identity string is tenant-tagged in multi-tenant mode. A tag
+    # naming another tenant (a session or remember-me cookie replayed onto this
+    # host) resolves to None here rather than loading the same-numbered user out
+    # of whichever database happens to be bound.
+    uid = parse_user_id(user_id)
+    if uid is None:
+        return None
+    return db.session.get(User, uid)
 
 class User(UserMixin, db.Model):
     __tablename__ = 'users'
@@ -125,6 +133,15 @@ class User(UserMixin, db.Model):
     def is_active(self):
         return self.active
 
+    # MT-21: Flask-Login derives BOTH the session '_user_id' and the
+    # remember-me cookie payload from get_id(), and feeds both back through
+    # load_user(). Tagging the tenant here is therefore the single seam that
+    # binds every persisted identity to the tenant that issued it. Returns a
+    # bare id (today's format) whenever no tenant is bound.
+    def get_id(self):
+        from app.tenancy.session_binding import tag_user_id
+        return tag_user_id(self.id)
+
     def set_password(self, password):
         self.password_hash = generate_password_hash(password)
 

app/routes/auth.py

@@ -8,6 +8,7 @@ from app.utils import mfa
 import logging
 from app.utils.audit import log_action, ACTION_CREATE, ACTION_UPDATE, ACTION_DELETE, ACTION_LOGIN, ACTION_LOGOUT
 from app.tenancy.gates import quota_soft_check
+from app.tenancy.session_binding import bind_session_tenant, SESSION_TENANT_KEY
 
 logger = logging.getLogger(__name__)
 
@@ -40,6 +41,7 @@ def login():
             # second-factor step. Password is verified; identity is NOT yet
             # established until the code is confirmed at /auth/mfa.
             if user.mfa_enabled and user.mfa_secret:
+                bind_session_tenant()   # MT-21: tag before identity enters the session
                 session['mfa_pending_user_id'] = user.id
                 session['mfa_pending_remember'] = bool(form.remember_me.data)
                 session['mfa_pending_next'] = safe_redirect_url(request.args.get('next'))
@@ -47,6 +49,7 @@ def login():
                 return redirect(url_for('auth.mfa_challenge'))
 
             login_user(user, remember=form.remember_me.data)
+            bind_session_tenant()   # MT-21
             # Use validated next URL — never redirect blindly to request.args['next']
             next_page = safe_redirect_url(request.args.get('next'))
             log_action(ACTION_LOGIN, 'User', user.id, user.username)
@@ -106,6 +109,7 @@ def mfa_challenge():
             next_page = session.pop('mfa_pending_next', None)
             session.pop('mfa_pending_user_id', None)
             login_user(user, remember=remember)
+            bind_session_tenant()   # MT-21
             log_action(ACTION_LOGIN, 'User', user.id, user.username, f'2fa via {via}')
             if via == 'recovery':
                 remaining_n = len(user.mfa_recovery_codes or [])
@@ -948,6 +952,23 @@ def impersonate_entry():
     flask_session['impersonating_tenant_id']     = tenant_id
     flask_session['impersonating_superadmin_id'] = superadmin_id
 
+    # ── MT-21: re-tag the session for the impersonated tenant ────────────────
+    # Identity in the session is tenant-tagged (User.get_id), and from the next
+    # request onward the middleware binds the impersonated tenant's database.
+    # Without re-tagging, load_user() would correctly reject the tag issued by
+    # the host tenant and the superadmin would land on a login page.
+    #
+    # This preserves the pre-existing impersonation semantics EXACTLY: the
+    # numeric user id carries over, so the superadmin is loaded as the
+    # same-numbered user in the target tenant's database. That behaviour is
+    # arbitrary and worth revisiting (see MULTI_TENANT_PLAN.md open items) —
+    # but changing it is a separate decision, not a security fix.
+    flask_session[SESSION_TENANT_KEY] = tenant_id
+    raw_uid = flask_session.get('_user_id')
+    if raw_uid is not None:
+        numeric_uid = str(raw_uid).rpartition(':')[2]
+        flask_session['_user_id'] = f'{tenant_id}:{numeric_uid}'
+
     logger.info('AUTH | impersonate_start | sa=%s tenant=%s', superadmin_id, tenant_id)
 
     import os
@@ -971,6 +992,11 @@ def impersonate_end():
     import os
     flask_session.pop('impersonating_tenant_id', None)
     flask_session.pop('impersonating_superadmin_id', None)
+    # MT-21: the session identity is still tagged for the impersonated tenant.
+    # Drop it rather than carrying it back to the superadmin's own host, where
+    # the middleware would clear it on tenant mismatch anyway.
+    logout_user()
+    flask_session.clear()
     panel_url = f"https://admin.{os.environ.get('TENANT_BASE_DOMAIN', 'jqc.app')}"
     logger.info('AUTH | impersonate_end | redirecting to panel')
     return redirect(panel_url)

app/tenancy/__init__.py

@@ -12,6 +12,10 @@ from app.tenancy.middleware import init_tenancy
 from app.tenancy.context import TenantContext
 from app.tenancy.gates import feature_required, quota_soft_check
 from app.tenancy.quota import check_quota, check_feature
+from app.tenancy.session_binding import (
+    bind_session_tenant, enforce_session_tenant, current_tenant_id,
+    tag_user_id, parse_user_id, SESSION_TENANT_KEY,
+)
 
 __all__ = [
     'RoutingSession',
@@ -21,4 +25,11 @@ __all__ = [
     'quota_soft_check',
     'check_quota',
     'check_feature',
+    # MT-21
+    'bind_session_tenant',
+    'enforce_session_tenant',
+    'current_tenant_id',
+    'tag_user_id',
+    'parse_user_id',
+    'SESSION_TENANT_KEY',
 ]

app/tenancy/engine_cache.py

@@ -5,40 +5,75 @@ Process-local cache of per-tenant SQLAlchemy engines, keyed by tenant id.
 
 Each tenant has its own database, hence its own engine + connection pool.
 Engines are created lazily on first use and reused across requests. Total
-backend connections ≈ workers × cached-tenants × pool_size, so pool sizing is
+backend connections ≈ workers × cached-tenants × (pool_size + max_overflow),
-a real scaling lever (see MULTI_TENANT_PLAN.md §4); tune via config, or set a
+so the cache is BOUNDED (MT-21): it holds at most
-small pool / switch to NullPool when the tenant count grows large.
+``TENANT_ENGINE_CACHE_MAX`` engines and evicts the least-recently-used one
+beyond that, disposing it. Without the bound, a process that has served N
+tenants holds N pools forever, and the connection count grows without limit
+until MySQL's ``max_connections`` (default 151) rejects new sessions.
+
+Worked example — 8 workers, pool_size 2 + max_overflow 3, cache cap 32:
+    8 × 32 × 5 = 1280 worst case, vs. unbounded before this change.
+
+Eviction disposes the engine, which closes its *idle* pooled connections.
+Connections already checked out by another thread stay valid and are closed
+when returned, so eviction is safe under concurrency; the evicted tenant
+simply rebuilds its engine on the next request.
 
 `invalidate(tenant_id)` drops a cached engine (e.g. after credential rotation
 or tenant suspension); the next request rebuilds it.
 """
 
 import threading
+from collections import OrderedDict
 
 from flask import current_app
 from sqlalchemy import create_engine
 
-_engines = {}
+_engines = OrderedDict()
 _lock = threading.Lock()
 
+# Fallback cap used when no application context is available (CLI/cron paths
+# that touch the cache outside a request). Mirrors the config default.
+_DEFAULT_CACHE_MAX = 32
+
+
+def _cache_max():
+    try:
+        return int(current_app.config.get('TENANT_ENGINE_CACHE_MAX', _DEFAULT_CACHE_MAX))
+    except Exception:
+        return _DEFAULT_CACHE_MAX
+
 
 def get_tenant_engine(tenant):
     """Return (building if needed) the cached engine for a TenantContext."""
-    engine = _engines.get(tenant.id)
+    evicted = []
-    if engine is not None:
-        return engine
     with _lock:
         engine = _engines.get(tenant.id)
         if engine is None:
             engine = create_engine(
                 tenant.db_uri,
                 pool_pre_ping=True,
-                pool_size=current_app.config.get('TENANT_ENGINE_POOL_SIZE', 5),
+                pool_size=current_app.config.get('TENANT_ENGINE_POOL_SIZE', 2),
-                max_overflow=current_app.config.get('TENANT_ENGINE_MAX_OVERFLOW', 5),
+                max_overflow=current_app.config.get('TENANT_ENGINE_MAX_OVERFLOW', 3),
                 pool_recycle=current_app.config.get('TENANT_ENGINE_POOL_RECYCLE', 1800),
                 future=True,
             )
             _engines[tenant.id] = engine
+            # Bound the cache: drop least-recently-used engines beyond the cap.
+            cap = _cache_max()
+            while cap > 0 and len(_engines) > cap:
+                old_id, old_engine = _engines.popitem(last=False)
+                evicted.append((old_id, old_engine))
+        else:
+            _engines.move_to_end(tenant.id)
+
+    # Dispose outside the lock — dispose() can block on socket teardown.
+    for old_id, old_engine in evicted:
+        try:
+            old_engine.dispose()
+        except Exception:
+            pass
     return engine
 
 

app/tenancy/middleware.py

@@ -139,6 +139,13 @@ def init_tenancy(app):
                         )
                         g.tenant = ctx
                         g.tenant_engine = get_tenant_engine(ctx)
+                        # MT-21: impersonation deliberately rebinds the session
+                        # to the impersonated tenant, so stamp it rather than
+                        # clearing it. /auth/impersonate has already written the
+                        # same marker; this keeps it correct if the superadmin's
+                        # target changes mid-session.
+                        from app.tenancy.session_binding import bind_session_tenant
+                        bind_session_tenant()
                         return  # skip normal Host resolution
             except Exception:
                 logger.warning('TENANCY | impersonation_failed | tenant_id=%s', imp_id)
@@ -156,6 +163,14 @@ def init_tenancy(app):
         g.tenant = tenant
         g.tenant_engine = get_tenant_engine(tenant)
 
+        # MT-21: a session cookie signed for a different tenant validates fine
+        # here — same app, same SECRET_KEY — so drop it before any downstream
+        # code reads identity out of it. Covers pre-auth session state
+        # (mfa_pending_user_id); the tenant tag in User.get_id() covers the
+        # authenticated session and the remember-me cookie.
+        from app.tenancy.session_binding import enforce_session_tenant
+        enforce_session_tenant()
+
     @app.before_request
     def _billing_gate():
         """MT-8: Enforce subscription status. Inert when BILLING_ENABLED=False."""

app/tenancy/session_binding.py

@@ -0,0 +1,154 @@
+"""
+app/tenancy/session_binding.py
+------------------------------
+MT-21: bind a browser session to the tenant that issued it.
+
+Why this exists
+---------------
+Every tenant is served by the same Flask application with the same
+``SECRET_KEY``. Before this module, nothing in a signed cookie identified
+*which* tenant it was issued by, so a cookie minted on one tenant host
+validated perfectly on another:
+
+    1. A legitimate user of tenant A logs in at a.jqc.app.
+    2. They copy their session cookie onto b.jqc.app (devtools / curl —
+       host-only cookie scoping stops the *browser* replaying it, but not a
+       person doing it by hand).
+    3. The tenancy middleware resolves Host → tenant B and binds tenant B's
+       database.
+    4. Flask-Login calls load_user('7') and gets **tenant B's user #7**.
+
+The signature was always valid, because it is the same key. The identity was
+never checked against the tenant. Result: authentication as an arbitrary user
+in any tenant whose hostname the attacker knows.
+
+Two layers close this, and both live here:
+
+``SESSION_TENANT_KEY``
+    A ``_tenant_id`` marker written into the session at every point where the
+    session starts carrying identity (login, MFA challenge hand-off,
+    impersonation). ``enforce_session_tenant()`` clears the whole session when
+    that marker disagrees with the resolved tenant, so pre-authentication
+    session state (``mfa_pending_user_id`` and friends) cannot cross tenants
+    either.
+
+``tag_user_id`` / ``parse_user_id``
+    Flask-Login derives BOTH the session ``_user_id`` and the "remember me"
+    cookie payload from ``User.get_id()``, and feeds both back through
+    ``user_loader``. Tagging the id there — ``"<tenant_id>:<user_id>"`` — is
+    therefore a single seam that covers both cookies. Clearing the session
+    alone would not have been enough: a remember-me cookie repopulates the
+    session immediately afterwards.
+
+Single-tenant behaviour is unchanged. When ``MULTI_TENANT_ENABLED`` is False,
+or no tenant is bound (cron, CLI, exempt paths), ids stay bare integers and
+the enforcement is a no-op.
+"""
+
+import logging
+
+from flask import current_app, g, session
+
+logger = logging.getLogger(__name__)
+
+SESSION_TENANT_KEY = '_tenant_id'
+
+
+def current_tenant_id():
+    """Resolved tenant id for this request, or None in single-tenant mode."""
+    if not current_app.config.get('MULTI_TENANT_ENABLED', False):
+        return None
+    tenant = getattr(g, 'tenant', None)
+    return tenant.id if tenant is not None else None
+
+
+def bind_session_tenant():
+    """Stamp the current session with the tenant that issued it.
+
+    Called at every point that puts identity into the session. No-op in
+    single-tenant mode so existing sessions keep working untouched.
+    """
+    tid = current_tenant_id()
+    if tid is not None:
+        session[SESSION_TENANT_KEY] = tid
+
+
+def enforce_session_tenant():
+    """Clear the session if it was issued by a different tenant.
+
+    Returns True when the session was cleared. Runs after tenant resolution,
+    from the tenancy middleware.
+    """
+    tid = current_tenant_id()
+    if tid is None:
+        return False
+
+    bound = session.get(SESSION_TENANT_KEY)
+
+    if bound is None:
+        # An untagged session carrying identity predates this binding, or was
+        # lifted from somewhere else. Either way it cannot be trusted here.
+        if '_user_id' in session or 'mfa_pending_user_id' in session:
+            logger.warning('TENANCY | session_untagged_cleared | tenant=%s', tid)
+            session.clear()
+            return True
+        return False
+
+    if bound != tid:
+        logger.warning('TENANCY | session_tenant_mismatch | bound=%s resolved=%s',
+                       bound, tid)
+        session.clear()
+        return True
+
+    return False
+
+
+def tag_user_id(user_id):
+    """Render a Flask-Login identity string, tenant-tagged when applicable.
+
+    Used by ``User.get_id()``. Feeds both the session ``_user_id`` and the
+    remember-me cookie.
+    """
+    tid = current_tenant_id()
+    if tid is None:
+        return str(user_id)
+    return f'{tid}:{user_id}'
+
+
+def parse_user_id(raw):
+    """Inverse of :func:`tag_user_id`, with the tenant check applied.
+
+    Returns the integer user id, or None when the identity must be rejected —
+    a foreign tenant tag, an untagged id arriving in multi-tenant mode, or
+    anything unparseable. ``user_loader`` turns None into an anonymous user,
+    which sends the caller back to the login page.
+    """
+    if raw is None:
+        return None
+    raw = str(raw)
+    tid = current_tenant_id()
+
+    if ':' in raw:
+        tag, _, uid = raw.partition(':')
+        if tid is None:
+            # Tagged id replayed at a single-tenant / unbound context.
+            return None
+        try:
+            if int(tag) != tid:
+                logger.warning('TENANCY | user_id_tenant_mismatch | tag=%s resolved=%s',
+                               tag, tid)
+                return None
+        except (TypeError, ValueError):
+            return None
+    else:
+        uid = raw
+        if tid is not None:
+            # Untagged id in multi-tenant mode: either a session predating
+            # MT-21 or one lifted from another host. Force re-authentication.
+            logger.warning('TENANCY | user_id_untagged_rejected | tenant=%s', tid)
+            return None
+
+    try:
+        return int(uid)
+    except (TypeError, ValueError):
+        return None

config.py

@@ -53,9 +53,17 @@ class Config:
     # by the provisioner to build <slug>.<base> subdomains.
     TENANT_BASE_DOMAIN = os.environ.get('TENANT_BASE_DOMAIN', 'jqc.app')
     # Per-tenant SQLAlchemy engine pool tuning (see MULTI_TENANT_PLAN.md §4).
-    TENANT_ENGINE_POOL_SIZE    = int(os.environ.get('TENANT_ENGINE_POOL_SIZE', 5))
+    # MT-21: defaults lowered from 5/5 to 2/3. Backend connections scale as
-    TENANT_ENGINE_MAX_OVERFLOW = int(os.environ.get('TENANT_ENGINE_MAX_OVERFLOW', 5))
+    # workers × cached-tenants × (pool_size + max_overflow); at the old
+    # defaults 8 workers × 32 tenants would reserve 2560 connections against a
+    # MySQL max_connections that defaults to 151.
+    TENANT_ENGINE_POOL_SIZE    = int(os.environ.get('TENANT_ENGINE_POOL_SIZE', 2))
+    TENANT_ENGINE_MAX_OVERFLOW = int(os.environ.get('TENANT_ENGINE_MAX_OVERFLOW', 3))
     TENANT_ENGINE_POOL_RECYCLE = int(os.environ.get('TENANT_ENGINE_POOL_RECYCLE', 1800))
+    # MT-21: hard cap on cached per-tenant engines per worker process. Beyond
+    # this the least-recently-used engine is disposed and rebuilt on demand.
+    # 0 disables the bound (previous behaviour — not recommended).
+    TENANT_ENGINE_CACHE_MAX    = int(os.environ.get('TENANT_ENGINE_CACHE_MAX', 32))
 
     # ── File uploads ────────────────────────────────────────────────────────
     UPLOAD_FOLDER = os.path.join(basedir, 'app/static/uploads')