diff --git a/api/.env.example b/api/.env.example index 75f0c8fdbfb..13cba5ca19b 100644 --- a/api/.env.example +++ b/api/.env.example @@ -4,6 +4,12 @@ # Alternatively you can set it with `SECRET_KEY` environment variable. SECRET_KEY= +# Optional server-side secret mixed into account password hashes. Leave empty to keep the legacy hashing. +# WARNING: setting, changing, or removing this value changes every password hash and existing passwords +# will stop working. Do not change it casually. +# Use a high-entropy random value, e.g. generate one with `openssl rand -base64 32`. +PASSWORD_PEPPER= + # Ensure UTF-8 encoding LANG=en_US.UTF-8 LC_ALL=en_US.UTF-8 diff --git a/api/configs/feature/__init__.py b/api/configs/feature/__init__.py index 80e26142786..ada427ac89f 100644 --- a/api/configs/feature/__init__.py +++ b/api/configs/feature/__init__.py @@ -76,6 +76,15 @@ class SecurityConfig(BaseSettings): default=None, ) + PASSWORD_PEPPER: str = Field( + description="Server-side secret mixed into account password hashes. " + "Leave empty to keep the legacy unpeppered hashing. " + "Use a high-entropy random value, e.g. `openssl rand -base64 32`. " + "WARNING: setting, changing, or removing this value changes every password hash, " + "so existing passwords will no longer verify. Do not change it casually.", + default="", + ) + class AppExecutionConfig(BaseSettings): """ diff --git a/api/controllers/console/workspace/__init__.py b/api/controllers/console/workspace/__init__.py index 09bb535e626..15f9b3ec97f 100644 --- a/api/controllers/console/workspace/__init__.py +++ b/api/controllers/console/workspace/__init__.py @@ -34,12 +34,13 @@ def plugin_permission_required( .limit(1) ) - if not permission: - # no permission set, allow access for everyone - return view(*args, **kwargs) + install_permission = ( + permission.install_permission if permission else TenantPluginInstallPermission.ADMINS + ) + debug_permission = permission.debug_permission if permission else TenantPluginDebugPermission.EVERYONE if install_required: - match permission.install_permission: + match install_permission: case TenantPluginInstallPermission.NOBODY: raise Forbidden() case TenantPluginInstallPermission.ADMINS: @@ -49,7 +50,7 @@ def plugin_permission_required( pass if debug_required: - match permission.debug_permission: + match debug_permission: case TenantPluginDebugPermission.NOBODY: raise Forbidden() case TenantPluginDebugPermission.ADMINS: diff --git a/api/controllers/console/workspace/plugin.py b/api/controllers/console/workspace/plugin.py index 2f35e42d334..9d3305dcc2f 100644 --- a/api/controllers/console/workspace/plugin.py +++ b/api/controllers/console/workspace/plugin.py @@ -1118,7 +1118,7 @@ class PluginFetchPermissionApi(Resource): if not permission: return jsonable_encoder( { - "install_permission": TenantPluginInstallPermission.EVERYONE, + "install_permission": TenantPluginInstallPermission.ADMINS, "debug_permission": TenantPluginDebugPermission.EVERYONE, } ) diff --git a/api/libs/password.py b/api/libs/password.py index 4170abbf225..e3752333faf 100644 --- a/api/libs/password.py +++ b/api/libs/password.py @@ -1,8 +1,11 @@ import base64 import binascii import hashlib +import hmac import re +from configs import dify_config + password_pattern = r"^(?=.*[a-zA-Z])(?=.*\d).{8,}$" @@ -18,6 +21,9 @@ def valid_password(password): def hash_password(password_str: str, salt_byte: bytes): dk = hashlib.pbkdf2_hmac("sha256", password_str.encode("utf-8"), salt_byte, 10000) + # The pepper wraps the legacy PBKDF2 digest, so changing PASSWORD_PEPPER invalidates every stored hash. + if pepper := dify_config.PASSWORD_PEPPER: + dk = hmac.new(pepper.encode("utf-8"), dk, hashlib.sha256).digest() return binascii.hexlify(dk) diff --git a/api/models/account.py b/api/models/account.py index 7503918c206..0000e6c96b3 100644 --- a/api/models/account.py +++ b/api/models/account.py @@ -315,7 +315,7 @@ class TenantPluginPermission(TypeBase): install_permission: Mapped[TenantPluginInstallPermission] = mapped_column( EnumText(TenantPluginInstallPermission, length=16), nullable=False, - default=TenantPluginInstallPermission.EVERYONE, + default=TenantPluginInstallPermission.ADMINS, ) debug_permission: Mapped[TenantPluginDebugPermission] = mapped_column( EnumText(TenantPluginDebugPermission, length=16), diff --git a/api/tests/test_containers_integration_tests/controllers/console/workspace/test_workspace_wraps.py b/api/tests/test_containers_integration_tests/controllers/console/workspace/test_workspace_wraps.py index 895f638380b..ecb0e5ebff3 100644 --- a/api/tests/test_containers_integration_tests/controllers/console/workspace/test_workspace_wraps.py +++ b/api/tests/test_containers_integration_tests/controllers/console/workspace/test_workspace_wraps.py @@ -60,6 +60,52 @@ class TestPluginPermissionRequired: assert handler() == "ok" + def test_install_without_permission_forbids_non_admin(self, db_session_with_containers: Session): + tenant = _create_tenant(db_session_with_containers) + user = SimpleNamespace(is_admin_or_owner=False) + + with patch( + "controllers.console.workspace.current_account_with_tenant", + return_value=(user, tenant.id), + ): + + @plugin_permission_required(install_required=True) + def handler(): + return "ok" + + with pytest.raises(Forbidden): + handler() + + def test_install_without_permission_allows_admin(self, db_session_with_containers: Session): + tenant = _create_tenant(db_session_with_containers) + user = SimpleNamespace(is_admin_or_owner=True) + + with patch( + "controllers.console.workspace.current_account_with_tenant", + return_value=(user, tenant.id), + ): + + @plugin_permission_required(install_required=True) + def handler(): + return "ok" + + assert handler() == "ok" + + def test_debug_without_permission_allows_non_admin(self, db_session_with_containers: Session): + tenant = _create_tenant(db_session_with_containers) + user = SimpleNamespace(is_admin_or_owner=False) + + with patch( + "controllers.console.workspace.current_account_with_tenant", + return_value=(user, tenant.id), + ): + + @plugin_permission_required(debug_required=True) + def handler(): + return "ok" + + assert handler() == "ok" + def test_install_nobody_forbidden(self, db_session_with_containers: Session): tenant = _create_tenant(db_session_with_containers) _create_permission( diff --git a/api/tests/unit_tests/controllers/console/workspace/test_plugin.py b/api/tests/unit_tests/controllers/console/workspace/test_plugin.py index 5424dc65634..122e249a630 100644 --- a/api/tests/unit_tests/controllers/console/workspace/test_plugin.py +++ b/api/tests/unit_tests/controllers/console/workspace/test_plugin.py @@ -754,7 +754,8 @@ class TestPluginFetchPermissionApi: ): result = method(api, "t1") - assert result["install_permission"] is not None + assert result["install_permission"] == TenantPluginInstallPermission.ADMINS + assert result["debug_permission"] == TenantPluginDebugPermission.EVERYONE class TestPluginFetchDynamicSelectOptionsApi: diff --git a/api/tests/unit_tests/libs/test_password.py b/api/tests/unit_tests/libs/test_password.py index 3cdf22e8051..b0ead5605e0 100644 --- a/api/tests/unit_tests/libs/test_password.py +++ b/api/tests/unit_tests/libs/test_password.py @@ -1,6 +1,9 @@ import base64 import binascii +import hashlib +import hmac import os +from collections.abc import Callable import pytest @@ -79,3 +82,44 @@ class TestPasswordHashing: """Test password case sensitivity""" result = compare_password(self.password.upper(), self.password_hash_base64, self.salt_base64) assert result is False + + +class TestPasswordPepper: + """Test PASSWORD_PEPPER handling""" + + password = "test123password" + salt = b"0123456789abcdef" + + @staticmethod + def _encode(password_hash: bytes) -> str: + return base64.b64encode(password_hash).decode() + + def test_empty_pepper_keeps_legacy_hash(self, config_overrides: Callable[..., None]): + config_overrides(PASSWORD_PEPPER="") + legacy = binascii.hexlify(hashlib.pbkdf2_hmac("sha256", self.password.encode(), self.salt, 10000)) + + assert hash_password(self.password, self.salt) == legacy + + def test_pepper_wraps_legacy_digest(self, config_overrides: Callable[..., None]): + config_overrides(PASSWORD_PEPPER="pepper-a") + legacy_digest = hashlib.pbkdf2_hmac("sha256", self.password.encode(), self.salt, 10000) + expected = binascii.hexlify(hmac.new(b"pepper-a", legacy_digest, hashlib.sha256).digest()) + + assert hash_password(self.password, self.salt) == expected + + def test_peppered_hash_verifies_with_same_pepper(self, config_overrides: Callable[..., None]): + config_overrides(PASSWORD_PEPPER="pepper-a") + password_hash = self._encode(hash_password(self.password, self.salt)) + + assert compare_password(self.password, password_hash, self._encode(self.salt)) is True + assert compare_password("wrongpassword1", password_hash, self._encode(self.salt)) is False + + @pytest.mark.parametrize(("hash_pepper", "verify_pepper"), [("", "pepper-a"), ("pepper-a", ""), ("pepper-a", "b")]) + def test_changing_pepper_invalidates_existing_hash( + self, config_overrides: Callable[..., None], hash_pepper: str, verify_pepper: str + ): + config_overrides(PASSWORD_PEPPER=hash_pepper) + password_hash = self._encode(hash_password(self.password, self.salt)) + + config_overrides(PASSWORD_PEPPER=verify_pepper) + assert compare_password(self.password, password_hash, self._encode(self.salt)) is False diff --git a/docker/envs/core-services/api.env.example b/docker/envs/core-services/api.env.example index 9e97522e395..0d5565374e8 100644 --- a/docker/envs/core-services/api.env.example +++ b/docker/envs/core-services/api.env.example @@ -11,6 +11,12 @@ PLUGIN_REMOTE_INSTALL_PORT=5003 PLUGIN_MAX_PACKAGE_SIZE=52428800 PLUGIN_DAEMON_TIMEOUT=600.0 INNER_API_KEY_FOR_PLUGIN=QaHbTe77CtuXmsfyhR7+vRjI/+XbV1AaFy691iy+kGDv2Jvy0/eAh8Y1 +# Optional server-side secret mixed into account password hashes. Leave empty to keep the legacy hashing. +# WARNING: setting, changing, or removing this value changes every password hash and existing passwords +# will stop working. Do not change it casually. +# Use a high-entropy random value, e.g. generate one with `openssl rand -base64 32`. +# Keep it in this file rather than docker/.env, which is also loaded by plugin_daemon and agent_backend. +PASSWORD_PEPPER= # Client deadline for converting a Binding file to a ToolFile through the Agent backend. AGENT_BACKEND_BINDING_FILE_DOWNLOAD_TIMEOUT_SECONDS=240 KNOWLEDGE_FS_ENABLED=${KNOWLEDGE_FS_ENABLED:-false}