Conversation
This fork integrates SurfSense with mPass (AWS Cognito-based OIDC) via
oauth2-proxy as a centralized authentication gateway, replacing
SurfSense's local fastapi-users authentication. The changes span
backend middleware, frontend redirect logic, and tests guarding the
SSO contract.
Backend
- Add Starlette ProxyAuthMiddleware (app/middleware/proxy_auth.py)
that reads X-Auth-Request-Email injected by oauth2-proxy via
Traefik ForwardAuth, JIT-provisions a local user on first SSO
login, and injects request.state.proxy_user so the FastAPI
dependency tree sees a fully authenticated user without needing
a JWT cookie.
- Add proxy_login GET endpoint at /auth/jwt/proxy-login
(app/routes/auth_routes.py) that issues a JWT after the SSO
header is validated and delivers it to the frontend via
short-lived surfsense_sso_token + surfsense_sso_refresh_token
cookies, then 302-redirects to / where the home-route splash
completes the cookie handoff to localStorage.
- Local fastapi-users routes (POST /auth/jwt/login, /auth/register,
/auth/forgot-password, /auth/reset-password,
/auth/request-verify-token, /auth/verify) are NOT registered, so
no code path can authenticate or create accounts without going
through Cognito.
- Refresh token machinery (POST /auth/jwt/refresh, /auth/jwt/revoke,
/auth/jwt/logout-all) is preserved as the SSO logout / token
rotation surface.
Frontend
- app/(home)/page.tsx is a neutral splash that runs the cookie
handoff in useEffect and routes to /dashboard. The upstream
marketing JSX is removed so SSO users never flash the homepage.
- app/(home)/layout.tsx hides the navbar + footer on the splash
route so the splash is fully blank during the redirect dance.
- app/(home)/login/page.tsx and app/(home)/register/page.tsx fall
back to a splash + window.location.replace() to oauth2-proxy/
sign_in when isSSOAuth() is true. The original LocalLoginForm
and registration form code is preserved unchanged for non-SSO
deployments.
- lib/auth-utils.ts handleUnauthorized() redirects to oauth2-proxy/
sign_in (instead of the dead /login route) when an in-app API
call returns 401, completing the SSO loop without flashing the
local form.
- lib/auth-utils.ts logout() implements the 3-layer logout flow:
revoke refresh token -> clear localStorage -> redirect to
oauth2-proxy/sign_out -> Cognito/logout -> back to /.
Tests
- tests/unit/routes/test_proxy_login.py adds:
- TestProxyLoginRouteRegistration: positive guards that
/auth/jwt/proxy-login is registered, accepts GET, and
dispatches to the proxy_login function.
- TestProxyLogin: behaviour tests for the 401/302/JIT-provision/
inactive-user paths, with a corrected SQLAlchemy mock chain
that previously skipped result.unique() and silently masked
the new-user provisioning bug.
- TestLocalAuthRoutesAreNotRegistered: negative guard that
asserts none of the standard fastapi-users local-auth
endpoints exist on the FastAPI app, catching accidental
re-introduction during a future upstream sync.
Configuration
- Reads NEXT_PUBLIC_FASTAPI_BACKEND_AUTH_TYPE (defaults to "SSO"
via lib/env-config.ts) so the same fork can be deployed in
LOCAL or GOOGLE auth modes without code changes — only the
splash redirects fire when isSSOAuth() returns true.
Known issue (deferred): direct visits to /login or /register can
flash the original form for ~100-200ms before the SSO redirect
fires, due to a Suspense streaming or useGlobalLoadingEffect
interaction with the SSR pre-render. Cosmetic only — the normal
SSO flow does not route through these pages directly.
- Default AUTH_TYPE to SSO everywhere (config, .env.example, compose files). Both .env.example files warn not to change the value since LOCAL/GOOGLE backend routes are not registered in this fork. - Refactor proxy_login to read request.state.proxy_user (set by ProxyAuthMiddleware) instead of querying the database directly. All user provisioning (JIT creation, on_after_register side effects) is now owned by the middleware. proxy_login only issues the JWT and sets the cookie-handoff cookies. - Remove RuntimeError guard that crashed the app on AUTH_TYPE != SSO. The SSO contract is enforced by the middleware + missing backend routes + frontend isSSOAuth() conditionals + the negative test guard — a hard crash on startup is redundant and prevents graceful fallback. - Remove unused imports (SECRET, auth_backend, PasswordHelper, uuid, secrets) from app.py and auth_routes.py. - Rewrite test_proxy_login.py to match the refactored proxy_login that reads request.state.proxy_user instead of touching the DB.
The logout flow's logout_uri was set to window.location.origin (https://foss-research.local.moneta.dev) which is behind ForwardAuth. After Cognito cleared the session, the user would bounce back to Cognito login instead of seeing the landing page. Now reads NEXT_PUBLIC_LOGOUT_REDIRECT_URL from the container env (set in docker-compose.yml to the platform landing page). Falls back to window.location.origin for non-devstack deployments.
…nstance crash ProxyAuthMiddleware resolves the user in its own async session which closes before the route handler runs. The User object left on request.state.proxy_user is detached — any downstream handler that tries session.refresh(user) or accesses lazy-loaded relationships in a new session gets "Instance is not persistent within this Session". Fix: current_active_user() and current_optional_user() now call _refetch_proxy_user() which re-fetches the user by ID in a fresh session then expunges it cleanly. Every route handler gets a User object that can be safely merged into any session context. Adds ~1-2ms overhead per authenticated request (one SELECT by ID). Verified: zero InvalidRequestError in backend logs since the fix.
logout() sets window.location.href to the oauth2-proxy sign_out chain, but callers immediately overwrote it with "/" before the browser navigated, sending users back to the dashboard via ForwardAuth re-auth. Also replaced encodeURIComponent with single-encoding to match Plane's pattern — double-encoding caused Cognito to reject the logout_uri as unregistered, stranding users on the Cognito page.
…tch helper Move session re-attachment to the two endpoints that modify user data (update_current_user_me, complete_task) and remove the _refetch_proxy_user helper. Fixes DetachedInstanceError when SSO proxy users hit these routes.
Pushes to feat/mpass-proxy-auth only triggered docker-build.yml; backend-tests and code-quality were gated on main/dev and never ran against the fork. Retarget both to foss-main so fork PRs exercise the unit/integration/quality gates before merge.
Unit: ProxyAuthMiddleware calls result.unique().scalar_one_or_none(), but tests stubbed scalar_one_or_none one level off. The unmocked .unique() returned a fresh MagicMock, so scalar_one_or_none() on it returned MagicMock (not None), skipping the insert branch and cascading into a TypeError when (now - user.last_login) ran against a mock. Mock the full chain: result.unique.return_value.scalar_one_or_none. Integration: /auth/register and /auth/jwt/login were removed in the SSO refactor (96a9ed6), so the test bootstrap 404'd before any test ran. Replace password-based register+login with the production path: GET /auth/jwt/proxy-login with X-Auth-Request-Email header, read JWT from the surfsense_sso_token cookie. Same code path ProxyAuthMiddleware serves real oauth2-proxy traffic. Also collapses the duplicate _authenticate_test_user in test_stripe_page_purchases.py onto the shared helper.
…ABLED The SSO auth refactor accidentally dropped TEST_EMAIL from the imports even though it's still used by the webhook and reconciliation assertions to look up the test user's page limit in the DB. The create-checkout-session tests also relied on the process-level default STRIPE_PAGE_BUYING_ENABLED=TRUE. That's fine in CI (no .env loaded) but breaks locally when .env sets it to FALSE, returning 503. Monkeypatch it to True alongside the other Stripe config overrides so the tests are hermetic.
The dict(**kwargs) → {...} rewrite is a ruff unsafe-fix because in
general dict() accepts non-string keys that literals can't express.
Here every key is a string literal so the rewrite would be safe, but
the change adds no runtime value and touches a load-bearing SSO
cookie site. Noqa the one site instead.
Biome reported format drift on two files: lib/auth-utils.ts — inside the logout() SSO branch the landing-page redirect block (comment + logoutRedirect + cognitoUrl.searchParams.set) was one tab short of the surrounding if-body. Re-tabbed to align. Also normalised three regex .replace() arguments from single to double quotes to match biome's configured quote style. lib/env-config.ts — removed a stray blank line between BACKEND_URL and the ETL placeholder comment. No behaviour change.
Code Quality CI runs ruff --fix, ruff format, and biome check on every
PR and fails when the hook rewrites files. Apply the rewrites upfront:
Python (ruff):
- app/app.py — drop unused UserCreate import
- app/middleware/proxy_auth.py, app/routes/auth_routes.py,
tests/unit/routes/test_proxy_login.py, tests/utils/helpers.py —
wrap long log/assert lines to satisfy ruff format
Web (biome):
- app/(home)/login/{LocalLoginForm,page}.tsx,
app/(home)/register/page.tsx, lib/apis/base-api.service.ts,
lib/auth-utils.ts — quote style + whitespace
No behaviour change.
Mirror the Plane middleware behavior: when oauth2-proxy forwards a bare
username in X-Auth-Request-Email (cognito:username claim with no email),
synthesize {username}@{SMB_NAME}.com so provisioning can proceed. Fall
back to X-Auth-Request-User if the email header is empty.
Also set display_name to the email local part on user creation so the UI
has a readable label without the caller having to fill it in.
feat(auth): mPass SSO via oauth2-proxy ForwardAuth with cookie-handoff
Context ------- SurfSense's `logout()` used to assume a 3-layer SSO chain — revoke JWT, clear oauth2-proxy cookie, then hit Cognito's hosted `/logout`. When deployments don't expose that hosted endpoint (NEXT_PUBLIC_OIDC_LOGOUT_URL unset) the function silently returned false without clearing the proxy cookie, so the next request re-authed the user via oauth2-proxy's still-valid cookie and landed them back on the dashboard. Also found: NEXT_PUBLIC_LOGOUT_REDIRECT_URL / OIDC_LOGOUT_URL / OIDC_CLIENT_ID were neither build-time ARGs nor in the runtime substitution list, so they resolved to `undefined` in the client bundle regardless of container env. Changes ------- * lib/auth-utils.ts — always hit `/oauth2/sign_out`; only prepend the Cognito hop when OIDC_LOGOUT_URL is configured. Else-branch uses NEXT_PUBLIC_LOGOUT_REDIRECT_URL as the post-logout landing page. Also cleaned unresolved merge-conflict markers in JSDoc blocks. * Dockerfile — add ARG/ENV for the 3 auth vars. Default ARG values are empty (not placeholder tokens like `__NEXT_PUBLIC_X__`): tokens look truthy to terser, causing it to dead-code-eliminate the no-Cognito branch. Empty string lets terser drop the branch that isn't configured at build time, matching the actual deploy. * .dockerignore — exclude .env.local / .env.*.local so developer-specific NEXT_PUBLIC_* values don't leak into shared builds via Next.js auto-loading .env.local at build time.
new_chat, threads/:id/resume, and regenerate endpoints were using raw fetch() with a manually attached Bearer token. If the JWT expired mid- session, the request returned 401 with no redirect handling — the chat UI hung in a loading state until the user refreshed. Swap to authenticatedFetch so 401s trigger token refresh (and fall back to handleUnauthorized → oauth2-proxy sign-in) like the rest of the app. The getBearerToken() guards at each call site stay as a defensive early- return, but the Authorization header is dropped (the wrapper injects it).
fix(chat): route streaming API calls through authenticatedFetch
Fix/logout without cognito hosted
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Motivation and Context
FIX #
Screenshots
API Changes
Change Type
Testing Performed
Checklist