Merge pull request #715 from MODSetter/dev

chore: fixed rback roles

surfsense_backend/alembic/versions/72_simplify_rbac_roles.py

@@ -0,0 +1,300 @@
+"""Simplify RBAC roles - Remove Admin role, keep only Owner, Editor, Viewer
+
+Revision ID: 72
+Revises: 71
+Create Date: 2025-01-20
+
+This migration:
+1. Moves any users with Admin role to Editor role
+2. Updates invites that reference Admin role to use Editor role
+3. Deletes the Admin role from all search spaces
+4. Updates Editor permissions to the new simplified set (everything except delete)
+5. Updates Viewer permissions to the new simplified set (read-only + comments)
+"""
+
+import sqlalchemy as sa
+
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision = "72"
+down_revision = "71"
+branch_labels = None
+depends_on = None
+
+# New Editor permissions (can do everything except delete, manage roles, and update settings)
+NEW_EDITOR_PERMISSIONS = [
+    "documents:create",
+    "documents:read",
+    "documents:update",
+    "chats:create",
+    "chats:read",
+    "chats:update",
+    "comments:create",
+    "comments:read",
+    "llm_configs:create",
+    "llm_configs:read",
+    "llm_configs:update",
+    "podcasts:create",
+    "podcasts:read",
+    "podcasts:update",
+    "connectors:create",
+    "connectors:read",
+    "connectors:update",
+    "logs:read",
+    "members:invite",
+    "members:view",
+    "roles:read",
+    "settings:view",
+]
+
+# New Viewer permissions (read-only + comments)
+NEW_VIEWER_PERMISSIONS = [
+    "documents:read",
+    "chats:read",
+    "comments:create",
+    "comments:read",
+    "llm_configs:read",
+    "podcasts:read",
+    "connectors:read",
+    "logs:read",
+    "members:view",
+    "roles:read",
+    "settings:view",
+]
+
+
+def upgrade():
+    connection = op.get_bind()
+
+    # Step 1: For each search space, get the Editor role ID and Admin role ID
+    search_spaces = connection.execute(
+        sa.text("SELECT id FROM searchspaces")
+    ).fetchall()
+
+    for (ss_id,) in search_spaces:
+        # Get Admin and Editor role IDs for this search space
+        admin_role = connection.execute(
+            sa.text("""
+                SELECT id FROM search_space_roles 
+                WHERE search_space_id = :ss_id AND name = 'Admin'
+            """),
+            {"ss_id": ss_id},
+        ).fetchone()
+
+        editor_role = connection.execute(
+            sa.text("""
+                SELECT id FROM search_space_roles 
+                WHERE search_space_id = :ss_id AND name = 'Editor'
+            """),
+            {"ss_id": ss_id},
+        ).fetchone()
+
+        if admin_role and editor_role:
+            admin_role_id = admin_role[0]
+            editor_role_id = editor_role[0]
+
+            # Step 2: Move all memberships from Admin to Editor
+            connection.execute(
+                sa.text("""
+                    UPDATE search_space_memberships 
+                    SET role_id = :editor_role_id 
+                    WHERE role_id = :admin_role_id
+                """),
+                {"editor_role_id": editor_role_id, "admin_role_id": admin_role_id},
+            )
+
+            # Step 3: Move all invites from Admin to Editor
+            connection.execute(
+                sa.text("""
+                    UPDATE search_space_invites 
+                    SET role_id = :editor_role_id 
+                    WHERE role_id = :admin_role_id
+                """),
+                {"editor_role_id": editor_role_id, "admin_role_id": admin_role_id},
+            )
+
+            # Step 4: Delete the Admin role
+            connection.execute(
+                sa.text("""
+                    DELETE FROM search_space_roles 
+                    WHERE id = :admin_role_id
+                """),
+                {"admin_role_id": admin_role_id},
+            )
+
+    # Step 5: Update Editor permissions for all search spaces
+    editor_perms_literal = (
+        "ARRAY[" + ",".join(f"'{p}'" for p in NEW_EDITOR_PERMISSIONS) + "]::TEXT[]"
+    )
+    connection.execute(
+        sa.text(f"""
+            UPDATE search_space_roles 
+            SET permissions = {editor_perms_literal},
+                description = 'Can create and update content (no delete, role management, or settings access)'
+            WHERE name = 'Editor' AND is_system_role = TRUE
+        """)
+    )
+
+    # Step 6: Update Viewer permissions for all search spaces
+    viewer_perms_literal = (
+        "ARRAY[" + ",".join(f"'{p}'" for p in NEW_VIEWER_PERMISSIONS) + "]::TEXT[]"
+    )
+    connection.execute(
+        sa.text(f"""
+            UPDATE search_space_roles 
+            SET permissions = {viewer_perms_literal}
+            WHERE name = 'Viewer' AND is_system_role = TRUE
+        """)
+    )
+
+
+def downgrade():
+    """
+    Downgrade recreates the Admin role and restores original permissions.
+    Note: Users who were moved from Admin to Editor will remain as Editor.
+    """
+    connection = op.get_bind()
+
+    # Old Admin permissions
+    old_admin_permissions = [
+        "documents:create",
+        "documents:read",
+        "documents:update",
+        "documents:delete",
+        "chats:create",
+        "chats:read",
+        "chats:update",
+        "chats:delete",
+        "comments:create",
+        "comments:read",
+        "comments:delete",
+        "llm_configs:create",
+        "llm_configs:read",
+        "llm_configs:update",
+        "llm_configs:delete",
+        "podcasts:create",
+        "podcasts:read",
+        "podcasts:update",
+        "podcasts:delete",
+        "connectors:create",
+        "connectors:read",
+        "connectors:update",
+        "connectors:delete",
+        "logs:read",
+        "logs:delete",
+        "members:invite",
+        "members:view",
+        "members:remove",
+        "members:manage_roles",
+        "roles:create",
+        "roles:read",
+        "roles:update",
+        "roles:delete",
+        "settings:view",
+        "settings:update",
+    ]
+
+    # Old Editor permissions
+    old_editor_permissions = [
+        "documents:create",
+        "documents:read",
+        "documents:update",
+        "documents:delete",
+        "chats:create",
+        "chats:read",
+        "chats:update",
+        "chats:delete",
+        "comments:create",
+        "comments:read",
+        "llm_configs:read",
+        "llm_configs:create",
+        "llm_configs:update",
+        "podcasts:create",
+        "podcasts:read",
+        "podcasts:update",
+        "podcasts:delete",
+        "connectors:create",
+        "connectors:read",
+        "connectors:update",
+        "logs:read",
+        "members:view",
+        "roles:read",
+        "settings:view",
+    ]
+
+    # Old Viewer permissions
+    old_viewer_permissions = [
+        "documents:read",
+        "chats:read",
+        "comments:create",
+        "comments:read",
+        "llm_configs:read",
+        "podcasts:read",
+        "connectors:read",
+        "logs:read",
+        "members:view",
+        "roles:read",
+        "settings:view",
+    ]
+
+    # Recreate Admin role for each search space
+    search_spaces = connection.execute(
+        sa.text("SELECT id FROM searchspaces")
+    ).fetchall()
+
+    admin_perms_literal = (
+        "ARRAY[" + ",".join(f"'{p}'" for p in old_admin_permissions) + "]::TEXT[]"
+    )
+
+    for (ss_id,) in search_spaces:
+        # Check if Admin role already exists
+        existing = connection.execute(
+            sa.text("""
+                SELECT id FROM search_space_roles 
+                WHERE search_space_id = :ss_id AND name = 'Admin'
+            """),
+            {"ss_id": ss_id},
+        ).fetchone()
+
+        if not existing:
+            connection.execute(
+                sa.text(f"""
+                    INSERT INTO search_space_roles 
+                    (name, description, permissions, is_default, is_system_role, search_space_id)
+                    VALUES (
+                        'Admin',
+                        'Can manage most resources except deleting the search space',
+                        {admin_perms_literal},
+                        FALSE,
+                        TRUE,
+                        :ss_id
+                    )
+                """),
+                {"ss_id": ss_id},
+            )
+
+    # Restore old Editor permissions
+    editor_perms_literal = (
+        "ARRAY[" + ",".join(f"'{p}'" for p in old_editor_permissions) + "]::TEXT[]"
+    )
+    connection.execute(
+        sa.text(f"""
+            UPDATE search_space_roles 
+            SET permissions = {editor_perms_literal},
+                description = 'Can create and edit documents, chats, and podcasts'
+            WHERE name = 'Editor' AND is_system_role = TRUE
+        """)
+    )
+
+    # Restore old Viewer permissions
+    viewer_perms_literal = (
+        "ARRAY[" + ",".join(f"'{p}'" for p in old_viewer_permissions) + "]::TEXT[]"
+    )
+    connection.execute(
+        sa.text(f"""
+            UPDATE search_space_roles 
+            SET permissions = {viewer_perms_literal}
+            WHERE name = 'Viewer' AND is_system_role = TRUE
+        """)
+    )

