gmakstutis/ragflow
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

[OND211-2329]: Updated remove member from team API and tests.

Browse source
This commit is contained in:
Hetavi Shah 2025-11-20 11:31:51 +05:30
parent 6c68429333
commit d5dcb4408e
3 changed files with 139 additions and 176 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

177
api/apps/tenant_app.py
View file

@ -1019,13 +1019,13 @@ def add_users(tenant_id: str) -> Response:
) )
@manager.route('/<tenant_id>/users/remove', methods=['POST']) # noqa: F821 @manager.route('/<tenant_id>/user/remove', methods=['POST']) # noqa: F821
@login_required @login_required
@validate_request("user_ids") @validate_request("user_id")
def remove_users(tenant_id: str) -> Response: def remove_user(tenant_id: str) -> Response:
""" """
Remove one or more users from a team. Only OWNER or ADMIN can remove users. Remove a user from a team. Only OWNER or ADMIN can remove users.
Owners cannot be removed. Supports both single user and bulk operations. Owners cannot be removed.
--- ---
tags: tags:
@ -1044,28 +1044,26 @@ def remove_users(tenant_id: str) -> Response:
schema: schema:
type: object type: object
required: required:
- user_ids - user_id
properties: properties:
user_ids: user_id:
type: array type: string
description: List of user IDs to remove description: User ID to remove
items:
type: string
responses: responses:
200: 200:
description: Users removed successfully description: User removed successfully
schema: schema:
type: object type: object
properties: properties:
data: data:
type: object type: object
properties: properties:
removed: user_id:
type: array type: string
description: Successfully removed user IDs description: Removed user ID
failed: email:
type: array type: string
description: Users that failed to be removed with error messages description: Removed user email
message: message:
type: string type: string
400: 400:
@ -1084,103 +1082,74 @@ def remove_users(tenant_id: str) -> Response:
) )
req: Dict[str, Any] = request.json if request.json is not None else {} req: Dict[str, Any] = request.json if request.json is not None else {}
user_ids: List[str] = req.get("user_ids", []) user_id: Optional[str] = req.get("user_id")
if not isinstance(user_ids, list) or len(user_ids) == 0: if not user_id or not isinstance(user_id, str):
return get_json_result( return get_json_result(
data=False, data=False,
message="'user_ids' must be a non-empty array.", message="'user_id' must be a non-empty string.",
code=RetCode.ARGUMENT_ERROR code=RetCode.ARGUMENT_ERROR
) )
removed_users: List[Dict[str, str]] = [] try:
failed_users: List[Dict[str, str]] = [] # Check if user exists in the team
user_tenant = UserTenantService.filter_by_tenant_and_user_id(tenant_id, user_id)
# Get all admins/owners for validation (check if removing would leave team without admin/owner) if not user_tenant:
all_user_tenants: List[Any] = UserTenantService.query(tenant_id=tenant_id) return get_json_result(
admin_owner_ids: Set[str] = { data=False,
ut.user_id for ut in all_user_tenants message="User is not a member of this team.",
if ut.role in [UserTenantRole.OWNER, UserTenantRole.ADMIN] and ut.status == StatusEnum.VALID.value code=RetCode.DATA_ERROR
} )
for user_id in user_ids:
if not isinstance(user_id, str):
failed_users.append({
"user_id": str(user_id),
"error": "Invalid user ID format."
})
continue
try: # Prevent removing the owner
# Check if user exists in the team if user_tenant.role == UserTenantRole.OWNER:
user_tenant = UserTenantService.filter_by_tenant_and_user_id(tenant_id, user_id) return get_json_result(
if not user_tenant: data=False,
failed_users.append({ message="Cannot remove the team owner.",
"user_id": user_id, code=RetCode.DATA_ERROR
"error": "User is not a member of this team." )
})
continue # Get all admins/owners for validation (check if removing would leave team without admin/owner)
all_user_tenants: List[Any] = UserTenantService.query(tenant_id=tenant_id)
# Prevent removing the owner admin_owner_ids: Set[str] = {
if user_tenant.role == UserTenantRole.OWNER: ut.user_id for ut in all_user_tenants
failed_users.append({ if ut.role in [UserTenantRole.OWNER, UserTenantRole.ADMIN] and ut.status == StatusEnum.VALID.value
"user_id": user_id, }
"error": "Cannot remove the team owner."
}) # Prevent removing yourself if you're the only admin
continue if user_id == current_user.id and user_tenant.role == UserTenantRole.ADMIN:
remaining_admins: Set[str] = admin_owner_ids - {user_id}
# Prevent removing yourself if you're the only admin if len(remaining_admins) == 0:
if user_id == current_user.id and user_tenant.role == UserTenantRole.ADMIN: return get_json_result(
remaining_admins: Set[str] = admin_owner_ids - {user_id} data=False,
if len(remaining_admins) == 0: message="Cannot remove yourself. At least one owner or admin must remain in the team.",
failed_users.append({ code=RetCode.DATA_ERROR
"user_id": user_id, )
"error": "Cannot remove yourself. At least one owner or admin must remain in the team."
}) # Remove user from team
continue UserTenantService.filter_delete([
UserTenant.tenant_id == tenant_id,
# Remove user from team UserTenant.user_id == user_id
UserTenantService.filter_delete([ ])
UserTenant.tenant_id == tenant_id,
UserTenant.user_id == user_id # Get user info for response
]) user: Optional[Any] = UserService.filter_by_id(user_id)
user_email: str = user.email if user else "Unknown"
# Get user info for response
user: Optional[Any] = UserService.filter_by_id(user_id) return get_json_result(
user_email: str = user.email if user else "Unknown" data={
removed_users.append({
"user_id": user_id, "user_id": user_id,
"email": user_email "email": user_email
}) },
message="User removed successfully."
except Exception as e:
logging.exception(f"Error removing user {user_id}: {e}")
failed_users.append({
"user_id": user_id,
"error": f"Failed to remove user: {str(e)}"
})
result: Dict[str, List[Dict[str, str]]] = {
"removed": removed_users,
"failed": failed_users
}
if failed_users and not removed_users:
return get_json_result(
data=result,
message=f"Failed to remove all users. {len(failed_users)} error(s).",
code=RetCode.DATA_ERROR
) )
elif failed_users:
except Exception as e:
logging.exception(f"Error removing user {user_id}: {e}")
return get_json_result( return get_json_result(
data=result, data=False,
message=f"Removed {len(removed_users)} user(s). {len(failed_users)} user(s) failed." message=f"Failed to remove user: {str(e)}",
) code=RetCode.EXCEPTION_ERROR
else:
return get_json_result(
data=result,
message=f"Successfully removed {len(removed_users)} user(s)."
) )

