mirror of https://github.com/langgenius/dify.git
refactor: optimize system features response payload for unauthenticated clients (#31392)
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: QuantumGhost <obelisk.reg+git@gmail.com>
This commit is contained in:
parent
f911199c8e
commit
b3a869b91b
|
|
@ -1,6 +1,7 @@
|
|||
from flask_restx import Resource, fields
|
||||
from werkzeug.exceptions import Unauthorized
|
||||
|
||||
from libs.login import current_account_with_tenant, login_required
|
||||
from libs.login import current_account_with_tenant, current_user, login_required
|
||||
from services.feature_service import FeatureService
|
||||
|
||||
from . import console_ns
|
||||
|
|
@ -48,4 +49,12 @@ class SystemFeatureApi(Resource):
|
|||
|
||||
Only non-sensitive configuration data should be returned by this endpoint.
|
||||
"""
|
||||
return FeatureService.get_system_features().model_dump()
|
||||
# NOTE(QuantumGhost): ideally we should access `current_user.is_authenticated`
|
||||
# without a try-catch. However, due to the implementation of user loader (the `load_user_from_request`
|
||||
# in api/extensions/ext_login.py), accessing `current_user.is_authenticated` will
|
||||
# raise `Unauthorized` exception if authentication token is not provided.
|
||||
try:
|
||||
is_authenticated = current_user.is_authenticated
|
||||
except Unauthorized:
|
||||
is_authenticated = False
|
||||
return FeatureService.get_system_features(is_authenticated=is_authenticated).model_dump()
|
||||
|
|
|
|||
|
|
@ -202,7 +202,7 @@ class FeatureService:
|
|||
return knowledge_rate_limit
|
||||
|
||||
@classmethod
|
||||
def get_system_features(cls) -> SystemFeatureModel:
|
||||
def get_system_features(cls, is_authenticated: bool = False) -> SystemFeatureModel:
|
||||
system_features = SystemFeatureModel()
|
||||
|
||||
cls._fulfill_system_params_from_env(system_features)
|
||||
|
|
@ -212,7 +212,7 @@ class FeatureService:
|
|||
system_features.webapp_auth.enabled = True
|
||||
system_features.enable_change_email = False
|
||||
system_features.plugin_manager.enabled = True
|
||||
cls._fulfill_params_from_enterprise(system_features)
|
||||
cls._fulfill_params_from_enterprise(system_features, is_authenticated)
|
||||
|
||||
if dify_config.MARKETPLACE_ENABLED:
|
||||
system_features.enable_marketplace = True
|
||||
|
|
@ -310,7 +310,7 @@ class FeatureService:
|
|||
features.next_credit_reset_date = billing_info["next_credit_reset_date"]
|
||||
|
||||
@classmethod
|
||||
def _fulfill_params_from_enterprise(cls, features: SystemFeatureModel):
|
||||
def _fulfill_params_from_enterprise(cls, features: SystemFeatureModel, is_authenticated: bool = False):
|
||||
enterprise_info = EnterpriseService.get_info()
|
||||
|
||||
if "SSOEnforcedForSignin" in enterprise_info:
|
||||
|
|
@ -347,19 +347,14 @@ class FeatureService:
|
|||
)
|
||||
features.webapp_auth.sso_config.protocol = enterprise_info.get("SSOEnforcedForWebProtocol", "")
|
||||
|
||||
if "License" in enterprise_info:
|
||||
license_info = enterprise_info["License"]
|
||||
if is_authenticated and (license_info := enterprise_info.get("License")):
|
||||
features.license.status = LicenseStatus(license_info.get("status", LicenseStatus.INACTIVE))
|
||||
features.license.expired_at = license_info.get("expiredAt", "")
|
||||
|
||||
if "status" in license_info:
|
||||
features.license.status = LicenseStatus(license_info.get("status", LicenseStatus.INACTIVE))
|
||||
|
||||
if "expiredAt" in license_info:
|
||||
features.license.expired_at = license_info["expiredAt"]
|
||||
|
||||
if "workspaces" in license_info:
|
||||
features.license.workspaces.enabled = license_info["workspaces"]["enabled"]
|
||||
features.license.workspaces.limit = license_info["workspaces"]["limit"]
|
||||
features.license.workspaces.size = license_info["workspaces"]["used"]
|
||||
if workspaces_info := license_info.get("workspaces"):
|
||||
features.license.workspaces.enabled = workspaces_info.get("enabled", False)
|
||||
features.license.workspaces.limit = workspaces_info.get("limit", 0)
|
||||
features.license.workspaces.size = workspaces_info.get("used", 0)
|
||||
|
||||
if "PluginInstallationPermission" in enterprise_info:
|
||||
plugin_installation_info = enterprise_info["PluginInstallationPermission"]
|
||||
|
|
|
|||
|
|
@ -4,7 +4,13 @@ import pytest
|
|||
from faker import Faker
|
||||
|
||||
from enums.cloud_plan import CloudPlan
|
||||
from services.feature_service import FeatureModel, FeatureService, KnowledgeRateLimitModel, SystemFeatureModel
|
||||
from services.feature_service import (
|
||||
FeatureModel,
|
||||
FeatureService,
|
||||
KnowledgeRateLimitModel,
|
||||
LicenseStatus,
|
||||
SystemFeatureModel,
|
||||
)
|
||||
|
||||
|
||||
class TestFeatureService:
|
||||
|
|
@ -274,7 +280,7 @@ class TestFeatureService:
|
|||
mock_config.PLUGIN_MAX_PACKAGE_SIZE = 100
|
||||
|
||||
# Act: Execute the method under test
|
||||
result = FeatureService.get_system_features()
|
||||
result = FeatureService.get_system_features(is_authenticated=True)
|
||||
|
||||
# Assert: Verify the expected outcomes
|
||||
assert result is not None
|
||||
|
|
@ -324,6 +330,61 @@ class TestFeatureService:
|
|||
# Verify mock interactions
|
||||
mock_external_service_dependencies["enterprise_service"].get_info.assert_called_once()
|
||||
|
||||
def test_get_system_features_unauthenticated(self, db_session_with_containers, mock_external_service_dependencies):
|
||||
"""
|
||||
Test system features retrieval for an unauthenticated user.
|
||||
|
||||
This test verifies that:
|
||||
- The response payload is minimized (e.g., verbose license details are excluded).
|
||||
- Essential UI configuration (Branding, SSO, Marketplace) remains available.
|
||||
- The response structure adheres to the public schema for unauthenticated clients.
|
||||
"""
|
||||
# Arrange: Setup test data with exact same config as success test
|
||||
with patch("services.feature_service.dify_config") as mock_config:
|
||||
mock_config.ENTERPRISE_ENABLED = True
|
||||
mock_config.MARKETPLACE_ENABLED = True
|
||||
mock_config.ENABLE_EMAIL_CODE_LOGIN = True
|
||||
mock_config.ENABLE_EMAIL_PASSWORD_LOGIN = True
|
||||
mock_config.ENABLE_SOCIAL_OAUTH_LOGIN = False
|
||||
mock_config.ALLOW_REGISTER = False
|
||||
mock_config.ALLOW_CREATE_WORKSPACE = False
|
||||
mock_config.MAIL_TYPE = "smtp"
|
||||
mock_config.PLUGIN_MAX_PACKAGE_SIZE = 100
|
||||
|
||||
# Act: Execute with is_authenticated=False
|
||||
result = FeatureService.get_system_features(is_authenticated=False)
|
||||
|
||||
# Assert: Basic structure
|
||||
assert result is not None
|
||||
assert isinstance(result, SystemFeatureModel)
|
||||
|
||||
# --- 1. Verify Response Payload Optimization (Data Minimization) ---
|
||||
# Ensure only essential UI flags are returned to unauthenticated clients
|
||||
# to keep the payload lightweight and adhere to architectural boundaries.
|
||||
assert result.license.status == LicenseStatus.NONE
|
||||
assert result.license.expired_at == ""
|
||||
assert result.license.workspaces.enabled is False
|
||||
assert result.license.workspaces.limit == 0
|
||||
assert result.license.workspaces.size == 0
|
||||
|
||||
# --- 2. Verify Public UI Configuration Availability ---
|
||||
# Ensure that data required for frontend rendering remains accessible.
|
||||
|
||||
# Branding should match the mock data
|
||||
assert result.branding.enabled is True
|
||||
assert result.branding.application_title == "Test Enterprise"
|
||||
assert result.branding.login_page_logo == "https://example.com/logo.png"
|
||||
|
||||
# SSO settings should be visible for login page rendering
|
||||
assert result.sso_enforced_for_signin is True
|
||||
assert result.sso_enforced_for_signin_protocol == "saml"
|
||||
|
||||
# General auth settings should be visible
|
||||
assert result.enable_email_code_login is True
|
||||
|
||||
# Marketplace should be visible
|
||||
assert result.enable_marketplace is True
|
||||
|
||||
def test_get_system_features_basic_config(self, db_session_with_containers, mock_external_service_dependencies):
|
||||
"""
|
||||
Test system features retrieval with basic configuration (no enterprise).
|
||||
|
|
@ -1031,7 +1092,7 @@ class TestFeatureService:
|
|||
}
|
||||
|
||||
# Act: Execute the method under test
|
||||
result = FeatureService.get_system_features()
|
||||
result = FeatureService.get_system_features(is_authenticated=True)
|
||||
|
||||
# Assert: Verify the expected outcomes
|
||||
assert result is not None
|
||||
|
|
@ -1400,7 +1461,7 @@ class TestFeatureService:
|
|||
}
|
||||
|
||||
# Act: Execute the method under test
|
||||
result = FeatureService.get_system_features()
|
||||
result = FeatureService.get_system_features(is_authenticated=True)
|
||||
|
||||
# Assert: Verify the expected outcomes
|
||||
assert result is not None
|
||||
|
|
|
|||
Loading…
Reference in New Issue