Curators can now update the curator relationship (#3536)

* Curators can now update the curator relationship

* mypy

* mypy

* whoops haha

backend/ee/onyx/db/user_group.py

@@ -122,7 +122,7 @@ def _cleanup_document_set__user_group_relationships__no_commit(
     )
 
 
-def validate_user_creation_permissions(
+def validate_object_creation_for_user(
     db_session: Session,
     user: User | None,
     target_group_ids: list[int] | None = None,
@@ -440,32 +440,108 @@ def remove_curator_status__no_commit(db_session: Session, user: User) -> None:
     _validate_curator_status__no_commit(db_session, [user])
 
 
-def update_user_curator_relationship(
+def _validate_curator_relationship_update_requester(
     db_session: Session,
     user_group_id: int,
-    set_curator_request: SetCuratorRequest,
+    user_making_change: User | None = None,
 ) -> None:
-    user = fetch_user_by_id(db_session, set_curator_request.user_id)
+    """
-    if not user:
+    This function validates that the user making the change has the necessary permissions
-        raise ValueError(f"User with id '{set_curator_request.user_id}' not found")
+    to update the curator relationship for the target user in the given user group.
+    """
 
-    if user.role == UserRole.ADMIN:
+    if user_making_change is None or user_making_change.role == UserRole.ADMIN:
+        return
+
+    # check if the user making the change is a curator in the group they are changing the curator relationship for
+    user_making_change_curator_groups = fetch_user_groups_for_user(
+        db_session=db_session,
+        user_id=user_making_change.id,
+        # only check if the user making the change is a curator if they are a curator
+        # otherwise, they are a global_curator and can update the curator relationship
+        # for any group they are a member of
+        only_curator_groups=user_making_change.role == UserRole.CURATOR,
+    )
+    requestor_curator_group_ids = [
+        group.id for group in user_making_change_curator_groups
+    ]
+    if user_group_id not in requestor_curator_group_ids:
         raise ValueError(
-            f"User '{user.email}' is an admin and therefore has all permissions "
+            f"user making change {user_making_change.email} is not a curator,"
+            f" admin, or global_curator for group '{user_group_id}'"
+        )
+
+
+def _validate_curator_relationship_update_request(
+    db_session: Session,
+    user_group_id: int,
+    target_user: User,
+) -> None:
+    """
+    This function validates that the curator_relationship_update request itself is valid.
+    """
+    if target_user.role == UserRole.ADMIN:
+        raise ValueError(
+            f"User '{target_user.email}' is an admin and therefore has all permissions "
             "of a curator. If you'd like this user to only have curator permissions, "
             "you must update their role to BASIC then assign them to be CURATOR in the "
             "appropriate groups."
         )
+    elif target_user.role == UserRole.GLOBAL_CURATOR:
+        raise ValueError(
+            f"User '{target_user.email}' is a global_curator and therefore has all "
+            "permissions of a curator for all groups. If you'd like this user to only "
+            "have curator permissions for a specific group, you must update their role "
+            "to BASIC then assign them to be CURATOR in the appropriate groups."
+        )
+    elif target_user.role not in [UserRole.CURATOR, UserRole.BASIC]:
+        raise ValueError(
+            f"This endpoint can only be used to update the curator relationship for "
+            "users with the CURATOR or BASIC role. \n"
+            f"Target user: {target_user.email} \n"
+            f"Target user role: {target_user.role} \n"
+        )
 
+    # check if the target user is in the group they are changing the curator relationship for
     requested_user_groups = fetch_user_groups_for_user(
         db_session=db_session,
-        user_id=set_curator_request.user_id,
+        user_id=target_user.id,
         only_curator_groups=False,
     )
-
     group_ids = [group.id for group in requested_user_groups]
     if user_group_id not in group_ids:
-        raise ValueError(f"user is not in group '{user_group_id}'")
+        raise ValueError(
+            f"target user {target_user.email} is not in group '{user_group_id}'"
+        )
+
+
+def update_user_curator_relationship(
+    db_session: Session,
+    user_group_id: int,
+    set_curator_request: SetCuratorRequest,
+    user_making_change: User | None = None,
+) -> None:
+    target_user = fetch_user_by_id(db_session, set_curator_request.user_id)
+    if not target_user:
+        raise ValueError(f"User with id '{set_curator_request.user_id}' not found")
+
+    _validate_curator_relationship_update_request(
+        db_session=db_session,
+        user_group_id=user_group_id,
+        target_user=target_user,
+    )
+
+    _validate_curator_relationship_update_requester(
+        db_session=db_session,
+        user_group_id=user_group_id,
+        user_making_change=user_making_change,
+    )
+
+    logger.info(
+        f"user_making_change={user_making_change.email if user_making_change else 'None'} is "
+        f"updating the curator relationship for user={target_user.email} "
+        f"in group={user_group_id} to is_curator={set_curator_request.is_curator}"
+    )
 
     relationship_to_update = (
         db_session.query(User__UserGroup)
@@ -486,7 +562,7 @@ def update_user_curator_relationship(
         )
         db_session.add(relationship_to_update)
 
-    _validate_curator_status__no_commit(db_session, [user])
+    _validate_curator_status__no_commit(db_session, [target_user])
     db_session.commit()
 
 

backend/ee/onyx/server/user_group/api.py

@@ -83,7 +83,7 @@ def patch_user_group(
 def set_user_curator(
     user_group_id: int,
     set_curator_request: SetCuratorRequest,
-    _: User | None = Depends(current_admin_user),
+    user: User | None = Depends(current_curator_or_admin_user),
     db_session: Session = Depends(get_session),
 ) -> None:
     try:
@@ -91,6 +91,7 @@ def set_user_curator(
             db_session=db_session,
             user_group_id=user_group_id,
             set_curator_request=set_curator_request,
+            user_making_change=user,
         )
     except ValueError as e:
         logger.error(f"Error setting user curator: {e}")

backend/onyx/server/documents/cc_pair.py

@@ -510,7 +510,7 @@ def associate_credential_to_connector(
     db_session: Session = Depends(get_session),
 ) -> StatusResponse[int]:
     fetch_ee_implementation_or_noop(
-        "onyx.db.user_group", "validate_user_creation_permissions", None
+        "onyx.db.user_group", "validate_object_creation_for_user", None
     )(
         db_session=db_session,
         user=user,

backend/onyx/server/documents/connector.py

@@ -680,7 +680,7 @@ def create_connector_from_model(
         _validate_connector_allowed(connector_data.source)
 
         fetch_ee_implementation_or_noop(
-            "onyx.db.user_group", "validate_user_creation_permissions", None
+            "onyx.db.user_group", "validate_object_creation_for_user", None
         )(
             db_session=db_session,
             user=user,
@@ -716,7 +716,7 @@ def create_connector_with_mock_credential(
     tenant_id: str = Depends(get_current_tenant_id),
 ) -> StatusResponse:
     fetch_ee_implementation_or_noop(
-        "onyx.db.user_group", "validate_user_creation_permissions", None
+        "onyx.db.user_group", "validate_object_creation_for_user", None
     )(
         db_session=db_session,
         user=user,
@@ -776,7 +776,7 @@ def update_connector_from_model(
     try:
         _validate_connector_allowed(connector_data.source)
         fetch_ee_implementation_or_noop(
-            "onyx.db.user_group", "validate_user_creation_permissions", None
+            "onyx.db.user_group", "validate_object_creation_for_user", None
         )(
             db_session=db_session,
             user=user,

backend/onyx/server/documents/credential.py

@@ -122,7 +122,7 @@ def create_credential_from_model(
 ) -> ObjectCreationIdResponse:
     if not _ignore_credential_permissions(credential_info.source):
         fetch_ee_implementation_or_noop(
-            "onyx.db.user_group", "validate_user_creation_permissions", None
+            "onyx.db.user_group", "validate_object_creation_for_user", None
         )(
             db_session=db_session,
             user=user,

backend/onyx/server/features/document_set/api.py

@@ -31,7 +31,7 @@ def create_document_set(
     db_session: Session = Depends(get_session),
 ) -> int:
     fetch_ee_implementation_or_noop(
-        "onyx.db.user_group", "validate_user_creation_permissions", None
+        "onyx.db.user_group", "validate_object_creation_for_user", None
     )(
         db_session=db_session,
         user=user,
@@ -56,7 +56,7 @@ def patch_document_set(
     db_session: Session = Depends(get_session),
 ) -> None:
     fetch_ee_implementation_or_noop(
-        "onyx.db.user_group", "validate_user_creation_permissions", None
+        "onyx.db.user_group", "validate_object_creation_for_user", None
     )(
         db_session=db_session,
         user=user,

web/src/app/ee/admin/groups/[groupId]/GroupDisplay.tsx

@@ -38,6 +38,7 @@ import { BookmarkIcon, RobotIcon } from "@/components/icons/icons";
 import { AddTokenRateLimitForm } from "./AddTokenRateLimitForm";
 import { GenericTokenRateLimitTable } from "@/app/admin/token-rate-limits/TokenRateLimitTables";
 import { useUser } from "@/components/user/UserProvider";
+import { GenericConfirmModal } from "@/components/modals/GenericConfirmModal";
 
 interface GroupDisplayProps {
   users: User[];
@@ -68,8 +69,13 @@ const UserRoleDropdown = ({
     return user.role;
   });
   const [isSettingRole, setIsSettingRole] = useState(false);
+  const [showDemoteConfirm, setShowDemoteConfirm] = useState(false);
+  const [pendingRoleChange, setPendingRoleChange] = useState<string | null>(
+    null
+  );
+  const { user: currentUser } = useUser();
 
-  const handleChange = async (value: string) => {
+  const applyRoleChange = async (value: string) => {
     if (value === localRole) return;
     if (value === UserRole.BASIC || value === UserRole.CURATOR) {
       setIsSettingRole(true);
@@ -95,30 +101,61 @@ const UserRoleDropdown = ({
     }
   };
 
-  const isEditable =
+  const handleChange = (value: string) => {
-    (user.role === UserRole.BASIC || user.role === UserRole.CURATOR) && isAdmin;
+    if (value === UserRole.BASIC && user.id === currentUser?.id) {
+      setPendingRoleChange(value);
+      setShowDemoteConfirm(true);
+    } else {
+      applyRoleChange(value);
+    }
+  };
 
-  if (isEditable) {
+  const isEditable =
-    return (
+    user.role === UserRole.BASIC || user.role === UserRole.CURATOR;
-      <div className="w-40">
+
-        <Select
+  return (
-          value={localRole}
+    <>
-          onValueChange={handleChange}
+      {/* Confirmation modal - only shown when users try to demote themselves */}
-          disabled={isSettingRole}
+      {showDemoteConfirm && pendingRoleChange && (
-        >
+        <GenericConfirmModal
-          <SelectTrigger>
+          title="Remove Yourself as a Curator for this Group?"
-            <SelectValue placeholder="Select role" />
+          message="Are you sure you want to change your role to Basic? This will remove your ability to curate this group."
-          </SelectTrigger>
+          confirmText="Yes, set me to Basic"
-          <SelectContent>
+          onClose={() => {
-            <SelectItem value={UserRole.BASIC}>Basic</SelectItem>
+            // Cancel the role change if user dismisses modal
-            <SelectItem value={UserRole.CURATOR}>Curator</SelectItem>
+            setShowDemoteConfirm(false);
-          </SelectContent>
+            setPendingRoleChange(null);
-        </Select>
+          }}
-      </div>
+          onConfirm={() => {
-    );
+            // Apply the role change if user confirms
-  } else {
+            setShowDemoteConfirm(false);
-    return <div>{USER_ROLE_LABELS[localRole]}</div>;
+            applyRoleChange(pendingRoleChange);
-  }
+            setPendingRoleChange(null);
+          }}
+        />
+      )}
+
+      {isEditable ? (
+        <div className="w-40">
+          <Select
+            value={localRole}
+            onValueChange={handleChange}
+            disabled={isSettingRole}
+          >
+            <SelectTrigger>
+              <SelectValue placeholder="Select role" />
+            </SelectTrigger>
+            <SelectContent>
+              <SelectItem value={UserRole.BASIC}>Basic</SelectItem>
+              <SelectItem value={UserRole.CURATOR}>Curator</SelectItem>
+            </SelectContent>
+          </Select>
+        </div>
+      ) : (
+        <div>{USER_ROLE_LABELS[localRole]}</div>
+      )}
+    </>
+  );
 };
 
 export const GroupDisplay = ({