Files
Chris CoutinhoandClaude Opus 4.7 b875eaf069 fix(auth): address PR #758 round-7 medium/minor review
- Gate browser session creation on a successful refresh token. When the
  IdP returns no refresh token, SessionAuthBackend would silently reject
  every subsequent request and bounce the user back to /oauth/login in a
  loop. The callback now bails with a 400 + correlation ID + actionable
  hint about offline_access *before* writing browser_sessions or setting
  the cookie. Pinned by a new end-to-end unit test.
- Evict orphaned browser_sessions rows in SessionAuthBackend when the
  associated refresh token is gone, instead of letting them accumulate
  until TTL cleanup. Best-effort; deletion errors stay non-fatal.
- Demote identity-bearing logs in the Flow 2 OAuth callback (user_id,
  scopes, audience, expires_at) from INFO to DEBUG so they don't leak
  into multi-tenant log aggregation on every provision.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-03 14:21:12 +02:00

122 lines
4.8 KiB
Python

"""Session-based authentication backend for Starlette routes.
Provides browser-based authentication for admin UI routes, separate from
MCP's OAuth authentication flow.
"""
import logging
import os
from starlette.authentication import (
AuthCredentials,
AuthenticationBackend,
SimpleUser,
)
from starlette.requests import HTTPConnection
logger = logging.getLogger(__name__)
class SessionAuthBackend(AuthenticationBackend):
"""Authentication backend using signed session cookies.
For BasicAuth mode: Always authenticates as the configured user.
For OAuth mode: Checks for valid session cookie with stored refresh token.
Behavior note — silent invalidation on refresh-token TTL expiry:
The OAuth path requires *both* a live ``browser_sessions`` row and a
live ``refresh_tokens`` row for the resolved user. Logout deletes
both atomically, so a logged-out user always fails closed here.
However, if the refresh token expires by TTL (without an explicit
logout) the row is removed by ``get_refresh_token`` and the browser
session becomes unusable — the user simply gets redirected to
``/oauth/login``. This is intentional defense-in-depth: the
refresh-token check is what makes a leaked or stale browser cookie
unusable after revocation. Do not relax this without first removing
the cleanup invariant on logout (PR #758 round-4 review medium 2).
"""
def __init__(self, oauth_enabled: bool = False):
"""Initialize session authentication backend.
Args:
oauth_enabled: Whether OAuth mode is enabled
"""
self.oauth_enabled = oauth_enabled
async def authenticate(
self, conn: HTTPConnection
) -> tuple[AuthCredentials, SimpleUser] | None:
"""Authenticate the request based on session cookie or BasicAuth mode.
This backend is only applied to browser routes (/user/*) via a separate
Starlette app mount. FastMCP routes use their own OAuth Bearer token
authentication.
Args:
conn: HTTP connection
Returns:
Tuple of (credentials, user) if authenticated, None otherwise
"""
# BasicAuth mode: Always authenticated as the configured user
if not self.oauth_enabled:
username = os.getenv("NEXTCLOUD_USERNAME", "admin")
return AuthCredentials(["authenticated", "admin"]), SimpleUser(username)
# OAuth mode: opaque random session_id cookie -> user_id mapping.
# Replaces the prior `mcp_session=<user_id>` cookie pattern (issue
# #626 finding 2). The cookie value is no longer the user identity;
# we look it up server-side and reject unknown / expired sessions.
session_id = conn.cookies.get("mcp_session")
if not session_id:
logger.info("No session cookie found - redirecting to login")
return None
oauth_context = getattr(conn.app.state, "oauth_context", None)
if not oauth_context:
logger.warning("OAuth context not available in app state")
return None
storage = oauth_context.get("storage")
if not storage:
logger.warning("OAuth storage not available")
return None
try:
user_id = await storage.get_browser_session_user(session_id)
if not user_id:
logger.info(
"Browser session not found or expired (sid=%s…)", session_id[:8]
)
return None
# Defense-in-depth: only authenticate sessions for users that
# actually have a refresh token persisted. Logout deletes both,
# so an expired/revoked user state will fail closed here.
token_data = await storage.get_refresh_token(user_id)
if not token_data:
logger.warning(
"Session %s… has no refresh token for user %s; rejecting",
session_id[:8],
user_id,
)
# Proactively evict the orphan so the table doesn't accumulate
# rows that the auth check will keep rejecting until TTL
# cleanup (PR #758 round-7 minor).
try:
await storage.delete_browser_session(session_id)
except Exception as e:
logger.warning(
"Failed to delete orphaned browser session %s…: %s",
session_id[:8],
e,
)
return None
return AuthCredentials(["authenticated"]), SimpleUser(user_id)
except Exception as e:
logger.warning("Session validation error: %s", e)
return None