From b3a869b91b49530808ce76c88cb142c0543f2687 Mon Sep 17 00:00:00 2001 From: Cursx <33718736+Cursx@users.noreply.github.com> Date: Fri, 23 Jan 2026 12:12:11 +0800 Subject: [PATCH] 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 --- api/controllers/console/feature.py | 13 +++- api/services/feature_service.py | 25 +++---- .../services/test_feature_service.py | 69 +++++++++++++++++-- 3 files changed, 86 insertions(+), 21 deletions(-) diff --git a/api/controllers/console/feature.py b/api/controllers/console/feature.py index d171c189ea..d3811e2d1b 100644 --- a/api/controllers/console/feature.py +++ b/api/controllers/console/feature.py @@ -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() diff --git a/api/services/feature_service.py b/api/services/feature_service.py index fc91f450b7..b2fb3784e8 100644 --- a/api/services/feature_service.py +++ b/api/services/feature_service.py @@ -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"] diff --git a/api/tests/test_containers_integration_tests/services/test_feature_service.py b/api/tests/test_containers_integration_tests/services/test_feature_service.py index 40380b09d2..bd2fd14ffa 100644 --- a/api/tests/test_containers_integration_tests/services/test_feature_service.py +++ b/api/tests/test_containers_integration_tests/services/test_feature_service.py @@ -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