opensource/dify
2
0
Fork 0
mirror of https://github.com/langgenius/dify.git synced 2026-09-05 00:31:19 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(api): enforce app-scoped access on indirect resources (#40893)

Browse Source
This commit is contained in:
WH-2099 2026-08-21 00:52:39 +00:00 committed by GitHub
parent 0ca99312c1
commit edc805c453
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
20 changed files with 519 additions and 154 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
api/controllers/console/app/workflow.py
View File

@ -20,6 +20,8 @@ from sqlalchemy.orm import Session, sessionmaker
from werkzeug.exceptions import BadRequest, Forbidden, InternalServerError, NotFound
import services
from configs import dify_config
from controllers.common.app_access import resolve_app_access_filter
from controllers.common.controller_schemas import DefaultBlockConfigQuery, WorkflowListQuery, WorkflowUpdatePayload
from controllers.common.errors import InvalidArgumentError
from controllers.common.fields import GeneratedAppResponse, NewAppResponse, SimpleResultResponse
@ -53,6 +55,7 @@ from core.app.apps.base_app_queue_manager import AppQueueManager
from core.app.apps.workflow.app_generator import SKIP_PREPARE_USER_INPUTS_KEY
from core.app.entities.app_invoke_entities import InvokeFrom
from core.app.file_access import DatabaseFileAccessController
from core.db.session_factory import session_factory
from core.helper import encrypter
from core.helper.trace_id_helper import get_external_trace_id
from core.plugin.impl.exc import PluginInvokeError
@ -1924,8 +1927,9 @@ class WorkflowOnlineUsersApi(Resource):
@setup_required
@login_required
@account_initialization_required
@with_current_user
@with_current_tenant_id
def post(self, current_tenant_id: str):
def post(self, current_tenant_id: str, current_user: Account):
args = WorkflowOnlineUsersPayload.model_validate(console_ns.payload or {})
app_ids = args.app_ids
@ -1935,8 +1939,20 @@ class WorkflowOnlineUsersApi(Resource):
if not app_ids:
return {"data": []}
access_filter = None
workflow_service = WorkflowService()
accessible_app_ids = workflow_service.get_accessible_app_ids(app_ids, current_tenant_id, session=db.session())
with session_factory.create_session() as session:
if dify_config.RBAC_ENABLED:
access_filter = resolve_app_access_filter(current_tenant_id, current_user.id, session=session)
app_maintainers = workflow_service.get_tenant_app_maintainers(app_ids, current_tenant_id, session=session)
accessible_app_ids = set(app_maintainers)
if access_filter is not None:
accessible_app_ids = {
app_id
for app_id, maintainer in app_maintainers.items()
if access_filter.is_app_accessible(app_id, maintainer, current_user.id)
}
ordered_accessible_app_ids = [app_id for app_id in app_ids if app_id in accessible_app_ids]
users_json_by_app_id: dict[str, Any] = {}

34
api/controllers/openapi/app_dsl.py
View File

@ -4,6 +4,7 @@ from typing import cast
from flask_restx import Resource
from sqlalchemy.orm import Session
from werkzeug.exceptions import Forbidden
from controllers.common.wraps import RBACPermission, RBACResourceScope
from controllers.openapi import openapi_ns
@ -17,6 +18,7 @@ from models import Account, App
from models.account import TenantAccountRole
from services.app_dsl_service import AppDslService, Import
from services.entities.dsl_entities import CheckDependenciesResult, ImportStatus
from services.errors.account import NoPermissionError
from services.errors.app import WorkflowNotFoundError
@ -53,18 +55,21 @@ class AppDslImportApi(Resource):
with Session(db.engine, expire_on_commit=False) as session:
service = AppDslService(session)
result = service.import_app(
account=account,
import_mode=body.mode,
yaml_content=body.yaml_content,
yaml_url=body.yaml_url,
name=body.name,
description=body.description,
icon_type=body.icon_type,
icon=body.icon,
icon_background=body.icon_background,
app_id=body.app_id,
)
try:
result = service.import_app(
account=account,
import_mode=body.mode,
yaml_content=body.yaml_content,
yaml_url=body.yaml_url,
name=body.name,
description=body.description,
icon_type=body.icon_type,
icon=body.icon,
icon_background=body.icon_background,
app_id=body.app_id,
)
except NoPermissionError as exc:
raise Forbidden(str(exc)) from exc
if result.status == ImportStatus.FAILED:
session.rollback()
else:
@ -108,7 +113,10 @@ class AppDslImportConfirmApi(Resource):
with Session(db.engine, expire_on_commit=False) as session:
service = AppDslService(session)
result = service.confirm_import(import_id=import_id, account=account)
try:
result = service.confirm_import(import_id=import_id, account=account)
except NoPermissionError as exc:
raise Forbidden(str(exc)) from exc
if result.status == ImportStatus.FAILED:
session.rollback()
else:

40
api/services/app_dsl_service.py
View File

