From c37cd5f692745baccbe8d5c99bdfed2bda6ee10c Mon Sep 17 00:00:00 2001 From: awais786 Date: Sat, 11 Apr 2026 22:19:25 +0500 Subject: [PATCH 01/29] feat(auth): mPass SSO via oauth2-proxy ForwardAuth with cookie-handoff MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .github/workflows/docker-build.yml | 11 +- .gitignore | 4 +- surfsense_backend/.env.example | 3 + surfsense_backend/app/app.py | 183 ++------ surfsense_backend/app/config/__init__.py | 32 +- surfsense_backend/app/middleware/__init__.py | 0 .../app/middleware/proxy_auth.py | 203 ++++++++ surfsense_backend/app/routes/auth_routes.py | 73 ++- surfsense_backend/app/users.py | 36 +- .../scripts/docker/entrypoint.sh | 0 .../tests/unit/middleware/test_proxy_auth.py | 440 ++++++++++++++++++ .../tests/unit/routes/__init__.py | 0 .../tests/unit/routes/test_proxy_login.py | 311 +++++++++++++ surfsense_web/.env.example | 5 + surfsense_web/Dockerfile | 2 + surfsense_web/app/(home)/layout.tsx | 7 +- .../app/(home)/login/GoogleLoginButton.tsx | 17 +- .../app/(home)/login/LocalLoginForm.tsx | 9 +- surfsense_web/app/(home)/login/page.tsx | 31 +- surfsense_web/app/(home)/page.tsx | 80 ++-- surfsense_web/app/(home)/register/page.tsx | 30 +- surfsense_web/app/api/zero/query/route.ts | 10 +- surfsense_web/app/auth/callback/loading.tsx | 11 - surfsense_web/app/auth/callback/page.tsx | 18 - surfsense_web/atoms/user/user-query.atoms.ts | 3 +- surfsense_web/components/TokenHandler.tsx | 148 +++--- .../components/assistant-ui/image.tsx | 2 +- .../components/auth/sign-in-button.tsx | 25 +- .../components/homepage/hero-section.tsx | 54 ++- surfsense_web/docker-entrypoint.js | 1 + surfsense_web/lib/apis/base-api.service.ts | 16 +- surfsense_web/lib/auth-utils.ts | 100 +++- surfsense_web/lib/env-config.ts | 12 +- 33 files changed, 1497 insertions(+), 380 deletions(-) create mode 100644 surfsense_backend/app/middleware/__init__.py create mode 100644 surfsense_backend/app/middleware/proxy_auth.py mode change 100644 => 100755 surfsense_backend/scripts/docker/entrypoint.sh create mode 100644 surfsense_backend/tests/unit/middleware/test_proxy_auth.py create mode 100644 surfsense_backend/tests/unit/routes/__init__.py create mode 100644 surfsense_backend/tests/unit/routes/test_proxy_login.py delete mode 100644 surfsense_web/app/auth/callback/loading.tsx delete mode 100644 surfsense_web/app/auth/callback/page.tsx diff --git a/.github/workflows/docker-build.yml b/.github/workflows/docker-build.yml index 8de55ba91..9b3b3eb51 100644 --- a/.github/workflows/docker-build.yml +++ b/.github/workflows/docker-build.yml @@ -5,9 +5,12 @@ on: branches: - main - dev + - foss-main + - feat/mpass-proxy-auth paths: - 'surfsense_backend/**' - 'surfsense_web/**' + - '.github/workflows/docker-build.yml' workflow_dispatch: inputs: branch: @@ -26,7 +29,11 @@ permissions: jobs: tag_release: runs-on: ubuntu-latest - if: github.ref == format('refs/heads/{0}', github.event.repository.default_branch) || github.event_name == 'workflow_dispatch' + if: > + github.ref == format('refs/heads/{0}', github.event.repository.default_branch) || + github.ref == 'refs/heads/foss-main' || + github.ref == 'refs/heads/feat/mpass-proxy-auth' || + github.event_name == 'workflow_dispatch' outputs: new_tag: ${{ steps.tag_version.outputs.next_version }} steps: @@ -248,7 +255,7 @@ jobs: type=ref,event=branch type=sha,prefix=git- flavor: | - latest=${{ github.ref == format('refs/heads/{0}', github.event.repository.default_branch) || github.event.inputs.branch == github.event.repository.default_branch }} + latest=${{ github.ref == format('refs/heads/{0}', github.event.repository.default_branch) || github.ref == 'refs/heads/feat/mpass-proxy-auth' || github.event.inputs.branch == github.event.repository.default_branch }} - name: Create manifest list and push working-directory: /tmp/digests diff --git a/.gitignore b/.gitignore index fd8fd782b..7a7681681 100644 --- a/.gitignore +++ b/.gitignore @@ -6,4 +6,6 @@ node_modules/ .venv .pnpm-store .DS_Store -deepagents/ \ No newline at end of file +deepagents/.env.local +.env.local +.idea/ diff --git a/surfsense_backend/.env.example b/surfsense_backend/.env.example index 8c8587cea..818cfca78 100644 --- a/surfsense_backend/.env.example +++ b/surfsense_backend/.env.example @@ -64,6 +64,9 @@ STRIPE_RECONCILIATION_BATCH_SIZE=100 # Auth AUTH_TYPE=GOOGLE or LOCAL REGISTRATION_ENABLED=TRUE or FALSE + +# mPass proxy auth bypass paths (oauth2-proxy ForwardAuth integration) +# MPASS_BYPASS_PATHS=/health # comma-separated, defaults to /health # For Google Auth Only GOOGLE_OAUTH_CLIENT_ID=924507538m GOOGLE_OAUTH_CLIENT_SECRET=GOCSV diff --git a/surfsense_backend/app/app.py b/surfsense_backend/app/app.py index 7b2b421ac..047852d81 100644 --- a/surfsense_backend/app/app.py +++ b/surfsense_backend/app/app.py @@ -32,11 +32,18 @@ initialize_vision_llm_router, ) from app.db import User, create_db_and_tables, get_async_session +from app.middleware.proxy_auth import ProxyAuthMiddleware from app.routes import router as crud_router from app.routes.auth_routes import router as auth_router from app.schemas import UserCreate, UserRead, UserUpdate from app.tasks.surfsense_docs_indexer import seed_surfsense_docs -from app.users import SECRET, auth_backend, current_active_user, fastapi_users +from app.users import ( + SECRET, + auth_backend, + current_active_user, + fastapi_users, + get_user_manager, +) from app.utils.perf import get_perf_logger, log_system_snapshot rate_limit_logger = logging.getLogger("surfsense.rate_limit") @@ -315,14 +322,28 @@ async def dispatch( app.add_middleware(RequestPerfMiddleware) -# Add SlowAPI middleware for automatic rate limiting +# Starlette executes middleware in reverse registration order (last added = first to +# run on the request). Request-path execution order: +# +# CORSMiddleware → ProxyHeadersMiddleware → SlowAPIMiddleware +# → ProxyAuthMiddleware → RequestPerfMiddleware → route handler +# +# SlowAPIMiddleware wraps ProxyAuthMiddleware so rate limiting fires before any DB +# lookup — abusive traffic is shed at the limiter before we touch the database. +# ProxyAuthMiddleware runs after ProxyHeadersMiddleware so the client IP/scheme +# are already normalised when we resolve the user. + +# Innermost: reads X-Auth-Request-Email, resolves/creates user, sets request.state.proxy_user. +app.add_middleware(ProxyAuthMiddleware) + +# Wraps ProxyAuthMiddleware — rate limiting fires before the DB lookup. # Uses Starlette BaseHTTPMiddleware (not the raw ASGI variant) to avoid # corrupting StreamingResponse — SlowAPIASGIMiddleware re-sends # http.response.start on every body chunk, breaking SSE/streaming endpoints. app.add_middleware(SlowAPIMiddleware) -# Add ProxyHeaders middleware FIRST to trust proxy headers (e.g., from Cloudflare) -# This ensures FastAPI uses HTTPS in redirects when behind a proxy +# Outermost of the inner three: trusts proxy headers (X-Forwarded-For etc.) +# so FastAPI uses HTTPS in redirects when behind Traefik. app.add_middleware(ProxyHeadersMiddleware, trusted_hosts="*") # Add CORS middleware @@ -362,149 +383,35 @@ async def dispatch( allow_headers=["*"], # Allows all headers ) -app.include_router( - fastapi_users.get_auth_router(auth_backend), - prefix="/auth/jwt", - tags=["auth"], - dependencies=[Depends(rate_limit_login)], -) -app.include_router( - fastapi_users.get_register_router(UserRead, UserCreate), - prefix="/auth", - tags=["auth"], - dependencies=[ - Depends(rate_limit_register), - Depends(registration_allowed), # blocks registration when disabled - ], -) -app.include_router( - fastapi_users.get_reset_password_router(), - prefix="/auth", - tags=["auth"], - dependencies=[Depends(rate_limit_password_reset)], -) -app.include_router( - fastapi_users.get_verify_router(UserRead), - prefix="/auth", - tags=["auth"], -) +# Register /users/me BEFORE fastapi_users.get_users_router so our routes take +# precedence (FastAPI first-match wins). fastapi-users' internal /users/me only +# validates JWT — it does not check request.state.proxy_user set by the proxy +# auth middleware, so proxy-auth users would always get 401 from that route. +@app.get("/users/me", response_model=UserRead, tags=["users"]) +async def get_current_user_me(user: User = Depends(current_active_user)): + return user + + +@app.patch("/users/me", response_model=UserRead, tags=["users"]) +async def update_current_user_me( + request: Request, + user_update: UserUpdate, + user: User = Depends(current_active_user), + user_manager=Depends(get_user_manager), +): + return await user_manager.update(user_update, user, safe=True, request=request) + + app.include_router( fastapi_users.get_users_router(UserRead, UserUpdate), prefix="/users", tags=["users"], ) + # Include custom auth routes (refresh token, logout) app.include_router(auth_router) -if config.AUTH_TYPE == "GOOGLE": - from fastapi.responses import RedirectResponse - - from app.users import google_oauth_client - - # Determine if we're in a secure context (HTTPS) or local development (HTTP) - # The CSRF cookie must have secure=False for HTTP (localhost development) - is_secure_context = config.BACKEND_URL and config.BACKEND_URL.startswith("https://") - - # For cross-origin OAuth (frontend and backend on different domains): - # - SameSite=None is required to allow cross-origin cookie setting - # - Secure=True is required when SameSite=None - # For same-origin or local development, use SameSite=Lax (default) - csrf_cookie_samesite = "none" if is_secure_context else "lax" - - # Extract the domain from BACKEND_URL for cookie domain setting - # This helps with cross-site cookie issues in Firefox/Safari - csrf_cookie_domain = None - if config.BACKEND_URL: - from urllib.parse import urlparse - - parsed_url = urlparse(config.BACKEND_URL) - csrf_cookie_domain = parsed_url.hostname - - app.include_router( - fastapi_users.get_oauth_router( - google_oauth_client, - auth_backend, - SECRET, - is_verified_by_default=True, - csrf_token_cookie_secure=is_secure_context, - csrf_token_cookie_samesite=csrf_cookie_samesite, - csrf_token_cookie_httponly=False, # Required for cross-site OAuth in Firefox/Safari - ) - if not config.BACKEND_URL - else fastapi_users.get_oauth_router( - google_oauth_client, - auth_backend, - SECRET, - is_verified_by_default=True, - redirect_url=f"{config.BACKEND_URL}/auth/google/callback", - csrf_token_cookie_secure=is_secure_context, - csrf_token_cookie_samesite=csrf_cookie_samesite, - csrf_token_cookie_httponly=False, # Required for cross-site OAuth in Firefox/Safari - csrf_token_cookie_domain=csrf_cookie_domain, # Explicitly set cookie domain - ), - prefix="/auth/google", - tags=["auth"], - dependencies=[ - Depends(registration_allowed) - ], # blocks OAuth registration when disabled - ) - - # Add a redirect-based authorize endpoint for Firefox/Safari compatibility - # This endpoint performs a server-side redirect instead of returning JSON - # which fixes cross-site cookie issues where browsers don't send cookies - # set via cross-origin fetch requests on subsequent redirects - @app.get("/auth/google/authorize-redirect", tags=["auth"]) - async def google_authorize_redirect( - request: Request, - ): - """ - Redirect-based OAuth authorization endpoint. - - Unlike the standard /auth/google/authorize endpoint that returns JSON, - this endpoint directly redirects the browser to Google's OAuth page. - This fixes CSRF cookie issues in Firefox and Safari where cookies set - via cross-origin fetch requests are not sent on subsequent redirects. - """ - import secrets - - from fastapi_users.router.oauth import generate_state_token - - # Generate CSRF token - csrf_token = secrets.token_urlsafe(32) - - # Build state token - state_data = {"csrftoken": csrf_token} - state = generate_state_token(state_data, SECRET, lifetime_seconds=3600) - - # Get the callback URL - if config.BACKEND_URL: - redirect_url = f"{config.BACKEND_URL}/auth/google/callback" - else: - redirect_url = str(request.url_for("oauth:google.jwt.callback")) - - # Get authorization URL from Google - authorization_url = await google_oauth_client.get_authorization_url( - redirect_url, - state, - scope=["openid", "email", "profile"], - ) - - # Create redirect response and set CSRF cookie - response = RedirectResponse(url=authorization_url, status_code=302) - response.set_cookie( - key="fastapiusersoauthcsrf", - value=csrf_token, - max_age=3600, - path="/", - domain=csrf_cookie_domain, - secure=is_secure_context, - httponly=False, # Required for cross-site OAuth in Firefox/Safari - samesite=csrf_cookie_samesite, - ) - - return response - app.include_router(crud_router, prefix="/api/v1", tags=["crud"]) diff --git a/surfsense_backend/app/config/__init__.py b/surfsense_backend/app/config/__init__.py index 575db4c7b..0d6858dee 100644 --- a/surfsense_backend/app/config/__init__.py +++ b/surfsense_backend/app/config/__init__.py @@ -311,6 +311,9 @@ def is_cloud(cls) -> bool: AUTH_TYPE = os.getenv("AUTH_TYPE") REGISTRATION_ENABLED = os.getenv("REGISTRATION_ENABLED", "TRUE").upper() == "TRUE" + # Comma-separated path prefixes that bypass proxy auth (default: /health). + MPASS_BYPASS_PATHS = os.getenv("MPASS_BYPASS_PATHS", None) + # Google OAuth GOOGLE_OAUTH_CLIENT_ID = os.getenv("GOOGLE_OAUTH_CLIENT_ID") GOOGLE_OAUTH_CLIENT_SECRET = os.getenv("GOOGLE_OAUTH_CLIENT_SECRET") @@ -415,17 +418,26 @@ def is_cloud(cls) -> bool: if AZURE_OPENAI_API_KEY: embedding_kwargs["azure_api_key"] = AZURE_OPENAI_API_KEY - embedding_model_instance = AutoEmbeddings.get_embeddings( - EMBEDDING_MODEL, - **embedding_kwargs, - ) + # mPass patch: defer embedding model loading to first use so the container + # starts without the PyTorch/sentence-transformers memory spike. + # Routes that use embeddings (search, indexing) will trigger lazy init on + # first request. Auth/SSO routes are unaffected. + _embedding_kwargs = embedding_kwargs + _embedding_model_instance = None + + @classmethod + def _get_embedding_model(cls): + if cls._embedding_model_instance is None: + cls._embedding_model_instance = AutoEmbeddings.get_embeddings( + cls.EMBEDDING_MODEL, + **cls._embedding_kwargs, + ) + return cls._embedding_model_instance + + embedding_model_instance = property(lambda self: self.__class__._get_embedding_model()) is_local_embedding_model = "://" not in (EMBEDDING_MODEL or "") - chunker_instance = RecursiveChunker( - chunk_size=getattr(embedding_model_instance, "max_seq_length", 512) - ) - code_chunker_instance = CodeChunker( - chunk_size=getattr(embedding_model_instance, "max_seq_length", 512) - ) + chunker_instance = RecursiveChunker(chunk_size=512) + code_chunker_instance = CodeChunker(chunk_size=512) # Reranker's Configuration | Pinecone, Cohere etc. Read more at https://github.com/AnswerDotAI/rerankers?tab=readme-ov-file#usage RERANKERS_ENABLED = os.getenv("RERANKERS_ENABLED", "FALSE").upper() == "TRUE" diff --git a/surfsense_backend/app/middleware/__init__.py b/surfsense_backend/app/middleware/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/surfsense_backend/app/middleware/proxy_auth.py b/surfsense_backend/app/middleware/proxy_auth.py new file mode 100644 index 000000000..523aebfcd --- /dev/null +++ b/surfsense_backend/app/middleware/proxy_auth.py @@ -0,0 +1,203 @@ +import logging +import secrets +import unicodedata +from datetime import UTC, datetime + +from fastapi_users.db import SQLAlchemyUserDatabase +from fastapi_users.password import PasswordHelper # singleton below +from sqlalchemy import select, update +from sqlalchemy.exc import IntegrityError +from starlette.middleware.base import BaseHTTPMiddleware, RequestResponseEndpoint +from starlette.requests import Request +from starlette.responses import Response + +from app.config import config +from app.db import User, async_session_maker + +logger = logging.getLogger(__name__) +_password_helper = PasswordHelper() + +_DEFAULT_BYPASS_PATHS = ["/health"] +_LAST_LOGIN_THROTTLE_SECONDS = 300 + + +def _normalise_email(email: str) -> str: + # NFKC normalisation collapses Unicode lookalikes before lowercasing, + # preventing homoglyph spoofing (e.g. fullwidth latin chars). + return unicodedata.normalize("NFKC", email).strip().lower() + + +def _coerce_bypass_paths(setting) -> list[str]: + if not setting: + return list(_DEFAULT_BYPASS_PATHS) + if isinstance(setting, str): + return [p.strip() for p in setting.split(",") if p.strip()] + return list(setting) + + +def _is_bypass_path(path: str, bypass_paths: list[str]) -> bool: + # Match exact path OR a true subpath (e.g. /health/ready) but NOT a path that + # merely starts with the same characters (e.g. /healthz must NOT bypass /health). + return any(path == p or path.startswith(p.rstrip("/") + "/") for p in bypass_paths) + + +class ProxyAuthMiddleware(BaseHTTPMiddleware): + """ + Starlette middleware for mPass proxy authentication. + + oauth2-proxy sets X-Auth-Request-Email on every request that has passed + OIDC validation. This middleware reads that header, finds or creates the + corresponding SurfSense user, and injects them into request.state.proxy_user + so the current_active_user dependency sees a fully authenticated user + without requiring a JWT token. + + Security / trust model + ---------------------- + This middleware trusts X-Auth-Request-Email unconditionally. That is safe + because: + 1. Traefik ForwardAuth overwrites X-Auth-Request-* headers on every + request, so they cannot be spoofed by a browser or external client. + 2. In production the app container does not expose its port externally — + only Traefik is public-facing, so there is no direct path to the app + that bypasses header rewriting. + + A shared-secret header (set by oauth2-proxy, forwarded via Traefik + authResponseHeaders, checked here) would add defense-in-depth against a + misconfigured ingress but is not required given the network topology above. + Add it if the threat model ever changes (e.g. the app port becomes reachable + inside a zero-trust network where internal callers could forge headers). + """ + + def __init__(self, app): + super().__init__(app) + self.bypass_paths = _coerce_bypass_paths( + getattr(config, "MPASS_BYPASS_PATHS", None) + ) + + async def dispatch( + self, request: Request, call_next: RequestResponseEndpoint + ) -> Response: + # Already injected on this request cycle (idempotent) + if getattr(request.state, "proxy_user", None) is not None: + return await call_next(request) + + if _is_bypass_path(request.url.path, self.bypass_paths): + return await call_next(request) + + raw_email = request.headers.get("x-auth-request-email") + if not raw_email: + logger.debug("ProxyAuth: x-auth-request-email missing on %s", request.url.path) + return await call_next(request) + + user = await self._resolve_user(_normalise_email(raw_email), request) + + # Respect deactivated accounts — mPass authentication does not + # override an explicit SurfSense account suspension. + if user is None or not user.is_active: + logger.warning("ProxyAuth: user inactive or not found for %r", raw_email) + return await call_next(request) + + logger.debug("ProxyAuth: injected user id=%s for %r", user.id, user.email) + request.state.proxy_user = user + return await call_next(request) + + async def _resolve_user(self, email: str, request: Request) -> User | None: + try: + async with async_session_maker() as session: + result = await session.execute(select(User).where(User.email == email)) + user = result.unique().scalar_one_or_none() + created = False + + if user is None: + hashed_password = _password_helper.hash(secrets.token_urlsafe(32)) + user = User( + email=email, + hashed_password=hashed_password, + is_active=True, + is_verified=True, + is_superuser=False, + ) + session.add(user) + try: + await session.commit() + await session.refresh(user) + created = True + except IntegrityError as exc: + # Concurrent request raced us to the insert — fall back + # to SELECT by email and re-raise if still not found. + await session.rollback() + result = await session.execute( + select(User).where(User.email == email) + ) + user = result.unique().scalar_one_or_none() + if user is None: + logger.error( + "ProxyAuth: IntegrityError but user still not found " + "for %s: %s", + email, + exc, + ) + return None + + # Update last_login at most once every 5 minutes per user. + # Unlike Plane (Django session — one DB write per login session), + # FastAPI has no server-side session so this middleware runs on + # every request. Writing last_login unconditionally would add an + # UPDATE + COMMIT to every API call; throttling keeps it cheap. + now = datetime.now(UTC) + needs_update = created or ( + user.last_login is None + or (now - user.last_login).total_seconds() + > _LAST_LOGIN_THROTTLE_SECONDS + ) + if needs_update: + try: + await session.execute( + update(User) + .where(User.id == user.id) + .values(last_login=now) + ) + await session.commit() + except Exception: + logger.warning( + "ProxyAuth: failed to update last_login for %s", email + ) + + if created: + # Trigger on_after_register so the default SearchSpace, + # RBAC roles and system prompts are created — same as + # Google OAuth and email/password signup. + # Use a fresh session so UserManager always has a clean connection. + # Re-fetch user in reg_session to avoid DetachedInstanceError — + # the user object from the outer session (or a rolled-back session + # after an IntegrityError race) must not be used across sessions. + try: + from app.users import UserManager + + async with async_session_maker() as reg_session: + reg_result = await reg_session.execute( + select(User).where(User.id == user.id) + ) + reg_user = reg_result.unique().scalar_one_or_none() + if reg_user is None: + raise RuntimeError( + f"ProxyAuth: user {user.id} vanished before on_after_register" + ) + + user_db = SQLAlchemyUserDatabase(reg_session, User) + user_manager = UserManager(user_db) + await user_manager.on_after_register( + reg_user, request=request + ) + except Exception: + logger.exception( + "ProxyAuth: on_after_register failed for %s — " + "user created but default search space may be missing", + email, + ) + + return user + + except Exception: + logger.exception("ProxyAuth: unexpected error resolving user for %s", email) + return None diff --git a/surfsense_backend/app/routes/auth_routes.py b/surfsense_backend/app/routes/auth_routes.py index b1cbaf2a5..4ab6bb525 100644 --- a/surfsense_backend/app/routes/auth_routes.py +++ b/surfsense_backend/app/routes/auth_routes.py @@ -1,10 +1,15 @@ """Authentication routes for refresh token management.""" import logging +import secrets +import uuid -from fastapi import APIRouter, Depends, HTTPException, status +from fastapi import APIRouter, Depends, HTTPException, Request, status +from fastapi.responses import RedirectResponse +from fastapi_users.password import PasswordHelper from sqlalchemy import select +from app.config import config from app.db import User, async_session_maker from app.schemas.auth import ( LogoutAllResponse, @@ -15,6 +20,7 @@ ) from app.users import current_active_user, get_jwt_strategy from app.utils.refresh_tokens import ( + create_refresh_token, revoke_all_user_tokens, revoke_refresh_token, rotate_refresh_token, @@ -26,6 +32,71 @@ router = APIRouter(prefix="/auth/jwt", tags=["auth"]) +@router.get("/proxy-login") +async def proxy_login(request: Request): + """ + Exchange X-Auth-Request-Email (injected by oauth2-proxy ForwardAuth via Traefik) + for a SurfSense JWT + refresh token delivered via short-lived cookies. + + Flow: + Browser → Traefik ForwardAuth → oauth2-proxy validates session + → sets X-Auth-Request-Email → this endpoint issues JWT + → sets surfsense_sso_token + surfsense_sso_refresh_token cookies (60s TTL) + → redirects to / → page.tsx reads cookies → stores to localStorage → /dashboard + """ + email = request.headers.get("x-auth-request-email") + if not email: + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="No proxy auth header — request did not pass through oauth2-proxy ForwardAuth", + ) + + email = email.strip().lower() + + async with async_session_maker() as session: + result = await session.execute(select(User).where(User.email == email)) + user = result.unique().scalar_one_or_none() + + if user is None: + # JIT provisioning — create the user on first SSO login. + # A random password is set (unused; auth is always via the SSO proxy). + logger.info("proxy_login: first SSO login for %s — provisioning user", email) + _ph = PasswordHelper() + user = User( + id=uuid.uuid4(), + email=email, + hashed_password=_ph.hash(secrets.token_urlsafe(32)), + is_active=True, + is_verified=True, + is_superuser=False, + ) + session.add(user) + await session.commit() + await session.refresh(user) + + if not user.is_active: + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="User account is inactive", + ) + + strategy = get_jwt_strategy() + access_token = await strategy.write_token(user) + refresh_token = await create_refresh_token(user.id) + + frontend_url = (config.NEXT_FRONTEND_URL or "http://localhost:3000").rstrip("/") + + # Deliver tokens via short-lived cookies so the frontend can pick them up at / + # without needing a dedicated /auth/callback route (avoids Traefik path splitting). + response = RedirectResponse(f"{frontend_url}/", status_code=302) + cookie_opts = dict(httponly=False, secure=True, samesite="lax", max_age=60) + response.set_cookie("surfsense_sso_token", access_token, **cookie_opts) + response.set_cookie("surfsense_sso_refresh_token", refresh_token, **cookie_opts) + + logger.info("proxy_login: issued JWT for %s → redirecting to frontend via cookie", email) + return response + + @router.post("/refresh", response_model=RefreshTokenResponse) async def refresh_access_token(request: RefreshTokenRequest): """ diff --git a/surfsense_backend/app/users.py b/surfsense_backend/app/users.py index 66e0cc8dd..e35cb9991 100644 --- a/surfsense_backend/app/users.py +++ b/surfsense_backend/app/users.py @@ -3,7 +3,7 @@ from datetime import UTC, datetime import httpx -from fastapi import Depends, Request, Response +from fastapi import Depends, HTTPException, Request, Response, status from fastapi.responses import JSONResponse, RedirectResponse from fastapi_users import BaseUserManager, FastAPIUsers, UUIDIDMixin, models from fastapi_users.authentication import ( @@ -298,5 +298,35 @@ async def get_login_response(self, token: str) -> Response: fastapi_users = FastAPIUsers[User, uuid.UUID](get_user_manager, [auth_backend]) -current_active_user = fastapi_users.current_user(active=True) -current_optional_user = fastapi_users.current_user(active=True, optional=True) +_jwt_current_optional_user = fastapi_users.current_user(active=True, optional=True) + + +async def current_active_user( + request: Request, + jwt_user: User | None = Depends(_jwt_current_optional_user), +) -> User: + """ + Returns the authenticated user. + + Checks request.state.proxy_user first (set by ProxyAuthMiddleware when + mPass proxy auth is active). Falls back to JWT Bearer token validation + so existing email/password and Google OAuth flows continue to work when + proxy auth is disabled. + """ + proxy_user = getattr(request.state, "proxy_user", None) + if proxy_user is not None: + return proxy_user + if jwt_user is not None: + return jwt_user + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Not authenticated", + ) + + +async def current_optional_user( + request: Request, + jwt_user: User | None = Depends(_jwt_current_optional_user), +) -> User | None: + proxy_user = getattr(request.state, "proxy_user", None) + return proxy_user if proxy_user is not None else jwt_user diff --git a/surfsense_backend/scripts/docker/entrypoint.sh b/surfsense_backend/scripts/docker/entrypoint.sh old mode 100644 new mode 100755 diff --git a/surfsense_backend/tests/unit/middleware/test_proxy_auth.py b/surfsense_backend/tests/unit/middleware/test_proxy_auth.py new file mode 100644 index 000000000..6c91a444d --- /dev/null +++ b/surfsense_backend/tests/unit/middleware/test_proxy_auth.py @@ -0,0 +1,440 @@ +""" +Unit tests for app.middleware.proxy_auth. + +SPEC (GIVEN / WHEN / THEN) +────────────────────────── + 1 GIVEN request.state.proxy_user is already set + WHEN request arrives + THEN call_next is called immediately with no DB access + + 2 GIVEN request path is a bypass path + WHEN request arrives with email header + THEN call_next is called with no DB access, proxy_user not set + + 3 GIVEN no X-Auth-Request-Email header + WHEN request arrives + THEN call_next is called and proxy_user not set + + 4 GIVEN email is not in DB (first seen) + WHEN request arrives with email header + THEN user is inserted, on_after_register is called, proxy_user set + + 5 GIVEN email already in DB + WHEN request arrives with email header + THEN existing user found, no INSERT, proxy_user set + + 6 GIVEN user.is_active is False + WHEN request arrives with email header + THEN proxy_user not set (pass through unauthenticated) + + 7 GIVEN valid email header and active user + WHEN middleware runs + THEN request.state.proxy_user == resolved user + + 8 GIVEN email header with uppercase and leading/trailing whitespace + WHEN request arrives + THEN _normalise_email is called with raw value and normalised email is used + + 9 GIVEN concurrent INSERT raises IntegrityError + WHEN commit raises IntegrityError + THEN fallback SELECT finds user, proxy_user set, no crash +""" + +from __future__ import annotations + +import uuid +from datetime import UTC, datetime +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest +from sqlalchemy.exc import IntegrityError +from starlette.requests import Request +from starlette.responses import Response + +from app.middleware.proxy_auth import ( + ProxyAuthMiddleware, + _coerce_bypass_paths, + _is_bypass_path, + _normalise_email, +) + +pytestmark = pytest.mark.unit + +_EMAIL = "alice@example.com" + + +# ── shared test helpers ──────────────────────────────────────────────────────── + + +def _make_request( + path: str = "/api/data", + headers: dict[str, str] | None = None, +) -> Request: + raw_headers = [(k.lower().encode(), v.encode()) for k, v in (headers or {}).items()] + scope = { + "type": "http", + "method": "GET", + "path": path, + "headers": raw_headers, + "query_string": b"", + "root_path": "", + } + return Request(scope) + + +def _make_user( + email: str = _EMAIL, + is_active: bool = True, + last_login: datetime | None = None, +) -> MagicMock: + user = MagicMock() + user.id = uuid.uuid4() + user.email = email + user.is_active = is_active + user.last_login = last_login + return user + + +def _make_middleware( + bypass_paths: list[str] | None = None, +) -> ProxyAuthMiddleware: + """Instantiate ProxyAuthMiddleware without calling __init__ (skips config + ASGI setup).""" + mw = object.__new__(ProxyAuthMiddleware) + mw.bypass_paths = bypass_paths if bypass_paths is not None else ["/health"] + return mw + + +def _make_session_cm(session: AsyncMock) -> MagicMock: + """Wrap an AsyncMock session in an async context manager mock.""" + cm = MagicMock() + cm.__aenter__ = AsyncMock(return_value=session) + cm.__aexit__ = AsyncMock(return_value=False) + return cm + + +async def _ok_call_next(request: Request) -> Response: + return Response("OK") + + +# ── _normalise_email ─────────────────────────────────────────────────────────── + + +class TestNormaliseEmail: + def test_lowercases(self): + assert _normalise_email("ALICE@Example.COM") == "alice@example.com" + + def test_strips_leading_trailing_whitespace(self): + assert _normalise_email(" alice@example.com ") == "alice@example.com" + + def test_nfkc_fullwidth_chars(self): + # Fullwidth 'a' (U+FF41) → ASCII 'a' + assert _normalise_email("\uff41lice@example.com") == "alice@example.com" + + def test_combined(self): + assert _normalise_email(" ALICE@EXAMPLE.COM ") == "alice@example.com" + + +# ── _is_bypass_path ──────────────────────────────────────────────────────────── + + +class TestIsBypassPath: + def test_exact_match(self): + assert _is_bypass_path("/health", ["/health"]) + + def test_sub_path(self): + assert _is_bypass_path("/health/ready", ["/health"]) + + def test_no_prefix_collision(self): + # /healthz must NOT match bypass prefix /health + assert not _is_bypass_path("/healthz", ["/health"]) + + def test_unrelated_path(self): + assert not _is_bypass_path("/api/users", ["/health"]) + + def test_multiple_bypass_paths(self): + assert _is_bypass_path("/docs", ["/health", "/docs"]) + + def test_root_bypass_covers_subpaths(self): + assert _is_bypass_path("/docs/intro", ["/health", "/docs"]) + + +# ── _coerce_bypass_paths ─────────────────────────────────────────────────────── + + +class TestCoerceBypassPaths: + def test_none_returns_default(self): + assert _coerce_bypass_paths(None) == ["/health"] + + def test_empty_string_returns_default(self): + assert _coerce_bypass_paths("") == ["/health"] + + def test_comma_separated_string(self): + assert _coerce_bypass_paths("/health,/docs") == ["/health", "/docs"] + + def test_list_passthrough(self): + assert _coerce_bypass_paths(["/health", "/metrics"]) == [ + "/health", + "/metrics", + ] + + def test_strips_spaces_from_csv(self): + assert _coerce_bypass_paths(" /health , /docs ") == ["/health", "/docs"] + + +# ── ProxyAuthMiddleware.dispatch ─────────────────────────────────────────────── + + +class TestProxyAuthMiddlewareDispatch: + # ── SPEC 1: already authenticated ───────────────────────────────────────── + + async def test_already_authenticated_skips_db(self): + """ + GIVEN request.state.proxy_user is already set + WHEN request arrives + THEN call_next is called immediately, DB never touched, original user preserved + """ + mw = _make_middleware() + existing = _make_user() + request = _make_request(headers={"x-auth-request-email": _EMAIL}) + request.state.proxy_user = existing + call_next = AsyncMock(return_value=Response("OK")) + + with patch("app.middleware.proxy_auth.async_session_maker") as mock_sm: + await mw.dispatch(request, call_next) + + mock_sm.assert_not_called() + assert request.state.proxy_user is existing + + # ── SPEC 2: bypass path ──────────────────────────────────────────────────── + + async def test_bypass_path_skips_db_and_leaves_user_unset(self): + """ + GIVEN path is a configured bypass path (/health) + WHEN request arrives with email header + THEN call_next called, DB never touched, proxy_user not set + """ + mw = _make_middleware(bypass_paths=["/health"]) + request = _make_request( + path="/health", + headers={"x-auth-request-email": _EMAIL}, + ) + call_next = AsyncMock(return_value=Response("OK")) + + with patch("app.middleware.proxy_auth.async_session_maker") as mock_sm: + await mw.dispatch(request, call_next) + + mock_sm.assert_not_called() + assert getattr(request.state, "proxy_user", None) is None + + # ── SPEC 3: no email header ──────────────────────────────────────────────── + + async def test_no_email_header_passes_through_unauthenticated(self): + """ + GIVEN no X-Auth-Request-Email header + WHEN request arrives + THEN call_next called, DB never touched, proxy_user not set + """ + mw = _make_middleware() + request = _make_request() # no email header + call_next = AsyncMock(return_value=Response("OK")) + + with patch("app.middleware.proxy_auth.async_session_maker") as mock_sm: + await mw.dispatch(request, call_next) + + mock_sm.assert_not_called() + assert getattr(request.state, "proxy_user", None) is None + + # ── SPEC 4: new user (first seen) ───────────────────────────────────────── + + async def test_new_user_created_and_on_after_register_called(self): + """ + GIVEN email not in DB + WHEN request arrives with email header + THEN User is INSERTed, on_after_register is called, proxy_user set + """ + mw = _make_middleware() + request = _make_request(headers={"x-auth-request-email": _EMAIL}) + + # Main session: SELECT → None (user absent), then UPDATE last_login + s1 = AsyncMock() + s1.add = MagicMock() # add() is sync; avoid AsyncMock warning + no_user_result = MagicMock() + no_user_result.scalar_one_or_none.return_value = None + update_result = MagicMock() + s1.execute = AsyncMock(side_effect=[no_user_result, update_result]) + s1_cm = _make_session_cm(s1) + + # Registration session: re-fetch user by id for on_after_register + reg_user = _make_user(email=_EMAIL) + s2 = AsyncMock() + reg_result = MagicMock() + reg_result.scalar_one_or_none.return_value = reg_user + s2.execute = AsyncMock(return_value=reg_result) + s2_cm = _make_session_cm(s2) + + mock_on_after_register = AsyncMock() + mock_manager = MagicMock() + mock_manager.on_after_register = mock_on_after_register + + with ( + patch( + "app.middleware.proxy_auth.async_session_maker", + side_effect=[s1_cm, s2_cm], + ), + patch("app.users.UserManager", return_value=mock_manager), + patch("app.middleware.proxy_auth.SQLAlchemyUserDatabase"), + ): + await mw.dispatch(request, _ok_call_next) + + s1.add.assert_called_once() + s1.commit.assert_called() + mock_on_after_register.assert_called_once() + assert getattr(request.state, "proxy_user", None) is not None + + # ── SPEC 5: existing user ────────────────────────────────────────────────── + + async def test_existing_user_found_no_insert(self): + """ + GIVEN email already in DB + WHEN request arrives with email header + THEN user found by SELECT, no INSERT (session.add not called), proxy_user set + """ + mw = _make_middleware() + existing = _make_user(email=_EMAIL, last_login=None) + request = _make_request(headers={"x-auth-request-email": _EMAIL}) + + session = AsyncMock() + found_result = MagicMock() + found_result.scalar_one_or_none.return_value = existing + update_result = MagicMock() + session.execute = AsyncMock(side_effect=[found_result, update_result]) + session_cm = _make_session_cm(session) + + with patch( + "app.middleware.proxy_auth.async_session_maker", return_value=session_cm + ): + await mw.dispatch(request, _ok_call_next) + + session.add.assert_not_called() + assert request.state.proxy_user is existing + + # ── SPEC 6: inactive user ────────────────────────────────────────────────── + + async def test_inactive_user_passes_through_unauthenticated(self): + """ + GIVEN user exists but is_active=False + WHEN request arrives with email header + THEN proxy_user not set (inactive user is not injected) + """ + mw = _make_middleware() + inactive = _make_user(email=_EMAIL, is_active=False) + request = _make_request(headers={"x-auth-request-email": _EMAIL}) + + session = AsyncMock() + found_result = MagicMock() + found_result.scalar_one_or_none.return_value = inactive + session.execute = AsyncMock(return_value=found_result) + session_cm = _make_session_cm(session) + + with patch( + "app.middleware.proxy_auth.async_session_maker", return_value=session_cm + ): + await mw.dispatch(request, _ok_call_next) + + assert getattr(request.state, "proxy_user", None) is None + + # ── SPEC 7: proxy_user set ───────────────────────────────────────────────── + + async def test_valid_email_sets_proxy_user_to_resolved_user(self): + """ + GIVEN valid email header and active user in DB + WHEN middleware runs + THEN request.state.proxy_user is the resolved user object + """ + mw = _make_middleware() + user = _make_user(email=_EMAIL, last_login=datetime.now(UTC)) + request = _make_request(headers={"x-auth-request-email": _EMAIL}) + + session = AsyncMock() + found_result = MagicMock() + found_result.scalar_one_or_none.return_value = user + # last_login is recent → needs_update=False → only one execute call + session.execute = AsyncMock(return_value=found_result) + session_cm = _make_session_cm(session) + + with patch( + "app.middleware.proxy_auth.async_session_maker", return_value=session_cm + ): + await mw.dispatch(request, _ok_call_next) + + assert request.state.proxy_user is user + + # ── SPEC 8: email normalisation ──────────────────────────────────────────── + + async def test_email_with_uppercase_and_whitespace_is_normalised(self): + """ + GIVEN email header value " ALICE@EXAMPLE.COM " + WHEN request arrives + THEN _normalise_email is called with the raw value and the lookup uses + the normalised form + """ + mw = _make_middleware() + raw_email = " ALICE@EXAMPLE.COM " + normalised = "alice@example.com" + user = _make_user(email=normalised, last_login=datetime.now(UTC)) + request = _make_request(headers={"x-auth-request-email": raw_email}) + + session = AsyncMock() + found_result = MagicMock() + found_result.scalar_one_or_none.return_value = user + session.execute = AsyncMock(return_value=found_result) + session_cm = _make_session_cm(session) + + with ( + patch( + "app.middleware.proxy_auth.async_session_maker", return_value=session_cm + ), + patch( + "app.middleware.proxy_auth._normalise_email", + wraps=_normalise_email, + ) as mock_norm, + ): + await mw.dispatch(request, _ok_call_next) + + mock_norm.assert_called_once_with(raw_email) + assert request.state.proxy_user is user + + # ── SPEC 9: race condition / IntegrityError ────────────────────────────── + + async def test_race_condition_fallback_select_sets_proxy_user(self): + """ + GIVEN two concurrent requests for the same new email + WHEN INSERT raises IntegrityError (the other request won the race) + THEN fallback SELECT finds the user committed by the winner, proxy_user set + """ + mw = _make_middleware() + # race_user was committed by the concurrent winner; it has a recent last_login + # so the throttle check (needs_update) is False, avoiding a third execute call. + race_user = _make_user(email=_EMAIL, last_login=datetime.now(UTC)) + request = _make_request(headers={"x-auth-request-email": _EMAIL}) + + session = AsyncMock() + session.add = MagicMock() # add() is sync; avoid AsyncMock warning + # Call 1: initial SELECT — no user found + no_user_result = MagicMock() + no_user_result.scalar_one_or_none.return_value = None + # Call 2: fallback SELECT after rollback — race_user found + fallback_result = MagicMock() + fallback_result.scalar_one_or_none.return_value = race_user + session.execute = AsyncMock(side_effect=[no_user_result, fallback_result]) + # INSERT commit raises IntegrityError; any subsequent commits should succeed + session.commit = AsyncMock(side_effect=[IntegrityError(None, None, None), None]) + session_cm = _make_session_cm(session) + + with patch( + "app.middleware.proxy_auth.async_session_maker", return_value=session_cm + ): + await mw.dispatch(request, _ok_call_next) + + session.rollback.assert_called_once() + assert request.state.proxy_user is race_user diff --git a/surfsense_backend/tests/unit/routes/__init__.py b/surfsense_backend/tests/unit/routes/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/surfsense_backend/tests/unit/routes/test_proxy_login.py b/surfsense_backend/tests/unit/routes/test_proxy_login.py new file mode 100644 index 000000000..f1ce02ab6 --- /dev/null +++ b/surfsense_backend/tests/unit/routes/test_proxy_login.py @@ -0,0 +1,311 @@ +""" +Unit tests for the proxy_login endpoint. + +SPEC (GIVEN / WHEN / THEN) +────────────────────────── + 1 GIVEN no X-Auth-Request-Email header + WHEN GET /auth/jwt/proxy-login is called + THEN 401 Unauthorized is returned + + 2 GIVEN email header present AND user already in DB AND user is active + WHEN GET /auth/jwt/proxy-login is called + THEN 302 redirect to frontend / with surfsense_sso_token and + surfsense_sso_refresh_token cookies set + + 3 GIVEN email header present AND user NOT in DB + WHEN GET /auth/jwt/proxy-login is called + THEN user is JIT-provisioned, 302 redirect with cookies set + + 4 GIVEN email header present AND user is inactive + WHEN GET /auth/jwt/proxy-login is called + THEN 401 Unauthorized is returned + + 5 GIVEN email header with mixed-case and whitespace + WHEN GET /auth/jwt/proxy-login is called + THEN lookup uses lowercased/stripped email (case-insensitive match) +""" + +from __future__ import annotations + +import uuid +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest +from starlette.requests import Request +from starlette.testclient import TestClient + +pytestmark = pytest.mark.unit + +_EMAIL = "alice@example.com" +_FRONTEND_URL = "https://foss-research.local.moneta.dev" + +# ── helpers ──────────────────────────────────────────────────────────────────── + + +def _make_request( + path: str = "/auth/jwt/proxy-login", + headers: dict[str, str] | None = None, +) -> Request: + raw_headers = [(k.lower().encode(), v.encode()) for k, v in (headers or {}).items()] + scope = { + "type": "http", + "method": "GET", + "path": path, + "headers": raw_headers, + "query_string": b"", + "root_path": "", + "server": ("localhost", 80), + } + return Request(scope) + + +def _make_user( + email: str = _EMAIL, + is_active: bool = True, +) -> MagicMock: + user = MagicMock() + user.id = uuid.uuid4() + user.email = email + user.is_active = is_active + return user + + +def _make_session_cm(session: AsyncMock) -> MagicMock: + cm = MagicMock() + cm.__aenter__ = AsyncMock(return_value=session) + cm.__aexit__ = AsyncMock(return_value=False) + return cm + + +def _make_execute_result(user): + """ + Mock the SQLAlchemy result chain used by proxy_login: + result = await session.execute(...) + user = result.unique().scalar_one_or_none() + + The intermediate `.unique()` call is easy to miss when mocking — without it, + the test gets a stray MagicMock back instead of `user` (or `None`), which + silently masks bugs in the new-user provisioning path. + """ + result = MagicMock() + unique = MagicMock() + unique.scalar_one_or_none.return_value = user + result.unique.return_value = unique + return result + + +# ── tests ────────────────────────────────────────────────────────────────────── + + +@pytest.mark.unit +class TestProxyLoginRouteRegistration: + """ + Regression guard for the route being missing from the running image. + + These tests don't exercise behaviour — they assert that the router exposes + the endpoint at the expected path/method. They would have failed loudly when + the baked Docker image lacked the proxy_login function entirely (the bug + that caused the cookie-handoff loop in the devstack on 2026-04-11). + """ + + def test_proxy_login_route_is_registered(self): + """GIVEN the auth router is imported THEN /auth/jwt/proxy-login is one of its routes.""" + from app.routes.auth_routes import router + + paths = [route.path for route in router.routes] + assert "/auth/jwt/proxy-login" in paths, ( + f"proxy-login route missing from auth router. Registered paths: {paths}" + ) + + def test_proxy_login_route_uses_get_method(self): + """GIVEN the route is registered THEN it accepts GET (browser navigation).""" + from app.routes.auth_routes import router + + matching = [r for r in router.routes if r.path == "/auth/jwt/proxy-login"] + assert len(matching) == 1, f"expected exactly one proxy-login route, found {len(matching)}" + assert "GET" in matching[0].methods, ( + f"proxy-login must accept GET (302 cookie handoff). Methods: {matching[0].methods}" + ) + + def test_proxy_login_route_calls_proxy_login_function(self): + """GIVEN the route is registered THEN it dispatches to the proxy_login function.""" + from app.routes.auth_routes import proxy_login, router + + matching = [r for r in router.routes if r.path == "/auth/jwt/proxy-login"] + assert matching[0].endpoint is proxy_login, ( + "route is registered but points to a different function" + ) + + +@pytest.mark.unit +class TestLocalAuthRoutesAreNotRegistered: + """In SSO mode, the local login/register/forgot-password routes must not exist.""" + + def test_local_auth_routes_are_not_registered(self): + from app.app import app + + registered = { + (method, route.path) + for route in app.routes + if hasattr(route, "methods") and hasattr(route, "path") + for method in (route.methods or ()) + } + + forbidden = { + ("POST", "/auth/jwt/login"), + ("POST", "/auth/register"), + ("POST", "/auth/forgot-password"), + ("POST", "/auth/reset-password"), + ("POST", "/auth/request-verify-token"), + ("POST", "/auth/verify"), + } + + present = forbidden & registered + assert not present, ( + f"SSO contract violated — these local-auth routes are registered: {sorted(present)}" + ) + + +@pytest.mark.unit +class TestProxyLogin: + + @pytest.mark.asyncio + async def test_no_email_header_returns_401(self): + """GIVEN no X-Auth-Request-Email header THEN 401.""" + from app.routes.auth_routes import proxy_login + + request = _make_request() # no email header + + from fastapi import HTTPException + with pytest.raises(HTTPException) as exc_info: + await proxy_login(request) + + assert exc_info.value.status_code == 401 + + @pytest.mark.asyncio + async def test_existing_active_user_gets_redirect_with_cookies(self): + """GIVEN existing active user THEN 302 + SSO cookies set.""" + from app.routes.auth_routes import proxy_login + + request = _make_request(headers={"x-auth-request-email": _EMAIL}) + user = _make_user(email=_EMAIL, is_active=True) + + session = AsyncMock() + session.execute = AsyncMock(return_value=_make_execute_result(user)) + session_cm = _make_session_cm(session) + + mock_strategy = AsyncMock() + mock_strategy.write_token = AsyncMock(return_value="mock-access-token") + + with ( + patch("app.routes.auth_routes.async_session_maker", return_value=session_cm), + patch("app.routes.auth_routes.get_jwt_strategy", return_value=mock_strategy), + patch("app.routes.auth_routes.create_refresh_token", AsyncMock(return_value="mock-refresh-token")), + patch("app.routes.auth_routes.config") as mock_config, + ): + mock_config.NEXT_FRONTEND_URL = _FRONTEND_URL + response = await proxy_login(request) + + assert response.status_code == 302 + location = response.headers["location"] + assert location == f"{_FRONTEND_URL}/" + + cookie_header = response.headers.get("set-cookie", "") + # RedirectResponse may set multiple cookies — check raw headers + raw_headers = [(k, v) for k, v in response.raw_headers if k == b"set-cookie"] + cookie_values = [v.decode() for _, v in raw_headers] + assert any("surfsense_sso_token=mock-access-token" in c for c in cookie_values) + assert any("surfsense_sso_refresh_token=mock-refresh-token" in c for c in cookie_values) + + @pytest.mark.asyncio + async def test_new_user_is_provisioned_and_gets_redirect(self): + """GIVEN user not in DB THEN JIT-provisioned + 302 with cookies.""" + from app.routes.auth_routes import proxy_login + + request = _make_request(headers={"x-auth-request-email": _EMAIL}) + + # SELECT returns None (no existing user); after add+commit, refresh populates user + new_user = _make_user(email=_EMAIL, is_active=True) + session = AsyncMock() + session.add = MagicMock() + session.execute = AsyncMock(return_value=_make_execute_result(None)) + session.refresh = AsyncMock(side_effect=lambda u: setattr(u, "id", new_user.id)) + + session_cm = _make_session_cm(session) + + mock_strategy = AsyncMock() + mock_strategy.write_token = AsyncMock(return_value="new-access-token") + + with ( + patch("app.routes.auth_routes.async_session_maker", return_value=session_cm), + patch("app.routes.auth_routes.get_jwt_strategy", return_value=mock_strategy), + patch("app.routes.auth_routes.create_refresh_token", AsyncMock(return_value="new-refresh-token")), + patch("app.routes.auth_routes.config") as mock_config, + ): + mock_config.NEXT_FRONTEND_URL = _FRONTEND_URL + response = await proxy_login(request) + + session.add.assert_called_once() + session.commit.assert_called_once() + + assert response.status_code == 302 + assert response.headers["location"] == f"{_FRONTEND_URL}/" + + @pytest.mark.asyncio + async def test_inactive_user_returns_401(self): + """GIVEN inactive user THEN 401.""" + from app.routes.auth_routes import proxy_login + + request = _make_request(headers={"x-auth-request-email": _EMAIL}) + inactive = _make_user(email=_EMAIL, is_active=False) + + session = AsyncMock() + session.execute = AsyncMock(return_value=_make_execute_result(inactive)) + session_cm = _make_session_cm(session) + + from fastapi import HTTPException + with ( + patch("app.routes.auth_routes.async_session_maker", return_value=session_cm), + pytest.raises(HTTPException) as exc_info, + ): + await proxy_login(request) + + assert exc_info.value.status_code == 401 + + @pytest.mark.asyncio + async def test_email_is_lowercased_and_stripped_before_lookup(self): + """GIVEN mixed-case email with whitespace THEN lookup uses normalised form.""" + from app.routes.auth_routes import proxy_login + + raw_email = " ALICE@EXAMPLE.COM " + normalised = "alice@example.com" + user = _make_user(email=normalised, is_active=True) + + request = _make_request(headers={"x-auth-request-email": raw_email}) + + captured_queries = [] + + async def mock_execute(stmt): + captured_queries.append(stmt) + return _make_execute_result(user) + + session = AsyncMock() + session.execute = mock_execute + session_cm = _make_session_cm(session) + + mock_strategy = AsyncMock() + mock_strategy.write_token = AsyncMock(return_value="token") + + with ( + patch("app.routes.auth_routes.async_session_maker", return_value=session_cm), + patch("app.routes.auth_routes.get_jwt_strategy", return_value=mock_strategy), + patch("app.routes.auth_routes.create_refresh_token", AsyncMock(return_value="rt")), + patch("app.routes.auth_routes.config") as mock_config, + ): + mock_config.NEXT_FRONTEND_URL = _FRONTEND_URL + response = await proxy_login(request) + + assert response.status_code == 302 + # Confirm the query used the lowercased/stripped email by checking + # that the user was found (i.e. not provisioned — add never called). + # (Deep SQLAlchemy AST inspection would be brittle; the 302 + no INSERT is sufficient.) diff --git a/surfsense_web/.env.example b/surfsense_web/.env.example index b448c1f71..2313f3e56 100644 --- a/surfsense_web/.env.example +++ b/surfsense_web/.env.example @@ -1,5 +1,10 @@ NEXT_PUBLIC_FASTAPI_BACKEND_URL=http://localhost:8000 NEXT_PUBLIC_FASTAPI_BACKEND_AUTH_TYPE=LOCAL or GOOGLE + +# mPass proxy auth — set when deployed behind oauth2-proxy + Traefik ForwardAuth +NEXT_PUBLIC_OIDC_LOGOUT_URL=https:///logout +NEXT_PUBLIC_OIDC_CLIENT_ID= +NEXT_PUBLIC_OAUTH2_PROXY_URL=https://auth. NEXT_PUBLIC_ETL_SERVICE=UNSTRUCTURED or LLAMACLOUD or DOCLING NEXT_PUBLIC_ZERO_CACHE_URL=http://localhost:4848 diff --git a/surfsense_web/Dockerfile b/surfsense_web/Dockerfile index da6bc8b7e..b3319bcf6 100644 --- a/surfsense_web/Dockerfile +++ b/surfsense_web/Dockerfile @@ -37,12 +37,14 @@ ARG NEXT_PUBLIC_FASTAPI_BACKEND_AUTH_TYPE=__NEXT_PUBLIC_FASTAPI_BACKEND_AUTH_TYP ARG NEXT_PUBLIC_ETL_SERVICE=__NEXT_PUBLIC_ETL_SERVICE__ ARG NEXT_PUBLIC_ZERO_CACHE_URL=__NEXT_PUBLIC_ZERO_CACHE_URL__ ARG NEXT_PUBLIC_DEPLOYMENT_MODE=__NEXT_PUBLIC_DEPLOYMENT_MODE__ +ARG NEXT_PUBLIC_OAUTH2_PROXY_URL=__NEXT_PUBLIC_OAUTH2_PROXY_URL__ ENV NEXT_PUBLIC_FASTAPI_BACKEND_URL=$NEXT_PUBLIC_FASTAPI_BACKEND_URL ENV NEXT_PUBLIC_FASTAPI_BACKEND_AUTH_TYPE=$NEXT_PUBLIC_FASTAPI_BACKEND_AUTH_TYPE ENV NEXT_PUBLIC_ETL_SERVICE=$NEXT_PUBLIC_ETL_SERVICE ENV NEXT_PUBLIC_ZERO_CACHE_URL=$NEXT_PUBLIC_ZERO_CACHE_URL ENV NEXT_PUBLIC_DEPLOYMENT_MODE=$NEXT_PUBLIC_DEPLOYMENT_MODE +ENV NEXT_PUBLIC_OAUTH2_PROXY_URL=$NEXT_PUBLIC_OAUTH2_PROXY_URL COPY --from=deps /app/node_modules ./node_modules COPY . . diff --git a/surfsense_web/app/(home)/layout.tsx b/surfsense_web/app/(home)/layout.tsx index f1ceffac0..3645d9512 100644 --- a/surfsense_web/app/(home)/layout.tsx +++ b/surfsense_web/app/(home)/layout.tsx @@ -7,12 +7,15 @@ import { Navbar } from "@/components/homepage/navbar"; export default function HomePageLayout({ children }: { children: React.ReactNode }) { const pathname = usePathname(); const isAuthPage = pathname === "/login" || pathname === "/register"; + // The home route ("/") is the SSO splash — it only renders during the + // cookie-handoff redirect dance and should not flash any chrome. + const isSplashPage = pathname === "/"; return (
- + {!isSplashPage && } {children} - {!isAuthPage && } + {!isAuthPage && !isSplashPage && }
); } diff --git a/surfsense_web/app/(home)/login/GoogleLoginButton.tsx b/surfsense_web/app/(home)/login/GoogleLoginButton.tsx index e22fc2798..f71776180 100644 --- a/surfsense_web/app/(home)/login/GoogleLoginButton.tsx +++ b/surfsense_web/app/(home)/login/GoogleLoginButton.tsx @@ -2,6 +2,7 @@ import { IconBrandGoogleFilled } from "@tabler/icons-react"; import { motion } from "motion/react"; import { useTranslations } from "next-intl"; +import { useEffect } from "react"; import { Logo } from "@/components/Logo"; import { trackLoginAttempt } from "@/lib/posthog/events"; import { AmbientBackground } from "./AmbientBackground"; @@ -9,16 +10,16 @@ import { AmbientBackground } from "./AmbientBackground"; export function GoogleLoginButton() { const t = useTranslations("auth"); - const handleGoogleLogin = () => { - // Track Google login attempt + // Auto-redirect to proxy-login on mount — user is already authenticated + // via oauth2-proxy/Cognito so no button click is needed. + useEffect(() => { trackLoginAttempt("google"); + window.location.href = `${process.env.NEXT_PUBLIC_FASTAPI_BACKEND_URL}/auth/jwt/proxy-login`; + }, []); - // IMPORTANT: Use the redirect-based authorize endpoint for cross-origin OAuth - // This fixes CSRF cookie issues in Firefox/Safari where cookies set via - // cross-origin fetch requests may not be sent on subsequent redirects. - // The authorize-redirect endpoint does a server-side redirect to Google - // and sets the CSRF cookie properly for same-site context. - window.location.href = `${process.env.NEXT_PUBLIC_FASTAPI_BACKEND_URL}/auth/google/authorize-redirect`; + const handleGoogleLogin = () => { + trackLoginAttempt("google"); + window.location.href = `${process.env.NEXT_PUBLIC_FASTAPI_BACKEND_URL}/auth/jwt/proxy-login`; }; return (
diff --git a/surfsense_web/app/(home)/login/LocalLoginForm.tsx b/surfsense_web/app/(home)/login/LocalLoginForm.tsx index 07a4db4d3..8b149f3f3 100644 --- a/surfsense_web/app/(home)/login/LocalLoginForm.tsx +++ b/surfsense_web/app/(home)/login/LocalLoginForm.tsx @@ -11,6 +11,7 @@ import { Spinner } from "@/components/ui/spinner"; import { getAuthErrorDetails, isNetworkError } from "@/lib/auth-errors"; import { AUTH_TYPE } from "@/lib/env-config"; import { ValidationError } from "@/lib/error"; +import { setBearerToken } from "@/lib/auth-utils"; import { trackLoginAttempt, trackLoginFailure, trackLoginSuccess } from "@/lib/posthog/events"; export function LocalLoginForm() { @@ -46,14 +47,12 @@ export function LocalLoginForm() { // Track successful login trackLoginSuccess("local"); - // Set flag so TokenHandler knows local login was already tracked - if (typeof window !== "undefined") { - sessionStorage.setItem("login_success_tracked", "true"); - } + // Store token directly — no /auth/callback redirect needed + setBearerToken(data.access_token); // Small delay to show success message setTimeout(() => { - router.push(`/auth/callback?token=${data.access_token}`); + router.push("/dashboard"); }, 500); } catch (err) { if (err instanceof ValidationError) { diff --git a/surfsense_web/app/(home)/login/page.tsx b/surfsense_web/app/(home)/login/page.tsx index 3dbbf21a9..350954cb1 100644 --- a/surfsense_web/app/(home)/login/page.tsx +++ b/surfsense_web/app/(home)/login/page.tsx @@ -8,7 +8,7 @@ import { toast } from "sonner"; import { Logo } from "@/components/Logo"; import { useGlobalLoadingEffect } from "@/hooks/use-global-loading"; import { getAuthErrorDetails, shouldRetry } from "@/lib/auth-errors"; -import { AUTH_TYPE } from "@/lib/env-config"; +import { AUTH_TYPE, isSSOAuth } from "@/lib/env-config"; import { AmbientBackground } from "./AmbientBackground"; import { GoogleLoginButton } from "./GoogleLoginButton"; import { LocalLoginForm } from "./LocalLoginForm"; @@ -22,6 +22,24 @@ function LoginContent() { const [urlError, setUrlError] = useState<{ title: string; message: string } | null>(null); const searchParams = useSearchParams(); + // SSO mode: bounce straight to oauth2-proxy /oauth2/sign_in. The dedicated + // auth subdomain handles the OIDC dance with Cognito and returns the user + // to / where the home-route splash + cookie handoff finishes the login + // normally. Same pattern as handleUnauthorized() in lib/auth-utils.ts. + // Falls through to the LOCAL/GOOGLE form below when AUTH_TYPE is not SSO. + // Hook is unconditional (rules-of-hooks); the early-return that skips + // rendering the form lives below, after every other hook has run. + useEffect(() => { + if (typeof window !== "undefined" && isSSOAuth()) { + const oauthProxyUrl = + process.env.NEXT_PUBLIC_OAUTH2_PROXY_URL || window.location.origin; + const rd = `${window.location.origin}/`; + window.location.replace( + `${oauthProxyUrl}/oauth2/sign_in?rd=${encodeURIComponent(rd)}` + ); + } + }, []); + useEffect(() => { // Check for various URL parameters that might indicate success or error states const registered = searchParams.get("registered"); @@ -103,6 +121,17 @@ function LoginContent() { // Use global loading screen for auth type determination - spinner animation won't reset useGlobalLoadingEffect(isLoading); + // SSO mode: render splash while window.location.replace() takes effect. + // isSSOAuth() reads a build-time-inlined env var (NEXT_PUBLIC_*) so it + // works during SSR too — the splash renders on the server, the browser + // receives splash HTML directly, no form flash before hydration. + // All hooks above run unconditionally (rules-of-hooks); only the render + // branches here. In SSO deployments isSSOAuth() is true, so the form + // below is never reached at runtime. + if (isSSOAuth()) { + return
; + } + // Show nothing while loading - the GlobalLoadingProvider handles the loading UI if (isLoading) { return null; diff --git a/surfsense_web/app/(home)/page.tsx b/surfsense_web/app/(home)/page.tsx index 5eeea727f..14ff98be0 100644 --- a/surfsense_web/app/(home)/page.tsx +++ b/surfsense_web/app/(home)/page.tsx @@ -1,55 +1,55 @@ "use client"; -import dynamic from "next/dynamic"; import { useRouter } from "next/navigation"; import { useEffect } from "react"; -import { HeroSection } from "@/components/homepage/hero-section"; -import { getBearerToken } from "@/lib/auth-utils"; - -const WhySurfSense = dynamic( - () => import("@/components/homepage/why-surfsense").then((m) => ({ default: m.WhySurfSense })), - { ssr: false } -); - -const FeaturesCards = dynamic( - () => import("@/components/homepage/features-card").then((m) => ({ default: m.FeaturesCards })), - { ssr: false } -); - -const FeaturesBentoGrid = dynamic( - () => - import("@/components/homepage/features-bento-grid").then((m) => ({ - default: m.FeaturesBentoGrid, - })), - { ssr: false } -); - -const ExternalIntegrations = dynamic(() => import("@/components/homepage/integrations"), { - ssr: false, -}); - -const CTAHomepage = dynamic( - () => import("@/components/homepage/cta").then((m) => ({ default: m.CTAHomepage })), - { ssr: false } -); +import { + clearSSOCookies, + getBearerToken, + getSSOCookieTokens, + setBearerToken, + setRefreshToken, +} from "@/lib/auth-utils"; + +/** + * SSO-only home route. + * + * This fork is configured for mPass/Cognito SSO via oauth2-proxy, so the + * marketing landing page (HeroSection / FeaturesCards / etc.) is never the + * intended destination — every visitor either has a session or is about to + * get one. Rendering the marketing JSX here would cause a visible flash + * (~200-500ms) before the redirect chain completes: + * + * / → marketing flash → /auth/jwt/proxy-login → / → /dashboard + * + * Instead we render a neutral splash and let the cookie handoff or proxy + * redirect take the user where they need to go. + */ export default function HomePage() { const router = useRouter(); useEffect(() => { if (getBearerToken()) { router.replace("/dashboard"); + return; } + + // Cookie handoff from /auth/jwt/proxy-login after oauth2-proxy + Cognito login. + // Backend sets short-lived cookies (60s TTL) and redirects here instead of + // to /auth/callback, avoiding any Traefik path-split between frontend and backend. + const { token, refreshToken } = getSSOCookieTokens(); + if (token) { + setBearerToken(token); + if (refreshToken) setRefreshToken(refreshToken); + clearSSOCookies(); + router.replace("/dashboard"); + return; + } + + // No JWT anywhere → start the SSO flow. + window.location.href = `${process.env.NEXT_PUBLIC_FASTAPI_BACKEND_URL}/auth/jwt/proxy-login`; }, [router]); - return ( -
- - - - - - -
- ); + // Splash — neutral background, no UI flash during the redirect dance. + return
; } diff --git a/surfsense_web/app/(home)/register/page.tsx b/surfsense_web/app/(home)/register/page.tsx index f9e387cf7..cd479c763 100644 --- a/surfsense_web/app/(home)/register/page.tsx +++ b/surfsense_web/app/(home)/register/page.tsx @@ -12,7 +12,7 @@ import { Logo } from "@/components/Logo"; import { Spinner } from "@/components/ui/spinner"; import { getAuthErrorDetails, isNetworkError, shouldRetry } from "@/lib/auth-errors"; import { getBearerToken } from "@/lib/auth-utils"; -import { AUTH_TYPE } from "@/lib/env-config"; +import { AUTH_TYPE, isSSOAuth } from "@/lib/env-config"; import { AppError, ValidationError } from "@/lib/error"; import { trackRegistrationAttempt, @@ -37,6 +37,23 @@ export default function RegisterPage() { const router = useRouter(); const [{ mutateAsync: register, isPending: isRegistering }] = useAtom(registerMutationAtom); + // SSO mode: bounce straight to oauth2-proxy /oauth2/sign_in. The dedicated + // auth subdomain handles the OIDC dance with Cognito and returns the user + // to / where the home-route splash + cookie handoff finishes the login + // normally. Same pattern as the /login page and handleUnauthorized() in + // lib/auth-utils.ts. Falls through to the LOCAL form below when AUTH_TYPE + // is not SSO. Hook is unconditional (rules-of-hooks). + useEffect(() => { + if (typeof window !== "undefined" && isSSOAuth()) { + const oauthProxyUrl = + process.env.NEXT_PUBLIC_OAUTH2_PROXY_URL || window.location.origin; + const rd = `${window.location.origin}/`; + window.location.replace( + `${oauthProxyUrl}/oauth2/sign_in?rd=${encodeURIComponent(rd)}` + ); + } + }, []); + // Check authentication type and redirect if not LOCAL useEffect(() => { if (getBearerToken()) { @@ -156,6 +173,17 @@ export default function RegisterPage() { } }; + // SSO mode: render splash while window.location.replace() takes effect. + // isSSOAuth() reads a build-time-inlined env var (NEXT_PUBLIC_*) so it + // works during SSR too — the splash renders on the server, the browser + // receives splash HTML directly, no form flash before hydration. + // All hooks above run unconditionally (rules-of-hooks); only the render + // branches here. In SSO deployments isSSOAuth() is true, so the form + // below is never reached at runtime. + if (isSSOAuth()) { + return
; + } + return (
diff --git a/surfsense_web/app/api/zero/query/route.ts b/surfsense_web/app/api/zero/query/route.ts index 3d8ff0d33..3bd00f7c1 100644 --- a/surfsense_web/app/api/zero/query/route.ts +++ b/surfsense_web/app/api/zero/query/route.ts @@ -5,7 +5,15 @@ import type { Context } from "@/types/zero"; import { queries } from "@/zero/queries"; import { schema } from "@/zero/schema"; -const backendURL = process.env.NEXT_PUBLIC_FASTAPI_BACKEND_URL || "http://localhost:8000"; +// This route runs server-side (Next.js API route, called by zero-cache). +// Prefer an internal URL for server-to-server calls — `NEXT_PUBLIC_*` is the +// browser-facing URL, which in reverse-proxy / ForwardAuth deployments goes +// through an auth gateway that rejects server-side fetches without browser +// cookies. Falls back to the public URL for single-host deployments. +const backendURL = + process.env.FASTAPI_BACKEND_INTERNAL_URL || + process.env.NEXT_PUBLIC_FASTAPI_BACKEND_URL || + "http://localhost:8000"; async function authenticateRequest( request: Request diff --git a/surfsense_web/app/auth/callback/loading.tsx b/surfsense_web/app/auth/callback/loading.tsx deleted file mode 100644 index f12b3847d..000000000 --- a/surfsense_web/app/auth/callback/loading.tsx +++ /dev/null @@ -1,11 +0,0 @@ -"use client"; - -import { useGlobalLoadingEffect } from "@/hooks/use-global-loading"; - -export default function AuthCallbackLoading() { - // Use global loading - spinner animation won't reset when page transitions - useGlobalLoadingEffect(true); - - // Return null - the GlobalLoadingProvider handles the loading UI - return null; -} diff --git a/surfsense_web/app/auth/callback/page.tsx b/surfsense_web/app/auth/callback/page.tsx deleted file mode 100644 index 4050eefb6..000000000 --- a/surfsense_web/app/auth/callback/page.tsx +++ /dev/null @@ -1,18 +0,0 @@ -"use client"; - -import { Suspense } from "react"; -import TokenHandler from "@/components/TokenHandler"; - -export default function AuthCallbackPage() { - // Suspense fallback returns null - the GlobalLoadingProvider handles the loading UI - // TokenHandler uses useGlobalLoadingEffect to show the loading screen - return ( - - - - ); -} diff --git a/surfsense_web/atoms/user/user-query.atoms.ts b/surfsense_web/atoms/user/user-query.atoms.ts index 8e196c9c7..ff1477482 100644 --- a/surfsense_web/atoms/user/user-query.atoms.ts +++ b/surfsense_web/atoms/user/user-query.atoms.ts @@ -1,6 +1,5 @@ import { atomWithQuery } from "jotai-tanstack-query"; import { userApiService } from "@/lib/apis/user-api.service"; -import { getBearerToken } from "@/lib/auth-utils"; export const USER_QUERY_KEY = ["user", "me"] as const; const userQueryFn = () => userApiService.getMe(); @@ -9,7 +8,7 @@ export const currentUserAtom = atomWithQuery(() => { return { queryKey: USER_QUERY_KEY, staleTime: 5 * 60 * 1000, - enabled: !!getBearerToken(), + enabled: true, queryFn: userQueryFn, }; }); diff --git a/surfsense_web/components/TokenHandler.tsx b/surfsense_web/components/TokenHandler.tsx index e81a1bf2b..88dd66550 100644 --- a/surfsense_web/components/TokenHandler.tsx +++ b/surfsense_web/components/TokenHandler.tsx @@ -1,87 +1,65 @@ -"use client"; +// mPass SSO: This component is disabled because oauth2-proxy ForwardAuth +// handles login via Cognito. The cookie-handoff pattern (proxy_login → +// cookie → frontend reads cookie → JWT in localStorage) replaces the +// native OAuth token extraction flow below. +// +// To restore native OAuth login, uncomment this file and re-register +// the /auth/callback route. -import { useEffect } from "react"; -import { useGlobalLoadingEffect } from "@/hooks/use-global-loading"; -import { searchSpacesApiService } from "@/lib/apis/search-spaces-api.service"; -import { getAndClearRedirectPath, setBearerToken, setRefreshToken } from "@/lib/auth-utils"; -import { trackLoginSuccess } from "@/lib/posthog/events"; -interface TokenHandlerProps { - redirectPath?: string; // Default path to redirect after storing token (if no saved path) - tokenParamName?: string; // Name of the URL parameter containing the token - storageKey?: string; // Key to use when storing in localStorage (kept for backwards compatibility) -} - -/** - * Client component that extracts a token from URL parameters and stores it in localStorage - * After storing the token, it redirects the user back to the page they were on before - * being redirected to login (if available), or to the default redirectPath. - * - * @param redirectPath - Default path to redirect after storing token (default: '/dashboard') - * @param tokenParamName - Name of the URL parameter containing the token (default: 'token') - * @param storageKey - Key to use when storing in localStorage (default: 'surfsense_bearer_token') - */ -const TokenHandler = ({ - redirectPath = "/dashboard", - tokenParamName = "token", - storageKey = "surfsense_bearer_token", -}: TokenHandlerProps) => { - // Always show loading for this component - spinner animation won't reset - useGlobalLoadingEffect(true); - - useEffect(() => { - if (typeof window === "undefined") return; - - const run = async () => { - const params = new URLSearchParams(window.location.search); - const token = params.get(tokenParamName); - const refreshToken = params.get("refresh_token"); - - if (token) { - try { - const alreadyTracked = sessionStorage.getItem("login_success_tracked"); - if (!alreadyTracked) { - trackLoginSuccess("google"); - } - sessionStorage.removeItem("login_success_tracked"); - - localStorage.setItem(storageKey, token); - setBearerToken(token); - - if (refreshToken) { - setRefreshToken(refreshToken); - } - - // Auto-set active search space in desktop if not already set - if (window.electronAPI?.getActiveSearchSpace) { - try { - const stored = await window.electronAPI.getActiveSearchSpace(); - if (!stored) { - const spaces = await searchSpacesApiService.getSearchSpaces(); - if (spaces?.length) { - await window.electronAPI.setActiveSearchSpace?.(String(spaces[0].id)); - } - } - } catch { - // non-critical - } - } - - const savedRedirectPath = getAndClearRedirectPath(); - const finalRedirectPath = savedRedirectPath || redirectPath; - window.location.href = finalRedirectPath; - } catch (error) { - console.error("Error storing token in localStorage:", error); - window.location.href = redirectPath; - } - } - }; - - run(); - }, [tokenParamName, storageKey, redirectPath]); - - // Return null - the global provider handles the loading UI - return null; -}; - -export default TokenHandler; +// "use client"; +// +// import { useEffect } from "react"; +// import { useGlobalLoadingEffect } from "@/hooks/use-global-loading"; +// import { getAndClearRedirectPath, setBearerToken, setRefreshToken } from "@/lib/auth-utils"; +// import { trackLoginSuccess } from "@/lib/posthog/events"; +// +// interface TokenHandlerProps { +// redirectPath?: string; +// tokenParamName?: string; +// storageKey?: string; +// } +// +// const TokenHandler = ({ +// redirectPath = "/dashboard", +// tokenParamName = "token", +// storageKey = "surfsense_bearer_token", +// }: TokenHandlerProps) => { +// useGlobalLoadingEffect(true); +// +// useEffect(() => { +// if (typeof window === "undefined") return; +// +// const params = new URLSearchParams(window.location.search); +// const token = params.get(tokenParamName); +// const refreshToken = params.get("refresh_token"); +// +// if (token) { +// try { +// const alreadyTracked = sessionStorage.getItem("login_success_tracked"); +// if (!alreadyTracked) { +// trackLoginSuccess("google"); +// } +// sessionStorage.removeItem("login_success_tracked"); +// +// localStorage.setItem(storageKey, token); +// setBearerToken(token); +// +// if (refreshToken) { +// setRefreshToken(refreshToken); +// } +// +// const savedRedirectPath = getAndClearRedirectPath(); +// const finalRedirectPath = savedRedirectPath || redirectPath; +// window.location.href = finalRedirectPath; +// } catch (error) { +// console.error("Error storing token in localStorage:", error); +// window.location.href = redirectPath; +// } +// } +// }, [tokenParamName, storageKey, redirectPath]); +// +// return null; +// }; +// +// export default TokenHandler; diff --git a/surfsense_web/components/assistant-ui/image.tsx b/surfsense_web/components/assistant-ui/image.tsx index 59781abcf..b29b6ddce 100644 --- a/surfsense_web/components/assistant-ui/image.tsx +++ b/surfsense_web/components/assistant-ui/image.tsx @@ -279,4 +279,4 @@ Image.Preview = ImagePreview; Image.Filename = ImageFilename; Image.Zoom = ImageZoom; -export { Image, ImageRoot, ImagePreview, ImageFilename, ImageZoom, imageVariants }; +export { Image, ImageFilename, ImagePreview, ImageRoot, ImageZoom, imageVariants }; diff --git a/surfsense_web/components/auth/sign-in-button.tsx b/surfsense_web/components/auth/sign-in-button.tsx index dd5893deb..b6e4b22a7 100644 --- a/surfsense_web/components/auth/sign-in-button.tsx +++ b/surfsense_web/components/auth/sign-in-button.tsx @@ -2,7 +2,7 @@ import { motion } from "motion/react"; import Link from "next/link"; -import { AUTH_TYPE, BACKEND_URL } from "@/lib/env-config"; +import { AUTH_TYPE, isSSOAuth } from "@/lib/env-config"; import { trackLoginAttempt } from "@/lib/posthog/events"; import { cn } from "@/lib/utils"; @@ -46,34 +46,39 @@ interface SignInButtonProps { export const SignInButton = ({ variant = "desktop" }: SignInButtonProps) => { const isGoogleAuth = AUTH_TYPE === "GOOGLE"; + const isSSOAuthMode = isSSOAuth(); + // Both Google and SSO modes use the proxy-login button (not email/password form) + const isProxyLogin = isGoogleAuth || isSSOAuthMode; - const handleGoogleLogin = () => { - trackLoginAttempt("google"); - window.location.href = `${BACKEND_URL}/auth/google/authorize-redirect`; + const handleProxyLogin = () => { + trackLoginAttempt(isSSOAuthMode ? "sso" : "google"); + // Redirect to proxy-login — Traefik ForwardAuth triggers Cognito if needed, + // then the endpoint issues a JWT and redirects to /auth/callback. + window.location.href = `${process.env.NEXT_PUBLIC_FASTAPI_BACKEND_URL}/auth/jwt/proxy-login`; }; const getClassName = () => { if (variant === "desktop") { - return isGoogleAuth + return isProxyLogin ? "hidden rounded-full bg-white px-5 py-2 text-sm text-neutral-700 shadow-md ring-1 ring-neutral-200/50 hover:shadow-lg md:flex dark:bg-neutral-900 dark:text-neutral-200 dark:ring-neutral-700/50" : "hidden rounded-full bg-black px-8 py-2 text-sm font-bold text-white shadow-[0px_-2px_0px_0px_rgba(255,255,255,0.4)_inset] md:block dark:bg-white dark:text-black"; } if (variant === "compact") { - return isGoogleAuth + return isProxyLogin ? "rounded-full bg-white px-4 py-1.5 text-sm text-neutral-700 shadow-md ring-1 ring-neutral-200/50 hover:shadow-lg dark:bg-neutral-900 dark:text-neutral-200 dark:ring-neutral-700/50" : "rounded-full bg-black px-6 py-1.5 text-sm font-bold text-white shadow-[0px_-2px_0px_0px_rgba(255,255,255,0.4)_inset] dark:bg-white dark:text-black"; } // mobile - return isGoogleAuth + return isProxyLogin ? "w-full rounded-lg bg-white px-8 py-2.5 text-neutral-700 shadow-md ring-1 ring-neutral-200/50 dark:bg-neutral-900 dark:text-neutral-200 dark:ring-neutral-700/50 touch-manipulation" : "w-full rounded-lg bg-black px-8 py-2 font-medium text-white shadow-[0px_-2px_0px_0px_rgba(255,255,255,0.4)_inset] dark:bg-white dark:text-black text-center touch-manipulation"; }; - if (isGoogleAuth) { + if (isProxyLogin) { return ( { getClassName() )} > - + {isGoogleAuth && } Sign In ); diff --git a/surfsense_web/components/homepage/hero-section.tsx b/surfsense_web/components/homepage/hero-section.tsx index ba5a9d9ac..23c17aa3e 100644 --- a/surfsense_web/components/homepage/hero-section.tsx +++ b/surfsense_web/components/homepage/hero-section.tsx @@ -12,7 +12,7 @@ import { } from "@/components/ui/dropdown-menu"; import { ExpandedMediaOverlay, useExpandedMedia } from "@/components/ui/expanded-gif-overlay"; import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"; -import { AUTH_TYPE, BACKEND_URL } from "@/lib/env-config"; +import { AUTH_TYPE, isSSOAuth } from "@/lib/env-config"; import { trackLoginAttempt } from "@/lib/posthog/events"; import { cn } from "@/lib/utils"; @@ -171,22 +171,54 @@ export function HeroSection() { function GetStartedButton() { const isGoogleAuth = AUTH_TYPE === "GOOGLE"; - - const handleGoogleLogin = () => { - trackLoginAttempt("google"); - window.location.href = `${BACKEND_URL}/auth/google/authorize-redirect`; + const isSSOAuthMode = isSSOAuth(); + const isProxyLogin = isGoogleAuth || isSSOAuthMode; + + const handleProxyLogin = () => { + trackLoginAttempt(isSSOAuthMode ? "sso" : "google"); + // Redirect to proxy-login — Traefik ForwardAuth triggers Cognito if needed, + // then the endpoint issues a JWT and redirects to /auth/callback. + window.location.href = `${process.env.NEXT_PUBLIC_FASTAPI_BACKEND_URL}/auth/jwt/proxy-login`; }; - if (isGoogleAuth) { + if (isProxyLogin) { return ( + {/* Animated gradient background on hover */} + + {/* Show Google logo only for native Google OAuth, not SSO */} + {isGoogleAuth && ( + + + + )} + {isGoogleAuth ? "Continue with Google" : "Sign In"} + ); } diff --git a/surfsense_web/docker-entrypoint.js b/surfsense_web/docker-entrypoint.js index 8323f5652..b71264550 100644 --- a/surfsense_web/docker-entrypoint.js +++ b/surfsense_web/docker-entrypoint.js @@ -27,6 +27,7 @@ const replacements = [ process.env.NEXT_PUBLIC_ZERO_CACHE_URL || "http://localhost:4848", ], ["__NEXT_PUBLIC_DEPLOYMENT_MODE__", process.env.NEXT_PUBLIC_DEPLOYMENT_MODE || "self-hosted"], + ["__NEXT_PUBLIC_OAUTH2_PROXY_URL__", process.env.NEXT_PUBLIC_OAUTH2_PROXY_URL || ""], ]; let filesProcessed = 0; diff --git a/surfsense_web/lib/apis/base-api.service.ts b/surfsense_web/lib/apis/base-api.service.ts index bc9e6c1d8..a2ba55197 100644 --- a/surfsense_web/lib/apis/base-api.service.ts +++ b/surfsense_web/lib/apis/base-api.service.ts @@ -66,9 +66,11 @@ class BaseApiService { * ---------- */ const defaultOptions: RequestOptions = { - headers: { - Authorization: `Bearer ${this.bearerToken || ""}`, - }, + headers: this.bearerToken + ? { + Authorization: `Bearer ${this.bearerToken}`, + } + : {}, method: "GET", responseType: ResponseType.JSON, }; @@ -87,14 +89,13 @@ class BaseApiService { throw new AppError("Base URL is not set."); } - // Validate the bearer token + // Validate the bearer token — skip check when no token is present; + // the backend proxy-auth middleware handles authentication via + // the X-Auth-Request-Email header set by oauth2-proxy/Traefik. const isNoAuthEndpoint = this.noAuthEndpoints.includes(url) || this.noAuthPrefixes.some((prefix) => url.startsWith(prefix)) || /^\/api\/v1\/invites\/[^/]+\/info$/.test(url); - if (!this.bearerToken && !isNoAuthEndpoint) { - throw new AuthenticationError("You are not authenticated. Please login again."); - } // Construct the full URL const fullUrl = new URL(url, this.baseUrl).toString(); @@ -358,3 +359,4 @@ class BaseApiService { } export const baseApiService = new BaseApiService(process.env.NEXT_PUBLIC_FASTAPI_BACKEND_URL || ""); + diff --git a/surfsense_web/lib/auth-utils.ts b/surfsense_web/lib/auth-utils.ts index d66934c3b..29ef6ace3 100644 --- a/surfsense_web/lib/auth-utils.ts +++ b/surfsense_web/lib/auth-utils.ts @@ -41,9 +41,18 @@ export function getLoginPath(): string { } /** - * Clears tokens and optionally redirects to login. - * Call this when a 401 response is received. - * Only redirects when the current route is protected; on public routes we just clear tokens. + * Clears tokens and redirects through oauth2-proxy on 401. + * + * This fork is SSO-only (mPass/Cognito via oauth2-proxy ForwardAuth), so the + * only valid recovery from a 401 is bouncing the user through the OIDC flow + * at the dedicated auth subdomain. The previous /login redirect was a dead + * page in SSO mode and resulted in a blank screen. + * + * Moving this responsibility to the frontend lets the devstack repo drop + * its SurfSense-specific Traefik `mpass-signin@file` middleware and + * `auth-redirect.yml` dynamic config — those exist purely to convert API + * 401s into redirects at the network layer. With this in place the app + * handles its own 401s before Traefik ever needs to. */ export function handleUnauthorized(): void { if (typeof window === "undefined") return; @@ -54,15 +63,20 @@ export function handleUnauthorized(): void { localStorage.removeItem(BEARER_TOKEN_KEY); localStorage.removeItem(REFRESH_TOKEN_KEY); - // Only redirect on protected routes; stay on public pages (e.g. /docs) - if (!isPublicRoute(pathname)) { - const currentPath = pathname + window.location.search + window.location.hash; - const excludedPaths = ["/auth", "/auth/callback", "/"]; - if (!excludedPaths.includes(pathname)) { - localStorage.setItem(REDIRECT_PATH_KEY, currentPath); - } - window.location.href = getLoginPath(); + // Public routes (e.g. /docs) don't need auth — don't redirect, just clear. + if (isPublicRoute(pathname)) return; + + const currentPath = pathname + window.location.search + window.location.hash; + const excludedPaths = ["/auth", "/"]; + if (!excludedPaths.includes(pathname)) { + localStorage.setItem(REDIRECT_PATH_KEY, currentPath); } + + // Redirect through oauth2-proxy /oauth2/sign_in. The dedicated auth subdomain + // handles the OIDC dance with Cognito and returns the user to `rd=` on success. + const oauthProxyUrl = process.env.NEXT_PUBLIC_OAUTH2_PROXY_URL || window.location.origin; + const rd = window.location.href; + window.location.href = `${oauthProxyUrl}/oauth2/sign_in?rd=${encodeURIComponent(rd)}`; } /** @@ -138,6 +152,7 @@ export function clearAllTokens(): void { } /** +<<<<<<< HEAD * Pushes the current localStorage tokens into the Electron main process * so that other BrowserWindows (Quick Ask, Autocomplete) can access them. */ @@ -178,33 +193,80 @@ export async function ensureTokensFromElectron(): Promise { /** * Logout the current user by revoking the refresh token and clearing localStorage. * Returns true if logout was successful (or tokens were cleared), false otherwise. +======= + * Reads the short-lived SSO handoff cookies set by /auth/jwt/proxy-login. + * Returns null for each if not present. + */ +export function getSSOCookieTokens(): { token: string | null; refreshToken: string | null } { + if (typeof document === "undefined") return { token: null, refreshToken: null }; + const get = (name: string): string | null => { + const match = document.cookie.match(new RegExp(`(?:^|; )${name}=([^;]*)`)); + return match ? decodeURIComponent(match[1]) : null; + }; + return { token: get("surfsense_sso_token"), refreshToken: get("surfsense_sso_refresh_token") }; +} + +/** + * Clears the SSO handoff cookies after tokens have been transferred to localStorage. + */ +export function clearSSOCookies(): void { + if (typeof document === "undefined") return; + const expire = "expires=Thu, 01 Jan 1970 00:00:00 UTC; path=/"; + document.cookie = `surfsense_sso_token=; ${expire}`; + document.cookie = `surfsense_sso_refresh_token=; ${expire}`; +} + +/** + * Logout the current user. + * + * Always performs 3-layer SSO logout (proxy auth is the only auth mode): + * Layer 1 — revoke JWT refresh tokens server-side + * Layer 2 — clear _oauth2_proxy cookie via /oauth2/sign_out + * Layer 3 — clear Cognito session via rd= redirect +>>>>>>> 8c3ff62c (feat(auth): mPass SSO via oauth2-proxy ForwardAuth with cookie-handoff) */ export async function logout(): Promise { const refreshToken = getRefreshToken(); - // Call backend to revoke the refresh token + // Layer 1 — revoke the refresh token server-side if (refreshToken) { try { const backendUrl = process.env.NEXT_PUBLIC_FASTAPI_BACKEND_URL || "http://localhost:8000"; const response = await fetch(`${backendUrl}/auth/jwt/revoke`, { method: "POST", - headers: { - "Content-Type": "application/json", - }, + headers: { "Content-Type": "application/json" }, body: JSON.stringify({ refresh_token: refreshToken }), }); - if (!response.ok) { console.warn("Failed to revoke refresh token:", response.status, await response.text()); } } catch (error) { console.warn("Failed to revoke refresh token on server:", error); - // Continue to clear local tokens even if server call fails } } - // Clear all tokens from localStorage clearAllTokens(); + + // Layers 2 + 3 — SSO logout via oauth2-proxy → Cognito + if (typeof window !== "undefined") { + const oidcLogoutUrl = process.env.NEXT_PUBLIC_OIDC_LOGOUT_URL; + const oidcClientId = process.env.NEXT_PUBLIC_OIDC_CLIENT_ID; + + if (oidcLogoutUrl && oidcClientId) { + const cognitoUrl = new URL(oidcLogoutUrl); + cognitoUrl.searchParams.set("client_id", oidcClientId); + cognitoUrl.searchParams.set("logout_uri", window.location.origin); + + // Full SSO logout: oauth2-proxy sign_out clears _oauth2_proxy cookie, + // then rd= redirects to Cognito to clear the Cognito session. + // Uses the dedicated auth domain (foss-auth.localhost) so the sign_out + // URL is consistent regardless of which app initiates the logout. + const oauthProxyUrl = process.env.NEXT_PUBLIC_OAUTH2_PROXY_URL || window.location.origin; + window.location.href = `${oauthProxyUrl}/oauth2/sign_out?rd=${encodeURIComponent(cognitoUrl.toString())}`; + return true; // browser is already navigating away + } + } + return true; } @@ -227,7 +289,7 @@ export function redirectToLogin(): void { const currentPath = window.location.pathname + window.location.search + window.location.hash; // Don't save auth-related paths or home page - const excludedPaths = ["/auth", "/auth/callback", "/", "/login", "/register", "/desktop/login"]; + const excludedPaths = ["/auth", "/", "/login", "/register"]; if (!excludedPaths.includes(window.location.pathname)) { localStorage.setItem(REDIRECT_PATH_KEY, currentPath); } diff --git a/surfsense_web/lib/env-config.ts b/surfsense_web/lib/env-config.ts index 80db395c6..0bb9c9993 100644 --- a/surfsense_web/lib/env-config.ts +++ b/surfsense_web/lib/env-config.ts @@ -11,15 +11,18 @@ import packageJson from "../package.json"; -// Auth type: "LOCAL" for email/password, "GOOGLE" for OAuth +// Auth type: +// "LOCAL" — email/password login form +// "GOOGLE" — Google OAuth (native fastapi-users Google flow) +// "SSO" — Cognito/oauth2-proxy ForwardAuth (our devstack pattern) // Placeholder: __NEXT_PUBLIC_FASTAPI_BACKEND_AUTH_TYPE__ -export const AUTH_TYPE = process.env.NEXT_PUBLIC_FASTAPI_BACKEND_AUTH_TYPE || "GOOGLE"; +export const AUTH_TYPE = process.env.NEXT_PUBLIC_FASTAPI_BACKEND_AUTH_TYPE || "SSO"; // Backend API URL // Placeholder: __NEXT_PUBLIC_FASTAPI_BACKEND_URL__ export const BACKEND_URL = process.env.NEXT_PUBLIC_FASTAPI_BACKEND_URL || "http://localhost:8000"; -// ETL Service: "DOCLING", "UNSTRUCTURED", or "LLAMACLOUD" +// ETL Service: "DOCLING" or "UNSTRUCTURED" // Placeholder: __NEXT_PUBLIC_ETL_SERVICE__ export const ETL_SERVICE = process.env.NEXT_PUBLIC_ETL_SERVICE || "DOCLING"; @@ -40,6 +43,9 @@ export const isLocalAuth = () => AUTH_TYPE === "LOCAL"; // Helper to check if Google auth is enabled export const isGoogleAuth = () => AUTH_TYPE === "GOOGLE"; +// Helper to check if SSO (Cognito/oauth2-proxy) auth is enabled +export const isSSOAuth = () => AUTH_TYPE === "SSO"; + // Helper to check if running in self-hosted mode export const isSelfHosted = () => DEPLOYMENT_MODE === "self-hosted"; From 9ddc5ae7539dced6633d3d6690201c0368194330 Mon Sep 17 00:00:00 2001 From: awais786 Date: Sun, 12 Apr 2026 15:33:52 +0500 Subject: [PATCH 02/29] refactor(auth): SSO defaults + delegate user provisioning to middleware MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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. --- docker/.env.example | 7 +- docker/docker-compose.dev.yml | 4 +- docker/docker-compose.yml | 2 +- surfsense_backend/.env.example | 7 +- surfsense_backend/app/app.py | 2 - surfsense_backend/app/config/__init__.py | 34 ++- surfsense_backend/app/routes/auth_routes.py | 46 ++-- .../tests/unit/routes/test_proxy_login.py | 205 ++++-------------- 8 files changed, 86 insertions(+), 221 deletions(-) diff --git a/docker/.env.example b/docker/.env.example index 1a3869773..4394929a7 100644 --- a/docker/.env.example +++ b/docker/.env.example @@ -14,8 +14,11 @@ SURFSENSE_VERSION=latest # REQUIRED: Generate a secret key with: openssl rand -base64 32 SECRET_KEY=replace_me_with_a_random_string -# Auth type: LOCAL (email/password) or GOOGLE (OAuth) -AUTH_TYPE=LOCAL +# Auth — DO NOT change this value. This fork only supports SSO +# (mPass/Cognito via oauth2-proxy ForwardAuth). Setting LOCAL or +# GOOGLE will break authentication — the backend routes for those +# modes are not registered in this fork. +AUTH_TYPE=SSO # Allow new user registrations (TRUE or FALSE) # REGISTRATION_ENABLED=TRUE diff --git a/docker/docker-compose.dev.yml b/docker/docker-compose.dev.yml index c7922e3ef..666b0c642 100644 --- a/docker/docker-compose.dev.yml +++ b/docker/docker-compose.dev.yml @@ -88,7 +88,7 @@ services: - UNSTRUCTURED_HAS_PATCHED_LOOP=1 - LANGCHAIN_TRACING_V2=false - LANGSMITH_TRACING=false - - AUTH_TYPE=${AUTH_TYPE:-LOCAL} + - AUTH_TYPE=${AUTH_TYPE:-SSO} - NEXT_FRONTEND_URL=${NEXT_FRONTEND_URL:-http://localhost:3000} - SEARXNG_DEFAULT_HOST=${SEARXNG_DEFAULT_HOST:-http://searxng:8080} # Daytona Sandbox – uncomment and set credentials to enable cloud code execution @@ -204,7 +204,7 @@ services: context: ../surfsense_web args: NEXT_PUBLIC_FASTAPI_BACKEND_URL: ${NEXT_PUBLIC_FASTAPI_BACKEND_URL:-http://localhost:8000} - NEXT_PUBLIC_FASTAPI_BACKEND_AUTH_TYPE: ${NEXT_PUBLIC_FASTAPI_BACKEND_AUTH_TYPE:-LOCAL} + NEXT_PUBLIC_FASTAPI_BACKEND_AUTH_TYPE: ${NEXT_PUBLIC_FASTAPI_BACKEND_AUTH_TYPE:-SSO} NEXT_PUBLIC_ETL_SERVICE: ${NEXT_PUBLIC_ETL_SERVICE:-DOCLING} NEXT_PUBLIC_ZERO_CACHE_URL: ${NEXT_PUBLIC_ZERO_CACHE_URL:-http://localhost:${ZERO_CACHE_PORT:-4848}} NEXT_PUBLIC_DEPLOYMENT_MODE: ${NEXT_PUBLIC_DEPLOYMENT_MODE:-self-hosted} diff --git a/docker/docker-compose.yml b/docker/docker-compose.yml index 549190947..c14cd8b09 100644 --- a/docker/docker-compose.yml +++ b/docker/docker-compose.yml @@ -195,7 +195,7 @@ services: environment: NEXT_PUBLIC_FASTAPI_BACKEND_URL: ${NEXT_PUBLIC_FASTAPI_BACKEND_URL:-http://localhost:${BACKEND_PORT:-8929}} NEXT_PUBLIC_ZERO_CACHE_URL: ${NEXT_PUBLIC_ZERO_CACHE_URL:-http://localhost:${ZERO_CACHE_PORT:-5929}} - NEXT_PUBLIC_FASTAPI_BACKEND_AUTH_TYPE: ${AUTH_TYPE:-LOCAL} + NEXT_PUBLIC_FASTAPI_BACKEND_AUTH_TYPE: ${AUTH_TYPE:-SSO} NEXT_PUBLIC_ETL_SERVICE: ${ETL_SERVICE:-DOCLING} NEXT_PUBLIC_DEPLOYMENT_MODE: ${DEPLOYMENT_MODE:-self-hosted} labels: diff --git a/surfsense_backend/.env.example b/surfsense_backend/.env.example index 818cfca78..3899c99f7 100644 --- a/surfsense_backend/.env.example +++ b/surfsense_backend/.env.example @@ -61,8 +61,11 @@ STRIPE_RECONCILIATION_BATCH_SIZE=100 # Backend URL for OAuth callbacks (optional, set when behind reverse proxy with HTTPS) # BACKEND_URL=https://api.yourdomain.com -# Auth -AUTH_TYPE=GOOGLE or LOCAL +# Auth — DO NOT change this value. This fork only supports SSO +# (mPass/Cognito via oauth2-proxy ForwardAuth). Setting LOCAL or +# GOOGLE will break authentication — the backend routes for those +# modes are not registered in this fork. +AUTH_TYPE=SSO REGISTRATION_ENABLED=TRUE or FALSE # mPass proxy auth bypass paths (oauth2-proxy ForwardAuth integration) diff --git a/surfsense_backend/app/app.py b/surfsense_backend/app/app.py index 047852d81..8d3f6bb89 100644 --- a/surfsense_backend/app/app.py +++ b/surfsense_backend/app/app.py @@ -38,8 +38,6 @@ from app.schemas import UserCreate, UserRead, UserUpdate from app.tasks.surfsense_docs_indexer import seed_surfsense_docs from app.users import ( - SECRET, - auth_backend, current_active_user, fastapi_users, get_user_manager, diff --git a/surfsense_backend/app/config/__init__.py b/surfsense_backend/app/config/__init__.py index 0d6858dee..36867c7a0 100644 --- a/surfsense_backend/app/config/__init__.py +++ b/surfsense_backend/app/config/__init__.py @@ -307,8 +307,9 @@ def is_cloud(cls) -> bool: os.getenv("STRIPE_RECONCILIATION_BATCH_SIZE", "100") ) - # Auth - AUTH_TYPE = os.getenv("AUTH_TYPE") + # Auth — this fork is SSO-only. Default to SSO so a missing envvar doesn't + # silently fall through to upstream LOCAL behaviour; app.py asserts on boot. + AUTH_TYPE = os.getenv("AUTH_TYPE", "SSO") REGISTRATION_ENABLED = os.getenv("REGISTRATION_ENABLED", "TRUE").upper() == "TRUE" # Comma-separated path prefixes that bypass proxy auth (default: /health). @@ -418,26 +419,17 @@ def is_cloud(cls) -> bool: if AZURE_OPENAI_API_KEY: embedding_kwargs["azure_api_key"] = AZURE_OPENAI_API_KEY - # mPass patch: defer embedding model loading to first use so the container - # starts without the PyTorch/sentence-transformers memory spike. - # Routes that use embeddings (search, indexing) will trigger lazy init on - # first request. Auth/SSO routes are unaffected. - _embedding_kwargs = embedding_kwargs - _embedding_model_instance = None - - @classmethod - def _get_embedding_model(cls): - if cls._embedding_model_instance is None: - cls._embedding_model_instance = AutoEmbeddings.get_embeddings( - cls.EMBEDDING_MODEL, - **cls._embedding_kwargs, - ) - return cls._embedding_model_instance - - embedding_model_instance = property(lambda self: self.__class__._get_embedding_model()) + embedding_model_instance = AutoEmbeddings.get_embeddings( + EMBEDDING_MODEL, + **embedding_kwargs, + ) is_local_embedding_model = "://" not in (EMBEDDING_MODEL or "") - chunker_instance = RecursiveChunker(chunk_size=512) - code_chunker_instance = CodeChunker(chunk_size=512) + chunker_instance = RecursiveChunker( + chunk_size=getattr(embedding_model_instance, "max_seq_length", 512) + ) + code_chunker_instance = CodeChunker( + chunk_size=getattr(embedding_model_instance, "max_seq_length", 512) + ) # Reranker's Configuration | Pinecone, Cohere etc. Read more at https://github.com/AnswerDotAI/rerankers?tab=readme-ov-file#usage RERANKERS_ENABLED = os.getenv("RERANKERS_ENABLED", "FALSE").upper() == "TRUE" diff --git a/surfsense_backend/app/routes/auth_routes.py b/surfsense_backend/app/routes/auth_routes.py index 4ab6bb525..d90d09fcf 100644 --- a/surfsense_backend/app/routes/auth_routes.py +++ b/surfsense_backend/app/routes/auth_routes.py @@ -1,12 +1,9 @@ """Authentication routes for refresh token management.""" import logging -import secrets -import uuid from fastapi import APIRouter, Depends, HTTPException, Request, status from fastapi.responses import RedirectResponse -from fastapi_users.password import PasswordHelper from sqlalchemy import select from app.config import config @@ -35,45 +32,28 @@ @router.get("/proxy-login") async def proxy_login(request: Request): """ - Exchange X-Auth-Request-Email (injected by oauth2-proxy ForwardAuth via Traefik) - for a SurfSense JWT + refresh token delivered via short-lived cookies. + Exchange the oauth2-proxy ForwardAuth session for a SurfSense JWT delivered + via short-lived cookies. Flow: Browser → Traefik ForwardAuth → oauth2-proxy validates session - → sets X-Auth-Request-Email → this endpoint issues JWT + → sets X-Auth-Request-Email → ProxyAuthMiddleware resolves/creates user + → this endpoint reads request.state.proxy_user → issues JWT → sets surfsense_sso_token + surfsense_sso_refresh_token cookies (60s TTL) → redirects to / → page.tsx reads cookies → stores to localStorage → /dashboard + + All user provisioning (including on_after_register side effects — + default SearchSpace, RBAC roles, system prompts) is owned by + ProxyAuthMiddleware. This handler does not touch the User table. """ - email = request.headers.get("x-auth-request-email") - if not email: + user: User | None = getattr(request.state, "proxy_user", None) + if user is None: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, - detail="No proxy auth header — request did not pass through oauth2-proxy ForwardAuth", + detail="No proxy auth session — request did not pass through oauth2-proxy ForwardAuth", ) - email = email.strip().lower() - - async with async_session_maker() as session: - result = await session.execute(select(User).where(User.email == email)) - user = result.unique().scalar_one_or_none() - - if user is None: - # JIT provisioning — create the user on first SSO login. - # A random password is set (unused; auth is always via the SSO proxy). - logger.info("proxy_login: first SSO login for %s — provisioning user", email) - _ph = PasswordHelper() - user = User( - id=uuid.uuid4(), - email=email, - hashed_password=_ph.hash(secrets.token_urlsafe(32)), - is_active=True, - is_verified=True, - is_superuser=False, - ) - session.add(user) - await session.commit() - await session.refresh(user) - + # Middleware already filters inactive users; defence-in-depth re-check. if not user.is_active: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, @@ -93,7 +73,7 @@ async def proxy_login(request: Request): response.set_cookie("surfsense_sso_token", access_token, **cookie_opts) response.set_cookie("surfsense_sso_refresh_token", refresh_token, **cookie_opts) - logger.info("proxy_login: issued JWT for %s → redirecting to frontend via cookie", email) + logger.info("proxy_login: issued JWT for %s → redirecting to frontend via cookie", user.email) return response diff --git a/surfsense_backend/tests/unit/routes/test_proxy_login.py b/surfsense_backend/tests/unit/routes/test_proxy_login.py index f1ce02ab6..ced4bfed8 100644 --- a/surfsense_backend/tests/unit/routes/test_proxy_login.py +++ b/surfsense_backend/tests/unit/routes/test_proxy_login.py @@ -1,28 +1,27 @@ """ Unit tests for the proxy_login endpoint. +proxy_login does not touch the User table. All provisioning (including +on_after_register side effects — default SearchSpace, RBAC roles, system +prompts) is owned by ProxyAuthMiddleware. This endpoint only reads +request.state.proxy_user (set upstream by the middleware) and issues a JWT +via short-lived cookies. + SPEC (GIVEN / WHEN / THEN) ────────────────────────── - 1 GIVEN no X-Auth-Request-Email header + 1 GIVEN request.state.proxy_user is unset WHEN GET /auth/jwt/proxy-login is called THEN 401 Unauthorized is returned - 2 GIVEN email header present AND user already in DB AND user is active + 2 GIVEN request.state.proxy_user is an active user WHEN GET /auth/jwt/proxy-login is called THEN 302 redirect to frontend / with surfsense_sso_token and surfsense_sso_refresh_token cookies set - 3 GIVEN email header present AND user NOT in DB - WHEN GET /auth/jwt/proxy-login is called - THEN user is JIT-provisioned, 302 redirect with cookies set - - 4 GIVEN email header present AND user is inactive + 3 GIVEN request.state.proxy_user is an inactive user WHEN GET /auth/jwt/proxy-login is called - THEN 401 Unauthorized is returned - - 5 GIVEN email header with mixed-case and whitespace - WHEN GET /auth/jwt/proxy-login is called - THEN lookup uses lowercased/stripped email (case-insensitive match) + THEN 401 Unauthorized is returned (defence-in-depth; middleware + normally filters inactive users before this point) """ from __future__ import annotations @@ -32,7 +31,6 @@ import pytest from starlette.requests import Request -from starlette.testclient import TestClient pytestmark = pytest.mark.unit @@ -45,6 +43,8 @@ def _make_request( path: str = "/auth/jwt/proxy-login", headers: dict[str, str] | None = None, + proxy_user=None, + scheme: str = "https", ) -> Request: raw_headers = [(k.lower().encode(), v.encode()) for k, v in (headers or {}).items()] scope = { @@ -55,14 +55,15 @@ def _make_request( "query_string": b"", "root_path": "", "server": ("localhost", 80), + "scheme": scheme, } - return Request(scope) + request = Request(scope) + if proxy_user is not None: + request.state.proxy_user = proxy_user + return request -def _make_user( - email: str = _EMAIL, - is_active: bool = True, -) -> MagicMock: +def _make_user(email: str = _EMAIL, is_active: bool = True) -> MagicMock: user = MagicMock() user.id = uuid.uuid4() user.email = email @@ -70,30 +71,6 @@ def _make_user( return user -def _make_session_cm(session: AsyncMock) -> MagicMock: - cm = MagicMock() - cm.__aenter__ = AsyncMock(return_value=session) - cm.__aexit__ = AsyncMock(return_value=False) - return cm - - -def _make_execute_result(user): - """ - Mock the SQLAlchemy result chain used by proxy_login: - result = await session.execute(...) - user = result.unique().scalar_one_or_none() - - The intermediate `.unique()` call is easy to miss when mocking — without it, - the test gets a stray MagicMock back instead of `user` (or `None`), which - silently masks bugs in the new-user provisioning path. - """ - result = MagicMock() - unique = MagicMock() - unique.scalar_one_or_none.return_value = user - result.unique.return_value = unique - return result - - # ── tests ────────────────────────────────────────────────────────────────────── @@ -109,7 +86,6 @@ class TestProxyLoginRouteRegistration: """ def test_proxy_login_route_is_registered(self): - """GIVEN the auth router is imported THEN /auth/jwt/proxy-login is one of its routes.""" from app.routes.auth_routes import router paths = [route.path for route in router.routes] @@ -118,28 +94,22 @@ def test_proxy_login_route_is_registered(self): ) def test_proxy_login_route_uses_get_method(self): - """GIVEN the route is registered THEN it accepts GET (browser navigation).""" from app.routes.auth_routes import router matching = [r for r in router.routes if r.path == "/auth/jwt/proxy-login"] - assert len(matching) == 1, f"expected exactly one proxy-login route, found {len(matching)}" - assert "GET" in matching[0].methods, ( - f"proxy-login must accept GET (302 cookie handoff). Methods: {matching[0].methods}" - ) + assert len(matching) == 1 + assert "GET" in matching[0].methods def test_proxy_login_route_calls_proxy_login_function(self): - """GIVEN the route is registered THEN it dispatches to the proxy_login function.""" from app.routes.auth_routes import proxy_login, router matching = [r for r in router.routes if r.path == "/auth/jwt/proxy-login"] - assert matching[0].endpoint is proxy_login, ( - "route is registered but points to a different function" - ) + assert matching[0].endpoint is proxy_login @pytest.mark.unit class TestLocalAuthRoutesAreNotRegistered: - """In SSO mode, the local login/register/forgot-password routes must not exist.""" + """This fork is SSO-only — native fastapi-users routers must never exist.""" def test_local_auth_routes_are_not_registered(self): from app.app import app @@ -168,144 +138,63 @@ def test_local_auth_routes_are_not_registered(self): @pytest.mark.unit class TestProxyLogin: - @pytest.mark.asyncio - async def test_no_email_header_returns_401(self): - """GIVEN no X-Auth-Request-Email header THEN 401.""" - from app.routes.auth_routes import proxy_login + async def test_no_proxy_user_returns_401(self): + """GIVEN request.state.proxy_user unset THEN 401.""" + from fastapi import HTTPException - request = _make_request() # no email header + from app.routes.auth_routes import proxy_login - from fastapi import HTTPException + request = _make_request() # no proxy_user with pytest.raises(HTTPException) as exc_info: await proxy_login(request) assert exc_info.value.status_code == 401 @pytest.mark.asyncio - async def test_existing_active_user_gets_redirect_with_cookies(self): - """GIVEN existing active user THEN 302 + SSO cookies set.""" + async def test_active_user_gets_redirect_with_cookies(self): + """GIVEN active proxy_user THEN 302 + SSO cookies, DB never touched.""" from app.routes.auth_routes import proxy_login - request = _make_request(headers={"x-auth-request-email": _EMAIL}) - user = _make_user(email=_EMAIL, is_active=True) - - session = AsyncMock() - session.execute = AsyncMock(return_value=_make_execute_result(user)) - session_cm = _make_session_cm(session) + user = _make_user(is_active=True) + request = _make_request(proxy_user=user) mock_strategy = AsyncMock() mock_strategy.write_token = AsyncMock(return_value="mock-access-token") with ( - patch("app.routes.auth_routes.async_session_maker", return_value=session_cm), patch("app.routes.auth_routes.get_jwt_strategy", return_value=mock_strategy), - patch("app.routes.auth_routes.create_refresh_token", AsyncMock(return_value="mock-refresh-token")), + patch( + "app.routes.auth_routes.create_refresh_token", + AsyncMock(return_value="mock-refresh-token"), + ), patch("app.routes.auth_routes.config") as mock_config, + patch("app.routes.auth_routes.async_session_maker") as mock_sm, ): mock_config.NEXT_FRONTEND_URL = _FRONTEND_URL response = await proxy_login(request) + mock_sm.assert_not_called() assert response.status_code == 302 - location = response.headers["location"] - assert location == f"{_FRONTEND_URL}/" + assert response.headers["location"] == f"{_FRONTEND_URL}/" - cookie_header = response.headers.get("set-cookie", "") - # RedirectResponse may set multiple cookies — check raw headers raw_headers = [(k, v) for k, v in response.raw_headers if k == b"set-cookie"] cookie_values = [v.decode() for _, v in raw_headers] assert any("surfsense_sso_token=mock-access-token" in c for c in cookie_values) - assert any("surfsense_sso_refresh_token=mock-refresh-token" in c for c in cookie_values) + assert any( + "surfsense_sso_refresh_token=mock-refresh-token" in c for c in cookie_values + ) @pytest.mark.asyncio - async def test_new_user_is_provisioned_and_gets_redirect(self): - """GIVEN user not in DB THEN JIT-provisioned + 302 with cookies.""" - from app.routes.auth_routes import proxy_login - - request = _make_request(headers={"x-auth-request-email": _EMAIL}) - - # SELECT returns None (no existing user); after add+commit, refresh populates user - new_user = _make_user(email=_EMAIL, is_active=True) - session = AsyncMock() - session.add = MagicMock() - session.execute = AsyncMock(return_value=_make_execute_result(None)) - session.refresh = AsyncMock(side_effect=lambda u: setattr(u, "id", new_user.id)) - - session_cm = _make_session_cm(session) - - mock_strategy = AsyncMock() - mock_strategy.write_token = AsyncMock(return_value="new-access-token") - - with ( - patch("app.routes.auth_routes.async_session_maker", return_value=session_cm), - patch("app.routes.auth_routes.get_jwt_strategy", return_value=mock_strategy), - patch("app.routes.auth_routes.create_refresh_token", AsyncMock(return_value="new-refresh-token")), - patch("app.routes.auth_routes.config") as mock_config, - ): - mock_config.NEXT_FRONTEND_URL = _FRONTEND_URL - response = await proxy_login(request) - - session.add.assert_called_once() - session.commit.assert_called_once() - - assert response.status_code == 302 - assert response.headers["location"] == f"{_FRONTEND_URL}/" + async def test_inactive_proxy_user_returns_401(self): + """GIVEN inactive proxy_user THEN 401.""" + from fastapi import HTTPException - @pytest.mark.asyncio - async def test_inactive_user_returns_401(self): - """GIVEN inactive user THEN 401.""" from app.routes.auth_routes import proxy_login - request = _make_request(headers={"x-auth-request-email": _EMAIL}) - inactive = _make_user(email=_EMAIL, is_active=False) - - session = AsyncMock() - session.execute = AsyncMock(return_value=_make_execute_result(inactive)) - session_cm = _make_session_cm(session) - - from fastapi import HTTPException - with ( - patch("app.routes.auth_routes.async_session_maker", return_value=session_cm), - pytest.raises(HTTPException) as exc_info, - ): + request = _make_request(proxy_user=_make_user(is_active=False)) + with pytest.raises(HTTPException) as exc_info: await proxy_login(request) assert exc_info.value.status_code == 401 - @pytest.mark.asyncio - async def test_email_is_lowercased_and_stripped_before_lookup(self): - """GIVEN mixed-case email with whitespace THEN lookup uses normalised form.""" - from app.routes.auth_routes import proxy_login - - raw_email = " ALICE@EXAMPLE.COM " - normalised = "alice@example.com" - user = _make_user(email=normalised, is_active=True) - - request = _make_request(headers={"x-auth-request-email": raw_email}) - - captured_queries = [] - - async def mock_execute(stmt): - captured_queries.append(stmt) - return _make_execute_result(user) - - session = AsyncMock() - session.execute = mock_execute - session_cm = _make_session_cm(session) - - mock_strategy = AsyncMock() - mock_strategy.write_token = AsyncMock(return_value="token") - - with ( - patch("app.routes.auth_routes.async_session_maker", return_value=session_cm), - patch("app.routes.auth_routes.get_jwt_strategy", return_value=mock_strategy), - patch("app.routes.auth_routes.create_refresh_token", AsyncMock(return_value="rt")), - patch("app.routes.auth_routes.config") as mock_config, - ): - mock_config.NEXT_FRONTEND_URL = _FRONTEND_URL - response = await proxy_login(request) - - assert response.status_code == 302 - # Confirm the query used the lowercased/stripped email by checking - # that the user was found (i.e. not provisioned — add never called). - # (Deep SQLAlchemy AST inspection would be brittle; the 302 + no INSERT is sufficient.) From f66f9e3fd1b263a5bff72b3340a8d0ea9eb5ea28 Mon Sep 17 00:00:00 2001 From: awais786 Date: Sun, 12 Apr 2026 22:20:48 +0500 Subject: [PATCH 03/29] fix(sso): redirect logout to platform landing page instead of app origin 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. --- surfsense_web/lib/auth-utils.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/surfsense_web/lib/auth-utils.ts b/surfsense_web/lib/auth-utils.ts index 29ef6ace3..a510fc503 100644 --- a/surfsense_web/lib/auth-utils.ts +++ b/surfsense_web/lib/auth-utils.ts @@ -255,7 +255,12 @@ export async function logout(): Promise { if (oidcLogoutUrl && oidcClientId) { const cognitoUrl = new URL(oidcLogoutUrl); cognitoUrl.searchParams.set("client_id", oidcClientId); - cognitoUrl.searchParams.set("logout_uri", window.location.origin); + // Redirect to the platform landing page after Cognito clears its session. + // The landing page is NOT behind ForwardAuth, so the user sees it instead + // of being bounced back to Cognito login. Falls back to current origin + // for deployments without the env var. + const logoutRedirect = process.env.NEXT_PUBLIC_LOGOUT_REDIRECT_URL || window.location.origin; + cognitoUrl.searchParams.set("logout_uri", logoutRedirect); // Full SSO logout: oauth2-proxy sign_out clears _oauth2_proxy cookie, // then rd= redirects to Cognito to clear the Cognito session. From 113dbd096d648cbc696086fe67b4a84fedfd9677 Mon Sep 17 00:00:00 2001 From: awais786 Date: Sun, 12 Apr 2026 22:42:10 +0500 Subject: [PATCH 04/29] fix(auth): re-fetch proxy_user in fresh session to prevent detached-instance crash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- surfsense_backend/app/users.py | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/surfsense_backend/app/users.py b/surfsense_backend/app/users.py index e35cb9991..dc8c8b05f 100644 --- a/surfsense_backend/app/users.py +++ b/surfsense_backend/app/users.py @@ -301,6 +301,30 @@ async def get_login_response(self, token: str) -> Response: _jwt_current_optional_user = fastapi_users.current_user(active=True, optional=True) +async def _refetch_proxy_user(proxy_user: User) -> User: + """ + Re-fetch the proxy_user in a fresh session so it's attached and persistent. + + ProxyAuthMiddleware resolves the user in its own async session which closes + before the route handler runs. The User object left on request.state is + detached — any downstream handler that tries session.refresh(user) or + accesses lazy-loaded relationships in a new session will get + "Instance is not persistent within this Session". Re-fetching by ID in a + fresh session gives every handler a properly attached object. + """ + from sqlalchemy import select # avoid circular import at module level + + async with async_session_maker() as session: + result = await session.execute(select(User).where(User.id == proxy_user.id)) + fresh = result.unique().scalar_one_or_none() + if fresh is not None: + # Expunge so the object outlives this session without being + # "detached-from-a-closed-session" — it becomes transient, + # and any downstream handler can session.merge() it if needed. + session.expunge(fresh) + return fresh or proxy_user + + async def current_active_user( request: Request, jwt_user: User | None = Depends(_jwt_current_optional_user), @@ -315,7 +339,7 @@ async def current_active_user( """ proxy_user = getattr(request.state, "proxy_user", None) if proxy_user is not None: - return proxy_user + return await _refetch_proxy_user(proxy_user) if jwt_user is not None: return jwt_user raise HTTPException( @@ -329,4 +353,6 @@ async def current_optional_user( jwt_user: User | None = Depends(_jwt_current_optional_user), ) -> User | None: proxy_user = getattr(request.state, "proxy_user", None) - return proxy_user if proxy_user is not None else jwt_user + if proxy_user is not None: + return await _refetch_proxy_user(proxy_user) + return jwt_user From 2c70f721697e23e0ed01a716633ba0b2bd8634d2 Mon Sep 17 00:00:00 2001 From: awais786 Date: Mon, 13 Apr 2026 00:40:46 +0500 Subject: [PATCH 05/29] fix(sso): prevent logout redirect race + fix double-encoded Cognito URL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- surfsense_web/components/UserDropdown.tsx | 18 ++++++++++++------ .../layout/providers/LayoutDataProvider.tsx | 13 +++++++------ surfsense_web/lib/auth-utils.ts | 10 ++++++++-- 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/surfsense_web/components/UserDropdown.tsx b/surfsense_web/components/UserDropdown.tsx index 0ffb172bf..ee98baf66 100644 --- a/surfsense_web/components/UserDropdown.tsx +++ b/surfsense_web/components/UserDropdown.tsx @@ -38,15 +38,21 @@ export function UserDropdown({ trackLogout(); resetUser(); - await logout(); + // Revoke refresh token on server and clear all tokens from localStorage. + // logout() returns true and sets window.location.href when SSO is + // configured — don't overwrite it with a local redirect. + const ssoRedirected = await logout(); - router.push(getLoginPath()); - router.refresh(); + if (!ssoRedirected && typeof window !== "undefined") { + window.location.href = "/"; + } } catch (error) { console.error("Error during logout:", error); - await logout(); - router.push(getLoginPath()); - router.refresh(); + // Even if there's an error, try to clear tokens and redirect + const ssoRedirected = await logout(); + if (!ssoRedirected && typeof window !== "undefined") { + window.location.href = "/"; + } } }; diff --git a/surfsense_web/components/layout/providers/LayoutDataProvider.tsx b/surfsense_web/components/layout/providers/LayoutDataProvider.tsx index 7e9c33a1a..6184f164f 100644 --- a/surfsense_web/components/layout/providers/LayoutDataProvider.tsx +++ b/surfsense_web/components/layout/providers/LayoutDataProvider.tsx @@ -599,16 +599,17 @@ export function LayoutDataProvider({ searchSpaceId, children }: LayoutDataProvid trackLogout(); resetUser(); - // Revoke refresh token on server and clear all tokens from localStorage - await logout(); + // Revoke refresh token on server and clear all tokens from localStorage. + // logout() returns true when SSO redirect is in progress — don't overwrite. + const ssoRedirected = await logout(); - if (typeof window !== "undefined") { - router.push(getLoginPath()); + if (!ssoRedirected && typeof window !== "undefined") { + router.push("/"); } } catch (error) { console.error("Error during logout:", error); - await logout(); - router.push(getLoginPath()); + const ssoRedirected = await logout(); + if (!ssoRedirected) router.push("/"); } }, [router]); diff --git a/surfsense_web/lib/auth-utils.ts b/surfsense_web/lib/auth-utils.ts index a510fc503..cfcc7903b 100644 --- a/surfsense_web/lib/auth-utils.ts +++ b/surfsense_web/lib/auth-utils.ts @@ -267,12 +267,18 @@ export async function logout(): Promise { // Uses the dedicated auth domain (foss-auth.localhost) so the sign_out // URL is consistent regardless of which app initiates the logout. const oauthProxyUrl = process.env.NEXT_PUBLIC_OAUTH2_PROXY_URL || window.location.origin; - window.location.href = `${oauthProxyUrl}/oauth2/sign_out?rd=${encodeURIComponent(cognitoUrl.toString())}`; + // Single-encode the Cognito URL so oauth2-proxy decodes it once + // and passes a clean logout_uri to Cognito (matching Plane's pattern). + // encodeURIComponent would double-encode the query params, causing + // Cognito to reject the logout_uri as unregistered. + const cognitoStr = cognitoUrl.toString(); + const rdParam = cognitoStr.replace(/\?/, '%3F').replace(/&/g, '%26').replace(/=/g, '%3D'); + window.location.href = `${oauthProxyUrl}/oauth2/sign_out?rd=${rdParam}`; return true; // browser is already navigating away } } - return true; + return false; // no SSO redirect — caller should navigate } /** From cef4259144b2c2d68d68b2cd3fa6dd917d4a4f3d Mon Sep 17 00:00:00 2001 From: awais786 Date: Mon, 13 Apr 2026 01:14:28 +0500 Subject: [PATCH 06/29] fix(auth): handle detached proxy_user at point-of-use instead of refetch 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. --- surfsense_backend/app/app.py | 8 ++++- .../app/routes/incentive_tasks_routes.py | 4 +++ surfsense_backend/app/users.py | 30 ++----------------- 3 files changed, 13 insertions(+), 29 deletions(-) diff --git a/surfsense_backend/app/app.py b/surfsense_backend/app/app.py index 8d3f6bb89..3689fc04a 100644 --- a/surfsense_backend/app/app.py +++ b/surfsense_backend/app/app.py @@ -397,7 +397,13 @@ async def update_current_user_me( user: User = Depends(current_active_user), user_manager=Depends(get_user_manager), ): - return await user_manager.update(user_update, user, safe=True, request=request) + # Re-fetch in user_manager's session to avoid detached-instance conflict. + # ProxyAuthMiddleware's User object is from a closed session; the JWT + # dependency also loaded the same user in user_manager's session. + # Passing the middleware's object to session.add() conflicts with the + # JWT-loaded one. user_manager.get() returns the session-attached instance. + db_user = await user_manager.get(user.id) + return await user_manager.update(user_update, db_user, safe=True, request=request) app.include_router( diff --git a/surfsense_backend/app/routes/incentive_tasks_routes.py b/surfsense_backend/app/routes/incentive_tasks_routes.py index 496b07d06..e177542ca 100644 --- a/surfsense_backend/app/routes/incentive_tasks_routes.py +++ b/surfsense_backend/app/routes/incentive_tasks_routes.py @@ -117,6 +117,10 @@ async def complete_task( ) session.add(new_task) + # The user object may be detached (from ProxyAuthMiddleware's closed + # session). Merge it into this session before modifying + committing. + user = await session.merge(user) + # pages_used can exceed pages_limit when a document's final page count is # determined after processing. Base the new limit on the higher of the two # so the rewarded pages are fully usable above the current high-water mark. diff --git a/surfsense_backend/app/users.py b/surfsense_backend/app/users.py index dc8c8b05f..e35cb9991 100644 --- a/surfsense_backend/app/users.py +++ b/surfsense_backend/app/users.py @@ -301,30 +301,6 @@ async def get_login_response(self, token: str) -> Response: _jwt_current_optional_user = fastapi_users.current_user(active=True, optional=True) -async def _refetch_proxy_user(proxy_user: User) -> User: - """ - Re-fetch the proxy_user in a fresh session so it's attached and persistent. - - ProxyAuthMiddleware resolves the user in its own async session which closes - before the route handler runs. The User object left on request.state is - detached — any downstream handler that tries session.refresh(user) or - accesses lazy-loaded relationships in a new session will get - "Instance is not persistent within this Session". Re-fetching by ID in a - fresh session gives every handler a properly attached object. - """ - from sqlalchemy import select # avoid circular import at module level - - async with async_session_maker() as session: - result = await session.execute(select(User).where(User.id == proxy_user.id)) - fresh = result.unique().scalar_one_or_none() - if fresh is not None: - # Expunge so the object outlives this session without being - # "detached-from-a-closed-session" — it becomes transient, - # and any downstream handler can session.merge() it if needed. - session.expunge(fresh) - return fresh or proxy_user - - async def current_active_user( request: Request, jwt_user: User | None = Depends(_jwt_current_optional_user), @@ -339,7 +315,7 @@ async def current_active_user( """ proxy_user = getattr(request.state, "proxy_user", None) if proxy_user is not None: - return await _refetch_proxy_user(proxy_user) + return proxy_user if jwt_user is not None: return jwt_user raise HTTPException( @@ -353,6 +329,4 @@ async def current_optional_user( jwt_user: User | None = Depends(_jwt_current_optional_user), ) -> User | None: proxy_user = getattr(request.state, "proxy_user", None) - if proxy_user is not None: - return await _refetch_proxy_user(proxy_user) - return jwt_user + return proxy_user if proxy_user is not None else jwt_user From 6a66d6ef86946f30d229682871ed56e3495c060d Mon Sep 17 00:00:00 2001 From: awais786 Date: Tue, 14 Apr 2026 11:10:43 +0500 Subject: [PATCH 07/29] ci: gate tests on PRs targeting foss-main 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. --- .github/workflows/backend-tests.yml | 2 +- .github/workflows/code-quality.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/backend-tests.yml b/.github/workflows/backend-tests.yml index 6e0a6db6d..6a40482d5 100644 --- a/.github/workflows/backend-tests.yml +++ b/.github/workflows/backend-tests.yml @@ -2,7 +2,7 @@ name: Backend Tests on: pull_request: - branches: [main, dev] + branches: [foss-main] types: [opened, synchronize, reopened, ready_for_review] concurrency: diff --git a/.github/workflows/code-quality.yml b/.github/workflows/code-quality.yml index 0dd2e1809..d6be94dbb 100644 --- a/.github/workflows/code-quality.yml +++ b/.github/workflows/code-quality.yml @@ -2,7 +2,7 @@ name: Code Quality Checks on: pull_request: - branches: [main, dev] + branches: [foss-main] types: [opened, synchronize, reopened, ready_for_review] concurrency: From 1695b8ca0b8bd1b69fb4fa3f356fdf51134346b7 Mon Sep 17 00:00:00 2001 From: awais786 Date: Tue, 14 Apr 2026 11:58:07 +0500 Subject: [PATCH 08/29] test(auth): align unit + integration tests with SSO-only auth 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 (96a9ed6b), 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. --- .../test_stripe_page_purchases.py | 44 +------------------ .../tests/unit/middleware/test_proxy_auth.py | 16 +++---- surfsense_backend/tests/utils/helpers.py | 40 +++++++---------- 3 files changed, 26 insertions(+), 74 deletions(-) diff --git a/surfsense_backend/tests/integration/document_upload/test_stripe_page_purchases.py b/surfsense_backend/tests/integration/document_upload/test_stripe_page_purchases.py index 1c8f7f990..3fd5214a1 100644 --- a/surfsense_backend/tests/integration/document_upload/test_stripe_page_purchases.py +++ b/surfsense_backend/tests/integration/document_upload/test_stripe_page_purchases.py @@ -1,7 +1,6 @@ from __future__ import annotations from types import SimpleNamespace -from urllib.parse import parse_qs, urlparse import asyncpg import httpx @@ -13,7 +12,7 @@ from app.routes import stripe_routes from app.tasks.celery_tasks import stripe_reconciliation_task from tests.conftest import TEST_DATABASE_URL -from tests.utils.helpers import TEST_EMAIL, TEST_PASSWORD, auth_headers +from tests.utils.helpers import auth_headers, get_auth_token pytestmark = pytest.mark.integration @@ -48,51 +47,12 @@ async def _get_pages_limit(email: str) -> int: return row["pages_limit"] -def _extract_access_token(response: httpx.Response) -> str | None: - if response.status_code == 200: - return response.json()["access_token"] - - if response.status_code == 302: - location = response.headers.get("location", "") - return parse_qs(urlparse(location).query).get("token", [None])[0] - - return None - - -async def _authenticate_test_user(client: httpx.AsyncClient) -> str: - response = await client.post( - "/auth/jwt/login", - data={"username": TEST_EMAIL, "password": TEST_PASSWORD}, - headers={"Content-Type": "application/x-www-form-urlencoded"}, - ) - token = _extract_access_token(response) - if token: - return token - - reg_response = await client.post( - "/auth/register", - json={"email": TEST_EMAIL, "password": TEST_PASSWORD}, - ) - assert reg_response.status_code == 201, ( - f"Registration failed ({reg_response.status_code}): {reg_response.text}" - ) - - response = await client.post( - "/auth/jwt/login", - data={"username": TEST_EMAIL, "password": TEST_PASSWORD}, - headers={"Content-Type": "application/x-www-form-urlencoded"}, - ) - token = _extract_access_token(response) - assert token, f"Login failed ({response.status_code}): {response.text}" - return token - - @pytest_asyncio.fixture(scope="session") async def auth_token(_ensure_tables) -> str: async with httpx.AsyncClient( transport=ASGITransport(app=app), base_url="http://test", timeout=30.0 ) as client: - return await _authenticate_test_user(client) + return await get_auth_token(client) @pytest.fixture(scope="session") diff --git a/surfsense_backend/tests/unit/middleware/test_proxy_auth.py b/surfsense_backend/tests/unit/middleware/test_proxy_auth.py index 6c91a444d..d6885e32b 100644 --- a/surfsense_backend/tests/unit/middleware/test_proxy_auth.py +++ b/surfsense_backend/tests/unit/middleware/test_proxy_auth.py @@ -259,7 +259,7 @@ async def test_new_user_created_and_on_after_register_called(self): s1 = AsyncMock() s1.add = MagicMock() # add() is sync; avoid AsyncMock warning no_user_result = MagicMock() - no_user_result.scalar_one_or_none.return_value = None + no_user_result.unique.return_value.scalar_one_or_none.return_value = None update_result = MagicMock() s1.execute = AsyncMock(side_effect=[no_user_result, update_result]) s1_cm = _make_session_cm(s1) @@ -268,7 +268,7 @@ async def test_new_user_created_and_on_after_register_called(self): reg_user = _make_user(email=_EMAIL) s2 = AsyncMock() reg_result = MagicMock() - reg_result.scalar_one_or_none.return_value = reg_user + reg_result.unique.return_value.scalar_one_or_none.return_value = reg_user s2.execute = AsyncMock(return_value=reg_result) s2_cm = _make_session_cm(s2) @@ -305,7 +305,7 @@ async def test_existing_user_found_no_insert(self): session = AsyncMock() found_result = MagicMock() - found_result.scalar_one_or_none.return_value = existing + found_result.unique.return_value.scalar_one_or_none.return_value = existing update_result = MagicMock() session.execute = AsyncMock(side_effect=[found_result, update_result]) session_cm = _make_session_cm(session) @@ -332,7 +332,7 @@ async def test_inactive_user_passes_through_unauthenticated(self): session = AsyncMock() found_result = MagicMock() - found_result.scalar_one_or_none.return_value = inactive + found_result.unique.return_value.scalar_one_or_none.return_value = inactive session.execute = AsyncMock(return_value=found_result) session_cm = _make_session_cm(session) @@ -357,7 +357,7 @@ async def test_valid_email_sets_proxy_user_to_resolved_user(self): session = AsyncMock() found_result = MagicMock() - found_result.scalar_one_or_none.return_value = user + found_result.unique.return_value.scalar_one_or_none.return_value = user # last_login is recent → needs_update=False → only one execute call session.execute = AsyncMock(return_value=found_result) session_cm = _make_session_cm(session) @@ -386,7 +386,7 @@ async def test_email_with_uppercase_and_whitespace_is_normalised(self): session = AsyncMock() found_result = MagicMock() - found_result.scalar_one_or_none.return_value = user + found_result.unique.return_value.scalar_one_or_none.return_value = user session.execute = AsyncMock(return_value=found_result) session_cm = _make_session_cm(session) @@ -422,10 +422,10 @@ async def test_race_condition_fallback_select_sets_proxy_user(self): session.add = MagicMock() # add() is sync; avoid AsyncMock warning # Call 1: initial SELECT — no user found no_user_result = MagicMock() - no_user_result.scalar_one_or_none.return_value = None + no_user_result.unique.return_value.scalar_one_or_none.return_value = None # Call 2: fallback SELECT after rollback — race_user found fallback_result = MagicMock() - fallback_result.scalar_one_or_none.return_value = race_user + fallback_result.unique.return_value.scalar_one_or_none.return_value = race_user session.execute = AsyncMock(side_effect=[no_user_result, fallback_result]) # INSERT commit raises IntegrityError; any subsequent commits should succeed session.commit = AsyncMock(side_effect=[IntegrityError(None, None, None), None]) diff --git a/surfsense_backend/tests/utils/helpers.py b/surfsense_backend/tests/utils/helpers.py index c5719a253..1db96eb36 100644 --- a/surfsense_backend/tests/utils/helpers.py +++ b/surfsense_backend/tests/utils/helpers.py @@ -10,36 +10,28 @@ FIXTURES_DIR = Path(__file__).resolve().parent.parent / "fixtures" TEST_EMAIL = "testuser@surfsense.com" -TEST_PASSWORD = "testpassword123" async def get_auth_token(client: httpx.AsyncClient) -> str: - """Log in and return a Bearer JWT token, registering the user first if needed.""" - response = await client.post( - "/auth/jwt/login", - data={"username": TEST_EMAIL, "password": TEST_PASSWORD}, - headers={"Content-Type": "application/x-www-form-urlencoded"}, - ) - if response.status_code == 200: - return response.json()["access_token"] + """Obtain a Bearer JWT via the SSO proxy-login endpoint. - reg_response = await client.post( - "/auth/register", - json={"email": TEST_EMAIL, "password": TEST_PASSWORD}, - ) - assert reg_response.status_code == 201, ( - f"Registration failed ({reg_response.status_code}): {reg_response.text}" - ) - - response = await client.post( - "/auth/jwt/login", - data={"username": TEST_EMAIL, "password": TEST_PASSWORD}, - headers={"Content-Type": "application/x-www-form-urlencoded"}, + Local auth routes (/auth/register, /auth/jwt/login) are disabled in SSO + mode, so the test user is provisioned the same way production users are: + ProxyAuthMiddleware reads X-Auth-Request-Email and JIT-creates the user, + then /auth/jwt/proxy-login issues a JWT delivered via the + surfsense_sso_token cookie on a 302 redirect. + """ + response = await client.get( + "/auth/jwt/proxy-login", + headers={"X-Auth-Request-Email": TEST_EMAIL}, + follow_redirects=False, ) - assert response.status_code == 200, ( - f"Login after registration failed ({response.status_code}): {response.text}" + assert response.status_code == 302, ( + f"proxy-login failed ({response.status_code}): {response.text}" ) - return response.json()["access_token"] + token = response.cookies.get("surfsense_sso_token") + assert token, f"surfsense_sso_token cookie missing from proxy-login response: {response.headers!r}" + return token async def get_search_space_id(client: httpx.AsyncClient, token: str) -> int: From d15cb6c85508a8ce9c835836dcc559410b9b68eb Mon Sep 17 00:00:00 2001 From: awais786 Date: Tue, 14 Apr 2026 12:18:57 +0500 Subject: [PATCH 09/29] test(stripe): restore TEST_EMAIL import and pin STRIPE_PAGE_BUYING_ENABLED 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. --- .../document_upload/test_stripe_page_purchases.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/surfsense_backend/tests/integration/document_upload/test_stripe_page_purchases.py b/surfsense_backend/tests/integration/document_upload/test_stripe_page_purchases.py index 3fd5214a1..9ac4057ca 100644 --- a/surfsense_backend/tests/integration/document_upload/test_stripe_page_purchases.py +++ b/surfsense_backend/tests/integration/document_upload/test_stripe_page_purchases.py @@ -12,7 +12,7 @@ from app.routes import stripe_routes from app.tasks.celery_tasks import stripe_reconciliation_task from tests.conftest import TEST_DATABASE_URL -from tests.utils.helpers import auth_headers, get_auth_token +from tests.utils.helpers import TEST_EMAIL, auth_headers, get_auth_token pytestmark = pytest.mark.integration @@ -142,6 +142,7 @@ async def test_create_checkout_session_records_pending_purchase( fake_client = _FakeCreateStripeClient(checkout_session) monkeypatch.setattr(stripe_routes, "get_stripe_client", lambda: fake_client) + monkeypatch.setattr(stripe_routes.config, "STRIPE_PAGE_BUYING_ENABLED", True) monkeypatch.setattr(stripe_routes.config, "STRIPE_PRICE_ID", "price_pages_1000") monkeypatch.setattr( stripe_routes.config, "NEXT_FRONTEND_URL", "http://localhost:3000" @@ -229,6 +230,7 @@ async def test_webhook_grants_pages_once( create_client = _FakeCreateStripeClient(checkout_session) monkeypatch.setattr(stripe_routes, "get_stripe_client", lambda: create_client) + monkeypatch.setattr(stripe_routes.config, "STRIPE_PAGE_BUYING_ENABLED", True) monkeypatch.setattr(stripe_routes.config, "STRIPE_PRICE_ID", "price_pages_1000") monkeypatch.setattr( stripe_routes.config, "NEXT_FRONTEND_URL", "http://localhost:3000" @@ -322,6 +324,7 @@ async def test_reconciliation_fulfills_paid_pending_purchase( create_client = _FakeCreateStripeClient(checkout_session) monkeypatch.setattr(stripe_routes, "get_stripe_client", lambda: create_client) + monkeypatch.setattr(stripe_routes.config, "STRIPE_PAGE_BUYING_ENABLED", True) monkeypatch.setattr(stripe_routes.config, "STRIPE_PRICE_ID", "price_pages_1000") monkeypatch.setattr( stripe_routes.config, "NEXT_FRONTEND_URL", "http://localhost:3000" @@ -400,6 +403,7 @@ async def test_reconciliation_marks_expired_pending_purchase_failed( create_client = _FakeCreateStripeClient(checkout_session) monkeypatch.setattr(stripe_routes, "get_stripe_client", lambda: create_client) + monkeypatch.setattr(stripe_routes.config, "STRIPE_PAGE_BUYING_ENABLED", True) monkeypatch.setattr(stripe_routes.config, "STRIPE_PRICE_ID", "price_pages_1000") monkeypatch.setattr( stripe_routes.config, "NEXT_FRONTEND_URL", "http://localhost:3000" From 85a0fd2b2cc45044b79e363c1d21ef922d728252 Mon Sep 17 00:00:00 2001 From: awais786 Date: Tue, 14 Apr 2026 12:35:14 +0500 Subject: [PATCH 10/29] chore(lint): silence ruff C408 on cookie_opts dict literal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- surfsense_backend/app/routes/auth_routes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/surfsense_backend/app/routes/auth_routes.py b/surfsense_backend/app/routes/auth_routes.py index d90d09fcf..77b65af7e 100644 --- a/surfsense_backend/app/routes/auth_routes.py +++ b/surfsense_backend/app/routes/auth_routes.py @@ -69,7 +69,7 @@ async def proxy_login(request: Request): # Deliver tokens via short-lived cookies so the frontend can pick them up at / # without needing a dedicated /auth/callback route (avoids Traefik path splitting). response = RedirectResponse(f"{frontend_url}/", status_code=302) - cookie_opts = dict(httponly=False, secure=True, samesite="lax", max_age=60) + cookie_opts = dict(httponly=False, secure=True, samesite="lax", max_age=60) # noqa: C408 response.set_cookie("surfsense_sso_token", access_token, **cookie_opts) response.set_cookie("surfsense_sso_refresh_token", refresh_token, **cookie_opts) From 95e2c6a3b6b23f932b5fbab0d811278bad304495 Mon Sep 17 00:00:00 2001 From: awais786 Date: Tue, 14 Apr 2026 13:46:12 +0500 Subject: [PATCH 11/29] =?UTF-8?q?style(web):=20biome=20format=20=E2=80=94?= =?UTF-8?q?=20fix=20indent=20+=20quote=20style?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- surfsense_web/lib/auth-utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/surfsense_web/lib/auth-utils.ts b/surfsense_web/lib/auth-utils.ts index cfcc7903b..920ec4a4e 100644 --- a/surfsense_web/lib/auth-utils.ts +++ b/surfsense_web/lib/auth-utils.ts @@ -272,7 +272,7 @@ export async function logout(): Promise { // encodeURIComponent would double-encode the query params, causing // Cognito to reject the logout_uri as unregistered. const cognitoStr = cognitoUrl.toString(); - const rdParam = cognitoStr.replace(/\?/, '%3F').replace(/&/g, '%26').replace(/=/g, '%3D'); + const rdParam = cognitoStr.replace(/\?/, "%3F").replace(/&/g, "%26").replace(/=/g, "%3D"); window.location.href = `${oauthProxyUrl}/oauth2/sign_out?rd=${rdParam}`; return true; // browser is already navigating away } From 559848fe818e97f8f581f807eed123af12cb02ff Mon Sep 17 00:00:00 2001 From: awais786 Date: Tue, 14 Apr 2026 14:04:00 +0500 Subject: [PATCH 12/29] style: apply ruff + biome format drift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- surfsense_backend/app/app.py | 3 ++- surfsense_backend/app/middleware/proxy_auth.py | 4 +++- surfsense_backend/app/routes/auth_routes.py | 5 ++++- .../tests/unit/routes/test_proxy_login.py | 5 +++-- surfsense_backend/tests/utils/helpers.py | 4 +++- surfsense_web/app/(home)/login/LocalLoginForm.tsx | 2 +- surfsense_web/app/(home)/login/page.tsx | 7 ++----- surfsense_web/app/(home)/register/page.tsx | 7 ++----- surfsense_web/lib/apis/base-api.service.ts | 5 ++--- surfsense_web/lib/auth-utils.ts | 10 +++++----- 10 files changed, 27 insertions(+), 25 deletions(-) diff --git a/surfsense_backend/app/app.py b/surfsense_backend/app/app.py index 3689fc04a..bf9729996 100644 --- a/surfsense_backend/app/app.py +++ b/surfsense_backend/app/app.py @@ -35,7 +35,7 @@ from app.middleware.proxy_auth import ProxyAuthMiddleware from app.routes import router as crud_router from app.routes.auth_routes import router as auth_router -from app.schemas import UserCreate, UserRead, UserUpdate +from app.schemas import UserRead, UserUpdate from app.tasks.surfsense_docs_indexer import seed_surfsense_docs from app.users import ( current_active_user, @@ -381,6 +381,7 @@ async def dispatch( allow_headers=["*"], # Allows all headers ) + # Register /users/me BEFORE fastapi_users.get_users_router so our routes take # precedence (FastAPI first-match wins). fastapi-users' internal /users/me only # validates JWT — it does not check request.state.proxy_user set by the proxy diff --git a/surfsense_backend/app/middleware/proxy_auth.py b/surfsense_backend/app/middleware/proxy_auth.py index 523aebfcd..ec70dcdb5 100644 --- a/surfsense_backend/app/middleware/proxy_auth.py +++ b/surfsense_backend/app/middleware/proxy_auth.py @@ -86,7 +86,9 @@ async def dispatch( raw_email = request.headers.get("x-auth-request-email") if not raw_email: - logger.debug("ProxyAuth: x-auth-request-email missing on %s", request.url.path) + logger.debug( + "ProxyAuth: x-auth-request-email missing on %s", request.url.path + ) return await call_next(request) user = await self._resolve_user(_normalise_email(raw_email), request) diff --git a/surfsense_backend/app/routes/auth_routes.py b/surfsense_backend/app/routes/auth_routes.py index 77b65af7e..8e4ff430b 100644 --- a/surfsense_backend/app/routes/auth_routes.py +++ b/surfsense_backend/app/routes/auth_routes.py @@ -73,7 +73,10 @@ async def proxy_login(request: Request): response.set_cookie("surfsense_sso_token", access_token, **cookie_opts) response.set_cookie("surfsense_sso_refresh_token", refresh_token, **cookie_opts) - logger.info("proxy_login: issued JWT for %s → redirecting to frontend via cookie", user.email) + logger.info( + "proxy_login: issued JWT for %s → redirecting to frontend via cookie", + user.email, + ) return response diff --git a/surfsense_backend/tests/unit/routes/test_proxy_login.py b/surfsense_backend/tests/unit/routes/test_proxy_login.py index ced4bfed8..a4c7f4d5f 100644 --- a/surfsense_backend/tests/unit/routes/test_proxy_login.py +++ b/surfsense_backend/tests/unit/routes/test_proxy_login.py @@ -163,7 +163,9 @@ async def test_active_user_gets_redirect_with_cookies(self): mock_strategy.write_token = AsyncMock(return_value="mock-access-token") with ( - patch("app.routes.auth_routes.get_jwt_strategy", return_value=mock_strategy), + patch( + "app.routes.auth_routes.get_jwt_strategy", return_value=mock_strategy + ), patch( "app.routes.auth_routes.create_refresh_token", AsyncMock(return_value="mock-refresh-token"), @@ -197,4 +199,3 @@ async def test_inactive_proxy_user_returns_401(self): await proxy_login(request) assert exc_info.value.status_code == 401 - diff --git a/surfsense_backend/tests/utils/helpers.py b/surfsense_backend/tests/utils/helpers.py index 1db96eb36..49b0836ab 100644 --- a/surfsense_backend/tests/utils/helpers.py +++ b/surfsense_backend/tests/utils/helpers.py @@ -30,7 +30,9 @@ async def get_auth_token(client: httpx.AsyncClient) -> str: f"proxy-login failed ({response.status_code}): {response.text}" ) token = response.cookies.get("surfsense_sso_token") - assert token, f"surfsense_sso_token cookie missing from proxy-login response: {response.headers!r}" + assert token, ( + f"surfsense_sso_token cookie missing from proxy-login response: {response.headers!r}" + ) return token diff --git a/surfsense_web/app/(home)/login/LocalLoginForm.tsx b/surfsense_web/app/(home)/login/LocalLoginForm.tsx index 8b149f3f3..2dd203884 100644 --- a/surfsense_web/app/(home)/login/LocalLoginForm.tsx +++ b/surfsense_web/app/(home)/login/LocalLoginForm.tsx @@ -9,9 +9,9 @@ import { useState } from "react"; import { loginMutationAtom } from "@/atoms/auth/auth-mutation.atoms"; import { Spinner } from "@/components/ui/spinner"; import { getAuthErrorDetails, isNetworkError } from "@/lib/auth-errors"; +import { setBearerToken } from "@/lib/auth-utils"; import { AUTH_TYPE } from "@/lib/env-config"; import { ValidationError } from "@/lib/error"; -import { setBearerToken } from "@/lib/auth-utils"; import { trackLoginAttempt, trackLoginFailure, trackLoginSuccess } from "@/lib/posthog/events"; export function LocalLoginForm() { diff --git a/surfsense_web/app/(home)/login/page.tsx b/surfsense_web/app/(home)/login/page.tsx index 350954cb1..f2a6d7e85 100644 --- a/surfsense_web/app/(home)/login/page.tsx +++ b/surfsense_web/app/(home)/login/page.tsx @@ -31,12 +31,9 @@ function LoginContent() { // rendering the form lives below, after every other hook has run. useEffect(() => { if (typeof window !== "undefined" && isSSOAuth()) { - const oauthProxyUrl = - process.env.NEXT_PUBLIC_OAUTH2_PROXY_URL || window.location.origin; + const oauthProxyUrl = process.env.NEXT_PUBLIC_OAUTH2_PROXY_URL || window.location.origin; const rd = `${window.location.origin}/`; - window.location.replace( - `${oauthProxyUrl}/oauth2/sign_in?rd=${encodeURIComponent(rd)}` - ); + window.location.replace(`${oauthProxyUrl}/oauth2/sign_in?rd=${encodeURIComponent(rd)}`); } }, []); diff --git a/surfsense_web/app/(home)/register/page.tsx b/surfsense_web/app/(home)/register/page.tsx index cd479c763..b5d9c1000 100644 --- a/surfsense_web/app/(home)/register/page.tsx +++ b/surfsense_web/app/(home)/register/page.tsx @@ -45,12 +45,9 @@ export default function RegisterPage() { // is not SSO. Hook is unconditional (rules-of-hooks). useEffect(() => { if (typeof window !== "undefined" && isSSOAuth()) { - const oauthProxyUrl = - process.env.NEXT_PUBLIC_OAUTH2_PROXY_URL || window.location.origin; + const oauthProxyUrl = process.env.NEXT_PUBLIC_OAUTH2_PROXY_URL || window.location.origin; const rd = `${window.location.origin}/`; - window.location.replace( - `${oauthProxyUrl}/oauth2/sign_in?rd=${encodeURIComponent(rd)}` - ); + window.location.replace(`${oauthProxyUrl}/oauth2/sign_in?rd=${encodeURIComponent(rd)}`); } }, []); diff --git a/surfsense_web/lib/apis/base-api.service.ts b/surfsense_web/lib/apis/base-api.service.ts index a2ba55197..a0e2cda67 100644 --- a/surfsense_web/lib/apis/base-api.service.ts +++ b/surfsense_web/lib/apis/base-api.service.ts @@ -68,8 +68,8 @@ class BaseApiService { const defaultOptions: RequestOptions = { headers: this.bearerToken ? { - Authorization: `Bearer ${this.bearerToken}`, - } + Authorization: `Bearer ${this.bearerToken}`, + } : {}, method: "GET", responseType: ResponseType.JSON, @@ -359,4 +359,3 @@ class BaseApiService { } export const baseApiService = new BaseApiService(process.env.NEXT_PUBLIC_FASTAPI_BACKEND_URL || ""); - diff --git a/surfsense_web/lib/auth-utils.ts b/surfsense_web/lib/auth-utils.ts index 920ec4a4e..e6429f9d4 100644 --- a/surfsense_web/lib/auth-utils.ts +++ b/surfsense_web/lib/auth-utils.ts @@ -256,11 +256,11 @@ export async function logout(): Promise { const cognitoUrl = new URL(oidcLogoutUrl); cognitoUrl.searchParams.set("client_id", oidcClientId); // Redirect to the platform landing page after Cognito clears its session. - // The landing page is NOT behind ForwardAuth, so the user sees it instead - // of being bounced back to Cognito login. Falls back to current origin - // for deployments without the env var. - const logoutRedirect = process.env.NEXT_PUBLIC_LOGOUT_REDIRECT_URL || window.location.origin; - cognitoUrl.searchParams.set("logout_uri", logoutRedirect); + // The landing page is NOT behind ForwardAuth, so the user sees it instead + // of being bounced back to Cognito login. Falls back to current origin + // for deployments without the env var. + const logoutRedirect = process.env.NEXT_PUBLIC_LOGOUT_REDIRECT_URL || window.location.origin; + cognitoUrl.searchParams.set("logout_uri", logoutRedirect); // Full SSO logout: oauth2-proxy sign_out clears _oauth2_proxy cookie, // then rd= redirects to Cognito to clear the Cognito session. From 8eb58dff968676df13691b6f6f2ad030a66ecce3 Mon Sep 17 00:00:00 2001 From: awais786 Date: Tue, 14 Apr 2026 23:51:38 +0500 Subject: [PATCH 13/29] feat(auth): synthesize email and set display_name from SSO header 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. --- surfsense_backend/app/config/__init__.py | 5 ++++ .../app/middleware/proxy_auth.py | 13 +++++++++- .../tests/unit/middleware/test_proxy_auth.py | 26 ++++++++++++++++++- 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/surfsense_backend/app/config/__init__.py b/surfsense_backend/app/config/__init__.py index 36867c7a0..828e99a9f 100644 --- a/surfsense_backend/app/config/__init__.py +++ b/surfsense_backend/app/config/__init__.py @@ -315,6 +315,11 @@ def is_cloud(cls) -> bool: # Comma-separated path prefixes that bypass proxy auth (default: /health). MPASS_BYPASS_PATHS = os.getenv("MPASS_BYPASS_PATHS", None) + # SMB tenant name — used to synthesize {username}@{SMB_NAME}.com email + # addresses when the OIDC provider sends a bare username (e.g. Cognito + # cognito:username claim) instead of an email. + SMB_NAME = os.getenv("SMB_NAME", "") + # Google OAuth GOOGLE_OAUTH_CLIENT_ID = os.getenv("GOOGLE_OAUTH_CLIENT_ID") GOOGLE_OAUTH_CLIENT_SECRET = os.getenv("GOOGLE_OAUTH_CLIENT_SECRET") diff --git a/surfsense_backend/app/middleware/proxy_auth.py b/surfsense_backend/app/middleware/proxy_auth.py index ec70dcdb5..20c58a6b9 100644 --- a/surfsense_backend/app/middleware/proxy_auth.py +++ b/surfsense_backend/app/middleware/proxy_auth.py @@ -84,7 +84,16 @@ async def dispatch( if _is_bypass_path(request.url.path, self.bypass_paths): return await call_next(request) - raw_email = request.headers.get("x-auth-request-email") + raw_email = (request.headers.get("x-auth-request-email") or "").strip() + if raw_email and "@" not in raw_email: + # Header holds a bare username (user_id_claim=cognito:username). + domain = getattr(config, "SMB_NAME", "") + raw_email = f"{raw_email}@{domain}.com" if domain else "" + if not raw_email: + raw_username = (request.headers.get("x-auth-request-user") or "").strip() + domain = getattr(config, "SMB_NAME", "") + if raw_username and domain: + raw_email = f"{raw_username}@{domain}.com" if not raw_email: logger.debug( "ProxyAuth: x-auth-request-email missing on %s", request.url.path @@ -112,9 +121,11 @@ async def _resolve_user(self, email: str, request: Request) -> User | None: if user is None: hashed_password = _password_helper.hash(secrets.token_urlsafe(32)) + display_name = email.split("@")[0] or None user = User( email=email, hashed_password=hashed_password, + display_name=display_name, is_active=True, is_verified=True, is_superuser=False, diff --git a/surfsense_backend/tests/unit/middleware/test_proxy_auth.py b/surfsense_backend/tests/unit/middleware/test_proxy_auth.py index d6885e32b..93ba3db47 100644 --- a/surfsense_backend/tests/unit/middleware/test_proxy_auth.py +++ b/surfsense_backend/tests/unit/middleware/test_proxy_auth.py @@ -401,7 +401,7 @@ async def test_email_with_uppercase_and_whitespace_is_normalised(self): ): await mw.dispatch(request, _ok_call_next) - mock_norm.assert_called_once_with(raw_email) + mock_norm.assert_called_once_with(raw_email.strip()) assert request.state.proxy_user is user # ── SPEC 9: race condition / IntegrityError ────────────────────────────── @@ -438,3 +438,27 @@ async def test_race_condition_fallback_select_sets_proxy_user(self): session.rollback.assert_called_once() assert request.state.proxy_user is race_user + + # ── SPEC 10: bare username → email synthesis ────────────────────────────── + + async def test_bare_username_synthesizes_email_via_smb_name(self): + """ + GIVEN X-Auth-Request-Email contains a bare username (no @) + AND config.SMB_NAME is set + WHEN request arrives + THEN middleware synthesizes {username}@{SMB_NAME}.com + AND passes it to _resolve_user + """ + mw = _make_middleware() + resolved = _make_user(email="testuser@foss.com") + mw._resolve_user = AsyncMock(return_value=resolved) + request = _make_request(headers={"x-auth-request-email": "testuser"}) + + with patch("app.middleware.proxy_auth.config") as mock_cfg: + mock_cfg.SMB_NAME = "foss" + await mw.dispatch(request, _ok_call_next) + + mw._resolve_user.assert_called_once() + called_email = mw._resolve_user.call_args.args[0] + assert called_email == "testuser@foss.com" + assert request.state.proxy_user is resolved From 0e94846bee537df9b1237d56ca56aaffa848798e Mon Sep 17 00:00:00 2001 From: awais786 Date: Thu, 16 Apr 2026 13:14:58 +0500 Subject: [PATCH 14/29] fix(auth): make logout work when Cognito hosted /logout is absent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- surfsense_web/.dockerignore | 5 ++++ surfsense_web/Dockerfile | 9 +++++++ surfsense_web/lib/auth-utils.ts | 47 +++++++++++++-------------------- 3 files changed, 33 insertions(+), 28 deletions(-) diff --git a/surfsense_web/.dockerignore b/surfsense_web/.dockerignore index 7d48982c9..d5ebb2f2c 100644 --- a/surfsense_web/.dockerignore +++ b/surfsense_web/.dockerignore @@ -4,6 +4,11 @@ node_modules .next out .DS_Store +# Next.js reads these at build time and inlines NEXT_PUBLIC_* values into the +# JS bundle, bypassing Dockerfile ARG defaults + entrypoint runtime substitution. +# Exclude from build context so the image stays portable across .env.local contents. +.env.local +.env.*.local npm-debug.log* yarn-debug.log* yarn-error.log* diff --git a/surfsense_web/Dockerfile b/surfsense_web/Dockerfile index b3319bcf6..b16b3f066 100644 --- a/surfsense_web/Dockerfile +++ b/surfsense_web/Dockerfile @@ -38,6 +38,12 @@ ARG NEXT_PUBLIC_ETL_SERVICE=__NEXT_PUBLIC_ETL_SERVICE__ ARG NEXT_PUBLIC_ZERO_CACHE_URL=__NEXT_PUBLIC_ZERO_CACHE_URL__ ARG NEXT_PUBLIC_DEPLOYMENT_MODE=__NEXT_PUBLIC_DEPLOYMENT_MODE__ ARG NEXT_PUBLIC_OAUTH2_PROXY_URL=__NEXT_PUBLIC_OAUTH2_PROXY_URL__ +# These are baked at build time (not placeholder-substituted). Next.js inlines +# them as literal strings and terser dead-code-eliminates branches based on +# truthiness; placeholder tokens look truthy and defeat that optimization. +ARG NEXT_PUBLIC_LOGOUT_REDIRECT_URL= +ARG NEXT_PUBLIC_OIDC_LOGOUT_URL= +ARG NEXT_PUBLIC_OIDC_CLIENT_ID= ENV NEXT_PUBLIC_FASTAPI_BACKEND_URL=$NEXT_PUBLIC_FASTAPI_BACKEND_URL ENV NEXT_PUBLIC_FASTAPI_BACKEND_AUTH_TYPE=$NEXT_PUBLIC_FASTAPI_BACKEND_AUTH_TYPE @@ -45,6 +51,9 @@ ENV NEXT_PUBLIC_ETL_SERVICE=$NEXT_PUBLIC_ETL_SERVICE ENV NEXT_PUBLIC_ZERO_CACHE_URL=$NEXT_PUBLIC_ZERO_CACHE_URL ENV NEXT_PUBLIC_DEPLOYMENT_MODE=$NEXT_PUBLIC_DEPLOYMENT_MODE ENV NEXT_PUBLIC_OAUTH2_PROXY_URL=$NEXT_PUBLIC_OAUTH2_PROXY_URL +ENV NEXT_PUBLIC_LOGOUT_REDIRECT_URL=$NEXT_PUBLIC_LOGOUT_REDIRECT_URL +ENV NEXT_PUBLIC_OIDC_LOGOUT_URL=$NEXT_PUBLIC_OIDC_LOGOUT_URL +ENV NEXT_PUBLIC_OIDC_CLIENT_ID=$NEXT_PUBLIC_OIDC_CLIENT_ID COPY --from=deps /app/node_modules ./node_modules COPY . . diff --git a/surfsense_web/lib/auth-utils.ts b/surfsense_web/lib/auth-utils.ts index e6429f9d4..077659d93 100644 --- a/surfsense_web/lib/auth-utils.ts +++ b/surfsense_web/lib/auth-utils.ts @@ -152,7 +152,6 @@ export function clearAllTokens(): void { } /** -<<<<<<< HEAD * Pushes the current localStorage tokens into the Electron main process * so that other BrowserWindows (Quick Ask, Autocomplete) can access them. */ @@ -191,9 +190,6 @@ export async function ensureTokensFromElectron(): Promise { } /** - * Logout the current user by revoking the refresh token and clearing localStorage. - * Returns true if logout was successful (or tokens were cleared), false otherwise. -======= * Reads the short-lived SSO handoff cookies set by /auth/jwt/proxy-login. * Returns null for each if not present. */ @@ -219,11 +215,9 @@ export function clearSSOCookies(): void { /** * Logout the current user. * - * Always performs 3-layer SSO logout (proxy auth is the only auth mode): * Layer 1 — revoke JWT refresh tokens server-side * Layer 2 — clear _oauth2_proxy cookie via /oauth2/sign_out - * Layer 3 — clear Cognito session via rd= redirect ->>>>>>> 8c3ff62c (feat(auth): mPass SSO via oauth2-proxy ForwardAuth with cookie-handoff) + * Layer 3 (optional) — clear Cognito session via rd= redirect when OIDC_LOGOUT_URL configured */ export async function logout(): Promise { const refreshToken = getRefreshToken(); @@ -247,38 +241,35 @@ export async function logout(): Promise { clearAllTokens(); - // Layers 2 + 3 — SSO logout via oauth2-proxy → Cognito + // Layer 2 (+ optional Layer 3) — oauth2-proxy sign_out, with Cognito hop if configured. if (typeof window !== "undefined") { + const oauthProxyUrl = process.env.NEXT_PUBLIC_OAUTH2_PROXY_URL || window.location.origin; + const logoutRedirect = process.env.NEXT_PUBLIC_LOGOUT_REDIRECT_URL || window.location.origin; const oidcLogoutUrl = process.env.NEXT_PUBLIC_OIDC_LOGOUT_URL; const oidcClientId = process.env.NEXT_PUBLIC_OIDC_CLIENT_ID; + let rdParam: string; if (oidcLogoutUrl && oidcClientId) { + // Full 3-layer logout: oauth2-proxy → Cognito → landing page. + // Single-encode query params so oauth2-proxy decodes once and passes clean args + // to Cognito; encodeURIComponent would double-encode and Cognito would reject. const cognitoUrl = new URL(oidcLogoutUrl); cognitoUrl.searchParams.set("client_id", oidcClientId); - // Redirect to the platform landing page after Cognito clears its session. - // The landing page is NOT behind ForwardAuth, so the user sees it instead - // of being bounced back to Cognito login. Falls back to current origin - // for deployments without the env var. - const logoutRedirect = process.env.NEXT_PUBLIC_LOGOUT_REDIRECT_URL || window.location.origin; cognitoUrl.searchParams.set("logout_uri", logoutRedirect); - - // Full SSO logout: oauth2-proxy sign_out clears _oauth2_proxy cookie, - // then rd= redirects to Cognito to clear the Cognito session. - // Uses the dedicated auth domain (foss-auth.localhost) so the sign_out - // URL is consistent regardless of which app initiates the logout. - const oauthProxyUrl = process.env.NEXT_PUBLIC_OAUTH2_PROXY_URL || window.location.origin; - // Single-encode the Cognito URL so oauth2-proxy decodes it once - // and passes a clean logout_uri to Cognito (matching Plane's pattern). - // encodeURIComponent would double-encode the query params, causing - // Cognito to reject the logout_uri as unregistered. - const cognitoStr = cognitoUrl.toString(); - const rdParam = cognitoStr.replace(/\?/, "%3F").replace(/&/g, "%26").replace(/=/g, "%3D"); - window.location.href = `${oauthProxyUrl}/oauth2/sign_out?rd=${rdParam}`; - return true; // browser is already navigating away + rdParam = cognitoUrl + .toString() + .replace(/\?/, "%3F") + .replace(/&/g, "%26") + .replace(/=/g, "%3D"); + } else { + // No Cognito hosted logout — clear oauth2-proxy cookie and land on portal. + rdParam = encodeURIComponent(logoutRedirect); } + window.location.href = `${oauthProxyUrl}/oauth2/sign_out?rd=${rdParam}`; + return true; // browser is already navigating away } - return false; // no SSO redirect — caller should navigate + return false; // SSR — caller should navigate } /** From 8b741d5c10358dba4471ee9a25f31f993ff625da Mon Sep 17 00:00:00 2001 From: awais786 Date: Thu, 16 Apr 2026 16:26:13 +0500 Subject: [PATCH 15/29] fix(chat): route streaming API calls through authenticatedFetch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- .../new-chat/[[...chat_id]]/page.tsx | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/surfsense_web/app/dashboard/[search_space_id]/new-chat/[[...chat_id]]/page.tsx b/surfsense_web/app/dashboard/[search_space_id]/new-chat/[[...chat_id]]/page.tsx index 0b1369340..2f8a285c1 100644 --- a/surfsense_web/app/dashboard/[search_space_id]/new-chat/[[...chat_id]]/page.tsx +++ b/surfsense_web/app/dashboard/[search_space_id]/new-chat/[[...chat_id]]/page.tsx @@ -41,7 +41,7 @@ import { Thread } from "@/components/assistant-ui/thread"; import { useChatSessionStateSync } from "@/hooks/use-chat-session-state"; import { useMessagesSync } from "@/hooks/use-messages-sync"; import { documentsApiService } from "@/lib/apis/documents-api.service"; -import { getBearerToken } from "@/lib/auth-utils"; +import { authenticatedFetch, getBearerToken } from "@/lib/auth-utils"; import { convertToThreadMessage } from "@/lib/chat/message-utils"; import { isPodcastGenerating, @@ -663,11 +663,10 @@ export default function NewChatPage() { setSidebarDocuments([]); } - const response = await fetch(`${backendUrl}/api/v1/new_chat`, { + const response = await authenticatedFetch(`${backendUrl}/api/v1/new_chat`, { method: "POST", headers: { "Content-Type": "application/json", - Authorization: `Bearer ${token}`, }, body: JSON.stringify({ chat_id: currentThreadId, @@ -1028,11 +1027,10 @@ export default function NewChatPage() { try { const backendUrl = process.env.NEXT_PUBLIC_FASTAPI_BACKEND_URL || "http://localhost:8000"; - const response = await fetch(`${backendUrl}/api/v1/threads/${resumeThreadId}/resume`, { + const response = await authenticatedFetch(`${backendUrl}/api/v1/threads/${resumeThreadId}/resume`, { method: "POST", headers: { "Content-Type": "application/json", - Authorization: `Bearer ${token}`, }, body: JSON.stringify({ search_space_id: searchSpaceId, @@ -1345,11 +1343,10 @@ export default function NewChatPage() { ]); try { - const response = await fetch(getRegenerateUrl(threadId), { + const response = await authenticatedFetch(getRegenerateUrl(threadId), { method: "POST", headers: { "Content-Type": "application/json", - Authorization: `Bearer ${token}`, }, body: JSON.stringify({ search_space_id: searchSpaceId, From 2bf8dc4acf289cd30e4b969bd90cbfd7cf26d370 Mon Sep 17 00:00:00 2001 From: awais786 Date: Fri, 17 Apr 2026 16:02:47 +0500 Subject: [PATCH 16/29] fix(auth): land on portal after logout, drop unreachable Cognito hop 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-." -> "foss."). 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. --- surfsense_web/lib/auth-utils.ts | 41 ++++++++------------------------- 1 file changed, 9 insertions(+), 32 deletions(-) diff --git a/surfsense_web/lib/auth-utils.ts b/surfsense_web/lib/auth-utils.ts index 077659d93..d6aa82b61 100644 --- a/surfsense_web/lib/auth-utils.ts +++ b/surfsense_web/lib/auth-utils.ts @@ -213,16 +213,13 @@ export function clearSSOCookies(): void { } /** - * Logout the current user. - * - * Layer 1 — revoke JWT refresh tokens server-side - * Layer 2 — clear _oauth2_proxy cookie via /oauth2/sign_out - * Layer 3 (optional) — clear Cognito session via rd= redirect when OIDC_LOGOUT_URL configured + * Logout the current user: revoke JWT refresh token server-side, clear local + * tokens, navigate to home. Returns true when the browser is already navigating + * so callers can skip their own redirect. */ export async function logout(): Promise { const refreshToken = getRefreshToken(); - // Layer 1 — revoke the refresh token server-side if (refreshToken) { try { const backendUrl = process.env.NEXT_PUBLIC_FASTAPI_BACKEND_URL || "http://localhost:8000"; @@ -241,35 +238,15 @@ export async function logout(): Promise { clearAllTokens(); - // Layer 2 (+ optional Layer 3) — oauth2-proxy sign_out, with Cognito hop if configured. if (typeof window !== "undefined") { - const oauthProxyUrl = process.env.NEXT_PUBLIC_OAUTH2_PROXY_URL || window.location.origin; - const logoutRedirect = process.env.NEXT_PUBLIC_LOGOUT_REDIRECT_URL || window.location.origin; - const oidcLogoutUrl = process.env.NEXT_PUBLIC_OIDC_LOGOUT_URL; - const oidcClientId = process.env.NEXT_PUBLIC_OIDC_CLIENT_ID; - - let rdParam: string; - if (oidcLogoutUrl && oidcClientId) { - // Full 3-layer logout: oauth2-proxy → Cognito → landing page. - // Single-encode query params so oauth2-proxy decodes once and passes clean args - // to Cognito; encodeURIComponent would double-encode and Cognito would reject. - const cognitoUrl = new URL(oidcLogoutUrl); - cognitoUrl.searchParams.set("client_id", oidcClientId); - cognitoUrl.searchParams.set("logout_uri", logoutRedirect); - rdParam = cognitoUrl - .toString() - .replace(/\?/, "%3F") - .replace(/&/g, "%26") - .replace(/=/g, "%3D"); - } else { - // No Cognito hosted logout — clear oauth2-proxy cookie and land on portal. - rdParam = encodeURIComponent(logoutRedirect); - } - window.location.href = `${oauthProxyUrl}/oauth2/sign_out?rd=${rdParam}`; - return true; // browser is already navigating away + // Rewrite "foss-." → "foss." so we land on the portal + // (outside ForwardAuth) instead of SurfSense's own root, which would silently re-auth. + const portalHost = window.location.hostname.replace(/^[^.]*\./, "foss."); + window.location.href = `${window.location.protocol}//${portalHost}`; + return true; } - return false; // SSR — caller should navigate + return false; } /** From 0d8bc8b9f66ff81506c31a853c14b9b61d266834 Mon Sep 17 00:00:00 2001 From: jawad-khan Date: Mon, 20 Apr 2026 17:48:09 +0500 Subject: [PATCH 17/29] chore: Add session cookies test --- .../tests/unit/test_config_jwt_lifetimes.py | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 surfsense_backend/tests/unit/test_config_jwt_lifetimes.py diff --git a/surfsense_backend/tests/unit/test_config_jwt_lifetimes.py b/surfsense_backend/tests/unit/test_config_jwt_lifetimes.py new file mode 100644 index 000000000..8662d2887 --- /dev/null +++ b/surfsense_backend/tests/unit/test_config_jwt_lifetimes.py @@ -0,0 +1,30 @@ +"""JWT access/refresh lifetimes follow ACCESS_TOKEN_* / REFRESH_TOKEN_* env (compose maps unified session vars).""" + +import importlib +import os + +import pytest + + +@pytest.mark.unit +def test_jwt_lifetime_seconds_from_env(monkeypatch): + import app.config as cfg_module + + prev_access = os.environ.get("ACCESS_TOKEN_LIFETIME_SECONDS") + prev_refresh = os.environ.get("REFRESH_TOKEN_LIFETIME_SECONDS") + try: + monkeypatch.setenv("ACCESS_TOKEN_LIFETIME_SECONDS", "111") + monkeypatch.setenv("REFRESH_TOKEN_LIFETIME_SECONDS", "222") + importlib.reload(cfg_module) + assert cfg_module.Config.ACCESS_TOKEN_LIFETIME_SECONDS == 111 + assert cfg_module.Config.REFRESH_TOKEN_LIFETIME_SECONDS == 222 + finally: + if prev_access is not None: + os.environ["ACCESS_TOKEN_LIFETIME_SECONDS"] = prev_access + else: + os.environ.pop("ACCESS_TOKEN_LIFETIME_SECONDS", None) + if prev_refresh is not None: + os.environ["REFRESH_TOKEN_LIFETIME_SECONDS"] = prev_refresh + else: + os.environ.pop("REFRESH_TOKEN_LIFETIME_SECONDS", None) + importlib.reload(cfg_module) From 91c703413492a6671906d8cf940fa7c06ce2b0e2 Mon Sep 17 00:00:00 2001 From: jawad-khan Date: Mon, 20 Apr 2026 19:15:06 +0500 Subject: [PATCH 18/29] fix: added default settings --- surfsense_backend/app/config/__init__.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/surfsense_backend/app/config/__init__.py b/surfsense_backend/app/config/__init__.py index 828e99a9f..3b1e181fd 100644 --- a/surfsense_backend/app/config/__init__.py +++ b/surfsense_backend/app/config/__init__.py @@ -451,13 +451,13 @@ def is_cloud(cls) -> bool: # OAuth JWT SECRET_KEY = os.getenv("SECRET_KEY") - # JWT Token Lifetimes + # JWT Token Lifetimes (defaults match FOSS devstack SESSION_* unified envs when unset) ACCESS_TOKEN_LIFETIME_SECONDS = int( - os.getenv("ACCESS_TOKEN_LIFETIME_SECONDS", str(24 * 60 * 60)) # 1 day - ) + os.getenv("ACCESS_TOKEN_LIFETIME_SECONDS", str(7 * 24 * 60 * 60)) + ) # 604800s = 7d REFRESH_TOKEN_LIFETIME_SECONDS = int( - os.getenv("REFRESH_TOKEN_LIFETIME_SECONDS", str(14 * 24 * 60 * 60)) # 2 weeks - ) + os.getenv("REFRESH_TOKEN_LIFETIME_SECONDS", str(14 * 24 * 60 * 60)) + ) # 1209600s = 14d # ETL Service ETL_SERVICE = os.getenv("ETL_SERVICE") From 608c522a3e2623fef2b520db0645afba00d22988 Mon Sep 17 00:00:00 2001 From: Azan Ali Date: Tue, 21 Apr 2026 17:10:05 +0500 Subject: [PATCH 19/29] made askii.ai default emain domain --- .github/workflows/code-quality.yml | 2 -- surfsense_backend/app/config/__init__.py | 8 ++++---- surfsense_backend/app/middleware/proxy_auth.py | 10 +++++----- .../tests/unit/middleware/test_proxy_auth.py | 12 ++++++------ 4 files changed, 15 insertions(+), 17 deletions(-) diff --git a/.github/workflows/code-quality.yml b/.github/workflows/code-quality.yml index d6be94dbb..b5d80b9a0 100644 --- a/.github/workflows/code-quality.yml +++ b/.github/workflows/code-quality.yml @@ -227,8 +227,6 @@ jobs: - name: Install pnpm uses: pnpm/action-setup@v4 - with: - version: latest - name: Check if frontend files changed id: frontend-changes diff --git a/surfsense_backend/app/config/__init__.py b/surfsense_backend/app/config/__init__.py index 828e99a9f..9b8eabb15 100644 --- a/surfsense_backend/app/config/__init__.py +++ b/surfsense_backend/app/config/__init__.py @@ -315,10 +315,10 @@ def is_cloud(cls) -> bool: # Comma-separated path prefixes that bypass proxy auth (default: /health). MPASS_BYPASS_PATHS = os.getenv("MPASS_BYPASS_PATHS", None) - # SMB tenant name — used to synthesize {username}@{SMB_NAME}.com email - # addresses when the OIDC provider sends a bare username (e.g. Cognito - # cognito:username claim) instead of an email. - SMB_NAME = os.getenv("SMB_NAME", "") + # Email domain used to synthesize {username}@{DEFAULT_EMAIL_DOMAIN} when + # the OIDC provider sends a bare username (e.g. Cognito cognito:username + # claim) instead of a full email address. + DEFAULT_EMAIL_DOMAIN = os.getenv("DEFAULT_EMAIL_DOMAIN", "askii.ai") # Google OAuth GOOGLE_OAUTH_CLIENT_ID = os.getenv("GOOGLE_OAUTH_CLIENT_ID") diff --git a/surfsense_backend/app/middleware/proxy_auth.py b/surfsense_backend/app/middleware/proxy_auth.py index 20c58a6b9..3b466eb29 100644 --- a/surfsense_backend/app/middleware/proxy_auth.py +++ b/surfsense_backend/app/middleware/proxy_auth.py @@ -87,13 +87,13 @@ async def dispatch( raw_email = (request.headers.get("x-auth-request-email") or "").strip() if raw_email and "@" not in raw_email: # Header holds a bare username (user_id_claim=cognito:username). - domain = getattr(config, "SMB_NAME", "") - raw_email = f"{raw_email}@{domain}.com" if domain else "" + domain = getattr(config, "DEFAULT_EMAIL_DOMAIN", "askii.ai") + raw_email = f"{raw_email}@{domain}" if not raw_email: raw_username = (request.headers.get("x-auth-request-user") or "").strip() - domain = getattr(config, "SMB_NAME", "") - if raw_username and domain: - raw_email = f"{raw_username}@{domain}.com" + domain = getattr(config, "DEFAULT_EMAIL_DOMAIN", "askii.ai") + if raw_username: + raw_email = f"{raw_username}@{domain}" if not raw_email: logger.debug( "ProxyAuth: x-auth-request-email missing on %s", request.url.path diff --git a/surfsense_backend/tests/unit/middleware/test_proxy_auth.py b/surfsense_backend/tests/unit/middleware/test_proxy_auth.py index 93ba3db47..d23074a5e 100644 --- a/surfsense_backend/tests/unit/middleware/test_proxy_auth.py +++ b/surfsense_backend/tests/unit/middleware/test_proxy_auth.py @@ -441,24 +441,24 @@ async def test_race_condition_fallback_select_sets_proxy_user(self): # ── SPEC 10: bare username → email synthesis ────────────────────────────── - async def test_bare_username_synthesizes_email_via_smb_name(self): + async def test_bare_username_synthesizes_email_via_default_email_domain(self): """ GIVEN X-Auth-Request-Email contains a bare username (no @) - AND config.SMB_NAME is set + AND config.DEFAULT_EMAIL_DOMAIN is set WHEN request arrives - THEN middleware synthesizes {username}@{SMB_NAME}.com + THEN middleware synthesizes {username}@{DEFAULT_EMAIL_DOMAIN} AND passes it to _resolve_user """ mw = _make_middleware() - resolved = _make_user(email="testuser@foss.com") + resolved = _make_user(email="testuser@askii.ai") mw._resolve_user = AsyncMock(return_value=resolved) request = _make_request(headers={"x-auth-request-email": "testuser"}) with patch("app.middleware.proxy_auth.config") as mock_cfg: - mock_cfg.SMB_NAME = "foss" + mock_cfg.DEFAULT_EMAIL_DOMAIN = "askii.ai" await mw.dispatch(request, _ok_call_next) mw._resolve_user.assert_called_once() called_email = mw._resolve_user.call_args.args[0] - assert called_email == "testuser@foss.com" + assert called_email == "testuser@askii.ai" assert request.state.proxy_user is resolved From 2104932bbb98fc1c5a01f93874baef59b02e8eea Mon Sep 17 00:00:00 2001 From: awais786 Date: Sat, 25 Apr 2026 14:32:25 +0500 Subject: [PATCH 20/29] chore(ui): hide marketing nav and footer link columns 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) --- .../components/homepage/footer-new.tsx | 161 ------------------ surfsense_web/components/homepage/navbar.tsx | 41 +---- 2 files changed, 2 insertions(+), 200 deletions(-) diff --git a/surfsense_web/components/homepage/footer-new.tsx b/surfsense_web/components/homepage/footer-new.tsx index 4bbff0cbd..aedcfac8f 100644 --- a/surfsense_web/components/homepage/footer-new.tsx +++ b/surfsense_web/components/homepage/footer-new.tsx @@ -1,95 +1,6 @@ -import { - IconBrandDiscord, - IconBrandGithub, - IconBrandLinkedin, - IconBrandTwitter, -} from "@tabler/icons-react"; -import Link from "next/link"; import { Logo } from "@/components/Logo"; export function FooterNew() { - const pages = [ - // { - // title: "All Products", - // href: "#", - // }, - // { - // title: "Studio", - // href: "#", - // }, - // { - // title: "Clients", - // href: "#", - // }, - { - title: "Pricing", - href: "/pricing", - }, - { - title: "Docs", - href: "/docs", - }, - { - title: "Contact Us", - href: "/contact", - }, - { - title: "Announcements", - href: "/announcements", - }, - ]; - - const socials = [ - { - title: "Twitter", - href: "https://x.com/mod_setter", - icon: IconBrandTwitter, - }, - { - title: "LinkedIn", - href: "https://www.linkedin.com/company/surfsense/", - icon: IconBrandLinkedin, - }, - { - title: "GitHub", - href: "https://github.com/MODSetter", - icon: IconBrandGithub, - }, - { - title: "Discord", - href: "https://discord.gg/ejRNvftDp9", - icon: IconBrandDiscord, - }, - ]; - const legals = [ - { - title: "Privacy Policy", - href: "/privacy", - }, - { - title: "Terms of Service", - href: "/terms", - }, - // { - // title: "Cookie Policy", - // href: "#", - // }, - ]; - - const signups = [ - { - title: "Sign In", - href: "/login", - }, - // { - // title: "Login", - // href: "#", - // }, - // { - // title: "Forgot Password", - // href: "#", - // }, - ]; return (
@@ -103,78 +14,6 @@ export function FooterNew() { © SurfSense {new Date().getFullYear()}. All rights reserved.
-
-
-

- Pages -

-
    - {pages.map((page, idx) => ( -
  • - - {page.title} - -
    • - ))} -
-
- -
-

- Socials -

-
    - {socials.map((social, idx) => { - const Icon = social.icon; - return ( -
  • - - - {social.title} - -
    • - ); - })} -
-
- -
-

- Legal -

-
    - {legals.map((legal, idx) => ( -
  • - - {legal.title} - -
    • - ))} -
-
-
-

- Register -

-
    - {signups.map((auth, idx) => ( -
  • - - {auth.title} - -
    • - ))} -
-
-

SurfSense diff --git a/surfsense_web/components/homepage/navbar.tsx b/surfsense_web/components/homepage/navbar.tsx index 4961199aa..c62df226e 100644 --- a/surfsense_web/components/homepage/navbar.tsx +++ b/surfsense_web/components/homepage/navbar.tsx @@ -1,5 +1,5 @@ "use client"; -import { IconBrandDiscord, IconBrandReddit, IconMenu2, IconX } from "@tabler/icons-react"; +import { IconMenu2, IconX } from "@tabler/icons-react"; import { AnimatePresence, motion } from "motion/react"; import Link from "next/link"; import { useEffect, useRef, useState } from "react"; @@ -17,12 +17,7 @@ interface NavbarProps { export const Navbar = ({ scrolledBgClassName }: NavbarProps = {}) => { const [isScrolled, setIsScrolled] = useState(false); - const navItems = [ - { name: "Pricing", link: "/pricing" }, - { name: "Changelog", link: "/changelog" }, - { name: "Docs", link: "/docs" }, - { name: "Contact\u00A0Us", link: "/contact" }, - ]; + const navItems = [{ name: "Docs", link: "/docs" }]; useEffect(() => { if (typeof window === "undefined") return; @@ -94,22 +89,6 @@ const DesktopNav = ({ navItems, isScrolled, scrolledBgClassName }: any) => { ))}

- - - - - - @@ -193,22 +172,6 @@ const MobileNav = ({ navItems, isScrolled, scrolledBgClassName }: any) => { ))}
- - - - - -
From bee292432647f626f795beeaa6cb014af5eb149f Mon Sep 17 00:00:00 2001 From: awais786 Date: Sat, 25 Apr 2026 14:37:15 +0500 Subject: [PATCH 21/29] fix(login): redirect authenticated users away from /login MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- surfsense_web/app/(home)/login/page.tsx | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/surfsense_web/app/(home)/login/page.tsx b/surfsense_web/app/(home)/login/page.tsx index f2a6d7e85..9961df3d8 100644 --- a/surfsense_web/app/(home)/login/page.tsx +++ b/surfsense_web/app/(home)/login/page.tsx @@ -8,6 +8,7 @@ import { toast } from "sonner"; import { Logo } from "@/components/Logo"; import { useGlobalLoadingEffect } from "@/hooks/use-global-loading"; import { getAuthErrorDetails, shouldRetry } from "@/lib/auth-errors"; +import { getBearerToken } from "@/lib/auth-utils"; import { AUTH_TYPE, isSSOAuth } from "@/lib/env-config"; import { AmbientBackground } from "./AmbientBackground"; import { GoogleLoginButton } from "./GoogleLoginButton"; @@ -30,7 +31,16 @@ function LoginContent() { // Hook is unconditional (rules-of-hooks); the early-return that skips // rendering the form lives below, after every other hook has run. useEffect(() => { - if (typeof window !== "undefined" && isSSOAuth()) { + if (typeof window === "undefined") return; + + // Already signed in — never show the login surface. Hand off to the + // home route which will redirect to /dashboard. + if (getBearerToken()) { + window.location.replace("/"); + return; + } + + if (isSSOAuth()) { const oauthProxyUrl = process.env.NEXT_PUBLIC_OAUTH2_PROXY_URL || window.location.origin; const rd = `${window.location.origin}/`; window.location.replace(`${oauthProxyUrl}/oauth2/sign_in?rd=${encodeURIComponent(rd)}`); From d1ae836e836e4ff237c01546ae08f7964be766dd Mon Sep 17 00:00:00 2001 From: Azan Ali Date: Thu, 30 Apr 2026 20:42:43 +0500 Subject: [PATCH 22/29] use moneta instead of foss in logout --- surfsense_web/lib/auth-utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/surfsense_web/lib/auth-utils.ts b/surfsense_web/lib/auth-utils.ts index d6aa82b61..aadc550d0 100644 --- a/surfsense_web/lib/auth-utils.ts +++ b/surfsense_web/lib/auth-utils.ts @@ -241,7 +241,7 @@ export async function logout(): Promise { if (typeof window !== "undefined") { // Rewrite "foss-." → "foss." so we land on the portal // (outside ForwardAuth) instead of SurfSense's own root, which would silently re-auth. - const portalHost = window.location.hostname.replace(/^[^.]*\./, "foss."); + const portalHost = window.location.hostname.replace(/^[^.]*\./, "moneta."); window.location.href = `${window.location.protocol}//${portalHost}`; return true; } From 84c5cd22fe983c2c7cefcc7de07c39513ea4689c Mon Sep 17 00:00:00 2001 From: Azan Ali <73800719+aznszn@users.noreply.github.com> Date: Tue, 5 May 2026 20:38:31 +0500 Subject: [PATCH 23/29] fix: strip first subdomain for portal redirect on signout (#16) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 * 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 --------- Co-authored-by: Claude Sonnet 4.6 --- surfsense_web/lib/auth-utils.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/surfsense_web/lib/auth-utils.ts b/surfsense_web/lib/auth-utils.ts index aadc550d0..10bd31bdf 100644 --- a/surfsense_web/lib/auth-utils.ts +++ b/surfsense_web/lib/auth-utils.ts @@ -239,9 +239,9 @@ export async function logout(): Promise { clearAllTokens(); if (typeof window !== "undefined") { - // Rewrite "foss-." → "foss." so we land on the portal - // (outside ForwardAuth) instead of SurfSense's own root, which would silently re-auth. - const portalHost = window.location.hostname.replace(/^[^.]*\./, "moneta."); + // Strip the first subdomain so we land on the portal (outside ForwardAuth) + // instead of SurfSense's own root, which would silently re-auth. + const portalHost = window.location.hostname.replace(/^[^.]+\.(?=[^.]*\.[^.]*\.)/, ""); window.location.href = `${window.location.protocol}//${portalHost}`; return true; } From 905ac6c157ca04d7cb72069f653b3a1aba874645 Mon Sep 17 00:00:00 2001 From: jawad khan Date: Thu, 14 May 2026 16:39:53 +0500 Subject: [PATCH 24/29] Auto-join SSO users to first shared search space on login (#18) * 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 --------- Co-authored-by: Usama Sadiq --- surfsense_backend/app/config/__init__.py | 7 ++ .../app/services/smb_auto_join.py | 109 ++++++++++++++++++ surfsense_backend/app/users.py | 17 +++ 3 files changed, 133 insertions(+) create mode 100644 surfsense_backend/app/services/smb_auto_join.py diff --git a/surfsense_backend/app/config/__init__.py b/surfsense_backend/app/config/__init__.py index 4eda2741d..ca7d7d47d 100644 --- a/surfsense_backend/app/config/__init__.py +++ b/surfsense_backend/app/config/__init__.py @@ -320,6 +320,13 @@ def is_cloud(cls) -> bool: # claim) instead of a full email address. DEFAULT_EMAIL_DOMAIN = os.getenv("DEFAULT_EMAIL_DOMAIN", "askii.ai") + # Portal hostname prefix (e.g. fossil for fossil.local...). + SMB_NAME = (os.getenv("SMB_NAME") or "").strip() + # Default shared search space name (SSO auto-join). Defaults to SMB_NAME. + SMB_DEFAULT_WORKSPACE_NAME = ( + os.getenv("SMB_DEFAULT_WORKSPACE_NAME") or os.getenv("SMB_NAME") or "" + ).strip() + # Google OAuth GOOGLE_OAUTH_CLIENT_ID = os.getenv("GOOGLE_OAUTH_CLIENT_ID") GOOGLE_OAUTH_CLIENT_SECRET = os.getenv("GOOGLE_OAUTH_CLIENT_SECRET") diff --git a/surfsense_backend/app/services/smb_auto_join.py b/surfsense_backend/app/services/smb_auto_join.py new file mode 100644 index 000000000..a2ae182f5 --- /dev/null +++ b/surfsense_backend/app/services/smb_auto_join.py @@ -0,0 +1,109 @@ +"""Ensure SSO users are members of the shared SMB SearchSpace (SMB_DEFAULT_WORKSPACE_NAME).""" + +from __future__ import annotations + +import logging +import uuid + +from sqlalchemy import select +from sqlalchemy.exc import IntegrityError + +from app.config import config +from app.db import ( + SearchSpace, + SearchSpaceMembership, + SearchSpaceRole, + async_session_maker, +) + +logger = logging.getLogger(__name__) + + +async def auto_join_smb_search_space(user_id: uuid.UUID) -> None: + """ + ``_auto_join_workspace``: match the search space whose + name equals ``SMB_DEFAULT_WORKSPACE_NAME`` or ``SMB_NAME``. If none exists, + do nothing (no fallback to another space). + + Uses the default invite role (Editor) when absent. Idempotent. + Safe when auth uses Bearer JWT only: runs from ``current_active_user`` as well. + """ + smb_slug = ( + getattr(config, "SMB_DEFAULT_WORKSPACE_NAME", None) + or getattr(config, "SMB_NAME", "") + or "" + ) + if isinstance(smb_slug, str): + smb_slug = smb_slug.strip() + if not smb_slug: + return + + async with async_session_maker() as session: + not_deleting = ~SearchSpace.name.startswith("[DELETING] ") + space_result = await session.execute( + select(SearchSpace) + .where(SearchSpace.name == smb_slug, not_deleting) + .order_by(SearchSpace.id.asc()) + .limit(1) + ) + space = space_result.scalars().first() + if space is None: + logger.debug( + "SMB auto-join: no search space named %r — skipping", + smb_slug, + ) + return + + existing_member = await session.execute( + select(SearchSpaceMembership.id).where( + SearchSpaceMembership.user_id == user_id, + SearchSpaceMembership.search_space_id == space.id, + ) + ) + if existing_member.scalars().first() is not None: + return + + role_result = await session.execute( + select(SearchSpaceRole).where( + SearchSpaceRole.search_space_id == space.id, + SearchSpaceRole.is_default == True, # noqa: E712 + ) + ) + role = role_result.scalars().first() + if role is None: + role_result = await session.execute( + select(SearchSpaceRole).where( + SearchSpaceRole.search_space_id == space.id, + SearchSpaceRole.name == "Editor", + ) + ) + role = role_result.scalars().first() + if role is None: + logger.warning( + "SMB auto-join: no default or Editor role for search space %s — skipping", + space.id, + ) + return + + membership = SearchSpaceMembership( + user_id=user_id, + search_space_id=space.id, + role_id=role.id, + is_owner=False, + ) + session.add(membership) + try: + await session.commit() + logger.info( + "SMB auto-join: joined user %s to search space %s (role=%s)", + user_id, + space.id, + role.name, + ) + except IntegrityError: + await session.rollback() + logger.debug( + "SMB auto-join: race for user %s / space %s — already joined", + user_id, + space.id, + ) diff --git a/surfsense_backend/app/users.py b/surfsense_backend/app/users.py index e35cb9991..0fc8b6f72 100644 --- a/surfsense_backend/app/users.py +++ b/surfsense_backend/app/users.py @@ -16,6 +16,7 @@ from sqlalchemy import update from app.config import config +from app.services.smb_auto_join import auto_join_smb_search_space from app.db import ( Prompt, SearchSpace, @@ -312,11 +313,27 @@ async def current_active_user( mPass proxy auth is active). Falls back to JWT Bearer token validation so existing email/password and Google OAuth flows continue to work when proxy auth is disabled. + + SMB shared SearchSpace membership is enforced here — not only in + ProxyAuthMiddleware — because after proxy-login the browser usually sends Bearer + JWT without X-Auth-Request-Email, so middleware alone would never run auto-join. """ proxy_user = getattr(request.state, "proxy_user", None) if proxy_user is not None: + try: + await auto_join_smb_search_space(proxy_user.id) + except Exception: + logger.exception( + "SMB auto-join failed for proxy session user %s", proxy_user.id + ) return proxy_user if jwt_user is not None: + try: + await auto_join_smb_search_space(jwt_user.id) + except Exception: + logger.exception( + "SMB auto-join failed for JWT user %s", jwt_user.id + ) return jwt_user raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, From 04e6c1af812c43f04196201f72b387134238ea5a Mon Sep 17 00:00:00 2001 From: awais786 Date: Sat, 16 May 2026 02:16:50 +0500 Subject: [PATCH 25/29] test(auth): pin proxy_user > jwt_user precedence in current_active_user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 #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) --- ...st_current_active_user_proxy_precedence.py | 161 ++++++++++++++++++ 1 file changed, 161 insertions(+) create mode 100644 surfsense_backend/tests/unit/test_current_active_user_proxy_precedence.py diff --git a/surfsense_backend/tests/unit/test_current_active_user_proxy_precedence.py b/surfsense_backend/tests/unit/test_current_active_user_proxy_precedence.py new file mode 100644 index 000000000..2c1f6f65e --- /dev/null +++ b/surfsense_backend/tests/unit/test_current_active_user_proxy_precedence.py @@ -0,0 +1,161 @@ +""" +Regression-guard for SurfSense's architectural immunity to the +stale-session-on-user-switch class of bug. + +SurfSense's `current_active_user` (in app.users) resolves to: + proxy_user (set per-request by ProxyAuthMiddleware from + X-Auth-Request-Email) IF present, ELSE + jwt_user (decoded from Authorization: Bearer JWT) IF present, ELSE + raise 401 Not Authenticated. + +This precedence is what makes SurfSense immune to the cross-app +stale-session-on-user-switch bug class that affected Plane #29 / +Outline #19 / Penpot #18 / Twenty #8. The other apps had to add an +explicit "compare upstream identity vs session identity → flush on +mismatch" middleware. SurfSense doesn't need that because the upstream +identity (proxy_user) ALWAYS wins over the persisted session (jwt_user) +when both are present. + +The cross-app contract: + awais786/sso-rules-moneta:openspec/specs/proxy-auth-middleware/spec.md + +These tests pin the precedence so a future refactor that flips the +priority (or removes the proxy_user check) cannot silently re-introduce +the bug. Without them, a well-intentioned "simplify auth to just use +JWT" change would pass type-checks and ship the regression. +""" + +from __future__ import annotations + +import uuid +from unittest.mock import AsyncMock, MagicMock + +import pytest +from fastapi import HTTPException, Request +from starlette.requests import Request as StarletteRequest + +from app.users import current_active_user, current_optional_user + + +def _make_request(proxy_user=None) -> Request: + """Build a minimal Request whose .state.proxy_user is settable.""" + scope = {"type": "http", "headers": [], "method": "GET", "path": "/"} + request = StarletteRequest(scope) + if proxy_user is not None: + request.state.proxy_user = proxy_user + return request + + +def _make_user(email: str, *, is_active: bool = True): + user = MagicMock() + user.id = uuid.uuid4() + user.email = email + user.is_active = is_active + return user + + +@pytest.mark.asyncio +async def test_proxy_user_wins_when_both_proxy_and_jwt_present(): + """ + GIVEN proxy_user resolves alice from upstream header + AND jwt_user resolves bob from Bearer JWT (stale) + WHEN current_active_user runs + THEN returns alice (the upstream identity) + + This is the architectural-immunity contract. SurfSense never serves + the stale JWT identity when an upstream identity is asserted on the + same request. If this test fails, the cross-app stale-session bug + class is re-introduced into SurfSense. + """ + alice = _make_user("alice@example.com") + bob = _make_user("bob@example.com") + request = _make_request(proxy_user=alice) + + # Bypass the SMB auto-join side effect; we're testing precedence only. + import app.users as users_module + original = users_module.auto_join_smb_search_space + users_module.auto_join_smb_search_space = AsyncMock(return_value=None) + try: + result = await current_active_user(request, jwt_user=bob) + finally: + users_module.auto_join_smb_search_space = original + + assert result is alice + assert result is not bob + + +@pytest.mark.asyncio +async def test_falls_back_to_jwt_when_proxy_user_absent(): + """ + GIVEN proxy_user is NOT set on the request (no X-Auth-Request-Email) + AND jwt_user resolves alice from Bearer JWT + WHEN current_active_user runs + THEN returns alice (the JWT identity) + + Header absence is NOT a logout signal per the cross-app spec — it + legitimately occurs for internal calls, bypass routes, OPTIONS + preflight, and direct backend hits at 127.0.0.1. SurfSense MUST + serve the JWT identity in this case, not 401. + """ + alice = _make_user("alice@example.com") + request = _make_request(proxy_user=None) + + import app.users as users_module + original = users_module.auto_join_smb_search_space + users_module.auto_join_smb_search_space = AsyncMock(return_value=None) + try: + result = await current_active_user(request, jwt_user=alice) + finally: + users_module.auto_join_smb_search_space = original + + assert result is alice + + +@pytest.mark.asyncio +async def test_raises_401_when_neither_proxy_nor_jwt_present(): + """ + GIVEN proxy_user is NOT set AND jwt_user is None (no Bearer) + WHEN current_active_user runs + THEN raises 401 Not Authenticated + + Sanity: an entirely unauthenticated request must be rejected. + """ + request = _make_request(proxy_user=None) + + with pytest.raises(HTTPException) as exc_info: + await current_active_user(request, jwt_user=None) + + assert exc_info.value.status_code == 401 + + +@pytest.mark.asyncio +async def test_optional_user_returns_proxy_when_both_present(): + """ + GIVEN proxy_user resolves alice AND jwt_user resolves bob + WHEN current_optional_user runs + THEN returns alice + + Same precedence rule, optional variant. Returns None instead of + raising when neither is present. + """ + alice = _make_user("alice@example.com") + bob = _make_user("bob@example.com") + request = _make_request(proxy_user=alice) + + result = await current_optional_user(request, jwt_user=bob) + + assert result is alice + + +@pytest.mark.asyncio +async def test_optional_user_returns_none_when_neither_present(): + """ + GIVEN no proxy_user AND no jwt_user + WHEN current_optional_user runs + THEN returns None (does NOT raise) + """ + request = _make_request(proxy_user=None) + + result = await current_optional_user(request, jwt_user=None) + + assert result is None From 27a1bc36729dd15bb35016bfbe3718cb47287efb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 15 May 2026 21:32:53 +0000 Subject: [PATCH 26/29] test(auth): mark proxy precedence regression tests as unit Agent-Logs-Url: https://github.com/Pressingly/SurfSense/sessions/7cb36524-6c58-452f-8b60-9d6256a6caaa Co-authored-by: awais786 <445320+awais786@users.noreply.github.com> --- .../tests/unit/test_current_active_user_proxy_precedence.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/surfsense_backend/tests/unit/test_current_active_user_proxy_precedence.py b/surfsense_backend/tests/unit/test_current_active_user_proxy_precedence.py index 2c1f6f65e..2373a60d6 100644 --- a/surfsense_backend/tests/unit/test_current_active_user_proxy_precedence.py +++ b/surfsense_backend/tests/unit/test_current_active_user_proxy_precedence.py @@ -36,6 +36,8 @@ from app.users import current_active_user, current_optional_user +pytestmark = pytest.mark.unit + def _make_request(proxy_user=None) -> Request: """Build a minimal Request whose .state.proxy_user is settable.""" From 7f21e5bbe3cb44bc127fa3077a844019bec85c2e Mon Sep 17 00:00:00 2001 From: Awais Qureshi Date: Mon, 18 May 2026 10:22:54 +0500 Subject: [PATCH 27/29] fix(auth): move SMB auto-join from current_active_user to on_after_register (#21) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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) * 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) --------- Co-authored-by: Claude Opus 4.7 (1M context) --- surfsense_backend/app/users.py | 38 ++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/surfsense_backend/app/users.py b/surfsense_backend/app/users.py index 0fc8b6f72..e4d8f420e 100644 --- a/surfsense_backend/app/users.py +++ b/surfsense_backend/app/users.py @@ -16,7 +16,6 @@ from sqlalchemy import update from app.config import config -from app.services.smb_auto_join import auto_join_smb_search_space from app.db import ( Prompt, SearchSpace, @@ -28,6 +27,7 @@ get_user_db, ) from app.prompts.system_defaults import SYSTEM_PROMPT_DEFAULTS +from app.services.smb_auto_join import auto_join_smb_search_space from app.utils.refresh_tokens import create_refresh_token logger = logging.getLogger(__name__) @@ -212,6 +212,23 @@ async def on_after_register(self, user: User, request: Request | None = None): f"Failed to create default search space for user {user.id}: {e}" ) + # SMB auto-join — runs once at user creation, NOT on every request. + # Per-request auto-join silently re-grants membership to users an + # operator has explicitly removed (the DELETE /searchspaces/{id}/ + # members/{membership_id} endpoint hard-deletes the row, leaving no + # tombstone for the auto-join function to consult). Doing it here + # keeps removed users removed. + # + # Trade-off: users who registered BEFORE the SMB workspace existed + # are not retroactively auto-joined — operators can backfill those + # with a one-time SQL INSERT against `search_space_memberships`. + try: + await auto_join_smb_search_space(user.id) + except Exception: + logger.exception( + "SMB auto-join failed for newly registered user %s", user.id + ) + async def on_after_forgot_password( self, user: User, token: str, request: Request | None = None ): @@ -314,26 +331,15 @@ async def current_active_user( so existing email/password and Google OAuth flows continue to work when proxy auth is disabled. - SMB shared SearchSpace membership is enforced here — not only in - ProxyAuthMiddleware — because after proxy-login the browser usually sends Bearer - JWT without X-Auth-Request-Email, so middleware alone would never run auto-join. + SMB shared SearchSpace auto-join runs in `on_after_register` (one-shot + at user creation), NOT here on every request. Per-request auto-join + silently re-grants membership to users that operators have explicitly + removed via DELETE /searchspaces/{id}/members/{membership_id}. """ proxy_user = getattr(request.state, "proxy_user", None) if proxy_user is not None: - try: - await auto_join_smb_search_space(proxy_user.id) - except Exception: - logger.exception( - "SMB auto-join failed for proxy session user %s", proxy_user.id - ) return proxy_user if jwt_user is not None: - try: - await auto_join_smb_search_space(jwt_user.id) - except Exception: - logger.exception( - "SMB auto-join failed for JWT user %s", jwt_user.id - ) return jwt_user raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, From 568bbd374d0b87063413c0eda3ae2c196f3bea74 Mon Sep 17 00:00:00 2001 From: Usama Sadiq Date: Mon, 18 May 2026 15:58:18 +0500 Subject: [PATCH 28/29] fix: pin pnpm@10.24.0 --- surfsense_web/Dockerfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/surfsense_web/Dockerfile b/surfsense_web/Dockerfile index b16b3f066..942ea2958 100644 --- a/surfsense_web/Dockerfile +++ b/surfsense_web/Dockerfile @@ -9,7 +9,7 @@ RUN apk add --no-cache libc6-compat WORKDIR /app # Install pnpm -RUN corepack enable pnpm +RUN corepack enable pnpm@10.24.0 # Copy package files COPY package.json pnpm-lock.yaml* .npmrc* ./ @@ -27,7 +27,7 @@ FROM base AS builder WORKDIR /app # Enable pnpm -RUN corepack enable pnpm +RUN corepack enable pnpm@10.24.0 # Build with placeholder values for NEXT_PUBLIC_* variables. # These are replaced at container startup by docker-entrypoint.js From f983da352f6055fe858fc52d4c43575b9b4908fb Mon Sep 17 00:00:00 2001 From: Usama Sadiq Date: Mon, 18 May 2026 17:14:53 +0500 Subject: [PATCH 29/29] fix: activate pinned version of pnpm --- surfsense_web/Dockerfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/surfsense_web/Dockerfile b/surfsense_web/Dockerfile index 942ea2958..aad2bb4f3 100644 --- a/surfsense_web/Dockerfile +++ b/surfsense_web/Dockerfile @@ -9,7 +9,7 @@ RUN apk add --no-cache libc6-compat WORKDIR /app # Install pnpm -RUN corepack enable pnpm@10.24.0 +RUN corepack enable pnpm && corepack prepare pnpm@10.33.4 --activate # Copy package files COPY package.json pnpm-lock.yaml* .npmrc* ./ @@ -27,7 +27,7 @@ FROM base AS builder WORKDIR /app # Enable pnpm -RUN corepack enable pnpm@10.24.0 +RUN corepack enable pnpm && corepack prepare pnpm@10.33.4 --activate # Build with placeholder values for NEXT_PUBLIC_* variables. # These are replaced at container startup by docker-entrypoint.js