From ecf4df88d31cdba2b32838a30193b4fd3970f206 Mon Sep 17 00:00:00 2001 From: Chris Farhood Date: Tue, 7 Jul 2026 10:39:05 -0400 Subject: [PATCH] security: require verified email for admin + harden sessions Admin (email allowlist) is now gated on a verified email and re-evaluated per request instead of trusting a frozen cookie flag; session max_age cut to 8h. Co-Authored-By: Claude Opus 4.8 (1M context) --- .coverage | Bin 53248 -> 53248 bytes src/intervalsicu_mcp_ui/app.py | 30 +++++++++++++++++++++++++----- tests/test_app.py | 24 ++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 5 deletions(-) diff --git a/.coverage b/.coverage index 4f48a44e1bad675de4a6fb9bc8e5f874bbfa5273..b1740dd5c0fd5f3645734660810c5e146a143a5d 100644 GIT binary patch delta 39 xcmV+?0NDS4paX!Q1F!*)6%zP!pO0Vr%*+e`0K?y}f9&t|?gEjKF0(|Byg+e<6Q=+G delta 36 ucmV+<0Nek7paX!Q1F!*)5>oE-@jf#%0{}4m{rboLUhgiEkukGGkGw!LUlE)D diff --git a/src/intervalsicu_mcp_ui/app.py b/src/intervalsicu_mcp_ui/app.py index 0e9c0f6..1c0b209 100644 --- a/src/intervalsicu_mcp_ui/app.py +++ b/src/intervalsicu_mcp_ui/app.py @@ -26,8 +26,19 @@ _TEMPLATES = Jinja2Templates(directory=str(Path(__file__).parent / "templates")) def current_user(request: Request) -> dict | None: - """Session user dict ({sub,email,name,is_admin}) or None. Overridable in tests.""" - return request.session.get("user") + """Session user dict ({sub,email,name,email_verified,is_admin}) or None. + + `is_admin` is re-evaluated on every request from the current allowlist and the + *verified* email — never trusted from the cookie. So removing someone from + ADMIN_EMAILS drops their admin rights immediately, and an unverified email can + never satisfy the admin check. Overridable in tests.""" + user = request.session.get("user") + if not user: + return None + cfg = request.app.state.cfg + email = (user.get("email") or "").lower() + is_admin = user.get("email_verified") is True and email in cfg.admin_emails + return {**user, "is_admin": is_admin} def create_app(config: Config | None = None) -> FastAPI: @@ -50,7 +61,14 @@ def create_app(config: Config | None = None) -> FastAPI: ) app = FastAPI(title="Intervals.icu MCP portal", root_path=cfg.root_path) - app.add_middleware(SessionMiddleware, secret_key=cfg.session_secret, https_only=True, same_site="lax") + app.add_middleware( + SessionMiddleware, + secret_key=cfg.session_secret, + https_only=True, + same_site="lax", + # Bound how long a leaked/stale signed cookie stays valid (default is 14d). + max_age=8 * 60 * 60, + ) app.state.cfg = cfg app.state.oauth = oauth @@ -92,8 +110,10 @@ def create_app(config: Config | None = None) -> FastAPI: "sub": sub, "email": email, "name": name, - # Admin is an email allowlist now (Better Auth has no group claims). - "is_admin": bool(email) and email.lower() in cfg.admin_emails, + # Store whether the IdP verified this email; admin (email allowlist, + # since Better Auth has no group claims) is gated on it per request in + # current_user. Google always sets email_verified for its accounts. + "email_verified": claims.get("email_verified") is True, } return redirect("/account") diff --git a/tests/test_app.py b/tests/test_app.py index 3d54087..329e90c 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -15,6 +15,30 @@ USER = {"sub": "sub-user", "email": "user@x.com", "name": "User", "is_admin": Fa ADMIN = {"sub": "sub-admin", "email": "admin@x.com", "name": "Admin", "is_admin": True} +def test_current_user_admin_requires_verified_email(): + """is_admin is recomputed per request and requires a VERIFIED allowlisted email.""" + from types import SimpleNamespace + + cfg = SimpleNamespace(admin_emails=frozenset({"admin@x.com"})) + + def req(session_user): + return SimpleNamespace( + session={"user": session_user} if session_user else {}, + app=SimpleNamespace(state=SimpleNamespace(cfg=cfg)), + ) + + # verified + allowlisted -> admin + assert appmod.current_user(req({"email": "admin@x.com", "email_verified": True}))["is_admin"] is True + # allowlisted but NOT verified -> not admin (the security fix) + assert appmod.current_user(req({"email": "admin@x.com", "email_verified": False}))["is_admin"] is False + # missing email_verified claim -> not admin + assert appmod.current_user(req({"email": "admin@x.com"}))["is_admin"] is False + # verified but not on the allowlist -> not admin + assert appmod.current_user(req({"email": "user@x.com", "email_verified": True}))["is_admin"] is False + # no session user -> None + assert appmod.current_user(req(None)) is None + + @pytest.fixture def app_url(tmp_path, monkeypatch): monkeypatch.setenv("INTERVALS_ENC_KEY", KEY)