mirror of
https://github.com/langgenius/dify.git
synced 2026-09-24 23:22:26 +08:00
feat: guard openapi with rbac (#37752)
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
co-authored by
autofix-ci[bot]
Copilot Autofix powered by AI
parent
0d7ca17cd1
commit
82d08851be
@@ -0,0 +1,161 @@
|
||||
"""Unit tests for controllers.common.app_access RBAC app-id access filtering."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
from controllers.common.app_access import (
|
||||
APP_LIST_PERMISSION_KEYS,
|
||||
AppAccessFilter,
|
||||
has_app_list_permission,
|
||||
resolve_app_access_filter,
|
||||
)
|
||||
from services.app_service import AppListParams
|
||||
from services.enterprise.rbac_service import (
|
||||
MyPermissionsResponse,
|
||||
ResourcePermissionKeys,
|
||||
ResourcePermissionSnapshot,
|
||||
ResourceWhitelistResources,
|
||||
WorkspacePermissionSnapshot,
|
||||
)
|
||||
|
||||
_RBAC_MODULE = "controllers.common.app_access.enterprise_rbac_service"
|
||||
|
||||
|
||||
def _permissions(
|
||||
*,
|
||||
workspace_keys: list[str] | None = None,
|
||||
app_default_keys: list[str] | None = None,
|
||||
app_overrides: list[ResourcePermissionKeys] | None = None,
|
||||
) -> MyPermissionsResponse:
|
||||
return MyPermissionsResponse(
|
||||
workspace=WorkspacePermissionSnapshot(permission_keys=workspace_keys or []),
|
||||
app=ResourcePermissionSnapshot(
|
||||
default_permission_keys=app_default_keys or [],
|
||||
overrides=app_overrides or [],
|
||||
),
|
||||
)
|
||||
|
||||
|
||||
class TestHasAppListPermission:
|
||||
def test_matches_known_preview_keys(self):
|
||||
for key in APP_LIST_PERMISSION_KEYS:
|
||||
assert has_app_list_permission([key])
|
||||
|
||||
def test_rejects_unknown_keys(self):
|
||||
assert not has_app_list_permission(["app.export", "app.delete"])
|
||||
assert not has_app_list_permission([])
|
||||
|
||||
|
||||
class TestAppAccessFilterIsAppAccessible:
|
||||
def test_unrestricted_sees_everything(self):
|
||||
flt = AppAccessFilter.unrestricted()
|
||||
assert flt.is_app_accessible("app-1", maintainer="someone", account_id="acc-1")
|
||||
|
||||
def test_whitelisted_app_is_visible(self):
|
||||
flt = AppAccessFilter(accessible_app_ids={"app-1"}, can_manage_own_apps=False)
|
||||
assert flt.is_app_accessible("app-1", maintainer=None, account_id="acc-1")
|
||||
assert not flt.is_app_accessible("app-2", maintainer=None, account_id="acc-1")
|
||||
|
||||
def test_own_app_visible_only_with_manage_permission(self):
|
||||
own = AppAccessFilter(accessible_app_ids=set(), can_manage_own_apps=True)
|
||||
assert own.is_app_accessible("app-1", maintainer="acc-1", account_id="acc-1")
|
||||
assert not own.is_app_accessible("app-1", maintainer="acc-2", account_id="acc-1")
|
||||
|
||||
no_manage = AppAccessFilter(accessible_app_ids=set(), can_manage_own_apps=False)
|
||||
assert not no_manage.is_app_accessible("app-1", maintainer="acc-1", account_id="acc-1")
|
||||
|
||||
|
||||
class TestAppAccessFilterApplyToParams:
|
||||
def test_unrestricted_leaves_params_untouched(self):
|
||||
params = AppListParams()
|
||||
AppAccessFilter.unrestricted().apply_to_params(params)
|
||||
assert params.accessible_app_ids is None
|
||||
assert params.include_own_apps is False
|
||||
assert params.is_created_by_me is None
|
||||
|
||||
def test_whitelisted_ids_are_sorted_with_own_apps_flag(self):
|
||||
params = AppListParams()
|
||||
AppAccessFilter(accessible_app_ids={"b", "a"}, can_manage_own_apps=True).apply_to_params(params)
|
||||
assert params.accessible_app_ids == ["a", "b"]
|
||||
assert params.include_own_apps is True
|
||||
|
||||
def test_empty_set_with_manage_falls_back_to_maintained_apps(self):
|
||||
# Own-app fallback must use maintainer (include_own_apps), consistent
|
||||
# with is_app_accessible — not created_by (is_created_by_me).
|
||||
params = AppListParams()
|
||||
AppAccessFilter(accessible_app_ids=set(), can_manage_own_apps=True).apply_to_params(params)
|
||||
assert params.accessible_app_ids == []
|
||||
assert params.include_own_apps is True
|
||||
assert params.is_created_by_me is None
|
||||
|
||||
def test_empty_set_without_manage_sees_nothing(self):
|
||||
params = AppListParams()
|
||||
AppAccessFilter(accessible_app_ids=set(), can_manage_own_apps=False).apply_to_params(params)
|
||||
assert params.accessible_app_ids == []
|
||||
assert params.include_own_apps is False
|
||||
assert params.is_created_by_me is None
|
||||
|
||||
|
||||
class TestResolveAppAccessFilter:
|
||||
def _patch_whitelist(self, monkeypatch: pytest.MonkeyPatch, whitelist: ResourceWhitelistResources) -> None:
|
||||
monkeypatch.setattr(
|
||||
f"{_RBAC_MODULE}.RBACService.AppAccess.whitelist_resources",
|
||||
lambda tenant_id, account_id: whitelist,
|
||||
)
|
||||
|
||||
def test_default_preview_is_unrestricted(self, monkeypatch: pytest.MonkeyPatch):
|
||||
self._patch_whitelist(monkeypatch, ResourceWhitelistResources(unrestricted=True))
|
||||
permissions = _permissions(app_default_keys=["app.preview"])
|
||||
|
||||
flt = resolve_app_access_filter("tenant-1", "acc-1", permissions=permissions)
|
||||
|
||||
assert flt.accessible_app_ids is None
|
||||
assert flt.can_manage_own_apps is False
|
||||
|
||||
def test_default_preview_overrides_whitelist_restriction(self, monkeypatch: pytest.MonkeyPatch):
|
||||
self._patch_whitelist(monkeypatch, ResourceWhitelistResources(unrestricted=False, resource_ids=["app-9"]))
|
||||
permissions = _permissions(
|
||||
workspace_keys=["app.full_access", "app.create_and_management"],
|
||||
)
|
||||
|
||||
flt = resolve_app_access_filter("tenant-1", "acc-1", permissions=permissions)
|
||||
|
||||
# Workspace-level preview grant defeats the whitelist restriction.
|
||||
assert flt.accessible_app_ids is None
|
||||
assert flt.can_manage_own_apps is True
|
||||
|
||||
def test_override_apps_collected_without_default_preview(self, monkeypatch: pytest.MonkeyPatch):
|
||||
self._patch_whitelist(monkeypatch, ResourceWhitelistResources(unrestricted=True))
|
||||
permissions = _permissions(
|
||||
app_overrides=[
|
||||
ResourcePermissionKeys(resource_id="app-1", permission_keys=["app.preview"]),
|
||||
ResourcePermissionKeys(resource_id="app-2", permission_keys=["app.export"]),
|
||||
],
|
||||
)
|
||||
|
||||
flt = resolve_app_access_filter("tenant-1", "acc-1", permissions=permissions)
|
||||
|
||||
assert flt.accessible_app_ids == {"app-1"}
|
||||
|
||||
def test_whitelist_union_with_override_apps(self, monkeypatch: pytest.MonkeyPatch):
|
||||
self._patch_whitelist(monkeypatch, ResourceWhitelistResources(unrestricted=False, resource_ids=["app-5"]))
|
||||
permissions = _permissions(
|
||||
app_overrides=[ResourcePermissionKeys(resource_id="app-1", permission_keys=["app.acl.preview"])],
|
||||
)
|
||||
|
||||
flt = resolve_app_access_filter("tenant-1", "acc-1", permissions=permissions)
|
||||
|
||||
assert flt.accessible_app_ids == {"app-1", "app-5"}
|
||||
|
||||
def test_fetches_permissions_when_not_supplied(self, monkeypatch: pytest.MonkeyPatch):
|
||||
self._patch_whitelist(monkeypatch, ResourceWhitelistResources(unrestricted=False, resource_ids=[]))
|
||||
monkeypatch.setattr(
|
||||
f"{_RBAC_MODULE}.RBACService.MyPermissions.get",
|
||||
lambda tenant_id, account_id: _permissions(workspace_keys=["app.create_and_management"]),
|
||||
)
|
||||
|
||||
flt = resolve_app_access_filter("tenant-1", "acc-1")
|
||||
|
||||
assert flt.accessible_app_ids == set()
|
||||
assert flt.can_manage_own_apps is True
|
||||
@@ -1,16 +1,21 @@
|
||||
import uuid
|
||||
|
||||
from controllers.openapi.auth.composition import account_pipeline, auth_router, external_sso_pipeline
|
||||
from controllers.openapi.auth.data import RequestContext
|
||||
from controllers.openapi.auth.data import RBACRequirement, RequestContext
|
||||
from controllers.openapi.auth.flow import When
|
||||
from controllers.openapi.auth.pipeline import AuthPipeline, PipelineRoute, PipelineRouter
|
||||
from controllers.openapi.auth.verify import (
|
||||
check_acl,
|
||||
check_private_app_permission,
|
||||
check_rbac_permission,
|
||||
check_workspace_member,
|
||||
check_workspace_mismatch,
|
||||
check_workspace_role,
|
||||
)
|
||||
from libs.oauth_bearer import TokenType
|
||||
from core.rbac import RBACPermission, RBACResourceScope
|
||||
from libs.oauth_bearer import Scope, TokenType
|
||||
from models.account import TenantAccountRole
|
||||
from services.enterprise.enterprise_service import WebAppAccessMode
|
||||
|
||||
|
||||
def test_account_pipeline_is_auth_pipeline():
|
||||
@@ -29,8 +34,8 @@ def test_account_pipeline_prepare_has_six_entries():
|
||||
assert len(account_pipeline._prepare) == 6
|
||||
|
||||
|
||||
def test_account_auth_list_has_seven_entries():
|
||||
assert len(account_pipeline._auth) == 7
|
||||
def test_account_auth_list_has_eight_entries():
|
||||
assert len(account_pipeline._auth) == 8
|
||||
|
||||
|
||||
def test_external_sso_pipeline_prepare_has_four_entries():
|
||||
@@ -132,3 +137,89 @@ def test_app_path_selects_workspace_mismatch_check():
|
||||
def test_workspace_path_skips_workspace_mismatch_check():
|
||||
steps = _selected_auth_steps(app_id=False, workspace_membership=True, allowed_roles=None)
|
||||
assert check_workspace_mismatch not in steps
|
||||
|
||||
|
||||
def _selected_webapp_steps(*, scope, app_access_mode):
|
||||
"""Select auth steps for an EE, webapp-auth-enabled, app-scoped request.
|
||||
|
||||
Patches the config-backed conditions (edition + webapp_auth) so the gating
|
||||
reduces to PATH_HAS_APP_ID, LOADED_APP_IS_PRIVATE, and the request scope.
|
||||
"""
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from controllers.openapi.auth.data import AuthData, Edition
|
||||
|
||||
ctx = RequestContext(
|
||||
token_type=TokenType.OAUTH_ACCOUNT,
|
||||
scope=scope,
|
||||
path_params={"app_id": str(uuid.uuid4())},
|
||||
)
|
||||
data = AuthData(
|
||||
token_type=TokenType.OAUTH_ACCOUNT,
|
||||
token_hash="x",
|
||||
scopes=frozenset({scope}) if scope is not None else frozenset(),
|
||||
app_access_mode=app_access_mode,
|
||||
)
|
||||
features = MagicMock()
|
||||
features.webapp_auth.enabled = True
|
||||
selected = []
|
||||
with (
|
||||
patch("controllers.openapi.auth.conditions.current_edition", return_value=Edition.EE),
|
||||
patch("controllers.openapi.auth.conditions.FeatureService.get_system_features", return_value=features),
|
||||
):
|
||||
for step in account_pipeline._auth:
|
||||
if isinstance(step, When):
|
||||
if step.applies(ctx, data):
|
||||
selected.append(step._step)
|
||||
else:
|
||||
selected.append(step)
|
||||
return selected
|
||||
|
||||
|
||||
def test_apps_run_scope_selects_webapp_checks():
|
||||
steps = _selected_webapp_steps(scope=Scope.APPS_RUN, app_access_mode=WebAppAccessMode.PRIVATE)
|
||||
assert check_acl in steps
|
||||
assert check_private_app_permission in steps
|
||||
|
||||
|
||||
def test_management_scope_skips_webapp_checks_on_private_app():
|
||||
# Export DSL et al. carry an app_id but use a management scope; the webapp
|
||||
# end-user ACL / private-app gate must not block workspace members.
|
||||
steps = _selected_webapp_steps(scope=Scope.APPS_READ, app_access_mode=WebAppAccessMode.PRIVATE)
|
||||
assert check_acl not in steps
|
||||
assert check_private_app_permission not in steps
|
||||
|
||||
|
||||
def _selected_auth_steps_with_rbac(rbac):
|
||||
ctx = RequestContext(
|
||||
token_type=TokenType.OAUTH_ACCOUNT,
|
||||
scope=Scope.APPS_READ,
|
||||
path_params={"app_id": str(uuid.uuid4())},
|
||||
rbac=rbac,
|
||||
)
|
||||
selected = []
|
||||
for step in account_pipeline._auth:
|
||||
if isinstance(step, When):
|
||||
if step.applies(ctx, None):
|
||||
selected.append(step._step)
|
||||
else:
|
||||
selected.append(step)
|
||||
return selected
|
||||
|
||||
|
||||
def test_account_pipeline_selects_rbac_step_when_required():
|
||||
rbac = RBACRequirement(resource_type=RBACResourceScope.APP, scene=RBACPermission.APP_VIEW_LAYOUT)
|
||||
assert check_rbac_permission in _selected_auth_steps_with_rbac(rbac)
|
||||
|
||||
|
||||
def test_account_pipeline_skips_rbac_step_without_requirement():
|
||||
assert check_rbac_permission not in _selected_auth_steps_with_rbac(None)
|
||||
|
||||
|
||||
def test_external_sso_pipeline_never_enforces_rbac():
|
||||
# RBAC is a console (account) concern; external SSO callers are scope-gated.
|
||||
rbac_steps = [
|
||||
s._step for s in external_sso_pipeline._auth if isinstance(s, When) and s._step is check_rbac_permission
|
||||
]
|
||||
assert rbac_steps == []
|
||||
assert check_rbac_permission not in external_sso_pipeline._auth
|
||||
|
||||
@@ -5,19 +5,22 @@ from controllers.openapi.auth.conditions import (
|
||||
EDITION_EE,
|
||||
EDITION_SAAS,
|
||||
HAS_ALLOWED_ROLES,
|
||||
HAS_RBAC,
|
||||
LOADED_APP_IS_PRIVATE,
|
||||
PATH_HAS_APP_ID,
|
||||
TOKEN_IS_OAUTH_ACCOUNT,
|
||||
TOKEN_IS_OAUTH_EXTERNAL_SSO,
|
||||
WEBAPP_AUTH_ENABLED,
|
||||
WEBAPP_RUN_SCOPED,
|
||||
WORKSPACE_MEMBERSHIP_REQUIRED,
|
||||
Cond,
|
||||
config_cond,
|
||||
data_cond,
|
||||
request_cond,
|
||||
)
|
||||
from controllers.openapi.auth.data import AuthData, Edition, RequestContext
|
||||
from libs.oauth_bearer import TokenType
|
||||
from controllers.openapi.auth.data import AuthData, Edition, RBACRequirement, RequestContext
|
||||
from core.rbac import RBACPermission, RBACResourceScope
|
||||
from libs.oauth_bearer import Scope, TokenType
|
||||
from models.account import TenantAccountRole
|
||||
from services.enterprise.enterprise_service import WebAppAccessMode
|
||||
|
||||
@@ -137,6 +140,34 @@ def test_webapp_auth_enabled():
|
||||
assert WEBAPP_AUTH_ENABLED(_ctx()) is True
|
||||
|
||||
|
||||
def test_webapp_run_scoped_true_for_apps_run():
|
||||
assert WEBAPP_RUN_SCOPED(_ctx(scope=Scope.APPS_RUN)) is True
|
||||
|
||||
|
||||
def test_webapp_run_scoped_false_for_management_scope():
|
||||
assert WEBAPP_RUN_SCOPED(_ctx(scope=Scope.APPS_READ)) is False
|
||||
|
||||
|
||||
def test_webapp_run_scoped_false_when_scope_none():
|
||||
assert WEBAPP_RUN_SCOPED(_ctx()) is False
|
||||
|
||||
|
||||
def _rbac_req():
|
||||
return RBACRequirement(resource_type=RBACResourceScope.APP, scene=RBACPermission.APP_TEST_AND_RUN)
|
||||
|
||||
|
||||
def test_has_rbac_true():
|
||||
assert HAS_RBAC(_ctx(rbac=_rbac_req())) is True
|
||||
|
||||
|
||||
def test_has_rbac_false():
|
||||
assert HAS_RBAC(_ctx(rbac=None)) is False
|
||||
|
||||
|
||||
def test_has_rbac_default():
|
||||
assert HAS_RBAC(_ctx()) is False
|
||||
|
||||
|
||||
def test_loaded_app_is_private():
|
||||
data_private = _data(app_access_mode=WebAppAccessMode.PRIVATE)
|
||||
data_public = _data(app_access_mode=WebAppAccessMode.PUBLIC)
|
||||
|
||||
@@ -5,17 +5,19 @@ import pytest
|
||||
from flask import Flask
|
||||
from werkzeug.exceptions import Forbidden, NotFound
|
||||
|
||||
from controllers.openapi.auth.data import AuthData
|
||||
from controllers.openapi.auth.data import AuthData, RBACRequirement
|
||||
from controllers.openapi.auth.verify import (
|
||||
check_acl,
|
||||
check_app_access,
|
||||
check_app_api_enabled,
|
||||
check_private_app_permission,
|
||||
check_rbac_permission,
|
||||
check_scope,
|
||||
check_workspace_member,
|
||||
check_workspace_mismatch,
|
||||
check_workspace_role,
|
||||
)
|
||||
from core.rbac import RBACPermission, RBACResourceScope
|
||||
from libs.oauth_bearer import Scope, TokenType
|
||||
from models.account import Tenant, TenantAccountRole
|
||||
from models.model import App
|
||||
@@ -75,6 +77,67 @@ def test_check_app_access_raises_when_not_member():
|
||||
check_app_access(data)
|
||||
|
||||
|
||||
# --- check_rbac_permission ---
|
||||
|
||||
_RBAC_REQ = RBACRequirement(resource_type=RBACResourceScope.APP, scene=RBACPermission.APP_VIEW_LAYOUT)
|
||||
|
||||
|
||||
def test_check_rbac_noop_when_no_requirement():
|
||||
with patch("controllers.openapi.auth.verify.enforce_rbac_access") as mock_enforce:
|
||||
check_rbac_permission(_data(rbac=None, caller_kind="account"))
|
||||
mock_enforce.assert_not_called()
|
||||
|
||||
|
||||
def test_check_rbac_noop_when_rbac_disabled():
|
||||
with (
|
||||
patch("controllers.openapi.auth.verify.dify_config.RBAC_ENABLED", False),
|
||||
patch("controllers.openapi.auth.verify.enforce_rbac_access") as mock_enforce,
|
||||
):
|
||||
check_rbac_permission(_data(rbac=_RBAC_REQ, caller_kind="account"))
|
||||
mock_enforce.assert_not_called()
|
||||
|
||||
|
||||
def test_check_rbac_skips_end_user_caller():
|
||||
with (
|
||||
patch("controllers.openapi.auth.verify.dify_config.RBAC_ENABLED", True),
|
||||
patch("controllers.openapi.auth.verify.enforce_rbac_access") as mock_enforce,
|
||||
):
|
||||
check_rbac_permission(_data(rbac=_RBAC_REQ, caller_kind="end_user"))
|
||||
mock_enforce.assert_not_called()
|
||||
|
||||
|
||||
def test_check_rbac_raises_when_context_missing():
|
||||
with patch("controllers.openapi.auth.verify.dify_config.RBAC_ENABLED", True):
|
||||
with pytest.raises(Forbidden, match="rbac context missing"):
|
||||
check_rbac_permission(_data(rbac=_RBAC_REQ, caller_kind="account", account_id=None, tenant=None))
|
||||
|
||||
|
||||
def test_check_rbac_enforces_for_account_caller():
|
||||
tenant = MagicMock(spec=Tenant)
|
||||
tenant.id = "t1"
|
||||
account_id = uuid.uuid4()
|
||||
data = _data(
|
||||
rbac=_RBAC_REQ,
|
||||
caller_kind="account",
|
||||
account_id=account_id,
|
||||
tenant=tenant,
|
||||
path_params={"app_id": "app-1"},
|
||||
)
|
||||
with (
|
||||
patch("controllers.openapi.auth.verify.dify_config.RBAC_ENABLED", True),
|
||||
patch("controllers.openapi.auth.verify.enforce_rbac_access") as mock_enforce,
|
||||
):
|
||||
check_rbac_permission(data)
|
||||
mock_enforce.assert_called_once_with(
|
||||
tenant_id="t1",
|
||||
account_id=str(account_id),
|
||||
resource_type=RBACResourceScope.APP,
|
||||
scene=RBACPermission.APP_VIEW_LAYOUT,
|
||||
resource_required=True,
|
||||
path_args={"app_id": "app-1"},
|
||||
)
|
||||
|
||||
|
||||
def test_check_acl_raises_when_app_or_mode_missing():
|
||||
with pytest.raises(Forbidden):
|
||||
check_acl(_data(app=None, app_access_mode=None))
|
||||
|
||||
@@ -20,6 +20,7 @@ def _stub_execute(
|
||||
edition=None,
|
||||
workspace_membership=False,
|
||||
allowed_roles=None,
|
||||
rbac=None,
|
||||
):
|
||||
"""Bypass all auth logic; inject minimal AuthData and call the view directly."""
|
||||
kwargs["auth_data"] = AuthData(
|
||||
@@ -30,6 +31,7 @@ def _stub_execute(
|
||||
scopes=frozenset({Scope.FULL}),
|
||||
required_scope=scope,
|
||||
allowed_roles=allowed_roles,
|
||||
rbac=rbac,
|
||||
)
|
||||
return view(*args, **kwargs)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user