From 7af391b482c52461c4794331553be99e56dbf4ce Mon Sep 17 00:00:00 2001 From: hjlarry Date: Wed, 12 Aug 2026 17:03:57 +0800 Subject: [PATCH] fix(api): normalize web app permission dependency errors --- api/controllers/web/app.py | 16 ++++++---- api/extensions/ext_application_services.py | 9 +++++- api/openapi/markdown/web-openapi.md | 1 + .../unit_tests/controllers/web/test_app.py | 29 +++++++++++++++++++ .../test_ext_application_services.py | 17 ++++++++++- .../contracts/generated/api/web/types.gen.ts | 1 + 6 files changed, 66 insertions(+), 7 deletions(-) diff --git a/api/controllers/web/app.py b/api/controllers/web/app.py index 188e331cb0b..d70a3f0621b 100644 --- a/api/controllers/web/app.py +++ b/api/controllers/web/app.py @@ -162,6 +162,7 @@ class AppWebAuthPermission(Resource): 400: "Bad Request", 401: "Unauthorized", 500: "Internal Server Error", + 503: "Web App Access Service Unavailable", } ) @web_ns.response(200, "Success", web_ns.models[BooleanResultResponse.__name__]) @@ -172,7 +173,11 @@ class AppWebAuthPermission(Resource): raise ValueError("appId must be provided") webapp_access = application_services().webapp_access - if not webapp_access.requires_permission_check(app_id): + try: + requires_permission_check = webapp_access.requires_permission_check(app_id) + except WebAppAccessUnavailableError: + raise WebAppAccessServiceUnavailableError() from None + if not requires_permission_check: return dump_response(BooleanResultResponse, {"result": True}) try: @@ -187,7 +192,8 @@ class AppWebAuthPermission(Resource): logger.exception("Unexpected error during auth verification") raise - return dump_response( - BooleanResultResponse, - {"result": webapp_access.is_user_allowed(user_id=str(user_id), app_id=app_id)}, - ) + try: + is_allowed = webapp_access.is_user_allowed(user_id=str(user_id), app_id=app_id) + except WebAppAccessUnavailableError: + raise WebAppAccessServiceUnavailableError() from None + return dump_response(BooleanResultResponse, {"result": is_allowed}) diff --git a/api/extensions/ext_application_services.py b/api/extensions/ext_application_services.py index 37e1192c6be..550d712a480 100644 --- a/api/extensions/ext_application_services.py +++ b/api/extensions/ext_application_services.py @@ -55,6 +55,13 @@ def _get_enterprise_webapp_access_mode(app_id: str) -> WebAppAccessMode: raise WebAppAccessUnavailableError from e +def _is_user_allowed_to_access_webapp(user_id: str, app_id: str) -> bool: + try: + return EnterpriseService.WebAppAuth.is_user_allowed_to_access_webapp(user_id, app_id) + except (EnterpriseServiceError, httpx.RequestError, json.JSONDecodeError, UnicodeDecodeError) as e: + raise WebAppAccessUnavailableError from e + + @dataclass(frozen=True, slots=True) class ApplicationServices: app_definitions: AppDefinitionQueryService @@ -87,7 +94,7 @@ def build_application_services( access=WebAppAccessQueryRepository(session_factory=database_client), webapp_auth_enabled=FeatureService.is_webapp_auth_enabled(), access_mode_for_app=_get_enterprise_webapp_access_mode, - is_user_allowed_for_app=EnterpriseService.WebAppAuth.is_user_allowed_to_access_webapp, + is_user_allowed_for_app=_is_user_allowed_to_access_webapp, ), explore_banner_queries=ExploreBannerQueryService( banners=ExploreBannerQueryRepository(client=database_client), diff --git a/api/openapi/markdown/web-openapi.md b/api/openapi/markdown/web-openapi.md index 0fa5dfe91ca..c353061ed80 100644 --- a/api/openapi/markdown/web-openapi.md +++ b/api/openapi/markdown/web-openapi.md @@ -847,6 +847,7 @@ Check if user has permission to access a web application. | 400 | Bad Request | | | 401 | Unauthorized | | | 500 | Internal Server Error | | +| 503 | Web App Access Service Unavailable | | ### [POST] /workflows/run **Run workflow** diff --git a/api/tests/unit_tests/controllers/web/test_app.py b/api/tests/unit_tests/controllers/web/test_app.py index 2e2f4a35691..696bd6ab70b 100644 --- a/api/tests/unit_tests/controllers/web/test_app.py +++ b/api/tests/unit_tests/controllers/web/test_app.py @@ -227,6 +227,35 @@ class TestAppWebAuthPermission: passport_service.return_value.verify.assert_called_once_with("passport") webapp_access.is_user_allowed.assert_called_once_with(user_id=expected_user_id, app_id="app-1") + @pytest.mark.parametrize("failing_method", ["requires_permission_check", "is_user_allowed"]) + @patch("controllers.web.app.application_services") + def test_maps_access_dependency_failure_to_service_unavailable( + self, application_services: MagicMock, failing_method: str, app: Flask + ) -> None: + webapp_access = MagicMock() + webapp_access.requires_permission_check.return_value = True + if failing_method == "requires_permission_check": + webapp_access.requires_permission_check.side_effect = WebAppAccessUnavailableError() + else: + webapp_access.is_user_allowed.side_effect = WebAppAccessUnavailableError() + application_services.return_value = SimpleNamespace(webapp_access=webapp_access) + + passport_service = MagicMock() + passport_service.return_value.verify.return_value = {"user_id": "user-1"} + with ( + app.test_request_context("/webapp/permission?appId=app-1", headers={"X-App-Code": "code1"}), + patch("controllers.web.app.extract_webapp_passport", return_value="passport"), + patch("controllers.web.app.PassportService", passport_service), + pytest.raises(WebAppAccessServiceUnavailableError) as raised, + ): + AppWebAuthPermission().get() + + assert raised.value.data == { + "code": "web_app_access_unavailable", + "message": "Web app access service is unavailable.", + "status": 503, + } + @patch("controllers.web.app.application_services") def test_private_app_requires_passport(self, application_services: MagicMock, app: Flask) -> None: webapp_access = MagicMock() diff --git a/api/tests/unit_tests/extensions/test_ext_application_services.py b/api/tests/unit_tests/extensions/test_ext_application_services.py index c9f3c7383bc..4dd82939b2b 100644 --- a/api/tests/unit_tests/extensions/test_ext_application_services.py +++ b/api/tests/unit_tests/extensions/test_ext_application_services.py @@ -266,9 +266,10 @@ def test_build_application_services_wires_webapp_permission( return_value=False, ) as is_user_allowed, ): - services = build_application_services( + services = ext_application_services.build_application_services( database_client=sqlite_session_factory, deployment_edition=DeploymentEdition.COMMUNITY, + initialization_password="", redis=MagicMock(spec=RedisClientWrapper), ) requires_permission = services.webapp_access.requires_permission_check("app-1") @@ -279,3 +280,17 @@ def test_build_application_services_wires_webapp_permission( enabled.assert_called_once_with() get_access_mode.assert_called_once_with("app-1") is_user_allowed.assert_called_once_with("user-1", "app-1") + + +def test_webapp_permission_adapter_maps_connection_failure() -> None: + failure = httpx.ConnectError("connection failed") + with ( + patch( + "extensions.ext_application_services.EnterpriseService.WebAppAuth.is_user_allowed_to_access_webapp", + side_effect=failure, + ), + pytest.raises(WebAppAccessUnavailableError) as raised, + ): + ext_application_services._is_user_allowed_to_access_webapp("user-1", "app-1") + + assert raised.value.__cause__ is failure diff --git a/packages/contracts/generated/api/web/types.gen.ts b/packages/contracts/generated/api/web/types.gen.ts index 5c9b6a578e8..990a9187512 100644 --- a/packages/contracts/generated/api/web/types.gen.ts +++ b/packages/contracts/generated/api/web/types.gen.ts @@ -1568,6 +1568,7 @@ export type GetWebappPermissionErrors = { 400: unknown 401: unknown 500: unknown + 503: unknown } export type GetWebappPermissionResponses = {