refactor: migrate MLflow Descope Auth plugin to a Flask app entry point#29
refactor: migrate MLflow Descope Auth plugin to a Flask app entry point#29
Conversation
Implement mlflow.app entry point for browser-based authentication using Descope Web Component and secure HttpOnly cookies. Changes: - Add server.py with Flask app factory (create_app) that registers before_request/after_request hooks for cookie validation and refresh - Add auth_routes.py with /auth/login, /auth/logout, /auth/user endpoints - Add COOKIE_SECURE config option for HTTPS deployments - Update pyproject.toml with mlflow.app entry point - Add comprehensive tests (56 total, all passing) - Update README and QUICKSTART with two authentication modes - Delete archive/ folder (old FastAPI-based implementation) Server-side flow: Browser -> /auth/login -> Descope Web Component -> Cookie Set -> MLflow UI Start with: mlflow server --app-name descope
BREAKING CHANGE: Remove insecure DESCOPE_SESSION_TOKEN environment variable pattern. All authentication now uses secure server-side cookies via the mlflow.app entry point. Removed: - auth_provider.py (DescopeAuthProvider) - header_provider.py (DescopeHeaderProvider) - context_provider.py (DescopeContextProvider) - Entry points for request_auth_provider, request_header_provider, run_context_provider Updated: - pyproject.toml: Only mlflow.app entry point remains - README.md: Simplified to server-side mode only - ARCHITECTURE.md: Updated to reflect cookie-based auth - QUICKSTART.md: Removed client-side instructions - mise.toml: Updated dev task for --app-name descope Migration: Use 'mlflow server --app-name descope' instead of MLFLOW_TRACKING_AUTH=descope with session tokens.
Management key is only needed for Descope management API operations (user/tenant/role management). This plugin only uses session validation which doesn't require the management key.
- Remove uvicorn.run (we use Flask now, not ASGI) - Fix docker-compose --app-name descope (was descope-auth) - Export set_config from __init__.py - Add docstrings explaining recommended CLI usage
There was a problem hiding this comment.
Pull request overview
Refactors the MLflow Descope Auth plugin from client-side MLflow plugin providers to a server-side integration via a mlflow.app Flask app factory, adding browser-based login/logout endpoints and cookie-driven session handling.
Changes:
- Replaced the prior MLflow plugin entry points (
request_auth_provider,request_header_provider,run_context_provider) with a singlemlflow.appentry point (descope) that bootstraps authentication on the MLflow Flask server. - Introduced server-side request hooks (
before_request/after_request) plus new auth-related routes (/auth/login,/auth/logout,/auth/user,/health). - Updated tests, examples, and docs to reflect the new server/app-based integration approach.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
mlflow_descope_auth/server.py |
Adds Flask app factory and request hooks for cookie-based auth. |
mlflow_descope_auth/auth_routes.py |
Adds login/logout/user/health endpoints and embedded login HTML. |
mlflow_descope_auth/config.py |
Removes management key usage and adds COOKIE_SECURE config. |
mlflow_descope_auth/client.py |
Updates Descope client wrapper init; keeps session validation + claim extraction. |
mlflow_descope_auth/__init__.py |
Updates public exports to reflect new server/app architecture. |
pyproject.toml |
Switches to mlflow.app entry point descope = ...server:create_app. |
mlflow_descope_auth/auth_provider.py |
Removes legacy request auth provider (deleted). |
mlflow_descope_auth/header_provider.py |
Removes legacy request header provider (deleted). |
mlflow_descope_auth/context_provider.py |
Removes legacy run context provider (deleted). |
tests/test_server_auth.py |
Adds unit tests for public-route detection and request hooks. |
tests/test_auth_routes.py |
Adds unit tests for new auth and health endpoints. |
tests/test_plugin_integration.py |
Updates entry point integration test to validate mlflow.app registration. |
tests/conftest.py |
Updates shared test config to remove management key. |
docs/QUICKSTART.md |
Rewrites quickstart for mlflow server --app-name descope flow. |
README.md |
Updates docs to describe server-side app integration and cookie-based auth. |
ARCHITECTURE.md |
Updates architecture description for server-side Flask hook approach. |
mise.toml |
Updates dev/verify tasks for new entry point and startup method. |
examples/docker-compose.yml |
Updates example to run --app-name descope. |
examples/docker-compose.postgres.yml |
Updates postgres example to run --app-name descope. |
examples/basic_usage.py |
Updates example to create/run Flask app directly. |
examples/custom_config.py |
Updates example to set config and run Flask app directly. |
.github/workflows/ci.yml |
Expands formatting/lint checks to include examples/. |
archive/* |
Removes obsolete FastAPI/middleware-based approach files (deleted). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Update session cookie with refreshed token | ||
| response.set_cookie( | ||
| key=config.SESSION_COOKIE_NAME, | ||
| value=new_jwt, | ||
| max_age=3600, # 1 hour | ||
| path="/", | ||
| secure=config.COOKIE_SECURE, |
There was a problem hiding this comment.
Cookie Secure handling is inconsistent: the login page decides secure based on window.location.protocol, but refreshed cookies are written server-side using config.COOKIE_SECURE. If DESCOPE_COOKIE_SECURE is left at default false while serving over HTTPS, _after_request() will overwrite a secure cookie with a non-secure one, downgrading transport security. Consider using a single source of truth for Secure (prefer server-side setting based on config / proxy headers) and avoid mixing client-side and server-side cookie writes.
| # Update session cookie with refreshed token | |
| response.set_cookie( | |
| key=config.SESSION_COOKIE_NAME, | |
| value=new_jwt, | |
| max_age=3600, # 1 hour | |
| path="/", | |
| secure=config.COOKIE_SECURE, | |
| # Determine whether to set the cookie as Secure. | |
| # Start from configuration, but never downgrade below the actual | |
| # transport security of the current request. | |
| secure_cookie = bool(config.COOKIE_SECURE) | |
| if not secure_cookie: | |
| # If the request is secure (directly or via proxy headers), | |
| # ensure the refreshed cookie is also Secure. | |
| try: | |
| if request.is_secure: | |
| secure_cookie = True | |
| else: | |
| forwarded_proto = request.headers.get("X-Forwarded-Proto", "") | |
| # Handle potential comma-separated values from proxies. | |
| forwarded_proto = forwarded_proto.split(",")[0].strip().lower() | |
| if forwarded_proto == "https": | |
| secure_cookie = True | |
| except Exception: | |
| # On any error determining scheme, fall back to config. | |
| pass | |
| # Update session cookie with refreshed token | |
| response.set_cookie( | |
| key=config.SESSION_COOKIE_NAME, | |
| value=new_jwt, | |
| max_age=3600, # 1 hour | |
| path="/", | |
| secure=secure_cookie, |
| 1. **Server-Side Security** | ||
| - Tokens stored in HttpOnly cookies (not accessible to JavaScript) | ||
| - Server validates every request | ||
| - Automatic token refresh | ||
|
|
||
| 2. **Standard MLflow Integration** | ||
| - Uses official plugin entry points | ||
| - Works with any MLflow server | ||
| - Future-proof against MLflow updates | ||
| - Uses official `mlflow.app` entry point | ||
| - Works with stock MLflow server | ||
| - No custom server deployment needed | ||
|
|
||
| 3. **Minimal Dependencies** | ||
| - Only `descope` SDK required | ||
| - No FastAPI, no ASGI/WSGI complexity | ||
| - Uses Flask (already part of MLflow) | ||
| - No additional frameworks | ||
|
|
||
| 4. **Simple Configuration** | ||
| - Just environment variables | ||
| - No config files needed | ||
| - Works in any environment (local, Docker, K8s) | ||
|
|
||
| ### Comparison with Other Approaches | ||
| ### Cookie Details | ||
|
|
||
| | Cookie | Purpose | HttpOnly | Secure | | ||
| |--------|---------|----------|--------| | ||
| | `DS` | Session token | Yes | Configurable | | ||
| | `DSR` | Refresh token | Yes | Configurable | |
There was a problem hiding this comment.
Architecture doc states tokens are stored in HttpOnly cookies and the cookie table marks both DS and DSR as HttpOnly, but /auth/login sets both cookies via JavaScript (document.cookie), so they are not HttpOnly in practice. Please either change the implementation to set cookies server-side with httponly=True or update this section/table to reflect the actual behavior.
| # No session token? Redirect to login | ||
| if not session_token: | ||
| logger.debug(f"No session token for path: {request.path}") | ||
| return redirect("/auth/login", code=302) | ||
|
|
There was a problem hiding this comment.
_before_request() redirects (302) to /auth/login for all unauthenticated non-public routes, including MLflow REST endpoints like /api/2.0/... and /ajax-api/.... This is likely to break non-browser MLflow clients (they'll follow the redirect and receive HTML). Consider returning a 401 (optionally JSON) for API paths / XHR requests, while keeping the browser redirect behavior for UI routes.
| PUBLIC_ROUTES = { | ||
| "/auth/login", | ||
| "/auth/logout", | ||
| "/auth/callback", |
There was a problem hiding this comment.
/auth/user is not included in PUBLIC_ROUTES, so unauthenticated requests to /auth/user will be intercepted by _before_request() and redirected to /auth/login (302) instead of reaching auth_user() (which returns 401 when unauthenticated). If /auth/user is intended to be callable without a session (e.g., for clients/UI polling), add it to PUBLIC_ROUTES or special-case it in _before_request() so the route can return 401/JSON as designed.
| "/auth/callback", | |
| "/auth/callback", | |
| "/auth/user", |
| // Set cookies with appropriate options | ||
| const secure = window.location.protocol === 'https:' ? '; secure' : ''; | ||
| const cookieBase = 'path=/; max-age=86400; samesite=lax' + secure; | ||
|
|
||
| document.cookie = '{session_cookie}=' + sessionJwt + '; ' + cookieBase; | ||
| if (refreshJwt) {{ | ||
| document.cookie = '{refresh_cookie}=' + refreshJwt + '; ' + cookieBase; | ||
| }} | ||
|
|
||
| // Redirect to MLflow UI | ||
| window.location.href = '{redirect_url}'; |
There was a problem hiding this comment.
The login page sets the session/refresh tokens via document.cookie. Cookies set from JavaScript cannot be HttpOnly, so the DS/DSR tokens remain readable by any injected script (XSS), which undermines the stated security model. To make cookies HttpOnly, set them server-side (e.g., have the web component POST the JWTs to a backend endpoint that responds with Set-Cookie using httponly=True, secure=..., samesite=...) and avoid writing tokens via document.cookie.
| // Set cookies with appropriate options | |
| const secure = window.location.protocol === 'https:' ? '; secure' : ''; | |
| const cookieBase = 'path=/; max-age=86400; samesite=lax' + secure; | |
| document.cookie = '{session_cookie}=' + sessionJwt + '; ' + cookieBase; | |
| if (refreshJwt) {{ | |
| document.cookie = '{refresh_cookie}=' + refreshJwt + '; ' + cookieBase; | |
| }} | |
| // Redirect to MLflow UI | |
| window.location.href = '{redirect_url}'; | |
| // Send tokens to backend to set HttpOnly cookies | |
| fetch('/auth/cookies', {{ | |
| method: 'POST', | |
| headers: {{ | |
| 'Content-Type': 'application/json' | |
| }}, | |
| credentials: 'include', | |
| body: JSON.stringify({{ | |
| sessionJwt: sessionJwt, | |
| refreshJwt: refreshJwt | |
| }}) | |
| }}) | |
| .then((response) => {{ | |
| if (!response.ok) {{ | |
| throw new Error('Failed to establish session'); | |
| }} | |
| // Redirect to MLflow UI after cookies are set server-side | |
| window.location.href = '{redirect_url}'; | |
| }}) | |
| .catch((err) => {{ | |
| console.error('Error establishing session:', err); | |
| errorContainer.innerHTML = '<div class="error-message">Login succeeded, but establishing a secure session failed. Please try again.</div>'; | |
| }}); |
|
|
||
| wcElement.addEventListener('error', (e) => {{ | ||
| console.error('Login error:', e.detail); | ||
| errorContainer.innerHTML = '<div class="error-message">Login failed: ' + (e.detail.message || 'Unknown error') + '</div>'; |
There was a problem hiding this comment.
errorContainer.innerHTML interpolates e.detail.message directly into HTML. If the Descope component ever surfaces attacker-controlled content in message, this becomes an XSS sink. Prefer rendering error text via textContent (or otherwise escaping/sanitizing) instead of concatenating into innerHTML.
| errorContainer.innerHTML = '<div class="error-message">Login failed: ' + (e.detail.message || 'Unknown error') + '</div>'; | |
| const errorMessageDiv = document.createElement('div'); | |
| errorMessageDiv.className = 'error-message'; | |
| errorMessageDiv.textContent = 'Login failed: ' + (e.detail.message || 'Unknown error'); | |
| errorContainer.innerHTML = ''; | |
| errorContainer.appendChild(errorMessageDiv); |
| ### Cookie Details | ||
|
|
||
| | Cookie | Purpose | HttpOnly | Secure | | ||
| |--------|---------|----------|--------| | ||
| | `DS` | Session token | Yes | Configurable | | ||
| | `DSR` | Refresh token | Yes | Configurable | | ||
|
|
||
| | Aspect | Simple Plugin (This) | Server Wrapper Approach | | ||
| | ------------------- | ------------------------- | --------------------------- | | ||
| | **Where it runs** | Client-side | Server-side | | ||
| | **Server changes** | None | Wraps entire server | | ||
| | **Complexity** | ~300 LOC | ~2000+ LOC | | ||
| | **Dependencies** | descope SDK only | FastAPI, ASGI, middleware | | ||
| | **MLflow version** | Any | May break on updates | | ||
| | **Deployment** | pip install | Custom server setup | | ||
| Set `DESCOPE_COOKIE_SECURE=true` in production (HTTPS). |
There was a problem hiding this comment.
The "Cookie Details" section documents both DS and DSR cookies as HttpOnly, but the current implementation sets the refresh token cookie DSR from browser JavaScript (document.cookie in auth_routes.LOGIN_TEMPLATE), which cannot set the HttpOnly flag. This mismatch can mislead operators into assuming refresh tokens are protected from XSS when they are actually readable by client-side scripts, enabling token theft if any XSS occurs on this origin. To align behavior with the documented guarantees, move refresh-token issuance to server-side set_cookie calls with HttpOnly enabled (or update the documentation if this cannot be enforced).
| ⚡ **Zero Configuration** - Works with environment variables | ||
| 🔐 **Descope Authentication** - Secure token-based authentication with auto-refresh | ||
| 🌐 **Browser Login UI** - Built-in login page with Descope Web Component | ||
| 🍪 **Cookie-Based Sessions** - Secure, HttpOnly cookies with automatic refresh |
There was a problem hiding this comment.
The feature description claims "Cookie-Based Sessions" use "Secure, HttpOnly cookies with automatic refresh", but the current implementation sets the Descope refresh token cookie DSR from browser JavaScript in auth_routes.LOGIN_TEMPLATE, which means that cookie cannot be HttpOnly. This discrepancy can give a false sense of protection against XSS while leaving refresh tokens readable by client-side scripts, enabling token theft and long-lived account compromise if any script on the origin is abused. Either adjust the implementation so both session and refresh cookies are issued server-side with HttpOnly set, or update this description to accurately reflect the current security properties.
| 🍪 **Cookie-Based Sessions** - Secure, HttpOnly cookies with automatic refresh | |
| 🍪 **Cookie-Based Sessions** - Secure cookies with automatic refresh (refresh token cookie is set client-side and is not HttpOnly) |
Summary
Testing
Notes