From 45711eb6dd0d95c2a15819eea335404a120773bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E7=8E=AE=20=28Jade=20Lin=29?= Date: Thu, 23 Jul 2026 11:34:49 +0800 Subject: [PATCH] chore(api): cloud use presigned download url on api site (#39436) Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- api/controllers/web/site.py | 24 +++++-- api/extensions/ext_storage.py | 13 ++++ api/extensions/storage/aws_s3_storage.py | 18 +++++ api/extensions/storage/base_storage.py | 10 +++ api/openapi/markdown/web-openapi.md | 2 +- api/services/file_service.py | 23 +++++++ .../controllers/web/test_site.py | 35 ++++++++++ .../unit_tests/controllers/web/test_site.py | 67 +++++++++++++++++++ .../extensions/storage/test_aws_s3_storage.py | 27 ++++++++ .../unit_tests/services/test_file_service.py | 27 ++++++++ .../contracts/generated/api/web/types.gen.ts | 42 +----------- .../contracts/generated/api/web/zod.gen.ts | 49 +------------- 12 files changed, 240 insertions(+), 97 deletions(-) create mode 100644 api/tests/unit_tests/controllers/web/test_site.py create mode 100644 api/tests/unit_tests/extensions/storage/test_aws_s3_storage.py diff --git a/api/controllers/web/site.py b/api/controllers/web/site.py index c2c56f9e6de..b5999bc093d 100644 --- a/api/controllers/web/site.py +++ b/api/controllers/web/site.py @@ -1,6 +1,6 @@ from typing import Any, Self -from pydantic import AliasChoices, Field, computed_field +from pydantic import AliasChoices, Field from sqlalchemy import select from werkzeug.exceptions import Forbidden @@ -9,11 +9,13 @@ from controllers.common.schema import register_response_schema_models from controllers.web import web_ns from controllers.web.wraps import WebApiResource from extensions.ext_database import db +from extensions.storage.storage_type import StorageType from fields.base import ResponseModel from libs.helper import build_icon_url from models.account import Tenant, TenantStatus -from models.model import App, EndUser, Site +from models.model import App, EndUser, IconType, Site from services.feature_service import FeatureModel, FeatureService +from services.file_service import FileService class WebSiteResponse(ResponseModel): @@ -32,11 +34,7 @@ class WebSiteResponse(ResponseModel): prompt_public: bool | None = None show_workflow_steps: bool | None = None use_icon_as_answer_icon: bool | None = None - - @computed_field(return_type=str | None) # type: ignore[prop-decorator] - @property - def icon_url(self) -> str | None: - return build_icon_url(self.icon_type, self.icon) + icon_url: str | None = None class WebModelConfigResponse(ResponseModel): @@ -88,6 +86,7 @@ class WebAppSiteResponse(ResponseModel): end_user_id: str | None, features: FeatureModel, can_replace_logo: bool, + icon_url: str | None = None, ) -> Self: custom_config = None if can_replace_logo: @@ -102,6 +101,7 @@ class WebAppSiteResponse(ResponseModel): ) site_response = WebSiteResponse.model_validate(site, from_attributes=True) + site_response.icon_url = icon_url if icon_url is not None else build_icon_url(site.icon_type, site.icon) if features.billing.enabled and not features.webapp_copyright_enabled: site_response.copyright = None site_response.input_placeholder = None @@ -123,6 +123,15 @@ register_response_schema_models( ) +def _build_site_icon_url(*, site: Site, tenant_id: str) -> str | None: + """Use direct S3 URLs only in Cloud Mode and preserve preview URLs elsewhere.""" + if site.icon_type != IconType.IMAGE or not site.icon: + return None + if dify_config.EDITION == "CLOUD" and StorageType(dify_config.STORAGE_TYPE) == StorageType.S3: + return FileService(db.engine).get_file_presigned_url(file_id=site.icon, tenant_id=tenant_id) + return build_icon_url(site.icon_type, site.icon) + + @web_ns.route("/site") class AppSiteApi(WebApiResource): @web_ns.doc("Get App Site Info") @@ -159,4 +168,5 @@ class AppSiteApi(WebApiResource): end_user_id=end_user.id, features=features, can_replace_logo=features.can_replace_logo, + icon_url=_build_site_icon_url(site=site, tenant_id=tenant.id), ).model_dump(mode="json") diff --git a/api/extensions/ext_storage.py b/api/extensions/ext_storage.py index db5a6e48124..bc8b83b268b 100644 --- a/api/extensions/ext_storage.py +++ b/api/extensions/ext_storage.py @@ -119,6 +119,19 @@ class Storage: def delete(self, filename: str): return self.storage_runner.delete(filename) + def generate_presigned_url( + self, + filename: str, + *, + expires_in: int, + content_type: str | None = None, + ) -> str: + return self.storage_runner.generate_presigned_url( + filename, + expires_in=expires_in, + content_type=content_type, + ) + def scan(self, path: str, files: bool = True, directories: bool = False) -> list[str]: return self.storage_runner.scan(path, files=files, directories=directories) diff --git a/api/extensions/storage/aws_s3_storage.py b/api/extensions/storage/aws_s3_storage.py index 018aa17ac45..cf69f82bce1 100644 --- a/api/extensions/storage/aws_s3_storage.py +++ b/api/extensions/storage/aws_s3_storage.py @@ -92,3 +92,21 @@ class AwsS3Storage(BaseStorage): @override def delete(self, filename: str): self.client.delete_object(Bucket=self.bucket_name, Key=filename) + + @override + def generate_presigned_url( + self, + filename: str, + *, + expires_in: int, + content_type: str | None = None, + ) -> str: + params = {"Bucket": self.bucket_name, "Key": filename} + if content_type: + params["ResponseContentType"] = content_type + + return self.client.generate_presigned_url( + "get_object", + Params=params, + ExpiresIn=expires_in, + ) diff --git a/api/extensions/storage/base_storage.py b/api/extensions/storage/base_storage.py index a73d429ccd2..d9c3ff6c4c3 100644 --- a/api/extensions/storage/base_storage.py +++ b/api/extensions/storage/base_storage.py @@ -31,6 +31,16 @@ class BaseStorage(ABC): def delete(self, filename: str): raise NotImplementedError + def generate_presigned_url( + self, + filename: str, + *, + expires_in: int, + content_type: str | None = None, + ) -> str: + """Generate a temporary direct-download URL when the backend supports it.""" + raise NotImplementedError("This storage backend doesn't support presigned URLs") + def scan(self, path, files=True, directories=False) -> list[str]: """ Scan files and directories in the given path. diff --git a/api/openapi/markdown/web-openapi.md b/api/openapi/markdown/web-openapi.md index e196aa2eb81..b826b5fe8e0 100644 --- a/api/openapi/markdown/web-openapi.md +++ b/api/openapi/markdown/web-openapi.md @@ -1734,7 +1734,7 @@ in form definiton, or a variable while the workflow is running. | icon | string | | No | | icon_background | string | | No | | icon_type | string | | No | -| icon_url | string | | Yes | +| icon_url | string | | No | | input_placeholder | string | | No | | privacy_policy | string | | No | | prompt_public | boolean | | No | diff --git a/api/services/file_service.py b/api/services/file_service.py index fba7a760dfa..9409d2ac64d 100644 --- a/api/services/file_service.py +++ b/api/services/file_service.py @@ -141,6 +141,29 @@ class FileService: blob = storage.load_once(upload_file_key) return base64.b64encode(blob).decode() + def get_file_presigned_url(self, *, file_id: str, tenant_id: str) -> str: + """Generate a direct storage URL for a tenant-owned upload file.""" + with self._session_maker(expire_on_commit=False) as session: + upload_file = session.scalar( + select(UploadFile) + .where( + UploadFile.id == file_id, + UploadFile.tenant_id == tenant_id, + ) + .limit(1) + ) + if upload_file is None: + raise NotFound("File not found") + + file_key = upload_file.key + content_type = upload_file.mime_type + + return storage.generate_presigned_url( + file_key, + expires_in=dify_config.FILES_ACCESS_TIMEOUT, + content_type=content_type, + ) + def upload_text(self, text: str, text_name: str, user_id: str, tenant_id: str) -> UploadFile: if len(text_name) > 200: text_name = text_name[:200] diff --git a/api/tests/test_containers_integration_tests/controllers/web/test_site.py b/api/tests/test_containers_integration_tests/controllers/web/test_site.py index fa85f68bc4e..7f4fd45d037 100644 --- a/api/tests/test_containers_integration_tests/controllers/web/test_site.py +++ b/api/tests/test_containers_integration_tests/controllers/web/test_site.py @@ -9,7 +9,9 @@ from flask import Flask from sqlalchemy.orm import Session from werkzeug.exceptions import Forbidden +from configs import dify_config from controllers.web.site import AppSiteApi, WebAppSiteResponse, WebModelConfigResponse +from extensions.storage.storage_type import StorageType from models import Tenant, TenantStatus from models.account import TenantCustomConfigDict from models.model import App, AppMode, AppModelConfig, CustomizeTokenStrategy, EndUser, Site @@ -96,6 +98,39 @@ class TestAppSiteApi: assert result["plan"] == "basic" assert result["enable_site"] is True + @patch("controllers.web.site.FileService.get_file_presigned_url") + @patch("controllers.web.site.FeatureService.get_features") + def test_image_icon_uses_s3_presigned_url( + self, + mock_features: MagicMock, + mock_get_file_presigned_url: MagicMock, + app: Flask, + db_session_with_containers: Session, + ) -> None: + app.config["RESTX_MASK_HEADER"] = "X-Fields" + tenant = _create_tenant(db_session_with_containers) + app_model = _create_app(db_session_with_containers, tenant.id) + site = _create_site(db_session_with_containers, app_model.id) + site.icon_type = "image" + site.icon = "11111111-1111-4111-8111-111111111111" + db_session_with_containers.commit() + end_user = _end_user(tenant.id, app_model.id) + mock_features.return_value = FeatureModel(can_replace_logo=False) + mock_get_file_presigned_url.return_value = "https://s3.example.com/icon.png?signature=test" + + with ( + patch.object(dify_config, "EDITION", "CLOUD"), + patch.object(dify_config, "STORAGE_TYPE", StorageType.S3), + app.test_request_context("/site"), + ): + result = AppSiteApi().get(app_model, end_user) + + assert result["site"]["icon_url"] == "https://s3.example.com/icon.png?signature=test" + mock_get_file_presigned_url.assert_called_once_with( + file_id="11111111-1111-4111-8111-111111111111", + tenant_id=tenant.id, + ) + def test_missing_site_raises_forbidden(self, app: Flask, db_session_with_containers: Session) -> None: app.config["RESTX_MASK_HEADER"] = "X-Fields" tenant = _create_tenant(db_session_with_containers) diff --git a/api/tests/unit_tests/controllers/web/test_site.py b/api/tests/unit_tests/controllers/web/test_site.py new file mode 100644 index 00000000000..1c2a403994f --- /dev/null +++ b/api/tests/unit_tests/controllers/web/test_site.py @@ -0,0 +1,67 @@ +from unittest.mock import MagicMock, patch + +from configs import dify_config +from controllers.web import site as site_module +from extensions.storage.storage_type import StorageType +from models.model import IconType, Site + + +def test_build_site_icon_url_uses_s3_presigned_url() -> None: + site = MagicMock(spec=Site) + site.icon_type = IconType.IMAGE + site.icon = "11111111-1111-4111-8111-111111111111" + + with ( + patch.object(dify_config, "EDITION", "CLOUD"), + patch.object(dify_config, "STORAGE_TYPE", StorageType.S3), + patch.object(site_module, "db") as mock_db, + patch.object(site_module, "FileService") as mock_file_service, + patch.object(site_module, "build_icon_url") as mock_build_icon_url, + ): + mock_file_service.return_value.get_file_presigned_url.return_value = ( + "https://s3.example.com/icon.png?signature=test" + ) + + result = site_module._build_site_icon_url(site=site, tenant_id="tenant-id") + + assert result == "https://s3.example.com/icon.png?signature=test" + mock_file_service.assert_called_once_with(mock_db.engine) + mock_file_service.return_value.get_file_presigned_url.assert_called_once_with( + file_id="11111111-1111-4111-8111-111111111111", + tenant_id="tenant-id", + ) + mock_build_icon_url.assert_not_called() + + +def test_build_site_icon_url_keeps_preview_url_for_self_hosted_s3() -> None: + site = MagicMock(spec=Site) + site.icon_type = IconType.IMAGE + site.icon = "11111111-1111-4111-8111-111111111111" + + with ( + patch.object(dify_config, "EDITION", "SELF_HOSTED"), + patch.object(dify_config, "STORAGE_TYPE", StorageType.S3), + patch.object(site_module, "FileService") as mock_file_service, + patch.object(site_module, "build_icon_url", return_value="https://api.example.com/files/icon/file-preview"), + ): + result = site_module._build_site_icon_url(site=site, tenant_id="tenant-id") + + assert result == "https://api.example.com/files/icon/file-preview" + mock_file_service.assert_not_called() + + +def test_build_site_icon_url_keeps_preview_url_for_non_s3_storage() -> None: + site = MagicMock(spec=Site) + site.icon_type = IconType.IMAGE + site.icon = "11111111-1111-4111-8111-111111111111" + + with ( + patch.object(dify_config, "EDITION", "CLOUD"), + patch.object(dify_config, "STORAGE_TYPE", StorageType.LOCAL), + patch.object(site_module, "FileService") as mock_file_service, + patch.object(site_module, "build_icon_url", return_value="https://api.example.com/files/icon/file-preview"), + ): + result = site_module._build_site_icon_url(site=site, tenant_id="tenant-id") + + assert result == "https://api.example.com/files/icon/file-preview" + mock_file_service.assert_not_called() diff --git a/api/tests/unit_tests/extensions/storage/test_aws_s3_storage.py b/api/tests/unit_tests/extensions/storage/test_aws_s3_storage.py new file mode 100644 index 00000000000..acb3c7c9449 --- /dev/null +++ b/api/tests/unit_tests/extensions/storage/test_aws_s3_storage.py @@ -0,0 +1,27 @@ +from unittest.mock import MagicMock + +from extensions.storage.aws_s3_storage import AwsS3Storage + + +def test_generate_presigned_url() -> None: + storage = AwsS3Storage.__new__(AwsS3Storage) + storage.bucket_name = "test-bucket" + storage.client = MagicMock() + storage.client.generate_presigned_url.return_value = "https://s3.example.com/icon.png?signature=test" + + result = storage.generate_presigned_url( + "upload_files/tenant/icon.png", + expires_in=300, + content_type="image/png", + ) + + assert result == "https://s3.example.com/icon.png?signature=test" + storage.client.generate_presigned_url.assert_called_once_with( + "get_object", + Params={ + "Bucket": "test-bucket", + "Key": "upload_files/tenant/icon.png", + "ResponseContentType": "image/png", + }, + ExpiresIn=300, + ) diff --git a/api/tests/unit_tests/services/test_file_service.py b/api/tests/unit_tests/services/test_file_service.py index 1954b4d8f0e..583509e20db 100644 --- a/api/tests/unit_tests/services/test_file_service.py +++ b/api/tests/unit_tests/services/test_file_service.py @@ -203,6 +203,33 @@ class TestFileService: with pytest.raises(NotFound, match="File not found"): file_service.get_file_base64("non_existent") + def test_get_file_presigned_url_success(self, file_service: FileService, mock_db_session): + upload_file = MagicMock(spec=UploadFile) + upload_file.key = "upload_files/tenant_id/icon.png" + upload_file.mime_type = "image/png" + mock_db_session.scalar.return_value = upload_file + + with ( + patch.object(dify_config, "FILES_ACCESS_TIMEOUT", 300), + patch("services.file_service.storage") as mock_storage, + ): + mock_storage.generate_presigned_url.return_value = "https://s3.example.com/icon.png?signature=test" + + result = file_service.get_file_presigned_url(file_id="file_id", tenant_id="tenant_id") + + assert result == "https://s3.example.com/icon.png?signature=test" + mock_storage.generate_presigned_url.assert_called_once_with( + "upload_files/tenant_id/icon.png", + expires_in=300, + content_type="image/png", + ) + + def test_get_file_presigned_url_not_found(self, file_service: FileService, mock_db_session): + mock_db_session.scalar.return_value = None + + with pytest.raises(NotFound, match="File not found"): + file_service.get_file_presigned_url(file_id="file_id", tenant_id="tenant_id") + def test_upload_text_success(self, file_service: FileService, mock_db_session): # Setup text = "sample text" diff --git a/packages/contracts/generated/api/web/types.gen.ts b/packages/contracts/generated/api/web/types.gen.ts index f98ac9a775d..a593faced5c 100644 --- a/packages/contracts/generated/api/web/types.gen.ts +++ b/packages/contracts/generated/api/web/types.gen.ts @@ -646,7 +646,7 @@ export type WebSiteResponse = { icon?: string | null icon_background?: string | null icon_type?: string | null - readonly icon_url: string | null + icon_url?: string | null input_placeholder?: string | null privacy_policy?: string | null prompt_public?: boolean | null @@ -669,32 +669,10 @@ export type WorkflowRunPayload = { export type GeneratedAppResponseWritable = JsonValue -export type HumanInputFormDefinitionResponseWritable = { - expiration_time: number - form_content: string - inputs: Array - resolved_default_values: { - [key: string]: string - } - site?: WebAppSiteResponseWritable | null - user_actions: Array -} - export type HumanInputFormSubmitResponseWritable = { [key: string]: unknown } -export type WebAppSiteResponseWritable = { - app_id: string - can_replace_logo: boolean - custom_config?: WebAppCustomConfigResponse | null - enable_site: boolean - end_user_id?: string | null - model_config?: WebModelConfigResponse | null - plan: string - site: WebSiteResponseWritable -} - export type WebMessageInfiniteScrollPaginationWritable = { data: Array has_more: boolean @@ -726,24 +704,6 @@ export type WebMessageListItemWritable = { total_price?: string | null } -export type WebSiteResponseWritable = { - chat_color_theme?: string | null - chat_color_theme_inverted: boolean - copyright?: string | null - custom_disclaimer?: string | null - default_language?: string | null - description?: string | null - icon?: string | null - icon_background?: string | null - icon_type?: string | null - input_placeholder?: string | null - privacy_policy?: string | null - prompt_public?: boolean | null - show_workflow_steps?: boolean | null - title: string - use_icon_as_answer_icon?: boolean | null -} - export type PostAudioToTextData = { body?: never path?: never diff --git a/packages/contracts/generated/api/web/zod.gen.ts b/packages/contracts/generated/api/web/zod.gen.ts index 36ed9501a97..b1504220f27 100644 --- a/packages/contracts/generated/api/web/zod.gen.ts +++ b/packages/contracts/generated/api/web/zod.gen.ts @@ -904,7 +904,7 @@ export const zWebSiteResponse = z.object({ icon: z.string().nullish(), icon_background: z.string().nullish(), icon_type: z.string().nullish(), - icon_url: z.string().nullable(), + icon_url: z.string().nullish(), input_placeholder: z.string().nullish(), privacy_policy: z.string().nullish(), prompt_public: z.boolean().nullish(), @@ -1004,53 +1004,6 @@ export const zWebMessageInfiniteScrollPaginationWritable = z.object({ limit: z.int(), }) -/** - * WebSiteResponse - */ -export const zWebSiteResponseWritable = z.object({ - chat_color_theme: z.string().nullish(), - chat_color_theme_inverted: z.boolean(), - copyright: z.string().nullish(), - custom_disclaimer: z.string().nullish(), - default_language: z.string().nullish(), - description: z.string().nullish(), - icon: z.string().nullish(), - icon_background: z.string().nullish(), - icon_type: z.string().nullish(), - input_placeholder: z.string().nullish(), - privacy_policy: z.string().nullish(), - prompt_public: z.boolean().nullish(), - show_workflow_steps: z.boolean().nullish(), - title: z.string(), - use_icon_as_answer_icon: z.boolean().nullish(), -}) - -/** - * WebAppSiteResponse - */ -export const zWebAppSiteResponseWritable = z.object({ - app_id: z.string(), - can_replace_logo: z.boolean(), - custom_config: zWebAppCustomConfigResponse.nullish(), - enable_site: z.boolean(), - end_user_id: z.string().nullish(), - model_config: zWebModelConfigResponse.nullish(), - plan: z.string(), - site: zWebSiteResponseWritable, -}) - -/** - * HumanInputFormDefinitionResponse - */ -export const zHumanInputFormDefinitionResponseWritable = z.object({ - expiration_time: z.int(), - form_content: z.string(), - inputs: z.array(zFormInputConfig), - resolved_default_values: z.record(z.string(), z.string()), - site: zWebAppSiteResponseWritable.nullish(), - user_actions: z.array(zUserActionConfig), -}) - /** * Success */