From 4f7590e3130c0b6600b1c61f2eb4e8eca763f742 Mon Sep 17 00:00:00 2001 From: awais786 Date: Fri, 15 May 2026 14:50:41 +0500 Subject: [PATCH 1/2] fix(auth): move SMB auto-join from current_active_user to on_after_register MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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) --- surfsense_backend/app/users.py | 36 ++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/surfsense_backend/app/users.py b/surfsense_backend/app/users.py index 0fc8b6f722..08738f615d 100644 --- a/surfsense_backend/app/users.py +++ b/surfsense_backend/app/users.py @@ -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 fd3dd869a9604572e14d246a840c130a1f9a51bf Mon Sep 17 00:00:00 2001 From: awais786 Date: Fri, 15 May 2026 15:19:59 +0500 Subject: [PATCH 2/2] chore: fix ruff import ordering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- surfsense_backend/app/users.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/surfsense_backend/app/users.py b/surfsense_backend/app/users.py index 08738f615d..e4d8f420e4 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__)