mirror of
https://github.com/langgenius/dify.git
synced 2026-06-10 18:24:09 +08:00
chore: filter unavailable apps from the installed apps list API (#37206)
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
This commit is contained in:
parent
d11e4eeaf7
commit
5bec8eb33a
@ -5,7 +5,7 @@ from typing import Any
|
||||
from flask import request
|
||||
from flask_restx import Resource
|
||||
from pydantic import BaseModel, Field, computed_field, field_validator
|
||||
from sqlalchemy import and_, select
|
||||
from sqlalchemy import and_, exists, or_, select
|
||||
from werkzeug.exceptions import BadRequest, Forbidden, NotFound
|
||||
|
||||
from controllers.common.fields import SimpleMessageResponse, SimpleResultMessageResponse
|
||||
@ -24,8 +24,8 @@ from graphon.file import helpers as file_helpers
|
||||
from libs.datetime_utils import naive_utc_now
|
||||
from libs.helper import to_timestamp
|
||||
from libs.login import login_required
|
||||
from models import Account, App, InstalledApp, RecommendedApp
|
||||
from models.model import IconType
|
||||
from models import Account, App, AppModelConfig, InstalledApp, RecommendedApp, Workflow
|
||||
from models.model import AppMode, IconType
|
||||
from services.account_service import TenantService
|
||||
from services.enterprise.enterprise_service import EnterpriseService
|
||||
from services.feature_service import FeatureService
|
||||
@ -61,6 +61,24 @@ def _safe_primitive(value: Any) -> Any:
|
||||
return None
|
||||
|
||||
|
||||
def _published_app_filter():
|
||||
"""Return the SQL predicate for installed-app web API availability.
|
||||
|
||||
The installed-app parameters endpoint reads the published workflow for
|
||||
workflow-style apps and the published app model config for easy UI apps.
|
||||
Keep the list endpoint aligned in SQL so it does not return entries that
|
||||
will immediately fail with app_unavailable when opened.
|
||||
"""
|
||||
workflow_app_modes = (AppMode.ADVANCED_CHAT, AppMode.WORKFLOW)
|
||||
has_published_workflow = exists(select(Workflow.id).where(Workflow.id == App.workflow_id))
|
||||
has_published_model_config = exists(select(AppModelConfig.id).where(AppModelConfig.id == App.app_model_config_id))
|
||||
|
||||
return or_(
|
||||
and_(App.mode.in_(workflow_app_modes), App.workflow_id.isnot(None), has_published_workflow),
|
||||
and_(~App.mode.in_(workflow_app_modes), App.app_model_config_id.isnot(None), has_published_model_config),
|
||||
)
|
||||
|
||||
|
||||
class InstalledAppInfoResponse(ResponseModel):
|
||||
id: str
|
||||
name: str | None = None
|
||||
@ -141,33 +159,32 @@ class InstalledAppsListApi(Resource):
|
||||
def get(self, current_tenant_id: str, current_user: Account):
|
||||
query = InstalledAppsListQuery.model_validate(request.args.to_dict())
|
||||
|
||||
stmt = (
|
||||
select(InstalledApp, App)
|
||||
.join(App, App.id == InstalledApp.app_id)
|
||||
.where(InstalledApp.tenant_id == current_tenant_id, _published_app_filter())
|
||||
)
|
||||
if query.app_id:
|
||||
installed_apps = db.session.scalars(
|
||||
select(InstalledApp).where(
|
||||
and_(InstalledApp.tenant_id == current_tenant_id, InstalledApp.app_id == query.app_id)
|
||||
)
|
||||
).all()
|
||||
else:
|
||||
installed_apps = db.session.scalars(
|
||||
select(InstalledApp).where(InstalledApp.tenant_id == current_tenant_id)
|
||||
).all()
|
||||
stmt = stmt.where(InstalledApp.app_id == query.app_id)
|
||||
|
||||
installed_apps = db.session.execute(stmt).all()
|
||||
|
||||
if current_user.current_tenant is None:
|
||||
raise ValueError("current_user.current_tenant must not be None")
|
||||
current_user.role = TenantService.get_user_role(current_user, current_user.current_tenant)
|
||||
installed_app_list: list[dict[str, Any]] = [
|
||||
{
|
||||
"id": installed_app.id,
|
||||
"app": installed_app.app,
|
||||
"app_owner_tenant_id": installed_app.app_owner_tenant_id,
|
||||
"is_pinned": installed_app.is_pinned,
|
||||
"last_used_at": installed_app.last_used_at,
|
||||
"editable": current_user.role in {"owner", "admin"},
|
||||
"uninstallable": current_tenant_id == installed_app.app_owner_tenant_id,
|
||||
}
|
||||
for installed_app in installed_apps
|
||||
if installed_app.app is not None
|
||||
]
|
||||
installed_app_list: list[dict[str, Any]] = []
|
||||
for installed_app, app_model in installed_apps:
|
||||
installed_app_list.append(
|
||||
{
|
||||
"id": installed_app.id,
|
||||
"app": app_model,
|
||||
"app_owner_tenant_id": installed_app.app_owner_tenant_id,
|
||||
"is_pinned": installed_app.is_pinned,
|
||||
"last_used_at": installed_app.last_used_at,
|
||||
"editable": current_user.role in {"owner", "admin"},
|
||||
"uninstallable": current_tenant_id == installed_app.app_owner_tenant_id,
|
||||
}
|
||||
)
|
||||
|
||||
# filter out apps that user doesn't have access to
|
||||
if FeatureService.get_system_features().webapp_auth.enabled:
|
||||
|
||||
@ -57,6 +57,14 @@ def payload_patch() -> PayloadPatch:
|
||||
|
||||
|
||||
class TestInstalledAppsListApi:
|
||||
def test_published_app_filter_checks_publish_targets(self) -> None:
|
||||
compiled_filter = str(module._published_app_filter().compile(compile_kwargs={"literal_binds": True}))
|
||||
|
||||
assert "workflows" in compiled_filter
|
||||
assert "app_model_configs" in compiled_filter
|
||||
assert "workflow_id" in compiled_filter
|
||||
assert "app_model_config_id" in compiled_filter
|
||||
|
||||
def test_get_installed_apps(
|
||||
self, app: Flask, current_user: MagicMock, tenant_id: str, installed_app: MagicMock
|
||||
) -> None:
|
||||
@ -64,7 +72,7 @@ class TestInstalledAppsListApi:
|
||||
method = unwrap(api.get)
|
||||
|
||||
session = MagicMock()
|
||||
session.scalars.return_value.all.return_value = [installed_app]
|
||||
session.execute.return_value.all.return_value = [(installed_app, installed_app.app)]
|
||||
|
||||
with (
|
||||
app.test_request_context("/"),
|
||||
@ -87,7 +95,7 @@ class TestInstalledAppsListApi:
|
||||
method = unwrap(api.get)
|
||||
|
||||
session = MagicMock()
|
||||
session.scalars.return_value.all.return_value = []
|
||||
session.execute.return_value.all.return_value = []
|
||||
|
||||
with (
|
||||
app.test_request_context("/?app_id=a1"),
|
||||
@ -111,7 +119,7 @@ class TestInstalledAppsListApi:
|
||||
method = unwrap(api.get)
|
||||
|
||||
session = MagicMock()
|
||||
session.scalars.return_value.all.return_value = [installed_app]
|
||||
session.execute.return_value.all.return_value = [(installed_app, installed_app.app)]
|
||||
|
||||
mock_webapp_setting = MagicMock()
|
||||
mock_webapp_setting.access_mode = "restricted"
|
||||
@ -148,7 +156,7 @@ class TestInstalledAppsListApi:
|
||||
method = unwrap(api.get)
|
||||
|
||||
session = MagicMock()
|
||||
session.scalars.return_value.all.return_value = [installed_app]
|
||||
session.execute.return_value.all.return_value = [(installed_app, installed_app.app)]
|
||||
|
||||
mock_webapp_setting = MagicMock()
|
||||
mock_webapp_setting.access_mode = "restricted"
|
||||
@ -185,7 +193,7 @@ class TestInstalledAppsListApi:
|
||||
method = unwrap(api.get)
|
||||
|
||||
session = MagicMock()
|
||||
session.scalars.return_value.all.return_value = [installed_app]
|
||||
session.execute.return_value.all.return_value = [(installed_app, installed_app.app)]
|
||||
|
||||
mock_webapp_setting = MagicMock()
|
||||
mock_webapp_setting.access_mode = "sso_verified"
|
||||
@ -214,11 +222,54 @@ class TestInstalledAppsListApi:
|
||||
api = module.InstalledAppsListApi()
|
||||
method = unwrap(api.get)
|
||||
|
||||
installed_app_with_null = MagicMock()
|
||||
installed_app_with_null.app = None
|
||||
session = MagicMock()
|
||||
session.execute.return_value.all.return_value = []
|
||||
|
||||
with (
|
||||
app.test_request_context("/"),
|
||||
patch.object(module.db, "session", session),
|
||||
patch.object(module.TenantService, "get_user_role", return_value="owner"),
|
||||
patch.object(
|
||||
module.FeatureService,
|
||||
"get_system_features",
|
||||
return_value=MagicMock(webapp_auth=MagicMock(enabled=False)),
|
||||
),
|
||||
):
|
||||
result = method(api, tenant_id, current_user)
|
||||
|
||||
assert result["installed_apps"] == []
|
||||
|
||||
def test_get_installed_apps_filters_unpublished_chat_apps(
|
||||
self, app: Flask, current_user: MagicMock, tenant_id: str
|
||||
) -> None:
|
||||
api = module.InstalledAppsListApi()
|
||||
method = unwrap(api.get)
|
||||
|
||||
session = MagicMock()
|
||||
session.scalars.return_value.all.return_value = [installed_app_with_null]
|
||||
session.execute.return_value.all.return_value = []
|
||||
|
||||
with (
|
||||
app.test_request_context("/"),
|
||||
patch.object(module.db, "session", session),
|
||||
patch.object(module.TenantService, "get_user_role", return_value="owner"),
|
||||
patch.object(
|
||||
module.FeatureService,
|
||||
"get_system_features",
|
||||
return_value=MagicMock(webapp_auth=MagicMock(enabled=False)),
|
||||
),
|
||||
):
|
||||
result = method(api, tenant_id, current_user)
|
||||
|
||||
assert result["installed_apps"] == []
|
||||
|
||||
def test_get_installed_apps_filters_unpublished_workflow_apps(
|
||||
self, app: Flask, current_user: MagicMock, tenant_id: str
|
||||
) -> None:
|
||||
api = module.InstalledAppsListApi()
|
||||
method = unwrap(api.get)
|
||||
|
||||
session = MagicMock()
|
||||
session.execute.return_value.all.return_value = []
|
||||
|
||||
with (
|
||||
app.test_request_context("/"),
|
||||
@ -243,7 +294,7 @@ class TestInstalledAppsListApi:
|
||||
current_user.current_tenant = None
|
||||
|
||||
session = MagicMock()
|
||||
session.scalars.return_value.all.return_value = [installed_app]
|
||||
session.execute.return_value.all.return_value = [(installed_app, installed_app.app)]
|
||||
|
||||
with (
|
||||
app.test_request_context("/"),
|
||||
|
||||
Loading…
Reference in New Issue
Block a user