youngkingdom/dify
1
0
Fork 0
mirror of https://github.com/langgenius/dify.git synced 2026-05-30 13:47:52 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix: fix permission check

Browse Source
This commit is contained in:
fatelei
2026-05-29 10:16:31 +08:00
parent 73cb24f9be
commit c09ea2fc3b
6 changed files with 190 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
.gitignore vendored
View File

@ -259,3 +259,4 @@ scripts/stress-test/reports/
.qoder/*
.context/
.eslintcache
node_modules/

1
api/controllers/openapi/workspaces.py
View File

@ -1,3 +1,4 @@
"""User-scoped workspace reads and member management under /openapi/v1/workspaces.
Bearer-authed counterparts to the cookie-authed /console/api/workspaces

45
api/services/account_service.py
View File

@ -193,6 +193,23 @@ class AccountService:
role_ids=[resolved_role_id],
)
@staticmethod
def get_workspace_permission_keys(tenant_id: str, account_id: str) -> set[str]:
permissions = RBACService.MyPermissions.get(tenant_id, account_id)
return set(getattr(getattr(permissions, "workspace", None), "permission_keys", []) or [])
@staticmethod
def is_rbac_workspace_owner(tenant_id: str, actor_account_id: str, member_account_id: str) -> bool:
roles = RBACService.MemberRoles.get(
tenant_id=tenant_id,
account_id=actor_account_id,
member_account_id=member_account_id,
).roles
return any(
role.is_builtin and role.category == "global_system_default" and role.name in {"所有者", "owner"}
for role in roles
)
@staticmethod
def _get_refresh_token_key(refresh_token: str) -> str:
return f"{REFRESH_TOKEN_PREFIX}{refresh_token}"
@ -1538,11 +1555,6 @@ class TenantService:
@staticmethod
def check_member_permission(tenant: Tenant, operator: Account, member: Account | None, action: str):
"""Check member permission"""
perms = {
"add": [TenantAccountRole.OWNER, TenantAccountRole.ADMIN],
"remove": [TenantAccountRole.OWNER, TenantAccountRole.ADMIN],
"update": [TenantAccountRole.OWNER, TenantAccountRole.ADMIN],
}
if action not in {"add", "remove", "update"}:
raise InvalidActionError("Invalid action.")
@ -1550,6 +1562,29 @@ class TenantService:
if operator.id == member.id:
raise CannotOperateSelfError("Cannot operate self.")
if dify_config.RBAC_ENABLED:
workspace_permission_keys = AccountService.get_workspace_permission_keys(
str(tenant.id),
str(operator.id),
)
required_permission_key = (
"workspace.member.manage" if action in {"add", "remove"} else "workspace.role.manage"
)
if required_permission_key not in workspace_permission_keys:
raise NoPermissionError(f"No permission to {action} member.")
if action == "remove" and member and AccountService.is_rbac_workspace_owner(
str(tenant.id), str(operator.id), str(member.id)
):
raise NoPermissionError(f"No permission to {action} member.")
return
perms = {
"add": [TenantAccountRole.OWNER, TenantAccountRole.ADMIN],
"remove": [TenantAccountRole.OWNER, TenantAccountRole.ADMIN],
"update": [TenantAccountRole.OWNER, TenantAccountRole.ADMIN],
}
ta_operator = db.session.scalar(
select(TenantAccountJoin)
.where(TenantAccountJoin.tenant_id == tenant.id, TenantAccountJoin.account_id == operator.id)

20
api/services/dataset_service.py
View File

@ -67,6 +67,7 @@ from models.source import DataSourceOauthBinding
from models.workflow import Workflow
from services.document_indexing_proxy.document_indexing_task_proxy import DocumentIndexingTaskProxy
from services.document_indexing_proxy.duplicate_document_indexing_task_proxy import DuplicateDocumentIndexingTaskProxy
from services.enterprise import rbac_service as enterprise_rbac_service
from services.entities.knowledge_entities.knowledge_entities import (
ChildChunkUpdateArgs,
KnowledgeConfig,
@ -229,6 +230,15 @@ class _EstimateArgs(BaseModel):
class DatasetService:
@staticmethod
def _can_manage_all_datasets(tenant_id: str, account_id: str) -> bool:
if not dify_config.RBAC_ENABLED:
return False
permissions = enterprise_rbac_service.RBACService.MyPermissions.get(tenant_id, account_id)
workspace_permission_keys = getattr(getattr(permissions, "workspace", None), "permission_keys", []) or []
return "dataset.create_and_management" in workspace_permission_keys
@staticmethod
def get_datasets(page, per_page, tenant_id=None, user=None, search=None, tag_ids=None, include_all=False):
query = select(Dataset).where(Dataset.tenant_id == tenant_id).order_by(Dataset.created_at.desc(), Dataset.id)
@ -242,7 +252,7 @@ class DatasetService:
).all()
permitted_dataset_ids = {dp.dataset_id for dp in dataset_permission} if dataset_permission else None
if user.current_role == TenantAccountRole.DATASET_OPERATOR:
if not dify_config.RBAC_ENABLED and user.current_role == TenantAccountRole.DATASET_OPERATOR:
# only show datasets that the user has permission to access
# Check if permitted_dataset_ids is not empty to avoid WHERE false condition
if permitted_dataset_ids and len(permitted_dataset_ids) > 0:
@ -250,7 +260,13 @@ class DatasetService:
else:
return [], 0
else:
if user.current_role != TenantAccountRole.OWNER or not include_all:
if dify_config.RBAC_ENABLED:
can_manage_all_datasets = DatasetService._can_manage_all_datasets(str(tenant_id), str(user.id))
should_show_all_datasets = include_all and can_manage_all_datasets
else:
should_show_all_datasets = user.current_role == TenantAccountRole.OWNER and include_all
if not should_show_all_datasets:
# show all datasets that the user has permission to access
# Check if permitted_dataset_ids is not empty to avoid WHERE false condition
if permitted_dataset_ids and len(permitted_dataset_ids) > 0:

58
api/tests/unit_tests/services/test_account_service.py
View File

@ -1061,6 +1061,64 @@ class TestTenantService:
with pytest.raises(NoPermissionError):
TenantService.check_member_permission(mock_tenant, mock_operator, mock_member, "remove")
def test_rbac_member_can_remove_non_owner_member(self):
"""Test RBAC workspace.member.manage allows removing a non-owner member."""
mock_tenant = MagicMock()
mock_tenant.id = "tenant-456"
mock_operator = TestAccountAssociatedDataFactory.create_account_mock(account_id="operator-123")
mock_member = TestAccountAssociatedDataFactory.create_account_mock(account_id="member-789")
mock_permissions = MagicMock()
mock_permissions.workspace = MagicMock(permission_keys=["workspace.member.manage"])
with (
patch("services.account_service.dify_config.RBAC_ENABLED", True),
patch("services.account_service.RBACService.MyPermissions.get", return_value=mock_permissions),
):
TenantService.check_member_permission(mock_tenant, mock_operator, mock_member, "remove")
def test_rbac_member_cannot_remove_without_permission(self):
"""Test RBAC permission check rejects removal without workspace.member.manage."""
mock_tenant = MagicMock()
mock_tenant.id = "tenant-456"
mock_operator = TestAccountAssociatedDataFactory.create_account_mock(account_id="operator-123")
mock_member = TestAccountAssociatedDataFactory.create_account_mock(account_id="member-789")
mock_permissions = MagicMock()
mock_permissions.workspace = MagicMock(permission_keys=["workspace.role.manage"])
with (
patch("services.account_service.dify_config.RBAC_ENABLED", True),
patch("services.account_service.RBACService.MyPermissions.get", return_value=mock_permissions),
):
with pytest.raises(NoPermissionError):
TenantService.check_member_permission(mock_tenant, mock_operator, mock_member, "remove")
def test_rbac_member_cannot_remove_owner_member(self):
"""Test RBAC permission check rejects removing an owner member."""
mock_tenant = MagicMock()
mock_tenant.id = "tenant-456"
mock_operator = TestAccountAssociatedDataFactory.create_account_mock(account_id="operator-123")
mock_member = TestAccountAssociatedDataFactory.create_account_mock(account_id="member-789")
mock_permissions = MagicMock()
mock_permissions.workspace = MagicMock(permission_keys=["workspace.member.manage"])
mock_owner_role = MagicMock()
mock_owner_role.is_builtin = True
mock_owner_role.category = "global_system_default"
mock_owner_role.name = "所有者"
mock_member_roles = MagicMock()
mock_member_roles.roles = [mock_owner_role]
with (
patch("services.account_service.dify_config.RBAC_ENABLED", True),
patch("services.account_service.RBACService.MyPermissions.get", return_value=mock_permissions),
patch("services.account_service.RBACService.MemberRoles.get", return_value=mock_member_roles),
):
with pytest.raises(NoPermissionError):
TenantService.check_member_permission(mock_tenant, mock_operator, mock_member, "remove")
class TestRegisterService:
"""

72
api/tests/unit_tests/services/test_dataset_service_dataset.py
View File

@ -15,6 +15,7 @@ from .dataset_service_test_helpers import (
ProviderTokenNotInitError,
RagPipelineDatasetCreateEntity,
SimpleNamespace,
TenantAccountRole,
_make_knowledge_configuration,
_make_retrieval_model,
_make_session_context,
@ -167,6 +168,77 @@ class TestDatasetServiceValidation:
DatasetService.check_is_multimodal_model("tenant-1", "provider", "embedding-model")
class TestDatasetServiceRetrievalPermissions:
"""Unit tests for dataset list permission branching."""
def test_get_datasets_rbac_include_all_uses_workspace_permission(self):
mock_db = MagicMock()
mock_db.session.scalars.return_value.all.return_value = []
mock_db.paginate.return_value.items = []
mock_db.paginate.return_value.total = 0
user = DatasetServiceUnitDataFactory.create_user_mock(role=TenantAccountRole.NORMAL)
mock_permissions = SimpleNamespace(workspace=SimpleNamespace(permission_keys=["dataset.create_and_management"]))
with (
patch("services.dataset_service.db", mock_db),
patch("services.dataset_service.dify_config.RBAC_ENABLED", True),
patch(
"services.dataset_service.enterprise_rbac_service.RBACService.MyPermissions.get",
return_value=mock_permissions,
),
):
DatasetService.get_datasets(page=1, per_page=20, tenant_id="tenant-1", user=user, include_all=True)
mock_db.session.scalars.assert_called_once()
mock_db.paginate.assert_called_once()
select_stmt = mock_db.paginate.call_args.kwargs["select"]
assert len(select_stmt._where_criteria) == 1
def test_get_datasets_rbac_include_all_falls_back_without_workspace_permission(self):
mock_db = MagicMock()
mock_db.session.scalars.return_value.all.return_value = []
mock_db.paginate.return_value.items = []
mock_db.paginate.return_value.total = 0
user = DatasetServiceUnitDataFactory.create_user_mock(role=TenantAccountRole.NORMAL)
mock_permissions = SimpleNamespace(workspace=SimpleNamespace(permission_keys=[]))
with (
patch("services.dataset_service.db", mock_db),
patch("services.dataset_service.dify_config.RBAC_ENABLED", True),
patch(
"services.dataset_service.enterprise_rbac_service.RBACService.MyPermissions.get",
return_value=mock_permissions,
),
):
DatasetService.get_datasets(page=1, per_page=20, tenant_id="tenant-1", user=user, include_all=True)
mock_db.session.scalars.assert_called_once()
mock_db.paginate.assert_called_once()
select_stmt = mock_db.paginate.call_args.kwargs["select"]
assert len(select_stmt._where_criteria) == 2
def test_get_datasets_legacy_owner_include_all_keeps_full_access(self):
mock_db = MagicMock()
mock_db.session.scalars.return_value.all.return_value = []
mock_db.paginate.return_value.items = []
mock_db.paginate.return_value.total = 0
user = DatasetServiceUnitDataFactory.create_user_mock(role=TenantAccountRole.OWNER)
with (
patch("services.dataset_service.db", mock_db),
patch("services.dataset_service.dify_config.RBAC_ENABLED", False),
):
DatasetService.get_datasets(page=1, per_page=20, tenant_id="tenant-1", user=user, include_all=True)
mock_db.session.scalars.assert_called_once()
mock_db.paginate.assert_called_once()
select_stmt = mock_db.paginate.call_args.kwargs["select"]
assert len(select_stmt._where_criteria) == 1
class TestDatasetServiceCreationAndUpdate:
"""Unit tests for dataset creation and update helpers."""