@ -19,7 +19,7 @@ from configs import dify_config
from constants.dsl_version import CURRENT_APP_DSL_VERSION
from core.file import remote_fetcher
from core.plugin.entities.plugin import PluginDependency
from core.rbac import RBACPermission
from core.rbac import RBACPermission, RBACResourceScope
from core.trigger.constants import (
TRIGGER_PLUGIN_NODE_TYPE,
TRIGGER_SCHEDULE_NODE_TYPE,
@ -226,9 +226,7 @@ class AppDslService:
# If app_id is provided, check if it exists
app = None
if app_id:
stmt = select(App).where(App.id == app_id, App.tenant_id == account.current_tenant_id)
app = self._session.scalar(stmt)
app = self._load_app_for_overwrite(account, app_id)
if not app:
return Import(
id=import_id,
@ -366,8 +364,13 @@ class AppDslService:
app = None
if pending_data.app_id:
stmt = select(App).where(App.id == pending_data.app_id, App.tenant_id == account.current_tenant_id)
app = self._session.scalar(stmt)
app = self._load_app_for_overwrite(account, pending_data.app_id)
if not app:
return Import(
id=import_id,
status=ImportStatus.FAILED,
error="App not found",
)
# Create or update app
app = self._create_or_update_app(
@ -428,6 +431,31 @@ class AppDslService:
leaked_dependencies=leaked_dependencies,
)
def _load_app_for_overwrite(self, account: Account, app_id: str) -> App | None:
if account.current_tenant_id is None:
raise ValueError("Current tenant is not set")
if dify_config.RBAC_ENABLED and self._session.in_transaction():
raise RuntimeError("App overwrite authorization requires a session without an active transaction")
rbac_allowed = not dify_config.RBAC_ENABLED or RBACService.CheckAccess.check(
account.current_tenant_id,
account.id,
scene=RBACPermission.APP_IMPORT_EXPORT_DSL,
resource_type=RBACResourceScope.APP,
resource_id=app_id,
)
app = self._session.scalar(
select(App)
.where(
App.id == app_id,
App.tenant_id == account.current_tenant_id,
App.status == "normal",
)
.execution_options(populate_existing=True)
)
if app is not None and not rbac_allowed and app.maintainer != account.id:
raise NoPermissionError("You do not have permission to overwrite this app")
return app
@staticmethod
def _ensure_agent_manage_permission(account: Account) -> None:
"""Importing an Agent DSL creates a roster Agent, which requires ``agent.manage``."""

6
api/services/data_migration/import_service.py
View File

@ -18,6 +18,7 @@ import yaml
from sqlalchemy import or_
from sqlalchemy.orm import Session, sessionmaker
from configs import dify_config
from core.entities.mcp_provider import IdentityMode, MCPAuthentication, MCPConfiguration
from core.tools.entities.tool_entities import ApiProviderSchemaType, WorkflowToolParameterConfiguration
from extensions.ext_database import db
@ -324,11 +325,14 @@ class MigrationImportService:
) -> str:
import_service = AppDslService(session)
if existing_app is not None:
existing_app_id = existing_app.id
if dify_config.RBAC_ENABLED:
session.commit()
import_result = import_service.import_app(
account=account,
import_mode="yaml-content",
yaml_content=dsl_content,
app_id=existing_app.id,
app_id=existing_app_id,
)
else:
import_app_id = app_id if self._should_preserve_source_app_id(options) else None

30
api/services/workflow_collaboration_service.py
View File

@ -12,9 +12,12 @@ from socketio.exceptions import TimeoutError as SocketIOTimeoutError # type: ig
from sqlalchemy import select
from sqlalchemy.orm import Session
from configs import dify_config
from core.rbac import RBACPermission, RBACResourceScope
from models.account import Account
from models.model import App
from repositories.workflow_collaboration_repository import WorkflowCollaborationRepository, WorkflowSessionInfo
from services.enterprise.rbac_service import RBACService
logger = logging.getLogger(__name__)
@ -112,7 +115,7 @@ class WorkflowCollaborationService:
if not user_id or not tenant_id:
return None
if not self._can_access_workflow(workflow_id, str(tenant_id), session=session):
if not self._can_access_workflow(workflow_id, str(tenant_id), str(user_id), session=session):
logger.warning(
"Workflow collaboration join rejected: workflow_id=%s tenant_id=%s user_id=%s sid=%s",
workflow_id,
@ -148,10 +151,27 @@ class WorkflowCollaborationService:
return str(user_id), is_leader
def _can_access_workflow(self, workflow_id: str, tenant_id: str, *, session: Session) -> bool:
"""Check room access without relying on Flask's app-context-bound scoped session."""
app_id = session.scalar(select(App.id).where(App.id == workflow_id, App.tenant_id == tenant_id).limit(1))
return app_id is not None
def _can_access_workflow(self, workflow_id: str, tenant_id: str, user_id: str, *, session: Session) -> bool:
"""Check tenant and app permission without relying on Flask's scoped session."""
with session.begin():
app = session.execute(
select(App.id, App.maintainer).where(
App.id == workflow_id, App.tenant_id == tenant_id, App.status == "normal"
)
).one_or_none()
if app is None:
return False
app_id, maintainer = app
if not dify_config.RBAC_ENABLED or maintainer == user_id:
return True
return RBACService.CheckAccess.check(
tenant_id,
user_id,
scene=RBACPermission.APP_EDIT,
resource_type=RBACResourceScope.APP,
resource_id=app_id,
)
def disconnect_session(self, sid: str) -> None:
mapping = self._repository.get_sid_mapping(sid)

16
api/services/workflow_service.py
View File

@ -338,15 +338,17 @@ class WorkflowService:
return workflow
def get_accessible_app_ids(self, app_ids: Sequence[str], tenant_id: str, *, session: Session) -> set[str]:
"""
Return app IDs that belong to the given tenant.
"""
def get_tenant_app_maintainers(
self, app_ids: Sequence[str], tenant_id: str, *, session: Session
) -> dict[str, str | None]:
"""Return requested normal apps and their maintainers within a tenant."""
if not app_ids:
return set()
return {}
stmt = select(App.id).where(App.id.in_(app_ids), App.tenant_id == tenant_id)
return {str(app_id) for app_id in session.scalars(stmt).all()}
stmt = select(App.id, App.maintainer).where(
App.id.in_(app_ids), App.tenant_id == tenant_id, App.status == "normal"
)
return {str(app_id): maintainer for app_id, maintainer in session.execute(stmt)}
def get_all_published_workflow(
self,

30
api/tests/unit_tests/controllers/console/app/test_workflow.py
View File

@ -846,12 +846,19 @@ def test_workflow_online_users_filters_inaccessible_workflow(app: Flask, monkeyp
app_id_2 = "22222222-2222-2222-2222-222222222222"
signed_avatar_url = "https://files.example.com/signed/avatar-1"
sign_avatar = Mock(return_value=signed_avatar_url)
get_tenant_app_maintainers = Mock(return_value={app_id_1: "owner-1", app_id_2: "owner-2"})
monkeypatch.setattr(
workflow_module,
"WorkflowService",
lambda: SimpleNamespace(get_accessible_app_ids=lambda app_ids, tenant_id, session: {app_id_1}),
lambda: SimpleNamespace(get_tenant_app_maintainers=get_tenant_app_maintainers),
)
access_filter = SimpleNamespace(is_app_accessible=lambda app_id, _maintainer, _account_id: app_id == app_id_1)
resolve_access = Mock(return_value=access_filter)
monkeypatch.setattr(workflow_module, "resolve_app_access_filter", resolve_access)
monkeypatch.setattr(workflow_module.dify_config, "RBAC_ENABLED", True)
monkeypatch.setattr(workflow_module.file_helpers, "get_signed_file_url", sign_avatar)
short_session = Mock()
monkeypatch.setattr(workflow_module.session_factory, "create_session", lambda: nullcontext(short_session))
redis_pipeline = Mock()
redis_pipeline.execute.return_value = [
@ -899,7 +906,7 @@ def test_workflow_online_users_filters_inaccessible_workflow(app: Flask, monkeyp
method="POST",
json={"app_ids": [app_id_1, app_id_2]},
):
response = handler(api, "tenant-1")
response = handler(api, "tenant-1", SimpleNamespace(id="account-1"))
assert response == {
"data": [
@ -924,6 +931,11 @@ def test_workflow_online_users_filters_inaccessible_workflow(app: Flask, monkeyp
redis_pipeline.hgetall.assert_called_once_with(f"{workflow_module.WORKFLOW_ONLINE_USERS_PREFIX}{app_id_1}")
redis_pipeline.execute.assert_called_once_with()
sign_avatar.assert_called_once_with("avatar-file-id")
get_tenant_app_maintainers.assert_called_once()
resolve_access.assert_called_once()
assert get_tenant_app_maintainers.call_args.args == ([app_id_1, app_id_2], "tenant-1")
assert resolve_access.call_args.args == ("tenant-1", "account-1")
assert get_tenant_app_maintainers.call_args.kwargs["session"] is resolve_access.call_args.kwargs["session"]
def test_workflow_online_users_batches_redis_reads(app: Flask, monkeypatch: pytest.MonkeyPatch) -> None:
@ -931,8 +943,10 @@ def test_workflow_online_users_batches_redis_reads(app: Flask, monkeypatch: pyte
monkeypatch.setattr(
workflow_module,
"WorkflowService",
lambda: SimpleNamespace(get_accessible_app_ids=lambda app_ids, tenant_id, session: set(app_ids)),
lambda: SimpleNamespace(get_tenant_app_maintainers=lambda app_ids, tenant_id, session: dict.fromkeys(app_ids)),
)
monkeypatch.setattr(workflow_module.dify_config, "RBAC_ENABLED", False)
monkeypatch.setattr(workflow_module.session_factory, "create_session", lambda: nullcontext(Mock()))
first_pipeline = Mock()
first_pipeline.execute.return_value = [{} for _ in range(workflow_module.WORKFLOW_ONLINE_USERS_REDIS_BATCH_SIZE)]
@ -949,7 +963,7 @@ def test_workflow_online_users_batches_redis_reads(app: Flask, monkeypatch: pyte
method="POST",
json={"app_ids": app_ids},
):
response = handler(api, "tenant-1")
response = handler(api, "tenant-1", SimpleNamespace(id="account-1"))
assert len(response["data"]) == len(app_ids)
assert redis_pipeline_factory.call_count == 2
@ -958,11 +972,11 @@ def test_workflow_online_users_batches_redis_reads(app: Flask, monkeypatch: pyte
def test_workflow_online_users_rejects_excessive_workflow_ids(app: Flask, monkeypatch: pytest.MonkeyPatch) -> None:
accessible_app_ids = Mock(return_value=set())
get_tenant_app_maintainers = Mock(return_value={})
monkeypatch.setattr(
workflow_module,
"WorkflowService",
lambda: SimpleNamespace(get_accessible_app_ids=accessible_app_ids),
lambda: SimpleNamespace(get_tenant_app_maintainers=get_tenant_app_maintainers),
)
excessive_ids = [f"wf-{index}" for index in range(workflow_module.MAX_WORKFLOW_ONLINE_USERS_REQUEST_IDS + 1)]
@ -976,9 +990,9 @@ def test_workflow_online_users_rejects_excessive_workflow_ids(app: Flask, monkey
json={"app_ids": excessive_ids},
):
with pytest.raises(HTTPException) as exc:
handler(api, "tenant-1")
handler(api, "tenant-1", SimpleNamespace(id="account-1"))
assert exc.value.code == 400
assert exc.value.description is not None
assert "Maximum" in exc.value.description
accessible_app_ids.assert_not_called()
get_tenant_app_maintainers.assert_not_called()

49
api/tests/unit_tests/controllers/openapi/test_app_dsl.py Normal file
View File

@ -0,0 +1,49 @@
from inspect import unwrap
from types import SimpleNamespace
from unittest.mock import Mock
import pytest
from flask import Flask
from sqlalchemy.engine import Engine
from werkzeug.exceptions import Forbidden
from controllers.openapi import app_dsl as app_dsl_module
from controllers.openapi._models import AppDslImportPayload
from controllers.openapi.app_dsl import AppDslImportApi, AppDslImportConfirmApi
from services.errors.account import NoPermissionError
@pytest.mark.parametrize(
("api", "kwargs"),
[
(
AppDslImportApi(),
{
"workspace_id": "workspace-1",
"body": AppDslImportPayload(mode="yaml-content", yaml_content="app: {}"),
},
),
(
AppDslImportConfirmApi(),
{"workspace_id": "workspace-1", "import_id": "import-1"},
),
],
)
def test_permission_denial_maps_to_forbidden(
app: Flask,
monkeypatch: pytest.MonkeyPatch,
sqlite_engine: Engine,
api: AppDslImportApi | AppDslImportConfirmApi,
kwargs: dict[str, object],
) -> None:
service = Mock()
service.import_app.side_effect = NoPermissionError("denied")
service.confirm_import.side_effect = NoPermissionError("denied")
monkeypatch.setattr(app_dsl_module, "AppDslService", Mock(return_value=service))
monkeypatch.setattr(app_dsl_module, "db", SimpleNamespace(engine=sqlite_engine))
with app.test_request_context("/openapi/v1/workspaces/workspace-1/apps/imports", method="POST"):
with pytest.raises(Forbidden, match="denied") as exc_info:
unwrap(api.post)(api, auth_data=SimpleNamespace(caller=Mock()), **kwargs)
assert isinstance(exc_info.value.__cause__, NoPermissionError)

35
api/tests/unit_tests/services/data_migration/test_import_service.py
View File

@ -335,7 +335,7 @@ def test_find_existing_mcp_tool_does_not_compare_invalid_uuid(database: Database
assert f"{MCPToolProvider.__tablename__}.name" not in where_clause
def test_workflow_app_import_does_not_wrap_app_dsl_import_in_nested_transaction(
def test_workflow_app_import_closes_read_transaction_before_dsl_overwrite(
monkeypatch: pytest.MonkeyPatch, database: Database
):
class StubAppDslService:
@ -343,32 +343,25 @@ def test_workflow_app_import_does_not_wrap_app_dsl_import_in_nested_transaction(
self.session = session
def import_app(self, **kwargs):
assert not self.session.in_transaction()
return Import(id="import-id", status=ImportStatus.COMPLETED, app_id="imported-app-id")
monkeypatch.setattr(import_service, "AppDslService", StubAppDslService)
nested_transactions = []
monkeypatch.setattr(import_service.dify_config, "RBAC_ENABLED", True)
existing_app = _persist_app(database.session, app_id="11111111-1111-4111-8111-111111111111")
database.session.begin()
def capture_transaction(_session, transaction) -> None:
if transaction.nested:
nested_transactions.append(transaction)
event.listen(database.session, "after_transaction_create", capture_transaction)
try:
imported_app_id = MigrationImportService()._import_workflow_app(
account=object(),
workflow_data={"name": "main_chatflow"},
dsl_content="app:\n mode: workflow\n",
app_id="source-app-id",
existing_app=None,
options=ImportOptions(id_strategy=IdStrategy.PRESERVE_ID),
session=database.session,
)
finally:
event.remove(database.session, "after_transaction_create", capture_transaction)
imported_app_id = MigrationImportService()._import_workflow_app(
account=object(),
workflow_data={"name": "main_chatflow"},
dsl_content="app:\n mode: workflow\n",
app_id="source-app-id",
existing_app=existing_app,
options=ImportOptions(id_strategy=IdStrategy.PRESERVE_ID),
session=database.session,
)
assert imported_app_id == "imported-app-id"
assert nested_transactions == []
def test_rewrite_workflow_dsl_replaces_tool_provider_ids():

129
api/tests/unit_tests/services/test_app_dsl_service.py
View File

@ -7,7 +7,7 @@ import yaml
from sqlalchemy import event, select
from sqlalchemy.orm import Session, sessionmaker
from core.rbac import RBACPermission
from core.rbac import RBACPermission, RBACResourceScope
from core.workflow.llm_environment_variable import LLMEnvironmentVariable
from models import App, AppMode
from models.model import AppModelConfig, AppModelConfigDict, IconType
@ -17,6 +17,40 @@ from services.entities.dsl_entities import ImportStatus
from services.errors.account import NoPermissionError
from services.errors.app import WorkflowNotFoundError
_OVERWRITE_APP_ID = "11111111-1111-4111-8111-111111111111"
_TENANT_ID = "22222222-2222-4222-8222-222222222222"
_CALLER_ID = "33333333-3333-4333-8333-333333333333"
_OTHER_ACCOUNT_ID = "44444444-4444-4444-8444-444444444444"
_PENDING_WORKFLOW_DSL = "version: 99.0.0\nkind: app\napp: {name: Test, mode: workflow}\n"
_PENDING_DATA_JSON = PendingData(
tenant_id=_TENANT_ID,
account_id=_CALLER_ID,
import_mode="yaml-content",
yaml_content=_PENDING_WORKFLOW_DSL,
app_id=_OVERWRITE_APP_ID,
).model_dump_json()
def _persist_overwrite_target(session: Session, *, maintainer: str = _OTHER_ACCOUNT_ID) -> App:
app = App(
id=_OVERWRITE_APP_ID,
tenant_id=_TENANT_ID,
name="Target",
description="",
mode=AppMode.WORKFLOW,
icon_type=IconType.EMOJI,
icon="robot",
icon_background="#FFFFFF",
enable_site=True,
enable_api=True,
created_by=maintainer,
maintainer=maintainer,
updated_by=maintainer,
)
session.add(app)
session.commit()
return app
def test_extract_workflow_dependencies_uses_llm_environment_variable_provider(monkeypatch: pytest.MonkeyPatch) -> None:
workflow = SimpleNamespace(
@ -145,6 +179,99 @@ def test_import_app_returns_decode_error_for_invalid_yaml_url_bytes(
assert not unbound_session.in_transaction()
def test_import_app_checks_overwrite_rbac_before_database_access(
monkeypatch: pytest.MonkeyPatch, sqlite_session: Session
) -> None:
_persist_overwrite_target(sqlite_session)
account = Mock(id=_CALLER_ID, current_tenant_id=_TENANT_ID)
def deny_before_transaction(*_args: object, **_kwargs: object) -> bool:
assert not sqlite_session.in_transaction()
return False
check = Mock(side_effect=deny_before_transaction)
setex = Mock()
monkeypatch.setattr("services.app_dsl_service.dify_config.RBAC_ENABLED", True)
monkeypatch.setattr("services.app_dsl_service.RBACService.CheckAccess.check", check)
monkeypatch.setattr("services.app_dsl_service.redis_client.setex", setex)
with pytest.raises(NoPermissionError, match="permission to overwrite"):
AppDslService(sqlite_session).import_app(
account=account,
import_mode="yaml-content",
yaml_content=_PENDING_WORKFLOW_DSL,
app_id=_OVERWRITE_APP_ID,
)
check.assert_called_once_with(
_TENANT_ID,
_CALLER_ID,
scene=RBACPermission.APP_IMPORT_EXPORT_DSL,
resource_type=RBACResourceScope.APP,
resource_id=_OVERWRITE_APP_ID,
)
setex.assert_not_called()
def test_confirm_import_rechecks_overwrite_rbac_before_database_access(
monkeypatch: pytest.MonkeyPatch, sqlite_session: Session
) -> None:
_persist_overwrite_target(sqlite_session)
monkeypatch.setattr("services.app_dsl_service.redis_client.get", Mock(return_value=_PENDING_DATA_JSON))
redis_delete = Mock()
monkeypatch.setattr("services.app_dsl_service.redis_client.delete", redis_delete)
create_or_update = Mock()
service = AppDslService(sqlite_session)
monkeypatch.setattr(service, "_create_or_update_app", create_or_update)
def deny_before_transaction(*_args: object, **_kwargs: object) -> bool:
assert not sqlite_session.in_transaction()
return False
check = Mock(side_effect=deny_before_transaction)
monkeypatch.setattr("services.app_dsl_service.dify_config.RBAC_ENABLED", True)
monkeypatch.setattr("services.app_dsl_service.RBACService.CheckAccess.check", check)
with pytest.raises(NoPermissionError, match="permission to overwrite"):
service.confirm_import(
import_id="import-1",
account=Mock(id=_CALLER_ID, current_tenant_id=_TENANT_ID),
)
check.assert_called_once_with(
_TENANT_ID,
_CALLER_ID,
scene=RBACPermission.APP_IMPORT_EXPORT_DSL,
resource_type=RBACResourceScope.APP,
resource_id=_OVERWRITE_APP_ID,
)
create_or_update.assert_not_called()
redis_delete.assert_not_called()
def test_confirm_import_does_not_create_when_overwrite_target_disappeared(
monkeypatch: pytest.MonkeyPatch, sqlite_session: Session
) -> None:
monkeypatch.setattr("services.app_dsl_service.redis_client.get", Mock(return_value=_PENDING_DATA_JSON))
monkeypatch.setattr("services.app_dsl_service.dify_config.RBAC_ENABLED", True)
monkeypatch.setattr("services.app_dsl_service.RBACService.CheckAccess.check", Mock(return_value=True))
redis_delete = Mock()
monkeypatch.setattr("services.app_dsl_service.redis_client.delete", redis_delete)
service = AppDslService(sqlite_session)
create_or_update = Mock()
monkeypatch.setattr(service, "_create_or_update_app", create_or_update)
result = service.confirm_import(
import_id="import-1",
account=Mock(id=_CALLER_ID, current_tenant_id=_TENANT_ID),
)
assert result.status == ImportStatus.FAILED
assert result.error == "App not found"
create_or_update.assert_not_called()
redis_delete.assert_not_called()
def test_pending_import_is_scoped_to_its_owner(monkeypatch: pytest.MonkeyPatch, unbound_session: Session) -> None:
pending_imports: dict[str, str] = {}
monkeypatch.setattr(

75
api/tests/unit_tests/services/test_workflow_collaboration_service.py
View File

@ -8,6 +8,7 @@ from socketio.exceptions import TimeoutError as SocketIOTimeoutError
from sqlalchemy import Engine
from sqlalchemy.orm import Session
from core.rbac import RBACPermission, RBACResourceScope
from models.account import Account, Tenant
from models.base import TypeBase
from models.model import App, AppMode, IconType
@ -24,7 +25,7 @@ def db_session(sqlite_engine: Engine) -> Iterator[Session]:
yield session
def _app(*, app_id: str, tenant_id: str) -> App:
def _app(*, app_id: str, tenant_id: str, maintainer: str | None = None) -> App:
return App(
id=app_id,
tenant_id=tenant_id,
@ -42,6 +43,7 @@ def _app(*, app_id: str, tenant_id: str) -> App:
is_public=False,
is_universal=False,
max_active_requests=None,
maintainer=maintainer,
use_icon_as_answer_icon=False,
)
@ -65,23 +67,37 @@ class TestWorkflowCollaborationService:
"avatar": None,
"tenant_id": "t-1",
}
db_session.add(_app(app_id="wf-1", tenant_id="t-1", maintainer="owner-1"))
db_session.commit()
with (
patch.object(collaboration_service, "_can_access_workflow", return_value=True),
patch.object(collaboration_service, "get_or_set_leader", return_value="sid-1"),
patch.object(collaboration_service, "broadcast_online_users"),
patch("services.workflow_collaboration_service.dify_config.RBAC_ENABLED", True),
patch(
"services.workflow_collaboration_service.RBACService.CheckAccess.check", return_value=True
) as check_access,
patch.object(collaboration_service, "get_or_set_leader", return_value="sid-1") as get_leader,
patch.object(collaboration_service, "broadcast_online_users") as broadcast_online_users,
):
# Act
result = collaboration_service.authorize_and_join_workflow_room("wf-1", "sid-1", session=db_session)
# Assert
assert result == ("u-1", True)
check_access.assert_called_once_with(
"t-1",
"u-1",
scene=RBACPermission.APP_EDIT,
resource_type=RBACResourceScope.APP,
resource_id="wf-1",
)
repository.set_session_info.assert_called_once()
session_info = repository.set_session_info.call_args.args[1]
assert session_info["server_id"] == "server-1"
repository.refresh_server_heartbeat.assert_called_once_with("server-1")
socketio.start_background_task.assert_called_once()
get_leader.assert_called_once_with("wf-1", "sid-1")
socketio.enter_room.assert_called_once_with("sid-1", "wf-1")
broadcast_online_users.assert_called_once_with("wf-1")
socketio.emit.assert_called_once_with("status", {"isLeader": True}, room="sid-1")
def test_authorize_and_join_workflow_room_returns_none_when_missing_user(
@ -120,13 +136,33 @@ class TestWorkflowCollaborationService:
"avatar": None,
"tenant_id": "t-1",
}
db_session.add(_app(app_id="wf-1", tenant_id="t-1", maintainer="owner-1"))
db_session.commit()
with patch.object(collaboration_service, "_can_access_workflow", return_value=False):
with (
patch("services.workflow_collaboration_service.dify_config.RBAC_ENABLED", True),
patch(
"services.workflow_collaboration_service.RBACService.CheckAccess.check", return_value=False
) as check_access,
patch.object(collaboration_service, "get_or_set_leader") as get_leader,
patch.object(collaboration_service, "broadcast_online_users") as broadcast_online_users,
):
result = collaboration_service.authorize_and_join_workflow_room("wf-1", "sid-1", session=db_session)
assert result is None
check_access.assert_called_once_with(
"t-1",
"u-1",
scene=RBACPermission.APP_EDIT,
resource_type=RBACResourceScope.APP,
resource_id="wf-1",
)
repository.refresh_server_heartbeat.assert_not_called()
repository.set_session_info.assert_not_called()
socketio.start_background_task.assert_not_called()
get_leader.assert_not_called()
socketio.enter_room.assert_not_called()
broadcast_online_users.assert_not_called()
socketio.emit.assert_not_called()
def test_repr_and_save_socket_identity(self, service: tuple[WorkflowCollaborationService, Mock, Mock]) -> None:
@ -159,11 +195,34 @@ class TestWorkflowCollaborationService:
)
db_session.commit()
result = collaboration_service._can_access_workflow("wf-1", "tenant-1", session=db_session)
with patch("services.workflow_collaboration_service.dify_config.RBAC_ENABLED", False):
result = collaboration_service._can_access_workflow("wf-1", "tenant-1", "user-1", session=db_session)
assert result is True
assert (
collaboration_service._can_access_workflow("wf-1", "tenant-other", "user-1", session=db_session)
is False
)
assert (
collaboration_service._can_access_workflow("wf-other", "tenant-other", "user-1", session=db_session)
is True
)
def test_can_access_workflow_allows_maintainer_without_rbac_call(
self, service: tuple[WorkflowCollaborationService, Mock, Mock], db_session: Session
) -> None:
collaboration_service, _repository, _socketio = service
db_session.add(_app(app_id="wf-1", tenant_id="tenant-1", maintainer="owner-1"))
db_session.commit()
with (
patch("services.workflow_collaboration_service.dify_config.RBAC_ENABLED", True),
patch("services.workflow_collaboration_service.RBACService.CheckAccess.check") as check_access,
):
result = collaboration_service._can_access_workflow("wf-1", "tenant-1", "owner-1", session=db_session)
assert result is True
assert collaboration_service._can_access_workflow("wf-1", "tenant-other", session=db_session) is False
assert collaboration_service._can_access_workflow("wf-other", "tenant-other", session=db_session) is True
check_access.assert_not_called()
def test_relay_collaboration_event_unauthorized(
self, service: tuple[WorkflowCollaborationService, Mock, Mock]

20
api/tests/unit_tests/services/test_workflow_service.py
View File

@ -231,6 +231,26 @@ class TestWorkflowService:
"""Create a WorkflowService whose repositories use the test SQLite engine."""
return WorkflowService(sessionmaker(bind=sqlite_engine, expire_on_commit=False))
def test_get_tenant_app_maintainers_scopes_requested_apps(
self, workflow_service: WorkflowService, sqlite_session: Session
) -> None:
sqlite_session.add_all(
[
TestWorkflowAssociatedDataFactory.create_app(
app_id="app-1", tenant_id="tenant-1", maintainer="owner-1"
),
TestWorkflowAssociatedDataFactory.create_app(app_id="app-2", tenant_id="tenant-1"),
TestWorkflowAssociatedDataFactory.create_app(
app_id="app-3", tenant_id="tenant-2", maintainer="owner-2"
),
]
)
sqlite_session.commit()
assert workflow_service.get_tenant_app_maintainers(
["app-1", "app-2", "app-3", "missing"], "tenant-1", session=sqlite_session
) == {"app-1": "owner-1", "app-2": None}
# ==================== Workflow Existence Tests ====================
# These tests verify the service can check if a draft workflow exists

8
oxlint-suppressions.json
View File

@ -3365,14 +3365,6 @@
"count": 1
}
},
"web/app/components/workflow/header/online-users.tsx": {
"jsx-a11y/click-events-have-key-events": {
"count": 2
},
"jsx-a11y/no-static-element-interactions": {
"count": 2
}
},
"web/app/components/workflow/header/test-run-menu.tsx": {
"erasable-syntax-only/enums": {
"count": 1

24
web/app/components/workflow-app/components/__tests__/workflow-main.spec.tsx
View File

@ -5,6 +5,7 @@ import { useStore as useAppStore } from '@/app/components/app/store'
import { ChatVarType } from '@/app/components/workflow/panel/chat-variable-panel/type'
import { BlockEnum } from '@/app/components/workflow/types'
import { renderWithAccountProfile as render } from '@/test/console/account-profile'
import { AppACLPermission } from '@/utils/permission'
import WorkflowMain from '../workflow-main'
const mockSetFeatures = vi.fn()
@ -24,6 +25,7 @@ const mockReplaceGraphFromReactFlow = vi.hoisted(() => vi.fn())
const mockCanPersistLocalGraph = vi.hoisted(() => vi.fn())
const mockIsGraphReloadCurrent = vi.hoisted(() => vi.fn())
const mockRetryGraphReload = vi.hoisted(() => vi.fn())
const mockUseCollaboration = vi.hoisted(() => vi.fn())
const hookFns = {
doSyncWorkflowDraft: vi.fn(),
@ -135,14 +137,10 @@ vi.mock('reactflow', () => ({
}))
vi.mock('@/app/components/workflow/collaboration/hooks/use-collaboration', () => ({
useCollaboration: () => ({
startCursorTracking: collaborationRuntime.startCursorTracking,
stopCursorTracking: collaborationRuntime.stopCursorTracking,
onlineUsers: collaborationRuntime.onlineUsers,
cursors: collaborationRuntime.cursors,
isConnected: collaborationRuntime.isConnected,
isEnabled: collaborationRuntime.isEnabled,
}),
useCollaboration: (...args: unknown[]) => {
mockUseCollaboration(...args)
return collaborationRuntime
},
}))
vi.mock('@/app/components/workflow/hooks/use-workflow-update', () => ({
@ -590,6 +588,16 @@ describe('WorkflowMain', () => {
expect(screen.queryByRole('status')).not.toBeInTheDocument()
})
it('disables collaboration for view-only apps', () => {
useAppStore.setState({
appDetail: { permission_keys: [AppACLPermission.ViewLayout] } as never,
})
render(<WorkflowMain nodes={[]} edges={[]} viewport={{ x: 0, y: 0, zoom: 1 }} />)
expect(mockUseCollaboration).toHaveBeenCalledWith('app-1', false, expect.any(Object))
})
it('subscribes collaboration listeners and handles sync/workflow update callbacks', async () => {
collaborationRuntime.isEnabled = true
mockFetchWorkflowDraft.mockResolvedValue({

32
web/app/components/workflow-app/components/workflow-main.tsx
View File

@ -74,22 +74,6 @@ const WorkflowMain = ({ nodes, edges, viewport }: WorkflowMainProps) => {
}),
[reactFlow],
)
const {
startCursorTracking,
stopCursorTracking,
onlineUsers,
cursors,
isConnected,
isEnabled: isCollaborationEnabled,
} = useCollaboration(appId || '', reactFlowStore)
const myUserId = useMemo(
() => (isCollaborationEnabled && isConnected ? 'current-user' : null),
[isCollaborationEnabled, isConnected],
)
const filteredCursors = Object.fromEntries(
Object.entries(cursors).filter(([userId]) => userId !== myUserId),
)
const { data: currentUserId } = useSuspenseQuery({
...userProfileQueryOptions(),
select: (data) => data.profile.id,
@ -104,6 +88,22 @@ const WorkflowMain = ({ nodes, edges, viewport }: WorkflowMainProps) => {
}),
[appDetail?.maintainer, appDetail?.permission_keys, currentUserId, workspacePermissionKeys],
)
const {
startCursorTracking,
stopCursorTracking,
onlineUsers,
cursors,
isConnected,
isEnabled: isCollaborationEnabled,
} = useCollaboration(appId || '', appACLCapabilities.canEdit, reactFlowStore)
const myUserId = useMemo(
() => (isCollaborationEnabled && isConnected ? 'current-user' : null),
[isCollaborationEnabled, isConnected],
)
const filteredCursors = Object.fromEntries(
Object.entries(cursors).filter(([userId]) => userId !== myUserId),
)
useEffect(() => {
if (!isCollaborationEnabled) return

34
web/app/components/workflow/collaboration/hooks/__tests__/use-collaboration.spec.ts
View File

@ -3,7 +3,7 @@ import { waitFor } from '@testing-library/react'
import { renderHookWithConsoleQuery } from '@/test/console/query-data'
import { useCollaboration } from '../use-collaboration'
type HookReactFlowStore = NonNullable<Parameters<typeof useCollaboration>[1]>
type HookReactFlowStore = NonNullable<Parameters<typeof useCollaboration>[2]>
type HookReactFlowInstance = Parameters<
ReturnType<typeof useCollaboration>['startCursorTracking']
>[1]
@ -105,7 +105,7 @@ describe('useCollaboration', () => {
getState: vi.fn(),
}
const { result, unmount } = renderHookWithConsoleQuery(
() => useCollaboration('app-1', reactFlowStore),
() => useCollaboration('app-1', true, reactFlowStore),
{
systemFeatures: { enable_collaboration_mode: isCollaborationEnabled },
},
@ -155,18 +155,24 @@ describe('useCollaboration', () => {
expect(mockDisconnect).toHaveBeenCalledWith('conn-1')
})
it('does not connect or start cursor tracking when collaboration is disabled', async () => {
isCollaborationEnabled = false
const { result } = renderHookWithConsoleQuery(() => useCollaboration('app-1'), {
systemFeatures: { enable_collaboration_mode: isCollaborationEnabled },
})
it.each([
[false, true],
[true, false],
])(
'does not connect or track cursors when a collaboration gate is disabled',
async (featureEnabled, canEdit) => {
isCollaborationEnabled = featureEnabled
const { result } = renderHookWithConsoleQuery(() => useCollaboration('app-1', canEdit), {
systemFeatures: { enable_collaboration_mode: featureEnabled },
})
await waitFor(() => {
expect(mockConnect).not.toHaveBeenCalled()
expect(result.current.isEnabled).toBe(false)
})
await waitFor(() => {
expect(mockConnect).not.toHaveBeenCalled()
expect(result.current.isEnabled).toBe(false)
})
result.current.startCursorTracking({ current: document.createElement('div') })
expect(mockStartTracking).not.toHaveBeenCalled()
})
result.current.startCursorTracking({ current: document.createElement('div') })
expect(mockStartTracking).not.toHaveBeenCalled()
},
)
})

10
web/app/components/workflow/collaboration/hooks/use-collaboration.ts
View File

@ -29,7 +29,7 @@ const initialState: CollaborationViewState = {
isLeader: false,
}
export function useCollaboration(appId: string, reactFlowStore?: ReactFlowStore) {
export function useCollaboration(appId: string, canEdit: boolean, reactFlowStore?: ReactFlowStore) {
const [state, setState] = useState<CollaborationViewState>(initialState)
const cursorServiceRef = useRef<CursorService | null>(null)
@ -40,7 +40,7 @@ export function useCollaboration(appId: string, reactFlowStore?: ReactFlowStore)
})
useEffect(() => {
if (!appId || !isCollaborationEnabled) {
if (!appId || !isCollaborationEnabled || !canEdit) {
Promise.resolve().then(() => {
setState(initialState)
})
@ -110,7 +110,7 @@ export function useCollaboration(appId: string, reactFlowStore?: ReactFlowStore)
cursorServiceRef.current?.stopTracking()
if (connectionId) collaborationManager.disconnect(connectionId)
}
}, [appId, reactFlowStore, isCollaborationEnabled])
}, [appId, canEdit, reactFlowStore, isCollaborationEnabled])
const prevIsConnected = useRef(false)
useEffect(() => {
@ -126,7 +126,7 @@ export function useCollaboration(appId: string, reactFlowStore?: ReactFlowStore)
containerRef: React.RefObject<HTMLElement>,
reactFlowInstance?: ReactFlowInstance,
) => {
if (!isCollaborationEnabled || !cursorServiceRef.current) return
if (!isCollaborationEnabled || !canEdit || !cursorServiceRef.current) return
if (cursorServiceRef.current) {
cursorServiceRef.current.startTracking(
@ -150,7 +150,7 @@ export function useCollaboration(appId: string, reactFlowStore?: ReactFlowStore)
nodePanelPresence: state.nodePanelPresence || {},
isLeader: state.isLeader || false,
leaderId: collaborationManager.getLeaderId(),
isEnabled: isCollaborationEnabled,
isEnabled: isCollaborationEnabled && canEdit,
startCursorTracking,
stopCursorTracking,
}

74
web/app/components/workflow/header/online-users.tsx
View File

@ -14,6 +14,7 @@ import { userProfileQueryOptions } from '@/features/account-profile/client'
import { getAvatar } from '@/service/common'
import { useCollaboration } from '../collaboration/hooks/use-collaboration'
import { getUserColor } from '../collaboration/utils/user-color'
import { useHooksStore } from '../hooks-store'
import { useStore } from '../store'
const useAvatarUrls = (users: OnlineUser[]) => {
@ -49,11 +50,12 @@ const useAvatarUrls = (users: OnlineUser[]) => {
const OnlineUsers = () => {
const { t } = useTranslation()
const appId = useStore((s) => s.appId)
const canEdit = useHooksStore((s) => s.accessControl.canEdit)
const {
onlineUsers,
cursors,
isEnabled: isCollaborationEnabled,
} = useCollaboration(appId as string)
} = useCollaboration(appId as string, canEdit)
const { data: currentUserId } = useSuspenseQuery({
...userProfileQueryOptions(),
select: (data) => data.profile.id,
@ -119,29 +121,43 @@ const OnlineUsers = () => {
const userColor = isCurrentUser ? undefined : getUserColor(user.user_id)
const avatarUrl = getAvatarUrl(user)
const displayName = user.username || fallbackUsername
const avatar = (
<AvatarRoot size="sm" className="ring-1 ring-components-panel-bg">
{avatarUrl && <AvatarImage src={avatarUrl} alt={displayName} />}
<AvatarFallback
size="sm"
style={userColor ? { backgroundColor: userColor } : undefined}
>
{displayName?.[0]?.toLocaleUpperCase()}
</AvatarFallback>
</AvatarRoot>
)
const triggerClassName = cn(
'relative flex size-6 items-center justify-center',
index > 0 && '-ml-1.5',
!isCurrentUser && 'cursor-pointer transition-transform hover:scale-110',
)
const triggerStyle = { zIndex: visibleUsers.length - index }
return (
<Tooltip key={`${user.sid}-${index}`}>
<TooltipTrigger>
<div
className={cn(
'relative flex size-6 items-center justify-center',
index > 0 && '-ml-1.5',
!isCurrentUser && 'cursor-pointer transition-transform hover:scale-110',
)}
style={{ zIndex: visibleUsers.length - index }}
onClick={() => !isCurrentUser && jumpToUserCursor(user.user_id)}
>
<AvatarRoot size="sm" className="ring-1 ring-components-panel-bg">
{avatarUrl && <AvatarImage src={avatarUrl} alt={displayName} />}
<AvatarFallback
size="sm"
style={userColor ? { backgroundColor: userColor } : undefined}
<Tooltip key={user.sid}>
<TooltipTrigger
render={
isCurrentUser ? (
<div className={triggerClassName} style={triggerStyle}>
{avatar}
</div>
) : (
<button
type="button"
className={triggerClassName}
style={triggerStyle}
onClick={() => jumpToUserCursor(user.user_id)}
>
{displayName?.[0]?.toLocaleUpperCase()}
</AvatarFallback>
</AvatarRoot>
</div>
</TooltipTrigger>
{avatar}
</button>
)
}
/>
<TooltipContent
placement="bottom"
sideOffset={4}
@ -189,18 +205,18 @@ const OnlineUsers = () => {
const avatarUrl = getAvatarUrl(user)
const displayName = user.username || fallbackUsername
return (
<div
<button
type="button"
key={user.sid}
disabled={isCurrentUser}
className={cn(
'flex items-center gap-2 rounded-lg px-3 py-1.5',
'flex w-full items-center gap-2 rounded-lg px-3 py-1.5 text-left',
!isCurrentUser &&
'cursor-pointer hover:bg-components-panel-on-panel-item-bg-hover',
)}
onClick={() => {
if (!isCurrentUser) {
jumpToUserCursor(user.user_id)
setDropdownOpen(false)
}
jumpToUserCursor(user.user_id)
setDropdownOpen(false)
}}
>
<div className="relative">
@ -219,7 +235,7 @@ const OnlineUsers = () => {
'system-xs-medium text-text-secondary',
'text-text-tertiary',
)}
</div>
</button>
)
})}
</PopoverContent>

3
web/app/components/workflow/nodes/_base/components/workflow-panel/index.tsx
View File

@ -100,7 +100,8 @@ const BasePanel: FC<BasePanelProps> = ({ id, data, children }) => {
...userProfileQueryOptions(),
select: (data) => data.profile,
})
const { isConnected, nodePanelPresence } = useCollaboration(appId as string)
const canEdit = useHooksStore((s) => s.accessControl.canEdit)
const { isConnected, nodePanelPresence } = useCollaboration(appId as string, canEdit)
const { showMessageLogModal } = useAppStore(
useShallow((state) => ({
showMessageLogModal: state.showMessageLogModal,

4
web/app/components/workflow/nodes/_base/node.tsx
View File

@ -9,6 +9,7 @@ import { UserAvatarList } from '@/app/components/base/user-avatar-list'
import BlockIcon from '@/app/components/workflow/block-icon'
import { ToolType } from '@/app/components/workflow/block-selector/types'
import { useCollaboration } from '@/app/components/workflow/collaboration/hooks/use-collaboration'
import { useHooksStore } from '@/app/components/workflow/hooks-store'
import { useNodeIterationInteractions } from '@/app/components/workflow/nodes/iteration/use-interactions'
import { useNodeLoopInteractions } from '@/app/components/workflow/nodes/loop/use-interactions'
import CopyID from '@/app/components/workflow/nodes/tool/components/copy-id'
@ -62,7 +63,8 @@ const BaseNode: FC<BaseNodeProps> = ({ id, data, children }) => {
select: (data) => data.profile,
})
const appId = useStore((s) => s.appId)
const { nodePanelPresence } = useCollaboration(appId as string)
const canEdit = useHooksStore((s) => s.accessControl.canEdit)
const { nodePanelPresence } = useCollaboration(appId as string, canEdit)
const controlMode = useStore((s) => s.controlMode)
const isContextMenuTarget = useStore(
(s) => s.contextMenuTarget?.type === 'node' && s.contextMenuTarget.nodeId === id,