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) <noreply@anthropic.com>
This commit is contained in:
@@ -26,8 +26,19 @@ _TEMPLATES = Jinja2Templates(directory=str(Path(__file__).parent / "templates"))
|
|||||||
|
|
||||||
|
|
||||||
def current_user(request: Request) -> dict | None:
|
def current_user(request: Request) -> dict | None:
|
||||||
"""Session user dict ({sub,email,name,is_admin}) or None. Overridable in tests."""
|
"""Session user dict ({sub,email,name,email_verified,is_admin}) or None.
|
||||||
return request.session.get("user")
|
|
||||||
|
`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:
|
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 = 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.cfg = cfg
|
||||||
app.state.oauth = oauth
|
app.state.oauth = oauth
|
||||||
|
|
||||||
@@ -92,8 +110,10 @@ def create_app(config: Config | None = None) -> FastAPI:
|
|||||||
"sub": sub,
|
"sub": sub,
|
||||||
"email": email,
|
"email": email,
|
||||||
"name": name,
|
"name": name,
|
||||||
# Admin is an email allowlist now (Better Auth has no group claims).
|
# Store whether the IdP verified this email; admin (email allowlist,
|
||||||
"is_admin": bool(email) and email.lower() in cfg.admin_emails,
|
# 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")
|
return redirect("/account")
|
||||||
|
|
||||||
|
|||||||
@@ -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}
|
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
|
@pytest.fixture
|
||||||
def app_url(tmp_path, monkeypatch):
|
def app_url(tmp_path, monkeypatch):
|
||||||
monkeypatch.setenv("INTERVALS_ENC_KEY", KEY)
|
monkeypatch.setenv("INTERVALS_ENC_KEY", KEY)
|
||||||
|
|||||||
Reference in New Issue
Block a user