mirror of
https://github.com/langgenius/dify.git
synced 2026-06-17 06:21:07 +08:00
feat(openapi): emit canonical ErrorBody on every /openapi/v1 error path
Install OpenApiErrorFormatter on the openapi blueprint's ExternalApi so
all non-2xx responses from /openapi/v1 carry the canonical ErrorBody shape
(code, message, status, optional details/hint). RFC 8628 device-flow
endpoints are unaffected — their flat {error: ...} shape is passed through
unchanged.
Also: set catch_all_404s=True when a formatter is present so unknown
routes return canonical JSON 404s (not Flask's default HTML 404).
Override _help_on_404 to suppress route suggestions, which would corrupt
the JSON contract and enumerate routes to unauthenticated callers.
Both behaviours are scoped by formatter presence — other blueprints that
construct ExternalApi without error_body_formatter are byte-identical.
Wire-level tests added to TestWireContract (3 tests, 18 total):
- 422 from @accepts validation carries code/status/details
- unknown-route 404 is canonical JSON without route suggestions
- device token POST returns RFC 8628 flat shape untouched by formatter
This commit is contained in:
parent
41f827b609
commit
f5c5dbaed5
@ -1,6 +1,7 @@
|
||||
from flask import Blueprint
|
||||
from flask_restx import Namespace
|
||||
|
||||
from controllers.openapi._errors import OpenApiErrorFormatter
|
||||
from libs.device_flow_security import attach_anti_framing
|
||||
from libs.external_api import ExternalApi
|
||||
|
||||
@ -12,6 +13,7 @@ api = ExternalApi(
|
||||
version="1.0",
|
||||
title="OpenAPI",
|
||||
description="User-scoped programmatic API (bearer auth)",
|
||||
error_body_formatter=OpenApiErrorFormatter(),
|
||||
)
|
||||
|
||||
openapi_ns = Namespace("openapi", description="User-scoped operations", path="/")
|
||||
|
||||
@ -2,7 +2,7 @@ import re
|
||||
from collections.abc import Mapping
|
||||
from typing import Any, Protocol
|
||||
|
||||
from flask import Blueprint, Flask, current_app, got_request_exception
|
||||
from flask import Blueprint, Flask, current_app, got_request_exception, request
|
||||
from flask_restx import Api
|
||||
from werkzeug.exceptions import HTTPException
|
||||
from werkzeug.http import HTTP_STATUS_CODES
|
||||
@ -133,13 +133,43 @@ class ExternalApi(Api):
|
||||
}
|
||||
|
||||
def __init__(self, app: Blueprint | Flask, *args, error_body_formatter: ErrorBodyFormatter | None = None, **kwargs):
|
||||
self._error_body_formatter = error_body_formatter
|
||||
patch_swagger_for_inline_nested_dicts()
|
||||
kwargs.setdefault("authorizations", self._authorizations)
|
||||
kwargs.setdefault("security", "Bearer")
|
||||
kwargs["add_specs"] = dify_config.SWAGGER_UI_ENABLED
|
||||
kwargs["doc"] = dify_config.SWAGGER_UI_PATH if dify_config.SWAGGER_UI_ENABLED else False
|
||||
if error_body_formatter is not None:
|
||||
kwargs.setdefault("catch_all_404s", True)
|
||||
# the overrides below patch private flask-restx methods; fail at
|
||||
# startup (not at the first 404) if an upgrade removes them
|
||||
for private_hook in ("_should_use_fr_error_handler", "_help_on_404"):
|
||||
if not callable(getattr(Api, private_hook, None)):
|
||||
raise RuntimeError(f"flask-restx no longer exposes {private_hook}; update ExternalApi overrides")
|
||||
|
||||
# manual separate call on construction and init_app to ensure configs in kwargs effective
|
||||
super().__init__(app=None, *args, **kwargs)
|
||||
self.init_app(app, **kwargs)
|
||||
register_external_error_handlers(self, body_formatter=error_body_formatter)
|
||||
|
||||
def _should_use_fr_error_handler(self):
|
||||
# catch_all_404s makes flask-restx claim NotFound for ANY app path
|
||||
# (it wraps the app-level handle_exception), so scope the claim to
|
||||
# this blueprint's url prefix; other surfaces keep their own 404s.
|
||||
if self._error_body_formatter is not None and not self._request_under_own_prefix():
|
||||
return False
|
||||
return super()._should_use_fr_error_handler()
|
||||
|
||||
def _request_under_own_prefix(self) -> bool:
|
||||
prefix = self.blueprint.url_prefix if self.blueprint is not None else None
|
||||
if not prefix:
|
||||
return True
|
||||
return request.path == prefix or request.path.startswith(prefix.rstrip("/") + "/")
|
||||
|
||||
def _help_on_404(self, message: str | None = None) -> str | None:
|
||||
# flask-restx appends route suggestions post-handler; with a canonical
|
||||
# formatter installed, that would corrupt the contract and enumerate
|
||||
# routes to unauthenticated callers.
|
||||
if self._error_body_formatter is not None:
|
||||
return message
|
||||
return super()._help_on_404(message)
|
||||
|
||||
@ -1,5 +1,7 @@
|
||||
"""Wire-contract tests for the canonical /openapi/v1 error body."""
|
||||
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
from werkzeug.exceptions import Conflict, NotFound, UnprocessableEntity
|
||||
|
||||
@ -173,3 +175,59 @@ class TestOpenApiErrorFormatter:
|
||||
for e, data, status in cases:
|
||||
wire = fmt.finalize(e, data, status)
|
||||
assert wire["code"] in {c.value for c in OpenApiErrorCode}
|
||||
|
||||
|
||||
class TestWireContract:
|
||||
"""End-to-end: request in, canonical JSON out, through the real openapi blueprint."""
|
||||
|
||||
def test_accepts_422_carries_code_status_details(self, openapi_app, bypass_pipeline):
|
||||
client = openapi_app.test_client()
|
||||
|
||||
resp = client.get("/openapi/v1/apps?page=0")
|
||||
|
||||
assert resp.status_code == 422
|
||||
wire = resp.get_json()
|
||||
ErrorBody.model_validate(wire)
|
||||
assert wire["code"] == "invalid_param"
|
||||
assert wire["status"] == 422
|
||||
assert wire["details"]
|
||||
|
||||
def test_unknown_route_404_is_canonical_without_route_suggestions(self, openapi_app):
|
||||
client = openapi_app.test_client()
|
||||
|
||||
resp = client.get("/openapi/v1/definitely-not-a-route")
|
||||
|
||||
assert resp.status_code == 404
|
||||
wire = resp.get_json()
|
||||
ErrorBody.model_validate(wire)
|
||||
assert wire["code"] == "not_found"
|
||||
assert "did you mean" not in wire["message"].lower()
|
||||
|
||||
def test_404_outside_blueprint_prefix_is_not_claimed(self, openapi_app):
|
||||
# catch_all_404s wraps the app-level exception handler; the prefix
|
||||
# guard must keep non-/openapi/v1 paths on the app's own 404 handling
|
||||
client = openapi_app.test_client()
|
||||
|
||||
resp = client.get("/console/definitely-not-a-route")
|
||||
|
||||
assert resp.status_code == 404
|
||||
# not intercepted → Flask's default HTML 404, not the canonical JSON body
|
||||
assert "application/json" not in (resp.content_type or "")
|
||||
|
||||
@patch("controllers.openapi.oauth_device.DeviceFlowRedis")
|
||||
def test_oauth_device_token_keeps_rfc8628_shape(self, mock_redis_cls, openapi_app):
|
||||
store = MagicMock()
|
||||
mock_redis_cls.return_value = store
|
||||
store.record_poll.return_value = None # not SlowDownDecision.SLOW_DOWN
|
||||
store.load_by_device_code.return_value = None # unknown code → expired_token
|
||||
|
||||
client = openapi_app.test_client()
|
||||
|
||||
resp = client.post(
|
||||
"/openapi/v1/oauth/device/token",
|
||||
json={"client_id": "difyctl", "device_code": "nope"},
|
||||
)
|
||||
|
||||
assert resp.status_code == 400
|
||||
wire = resp.get_json()
|
||||
assert wire == {"error": "expired_token"}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user