10
test/testcases/test_http_api/common.py
View file

@ -450,19 +450,19 @@ def add_users_to_team(
return res.json() return res.json()
def remove_users_from_team( def remove_user_from_team(
auth: Union[AuthBase, str, None], auth: Union[AuthBase, str, None],
tenant_id: str, tenant_id: str,
payload: Optional[Dict[str, Any]] = None, payload: Optional[Dict[str, Any]] = None,
*, *,
headers: Dict[str, str] = HEADERS, headers: Dict[str, str] = HEADERS,
) -> Dict[str, Any]: ) -> Dict[str, Any]:
"""Remove users from a team (tenant). """Remove a user from a team (tenant).
Args: Args:
auth: Authentication object (AuthBase subclass), token string, or None. auth: Authentication object (AuthBase subclass), token string, or None.
tenant_id: The tenant/team ID to remove users from. tenant_id: The tenant/team ID to remove user from.
payload: Optional JSON payload containing user_ids list. payload: Optional JSON payload containing user_id string.
headers: Optional HTTP headers. Defaults to HEADERS. headers: Optional HTTP headers. Defaults to HEADERS.
Returns: Returns:
@ -471,7 +471,7 @@ def remove_users_from_team(
Raises: Raises:
requests.RequestException: If the HTTP request fails. requests.RequestException: If the HTTP request fails.
""" """
url: str = f"{HOST_ADDRESS}{TEAM_API_URL}/{tenant_id}/users/remove" url: str = f"{HOST_ADDRESS}{TEAM_API_URL}/{tenant_id}/user/remove"
res: requests.Response = requests.post( res: requests.Response = requests.post(
url=url, headers=headers, auth=auth, json=payload url=url, headers=headers, auth=auth, json=payload
) )

128
test/testcases/test_http_api/test_team_management/test_remove_users.py
View file

@ -27,7 +27,7 @@ from common import (
create_user, create_user,
encrypt_password, encrypt_password,
login_as_user, login_as_user,
remove_users_from_team, remove_user_from_team,
) )
from configs import INVALID_API_TOKEN from configs import INVALID_API_TOKEN
from libs.auth import RAGFlowWebApiAuth from libs.auth import RAGFlowWebApiAuth
@ -83,16 +83,16 @@ class TestAuthorization:
add_users_to_team(web_api_auth, tenant_id, add_payload) add_users_to_team(web_api_auth, tenant_id, add_payload)
# Try to remove user with invalid auth # Try to remove user with invalid auth
remove_payload: dict[str, list[str]] = {"user_ids": [user_id]} remove_payload: dict[str, str] = {"user_id": user_id}
res: dict[str, Any] = remove_users_from_team(invalid_auth, tenant_id, remove_payload) res: dict[str, Any] = remove_user_from_team(invalid_auth, tenant_id, remove_payload)
assert res["code"] == expected_code, res assert res["code"] == expected_code, res
if expected_message: if expected_message:
assert expected_message in res["message"] assert expected_message in res["message"]
@pytest.mark.p1 @pytest.mark.p1
class TestRemoveUsers: class TestRemoveUser:
"""Comprehensive tests for removing users from a team.""" """Comprehensive tests for removing a user from a team."""
@pytest.fixture @pytest.fixture
def test_team(self, web_api_auth: RAGFlowWebApiAuth) -> dict[str, Any]: def test_team(self, web_api_auth: RAGFlowWebApiAuth) -> dict[str, Any]:
@ -151,36 +151,36 @@ class TestRemoveUsers:
tenant_id: str = team_with_users["team"]["id"] tenant_id: str = team_with_users["team"]["id"]
user_id: str = team_with_users["users"][0]["id"] user_id: str = team_with_users["users"][0]["id"]
remove_payload: dict[str, list[str]] = {"user_ids": [user_id]} remove_payload: dict[str, str] = {"user_id": user_id}
res: dict[str, Any] = remove_users_from_team(web_api_auth, tenant_id, remove_payload) res: dict[str, Any] = remove_user_from_team(web_api_auth, tenant_id, remove_payload)
assert res["code"] == 0, res assert res["code"] == 0, res
assert "data" in res assert "data" in res
assert "removed" in res["data"] assert res["data"]["user_id"] == user_id
assert len(res["data"]["removed"]) == 1 assert "email" in res["data"]
assert res["data"]["removed"][0]["user_id"] == user_id
assert "failed" in res["data"]
assert len(res["data"]["failed"]) == 0
@pytest.mark.p1 @pytest.mark.p1
def test_remove_multiple_users( def test_remove_multiple_users(
self, web_api_auth: RAGFlowWebApiAuth, team_with_users: dict[str, Any] self, web_api_auth: RAGFlowWebApiAuth, team_with_users: dict[str, Any]
) -> None: ) -> None:
"""Test removing multiple users in bulk.""" """Test removing multiple users one by one."""
if len(team_with_users["users"]) < 2: if len(team_with_users["users"]) < 2:
pytest.skip("Need at least 2 users in team") pytest.skip("Need at least 2 users in team")
tenant_id: str = team_with_users["team"]["id"] tenant_id: str = team_with_users["team"]["id"]
user_ids: list[str] = [user["id"] for user in team_with_users["users"][:2]] user_ids: list[str] = [user["id"] for user in team_with_users["users"][:2]]
remove_payload: dict[str, list[str]] = {"user_ids": user_ids} # Remove first user
res: dict[str, Any] = remove_users_from_team(web_api_auth, tenant_id, remove_payload) remove_payload1: dict[str, str] = {"user_id": user_ids[0]}
res1: dict[str, Any] = remove_user_from_team(web_api_auth, tenant_id, remove_payload1)
assert res1["code"] == 0, res1
assert res1["data"]["user_id"] == user_ids[0]
assert res["code"] == 0, res # Remove second user
assert len(res["data"]["removed"]) == 2 remove_payload2: dict[str, str] = {"user_id": user_ids[1]}
assert len(res["data"]["failed"]) == 0 res2: dict[str, Any] = remove_user_from_team(web_api_auth, tenant_id, remove_payload2)
removed_ids = {user["user_id"] for user in res["data"]["removed"]} assert res2["code"] == 0, res2
assert removed_ids == set(user_ids) assert res2["data"]["user_id"] == user_ids[1]
@pytest.mark.p1 @pytest.mark.p1
def test_remove_user_not_in_team( def test_remove_user_not_in_team(
@ -194,14 +194,12 @@ class TestRemoveUsers:
# Use a user that was not added to the team # Use a user that was not added to the team
user_id: str = test_users[3]["id"] user_id: str = test_users[3]["id"]
remove_payload: dict[str, list[str]] = {"user_ids": [user_id]} remove_payload: dict[str, str] = {"user_id": user_id}
res: dict[str, Any] = remove_users_from_team(web_api_auth, tenant_id, remove_payload) res: dict[str, Any] = remove_user_from_team(web_api_auth, tenant_id, remove_payload)
# API returns error code when all removals fail # API returns error code when removal fails
assert res["code"] == 102 # DATA_ERROR assert res["code"] == 102 # DATA_ERROR
assert len(res["data"]["removed"]) == 0 assert "not a member" in res["message"].lower()
assert len(res["data"]["failed"]) == 1
assert "not a member" in res["data"]["failed"][0]["error"].lower()
@pytest.mark.p1 @pytest.mark.p1
def test_remove_owner( def test_remove_owner(
@ -211,20 +209,18 @@ class TestRemoveUsers:
tenant_id: str = test_team["id"] tenant_id: str = test_team["id"]
owner_id: str = test_team["owner_id"] owner_id: str = test_team["owner_id"]
remove_payload: dict[str, list[str]] = {"user_ids": [owner_id]} remove_payload: dict[str, str] = {"user_id": owner_id}
res: dict[str, Any] = remove_users_from_team(web_api_auth, tenant_id, remove_payload) res: dict[str, Any] = remove_user_from_team(web_api_auth, tenant_id, remove_payload)
# API returns error code when all removals fail # API returns error code when removal fails
assert res["code"] == 102 # DATA_ERROR assert res["code"] == 102 # DATA_ERROR
assert len(res["data"]["removed"]) == 0 assert "owner" in res["message"].lower()
assert len(res["data"]["failed"]) == 1
assert "owner" in res["data"]["failed"][0]["error"].lower()
@pytest.mark.p1 @pytest.mark.p1
def test_remove_users_partial_success( def test_remove_users_partial_success(
self, web_api_auth: RAGFlowWebApiAuth, team_with_users: dict[str, Any], test_users: list[dict[str, Any]] self, web_api_auth: RAGFlowWebApiAuth, team_with_users: dict[str, Any], test_users: list[dict[str, Any]]
) -> None: ) -> None:
"""Test removing users where some succeed and some fail.""" """Test removing users one by one where some succeed and some fail."""
if not team_with_users["users"] or len(test_users) < 4: if not team_with_users["users"] or len(test_users) < 4:
pytest.skip("Need users in team and at least 4 test users") pytest.skip("Need users in team and at least 4 test users")
@ -233,52 +229,52 @@ class TestRemoveUsers:
valid_user_id: str = team_with_users["users"][0]["id"] valid_user_id: str = team_with_users["users"][0]["id"]
invalid_user_id: str = test_users[3]["id"] # Not in team invalid_user_id: str = test_users[3]["id"] # Not in team
remove_payload: dict[str, list[str]] = {"user_ids": [valid_user_id, invalid_user_id]} # First remove valid user - should succeed
res: dict[str, Any] = remove_users_from_team(web_api_auth, tenant_id, remove_payload) remove_payload1: dict[str, str] = {"user_id": valid_user_id}
res1: dict[str, Any] = remove_user_from_team(web_api_auth, tenant_id, remove_payload1)
assert res1["code"] == 0, res1
assert res1["data"]["user_id"] == valid_user_id
assert res["code"] == 0, res # Then try to remove invalid user - should fail
assert len(res["data"]["removed"]) == 1 remove_payload2: dict[str, str] = {"user_id": invalid_user_id}
assert len(res["data"]["failed"]) == 1 res2: dict[str, Any] = remove_user_from_team(web_api_auth, tenant_id, remove_payload2)
assert res["data"]["removed"][0]["user_id"] == valid_user_id assert res2["code"] == 102 # DATA_ERROR
assert "not a member" in res["data"]["failed"][0]["error"].lower() assert "not a member" in res2["message"].lower()
@pytest.mark.p1 @pytest.mark.p1
def test_remove_users_empty_list( def test_remove_users_empty_user_id(
self, web_api_auth: RAGFlowWebApiAuth, test_team: dict[str, Any] self, web_api_auth: RAGFlowWebApiAuth, test_team: dict[str, Any]
) -> None: ) -> None:
"""Test removing users with empty list.""" """Test removing user with empty user_id."""
tenant_id: str = test_team["id"] tenant_id: str = test_team["id"]
remove_payload: dict[str, list[str]] = {"user_ids": []} remove_payload: dict[str, str] = {"user_id": ""}
res: dict[str, Any] = remove_users_from_team(web_api_auth, tenant_id, remove_payload) res: dict[str, Any] = remove_user_from_team(web_api_auth, tenant_id, remove_payload)
assert res["code"] == 101 # ARGUMENT_ERROR assert res["code"] == 101 # ARGUMENT_ERROR
assert "non-empty" in res["message"].lower() or "empty" in res["message"].lower() assert "non-empty" in res["message"].lower() or "empty" in res["message"].lower()
@pytest.mark.p1 @pytest.mark.p1
def test_remove_users_missing_user_ids_field( def test_remove_users_missing_user_id_field(
self, web_api_auth: RAGFlowWebApiAuth, test_team: dict[str, Any] self, web_api_auth: RAGFlowWebApiAuth, test_team: dict[str, Any]
) -> None: ) -> None:
"""Test removing users without 'user_ids' field.""" """Test removing user without 'user_id' field."""
tenant_id: str = test_team["id"] tenant_id: str = test_team["id"]
remove_payload: dict[str, Any] = {} remove_payload: dict[str, Any] = {}
res: dict[str, Any] = remove_users_from_team(web_api_auth, tenant_id, remove_payload) res: dict[str, Any] = remove_user_from_team(web_api_auth, tenant_id, remove_payload)
assert res["code"] == 101 # ARGUMENT_ERROR assert res["code"] == 101 # ARGUMENT_ERROR
@pytest.mark.p1 @pytest.mark.p1
def test_remove_users_invalid_user_id_format( def test_remove_users_invalid_user_id_format(
self, web_api_auth: RAGFlowWebApiAuth, test_team: dict[str, Any] self, web_api_auth: RAGFlowWebApiAuth, test_team: dict[str, Any]
) -> None: ) -> None:
"""Test removing users with invalid user ID format.""" """Test removing user with invalid user ID format."""
tenant_id: str = test_team["id"] tenant_id: str = test_team["id"]
remove_payload: dict[str, list[Any]] = {"user_ids": [12345]} # Not a string remove_payload: dict[str, Any] = {"user_id": 12345} # Not a string
res: dict[str, Any] = remove_users_from_team(web_api_auth, tenant_id, remove_payload) res: dict[str, Any] = remove_user_from_team(web_api_auth, tenant_id, remove_payload)
# API returns error code when all removals fail # API returns error code when removal fails
assert res["code"] == 102 # DATA_ERROR assert res["code"] == 101 # ARGUMENT_ERROR (validation should catch non-string)
assert len(res["data"]["removed"]) == 0
assert len(res["data"]["failed"]) == 1
assert "invalid" in res["data"]["failed"][0]["error"].lower()
@pytest.mark.p1 @pytest.mark.p1
def test_remove_users_not_owner_or_admin( def test_remove_users_not_owner_or_admin(
@ -305,8 +301,8 @@ class TestRemoveUsers:
# Try to remove the other user (normal user should not be able to) # Try to remove the other user (normal user should not be able to)
other_user_id: str = test_users[1]["id"] other_user_id: str = test_users[1]["id"]
remove_payload: dict[str, list[str]] = {"user_ids": [other_user_id]} remove_payload: dict[str, str] = {"user_id": other_user_id}
res: dict[str, Any] = remove_users_from_team(normal_user_auth, tenant_id, remove_payload) res: dict[str, Any] = remove_user_from_team(normal_user_auth, tenant_id, remove_payload)
assert res["code"] == 108 # PERMISSION_ERROR assert res["code"] == 108 # PERMISSION_ERROR
assert "owner" in res["message"].lower() or "admin" in res["message"].lower() assert "owner" in res["message"].lower() or "admin" in res["message"].lower()
@ -346,15 +342,13 @@ class TestRemoveUsers:
admin_user_id: str = test_users[0]["id"] admin_user_id: str = test_users[0]["id"]
# Try to remove the admin (should fail - last admin cannot remove themselves) # Try to remove the admin (should fail - last admin cannot remove themselves)
remove_payload: dict[str, list[str]] = {"user_ids": [admin_user_id]} remove_payload: dict[str, str] = {"user_id": admin_user_id}
res: dict[str, Any] = remove_users_from_team(admin_auth, tenant_id, remove_payload) res: dict[str, Any] = remove_user_from_team(admin_auth, tenant_id, remove_payload)
# API may return error code when all removals fail, or permission error if role not updated # API may return error code when removal fails, or permission error if role not updated
assert res["code"] in [102, 108] # DATA_ERROR or PERMISSION_ERROR assert res["code"] in [102, 108] # DATA_ERROR or PERMISSION_ERROR
if res["code"] == 102: if res["code"] == 102:
# If we get DATA_ERROR, check the failed entry # If we get DATA_ERROR, check the message
assert len(res["data"]["removed"]) == 0 assert "cannot remove yourself" in res["message"].lower() or "at least one" in res["message"].lower()
assert len(res["data"]["failed"]) == 1
assert "cannot remove yourself" in res["data"]["failed"][0]["error"].lower() or "at least one" in res["data"]["failed"][0]["error"].lower()
else: else:
# If we get PERMISSION_ERROR, the user might not have admin role yet # If we get PERMISSION_ERROR, the user might not have admin role yet
assert "owner" in res["message"].lower() or "admin" in res["message"].lower() assert "owner" in res["message"].lower() or "admin" in res["message"].lower()
@ -363,14 +357,14 @@ class TestRemoveUsers:
def test_remove_users_invalid_tenant_id( def test_remove_users_invalid_tenant_id(
self, web_api_auth: RAGFlowWebApiAuth, test_users: list[dict[str, Any]] self, web_api_auth: RAGFlowWebApiAuth, test_users: list[dict[str, Any]]
) -> None: ) -> None:
"""Test removing users from a non-existent team.""" """Test removing user from a non-existent team."""
if not test_users: if not test_users:
pytest.skip("No test users created") pytest.skip("No test users created")
invalid_tenant_id: str = f"invalid_{uuid.uuid4().hex[:8]}" invalid_tenant_id: str = f"invalid_{uuid.uuid4().hex[:8]}"
remove_payload: dict[str, list[str]] = {"user_ids": [test_users[0]["id"]]} remove_payload: dict[str, str] = {"user_id": test_users[0]["id"]}
res: dict[str, Any] = remove_users_from_team(web_api_auth, invalid_tenant_id, remove_payload) res: dict[str, Any] = remove_user_from_team(web_api_auth, invalid_tenant_id, remove_payload)
assert res["code"] != 0 assert res["code"] != 0
assert "not found" in res["message"].lower() or res["code"] in [100, 102, 108] assert "not found" in res["message"].lower() or res["code"] in [100, 102, 108]