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
The logout() helper was computing a Cognito hosted /logout URL and
routing it through /oauth2/sign_out?rd=. In this deployment the app
client has no hosted /logout endpoint, so that layer could never fully
clear the Cognito session and the URL-building was dead code. It also
required NEXT_PUBLIC_OIDC_LOGOUT_URL + NEXT_PUBLIC_OIDC_CLIENT_ID at
build time, coupling the web bundle to identity-provider config.
Simplified to:
1. Revoke the JWT refresh token server-side (/auth/jwt/revoke).
2. Clear local JWT tokens.
3. Navigate top-level to the portal host derived from the current
hostname ("foss-<app>.<domain>" -> "foss.<domain>").
The portal host is outside ForwardAuth, so the user lands on the
landing page instead of being silently re-authed into the dashboard.
Re-auth still happens the next time the user clicks into a gated app,
which is the expected behavior while Cognito hosted /logout is absent.
Existing return-value contract is preserved: logout() returns true
when the browser is already navigating so UserDropdown /
LayoutDataProvider callers skip their own redirect.
fix(auth): land on portal after logout, drop unreachable Cognito hop
chore: Add session cookies test
made askii.ai default email domain
Removes Pricing / Changelog / Contact / Discord / Reddit entries from both desktop and mobile navbars, leaving only Docs + GitHub stars. Strips the Pages / Socials / Legal / Register columns from the footer, keeping just the logo, copyright, and the brand name. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
When a user lands on /login while already holding a bearer token, the page used to flash the splash and bounce through oauth2-proxy before ending up on the dashboard. Send them straight to / instead — the home route already routes authenticated users to /dashboard, so this short-circuits the unnecessary OIDC round-trip and keeps the login surface from being reachable post-login. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
use moneta instead of foss in logout
* fix: strip first subdomain for portal redirect on signout Replace hardcoded "moneta." prefix with an empty string so the regex strips just the leading subdomain (e.g. app.moneta.askii.ai → moneta.askii.ai) regardless of the deployment domain. Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * fix: use lookahead regex to safely strip leading subdomain Switch to /^[^.]+\.(?=[^.]*\.[^.]*\.)/ so the subdomain is only stripped when at least two dot-separated parts remain, preventing over-stripping on bare domains. Co-Authored-By: Claude Sonnet 4.6 <[email protected]> --------- Co-authored-by: Claude Sonnet 4.6 <[email protected]>
* feat: Add user to default workspace on signin * fix: deleted unwanted file * fix: fixed sso workspace * fix: apply suggestions from code review Co-authored-by: Usama Sadiq <[email protected]> --------- Co-authored-by: Usama Sadiq <[email protected]>
Regression-guard for SurfSense's architectural immunity to the cross-app stale-session-on-user-switch class of bug. SurfSense doesn't need an explicit Rule-2-style "compare upstream identity vs session identity → flush on mismatch" middleware (like Plane MODSetter#29, Outline #19, Penpot #18, Twenty #8 do) because `current_active_user` in app.users already resolves to the upstream identity (proxy_user) over the persisted session (jwt_user) whenever both are present. That precedence IS the contract. If a future refactor flips it (or removes the proxy_user check during a "simplify auth" pass) the stale-session bug class is silently re-introduced — and type-checks pass, so it would ship. These five tests pin the contract: 1. proxy_user wins when both proxy_user and jwt_user are present with different identities (the user-switch scenario) 2. Falls back to jwt_user when proxy_user is absent (header-absent is NOT a logout signal — internal calls, OPTIONS preflight, direct backend hits at 127.0.0.1 legitimately arrive without a proxy header) 3. Raises 401 when neither is present (sanity) 4. Same precedence for current_optional_user 5. current_optional_user returns None (does not raise) when neither is present Cross-app contract: awais786/sso-rules-moneta:openspec/specs/proxy-auth-middleware/spec.md SurfSense's architectural-immunity reasoning: awais786/sso-rules-moneta:surfsense-security.md Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Agent-Logs-Url: https://github.com/Pressingly/SurfSense/sessions/7cb36524-6c58-452f-8b60-9d6256a6caaa Co-authored-by: awais786 <[email protected]>
…n-user-switch test(auth): pin proxy_user > jwt_user precedence in current_active_user
…gister (#21) * fix(auth): move SMB auto-join from current_active_user to on_after_register `auto_join_smb_search_space` was called from `current_active_user`, FastAPI's per-request auth dependency. That dependency runs on every authenticated API request — both proxy-auth and JWT paths. Combined with `DELETE /searchspaces/{id}/members/{membership_id}` (`rbac_routes.py:705`) which hard-deletes the membership row with no tombstone (the SearchSpaceMembership model has no `removed_at` / `is_blocked` / `deleted_at` column), this made admin removal a no-op: 1. Owner Alice calls `DELETE /api/v1/searchspaces/42/members/77` to evict Editor Bob → row deleted, API returns 200. 2. Bob's next request (`GET /api/v1/searchspaces/42/documents`) hits `current_active_user` → triggers `auto_join_smb_search_space(bob.id)` → no membership row found → INSERT (Editor again). 3. Bob is back inside the workspace, can re-read every document, mint invite links, etc. He cannot be removed at all while his SSO sign-in still works at the IdP. Editor permissions include documents:create/read/update, chats:create/read/update, members:invite, connectors:create/update — significant write surface to silently restore. Fix: move the call to `on_after_register` so it fires exactly once per user, at user creation. Both registration paths land here: - Standard FastAPI Users register flow - ProxyAuthMiddleware (already invokes `on_after_register` after creating a user via header-trust — see proxy_auth.py:179-209) Trade-off documented in the comment: users who registered BEFORE the SMB workspace existed are not retroactively auto-joined. Operators can backfill with a one-time SQL INSERT against `search_space_memberships`. The previous design intentionally caught that case via per-request enforcement; the un-revokability cost was not worth the convenience. Refs: awais786/sso-rules surfsense-security.md §"Finding 1" Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * chore: fix ruff import ordering CI's ruff-check moved `from app.services.smb_auto_join import auto_join_smb_search_space` from between `app.config` and `app.db` to between `app.prompts.system_defaults` and `app.utils.refresh_tokens` — correct alphabetical position within the `app.*` block. Auto-fix from `ruff check --fix surfsense_backend/app/users.py`. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
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
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.
OLD diff – for comparison purposes