From e163708bad2f02f81ffe247338392a57a83f56d5 Mon Sep 17 00:00:00 2001 From: "DESKTOP-RTLN3BA\\$punk" Date: Tue, 20 Jan 2026 02:59:32 -0800 Subject: [PATCH] feat: Refactor role permissions and access checks - Removed the Admin role and updated the permission sets for Owner, Editor, and Viewer roles in db.py. - Clarified access requirements for chat threads in new_chat_routes.py, ensuring ownership checks are prioritized. - Added preset permission options for quick role creation in the CreateRoleDialog component of the team page. --- .../versions/72_simplify_rbac_roles.py | 300 ++++++++++++++++++ surfsense_backend/app/db.py | 91 ++---- .../app/routes/new_chat_routes.py | 31 +- .../dashboard/[search_space_id]/team/page.tsx | 95 +++++- 4 files changed, 417 insertions(+), 100 deletions(-) create mode 100644 surfsense_backend/alembic/versions/72_simplify_rbac_roles.py diff --git a/surfsense_backend/alembic/versions/72_simplify_rbac_roles.py b/surfsense_backend/alembic/versions/72_simplify_rbac_roles.py new file mode 100644 index 000000000..e7d5ff019 --- /dev/null +++ b/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 + """) + ) diff --git a/surfsense_backend/app/db.py b/surfsense_backend/app/db.py index 2b514483a..38e27ecf2 100644 --- a/surfsense_backend/app/db.py +++ b/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 + # Chats (no delete) 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) - Permission.LLM_CONFIGS_READ.value, + # LLM Configs (no delete) 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 (full access for editors) + # Connectors (no delete) 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, diff --git a/surfsense_backend/app/routes/new_chat_routes.py b/surfsense_backend/app/routes/new_chat_routes.py index 7a5224ba6..8fddc55c4 100644 --- a/surfsense_backend/app/routes/new_chat_routes.py +++ b/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) - For SEARCH_SPACE threads, any member with permission can access - Legacy threads (NULL creator) are accessible by search space owner + require_ownership: If True, ONLY the creator can perform this action (e.g., changing visibility). + This is checked FIRST, before visibility rules. 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 - # This check comes first so shared threads are always accessible + # If ownership is required (e.g., changing visibility), ONLY the creator can do it + # 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 diff --git a/surfsense_web/app/dashboard/[search_space_id]/team/page.tsx b/surfsense_web/app/dashboard/[search_space_id]/team/page.tsx index b8ba2d77b..f00982555 100644 --- a/surfsense_web/app/dashboard/[search_space_id]/team/page.tsx +++ b/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" )} > @@ -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 ( @@ -1416,7 +1462,34 @@ function CreateRoleDialog({ />
- +
+ +
+ + +
+
+

+ Use presets to quickly apply Editor (create/read/update) or Viewer (read-only) permissions +

{Object.entries(groupedPermissions).map(([category, perms]) => { @@ -1427,10 +1500,8 @@ function CreateRoleDialog({ return (
- +
{perms.map((perm) => ( - + ))}