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

@@ -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")

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)
 

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,
+        )

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.

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 |

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]

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)

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()

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,
+    )

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"

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<FormInputConfig>
-  resolved_default_values: {
-    [key: string]: string
-  }
-  site?: WebAppSiteResponseWritable | null
-  user_actions: Array<UserActionConfig>
-}
-
 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<WebMessageListItemWritable>
   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

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
  */