surfsense_backend/app/agents/new_chat/tools/mcp_client.py

@@ -292,9 +292,7 @@ async def test_mcp_http_connection(
                     }
                 )
 
-            logger.info(
+            logger.info("HTTP MCP connection successful. Found %d tools.", len(tools))
-                "HTTP MCP connection successful. Found %d tools.", len(tools)
-            )
             return {
                 "status": "success",
                 "message": f"Connected successfully. Found {len(tools)} tools.",

surfsense_backend/app/db.py

@@ -201,89 +201,42 @@ class Permission(str, Enum):
 
 
 # Predefined role permission sets for convenience
+# Note: Only Owner, Editor, and Viewer roles are supported.
+# Owner has full access (*), Editor can do everything except delete, Viewer has read-only access.
 DEFAULT_ROLE_PERMISSIONS = {
     "Owner": [Permission.FULL_ACCESS.value],
-    "Admin": [
-        # Documents
-        Permission.DOCUMENTS_CREATE.value,
-        Permission.DOCUMENTS_READ.value,
-        Permission.DOCUMENTS_UPDATE.value,
-        Permission.DOCUMENTS_DELETE.value,
-        # Chats
-        Permission.CHATS_CREATE.value,
-        Permission.CHATS_READ.value,
-        Permission.CHATS_UPDATE.value,
-        Permission.CHATS_DELETE.value,
-        # Comments
-        Permission.COMMENTS_CREATE.value,
-        Permission.COMMENTS_READ.value,
-        Permission.COMMENTS_DELETE.value,
-        # LLM Configs
-        Permission.LLM_CONFIGS_CREATE.value,
-        Permission.LLM_CONFIGS_READ.value,
-        Permission.LLM_CONFIGS_UPDATE.value,
-        Permission.LLM_CONFIGS_DELETE.value,
-        # Podcasts
-        Permission.PODCASTS_CREATE.value,
-        Permission.PODCASTS_READ.value,
-        Permission.PODCASTS_UPDATE.value,
-        Permission.PODCASTS_DELETE.value,
-        # Connectors
-        Permission.CONNECTORS_CREATE.value,
-        Permission.CONNECTORS_READ.value,
-        Permission.CONNECTORS_UPDATE.value,
-        Permission.CONNECTORS_DELETE.value,
-        # Logs
-        Permission.LOGS_READ.value,
-        Permission.LOGS_DELETE.value,
-        # Members
-        Permission.MEMBERS_INVITE.value,
-        Permission.MEMBERS_VIEW.value,
-        Permission.MEMBERS_REMOVE.value,
-        Permission.MEMBERS_MANAGE_ROLES.value,
-        # Roles
-        Permission.ROLES_CREATE.value,
-        Permission.ROLES_READ.value,
-        Permission.ROLES_UPDATE.value,
-        Permission.ROLES_DELETE.value,
-        # Settings (no delete)
-        Permission.SETTINGS_VIEW.value,
-        Permission.SETTINGS_UPDATE.value,
-    ],
     "Editor": [
-        # Documents
+        # Documents (no delete)
         Permission.DOCUMENTS_CREATE.value,
         Permission.DOCUMENTS_READ.value,
         Permission.DOCUMENTS_UPDATE.value,
-        Permission.DOCUMENTS_DELETE.value,
+        # Chats (no delete)
-        # Chats
         Permission.CHATS_CREATE.value,
         Permission.CHATS_READ.value,
         Permission.CHATS_UPDATE.value,
-        Permission.CHATS_DELETE.value,
         # Comments (no delete)
         Permission.COMMENTS_CREATE.value,
         Permission.COMMENTS_READ.value,
-        # LLM Configs (read only)
+        # LLM Configs (no delete)
-        Permission.LLM_CONFIGS_READ.value,
         Permission.LLM_CONFIGS_CREATE.value,
+        Permission.LLM_CONFIGS_READ.value,
         Permission.LLM_CONFIGS_UPDATE.value,
-        # Podcasts
+        # Podcasts (no delete)
         Permission.PODCASTS_CREATE.value,
         Permission.PODCASTS_READ.value,
         Permission.PODCASTS_UPDATE.value,
-        Permission.PODCASTS_DELETE.value,
+        # Connectors (no delete)
-        # Connectors (full access for editors)
         Permission.CONNECTORS_CREATE.value,
         Permission.CONNECTORS_READ.value,
         Permission.CONNECTORS_UPDATE.value,
-        # Logs
+        # Logs (read only)
         Permission.LOGS_READ.value,
-        # Members (view only)
+        # Members (can invite and view only, cannot manage roles or remove)
+        Permission.MEMBERS_INVITE.value,
         Permission.MEMBERS_VIEW.value,
-        # Roles (read only)
+        # Roles (read only - cannot create, update, or delete)
         Permission.ROLES_READ.value,
-        # Settings (view only)
+        # Settings (view only, no update or delete)
         Permission.SETTINGS_VIEW.value,
     ],
     "Viewer": [
@@ -291,7 +244,7 @@ DEFAULT_ROLE_PERMISSIONS = {
         Permission.DOCUMENTS_READ.value,
         # Chats (read only)
         Permission.CHATS_READ.value,
-        # Comments (no delete)
+        # Comments (can create and read, but not delete)
         Permission.COMMENTS_CREATE.value,
         Permission.COMMENTS_READ.value,
         # LLM Configs (read only)
@@ -865,7 +818,7 @@ class SearchSpaceRole(BaseModel, TimestampMixin):
     permissions = Column(ARRAY(String), nullable=False, default=[])
     # Whether this role is assigned to new members by default when they join via invite
     is_default = Column(Boolean, nullable=False, default=False)
-    # System roles (Owner, Admin, Editor, Viewer) cannot be deleted
+    # System roles (Owner, Editor, Viewer) cannot be deleted
     is_system_role = Column(Boolean, nullable=False, default=False)
 
     search_space_id = Column(
@@ -1221,6 +1174,11 @@ def get_default_roles_config() -> list[dict]:
     Get the configuration for default system roles.
     These roles are created automatically when a search space is created.
 
+    Only 3 roles are supported:
+    - Owner: Full access to everything (assigned to search space creator)
+    - Editor: Can create/update content but cannot delete, manage roles, or change settings
+    - Viewer: Read-only access to resources (can add comments)
+
     Returns:
         List of role configurations with name, description, permissions, and flags
     """
@@ -1232,16 +1190,9 @@ def get_default_roles_config() -> list[dict]:
             "is_default": False,
             "is_system_role": True,
         },
-        {
-            "name": "Admin",
-            "description": "Can manage most resources except deleting the search space",
-            "permissions": DEFAULT_ROLE_PERMISSIONS["Admin"],
-            "is_default": False,
-            "is_system_role": True,
-        },
         {
             "name": "Editor",
-            "description": "Can create and edit documents, chats, and podcasts",
+            "description": "Can create and update content (no delete, role management, or settings access)",
             "permissions": DEFAULT_ROLE_PERMISSIONS["Editor"],
             "is_default": True,  # Default role for new members via invite
             "is_system_role": True,

surfsense_backend/app/routes/new_chat_routes.py

@@ -67,16 +67,15 @@ async def check_thread_access(
 
     Access is granted if:
     - User is the creator of the thread
-    - Thread visibility is SEARCH_SPACE (any member can access)
+    - Thread visibility is SEARCH_SPACE (any member can access) - for read/update operations only
     - Thread is a legacy thread (created_by_id is NULL) - only if user is search space owner
 
     Args:
         session: Database session
         thread: The thread to check access for
         user: The user requesting access
-        require_ownership: If True, only the creator can access (for edit/delete operations)
+        require_ownership: If True, ONLY the creator can perform this action (e.g., changing visibility).
-                          For SEARCH_SPACE threads, any member with permission can access
+                          This is checked FIRST, before visibility rules.
-                          Legacy threads (NULL creator) are accessible by search space owner
 
     Returns:
         True if access is granted
@@ -87,11 +86,18 @@ async def check_thread_access(
     is_owner = thread.created_by_id == user.id
     is_legacy = thread.created_by_id is None
 
-    # Shared threads (SEARCH_SPACE) are accessible by any member
+    # If ownership is required (e.g., changing visibility), ONLY the creator can do it
-    # This check comes first so shared threads are always accessible
+    # This check comes first to ensure ownership-required operations are always creator-only
+    if require_ownership:
+        if not is_owner:
+            raise HTTPException(
+                status_code=403,
+                detail="Only the creator of this chat can perform this action",
+            )
+        return True
+
+    # Shared threads (SEARCH_SPACE) are accessible by any member for read/update operations
     if thread.visibility == ChatVisibility.SEARCH_SPACE:
-        # For ownership-required operations on shared threads, any member can proceed
-        # (permission check is done at route level)
         return True
 
     # For legacy threads (created before visibility feature),
@@ -112,15 +118,6 @@ async def check_thread_access(
             detail="You don't have access to this chat",
         )
 
-    # If ownership is required, only the creator can access
-    if require_ownership:
-        if not is_owner:
-            raise HTTPException(
-                status_code=403,
-                detail="Only the creator of this chat can perform this action",
-            )
-        return True
-
     # For read access: owner can access their own private threads
     if is_owner:
         return True

surfsense_web/app/dashboard/[search_space_id]/team/page.tsx

@@ -767,20 +767,18 @@ function RolesTab({
 										className={cn(
 											"h-10 w-10 rounded-lg flex items-center justify-center",
 											role.name === "Owner" && "bg-amber-500/20",
-											role.name === "Admin" && "bg-red-500/20",
 											role.name === "Editor" && "bg-blue-500/20",
 											role.name === "Viewer" && "bg-gray-500/20",
-											!["Owner", "Admin", "Editor", "Viewer"].includes(role.name) && "bg-primary/20"
+											!["Owner", "Editor", "Viewer"].includes(role.name) && "bg-primary/20"
 										)}
 									>
 										<ShieldCheck
 											className={cn(
 												"h-5 w-5",
 												role.name === "Owner" && "text-amber-600",
-												role.name === "Admin" && "text-red-600",
 												role.name === "Editor" && "text-blue-600",
 												role.name === "Viewer" && "text-gray-600",
-												!["Owner", "Admin", "Editor", "Viewer"].includes(role.name) &&
+												!["Owner", "Editor", "Viewer"].includes(role.name) &&
 													"text-primary"
 											)}
 										/>
@@ -1310,6 +1308,49 @@ function CreateInviteDialog({
 
 // ============ Create Role Dialog ============
 
+// Preset permission sets for quick role creation
+// Editor: can create/read/update content, but cannot manage roles, remove members, or change settings
+// Viewer: read-only access with ability to create comments
+const PRESET_PERMISSIONS = {
+	editor: [
+		"documents:create",
+		"documents:read",
+		"documents:update",
+		"chats:create",
+		"chats:read",
+		"chats:update",
+		"comments:create",
+		"comments:read",
+		"llm_configs:create",
+		"llm_configs:read",
+		"llm_configs:update",
+		"podcasts:create",
+		"podcasts:read",
+		"podcasts:update",
+		"connectors:create",
+		"connectors:read",
+		"connectors:update",
+		"logs:read",
+		"members:invite",
+		"members:view",
+		"roles:read",
+		"settings:view",
+	],
+	viewer: [
+		"documents:read",
+		"chats:read",
+		"comments:create",
+		"comments:read",
+		"llm_configs:read",
+		"podcasts:read",
+		"connectors:read",
+		"logs:read",
+		"members:view",
+		"roles:read",
+		"settings:view",
+	],
+};
+
 function CreateRoleDialog({
 	groupedPermissions,
 	onCreateRole,
@@ -1369,6 +1410,11 @@ function CreateRoleDialog({
 		}
 	};
 
+	const applyPreset = (preset: "editor" | "viewer") => {
+		setSelectedPermissions(PRESET_PERMISSIONS[preset]);
+		toast.success(`Applied ${preset === "editor" ? "Editor" : "Viewer"} preset permissions`);
+	};
+
 	return (
 		<Dialog open={open} onOpenChange={setOpen}>
 			<DialogTrigger asChild>
@@ -1416,7 +1462,34 @@ function CreateRoleDialog({
 						/>
 					</div>
 					<div className="space-y-2">
-						<Label>Permissions ({selectedPermissions.length} selected)</Label>
+						<div className="flex items-center justify-between">
+							<Label>Permissions ({selectedPermissions.length} selected)</Label>
+							<div className="flex gap-2">
+								<Button
+									type="button"
+									variant="outline"
+									size="sm"
+									className="h-7 text-xs gap-1"
+									onClick={() => applyPreset("editor")}
+								>
+									<ShieldCheck className="h-3 w-3 text-blue-600" />
+									Editor Preset
+								</Button>
+								<Button
+									type="button"
+									variant="outline"
+									size="sm"
+									className="h-7 text-xs gap-1"
+									onClick={() => applyPreset("viewer")}
+								>
+									<ShieldCheck className="h-3 w-3 text-gray-600" />
+									Viewer Preset
+								</Button>
+							</div>
+						</div>
+						<p className="text-xs text-muted-foreground">
+							Use presets to quickly apply Editor (create/read/update) or Viewer (read-only) permissions
+						</p>
 						<ScrollArea className="h-64 rounded-lg border p-4">
 							<div className="space-y-4">
 								{Object.entries(groupedPermissions).map(([category, perms]) => {
@@ -1427,10 +1500,8 @@ function CreateRoleDialog({
 
 									return (
 										<div key={category} className="space-y-2">
-											<button
+											<label
-												type="button"
 												className="flex items-center gap-2 cursor-pointer hover:bg-muted/50 p-1 rounded w-full text-left"
-												onClick={() => toggleCategory(category)}
 											>
 												<Checkbox
 													checked={allSelected}
@@ -1439,21 +1510,19 @@ function CreateRoleDialog({
 												<span className="text-sm font-medium capitalize">
 													{category} ({categorySelected}/{perms.length})
 												</span>
-											</button>
+											</label>
 											<div className="grid grid-cols-2 gap-2 ml-6">
 												{perms.map((perm) => (
-													<button
+													<label
-														type="button"
 														key={perm.value}
 														className="flex items-center gap-2 cursor-pointer text-left"
-														onClick={() => togglePermission(perm.value)}
 													>
 														<Checkbox
 															checked={selectedPermissions.includes(perm.value)}
 															onCheckedChange={() => togglePermission(perm.value)}
 														/>
 														<span className="text-xs">{perm.value.split(":")[1]}</span>
-													</button>
+													</label>
 												))}
 											</div>
 										</div>