This is an automated email from the ASF dual-hosted git repository.
vincbeck pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new e87c35e344e Fix 400 and 500s on POST /connections/defaults in
multi-team mode (#74615)
e87c35e344e is described below
commit e87c35e344ee4ba6ef41d879d401e2b77d4dcf86
Author: Pushkal Gupta <[email protected]>
AuthorDate: Tue Oct 13 12:30:07 2026 +0530
Fix 400 and 500s on POST /connections/defaults in multi-team mode (#74615)
---
.../src/airflow/api_fastapi/core_api/security.py | 26 +++++---
.../core_api/routes/public/test_connections.py | 8 +++
.../unit/api_fastapi/core_api/test_security.py | 75 ++++++++++++++++++++++
3 files changed, 100 insertions(+), 9 deletions(-)
diff --git a/airflow-core/src/airflow/api_fastapi/core_api/security.py
b/airflow-core/src/airflow/api_fastapi/core_api/security.py
index 52d2be28b97..bf2821e972e 100644
--- a/airflow-core/src/airflow/api_fastapi/core_api/security.py
+++ b/airflow-core/src/airflow/api_fastapi/core_api/security.py
@@ -19,7 +19,6 @@ from __future__ import annotations
import posixpath
from collections.abc import Callable, Coroutine
from contextlib import suppress
-from json import JSONDecodeError
from typing import TYPE_CHECKING, Annotated, Any, cast
from urllib.parse import ParseResult, unquote, urljoin, urlparse
@@ -1326,14 +1325,23 @@ async def _collect_teams_to_check(
if method != "POST":
teams.add(get_existing_team(resource_id) if resource_id else None)
if method in ("POST", "PUT"):
- try:
- body = await request.json()
- except JSONDecodeError:
- # Fail closed: reject unparsable bodies before any authz decision.
- raise HTTPException(
- status_code=status.HTTP_400_BAD_REQUEST,
- detail="Request body is not valid JSON",
- )
+ body: Any = None
+ # ``POST /connections/defaults`` declares no body model, so FastAPI
parses nothing for it
+ # and the read below is its only parse: a bodyless request there names
no team and must
+ # still be served. Every other route arriving here declares a body,
which FastAPI has
+ # already parsed and cached by the time this dependency runs.
+ if await request.body():
+ try:
+ body = await request.json()
+ except Exception as e:
+ # Broad because ``json.loads`` also raises UnicodeDecodeError,
a bare ValueError
+ # (a number over ``int_max_str_digits``) and RecursionError,
none of them
+ # JSONDecodeError. Fail closed: a body that cannot be read may
carry a
+ # ``team_name``, so reject it rather than treat it as naming
no team.
+ raise HTTPException(
+ status_code=status.HTTP_400_BAD_REQUEST,
+ detail="Request body is not valid JSON",
+ ) from e
raw = body.get("team_name") if isinstance(body, dict) else None
if raw is not None and not isinstance(raw, str):
# Fail closed: reject non-string team_name before authz / DB
lookup.
diff --git
a/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_connections.py
b/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_connections.py
index cb94ac41c1e..67c276b7300 100644
---
a/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_connections.py
+++
b/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_connections.py
@@ -1798,6 +1798,14 @@ class
TestCreateDefaultConnections(TestConnectionEndpoint):
assert response.content == b""
_check_last_log(session, dag_id=None,
event="create_default_connections", logical_date=None)
+ @conf_vars({("core", "multi_team"): "True"})
+ def test_should_respond_204_in_multi_team_mode(self, test_client, session):
+ """The route declares no body model, so the team-scoping dependency's
read is the only
+ parse of the body: a correct bodyless request must not be rejected as
unparsable."""
+ response = test_client.post("/connections/defaults")
+ assert response.status_code == 204
+ assert response.content == b""
+
def test_should_respond_401(self, unauthenticated_test_client):
response = unauthenticated_test_client.post("/connections/defaults")
assert response.status_code == 401
diff --git a/airflow-core/tests/unit/api_fastapi/core_api/test_security.py
b/airflow-core/tests/unit/api_fastapi/core_api/test_security.py
index c5ca94e5b26..8f937ca095d 100644
--- a/airflow-core/tests/unit/api_fastapi/core_api/test_security.py
+++ b/airflow-core/tests/unit/api_fastapi/core_api/test_security.py
@@ -894,6 +894,7 @@ class TestFastApiSecurity:
mock_get_name_if_exists.return_value = team_name
fastapi_request = Mock()
fastapi_request.path_params = {"connection_id": "conn_id"}
+ fastapi_request.body = AsyncMock(return_value=b"{}")
fastapi_request.json = AsyncMock(return_value={})
user = Mock()
@@ -920,6 +921,7 @@ class TestFastApiSecurity:
mock_get_name_if_exists.return_value = "team1"
fastapi_request = Mock()
fastapi_request.path_params = {}
+ fastapi_request.body = AsyncMock(return_value=b"{}")
fastapi_request.json = AsyncMock(return_value={"team_name": "team1"})
user = Mock()
@@ -948,6 +950,7 @@ class TestFastApiSecurity:
mock_get_name_if_exists.side_effect = lambda name: name
fastapi_request = Mock()
fastapi_request.path_params = {"connection_id": "conn_id"}
+ fastapi_request.body = AsyncMock(return_value=b"{}")
fastapi_request.json = AsyncMock(return_value={"team_name":
"new-team"})
user = Mock()
@@ -976,6 +979,7 @@ class TestFastApiSecurity:
mock_get_name_if_exists.return_value = None
fastapi_request = Mock()
fastapi_request.path_params = {}
+ fastapi_request.body = AsyncMock(return_value=b"{}")
fastapi_request.json = AsyncMock(return_value={"team_name":
"nonexistent"})
user = Mock()
@@ -1001,6 +1005,7 @@ class TestFastApiSecurity:
mock_get_team_name.return_value = None
fastapi_request = Mock()
fastapi_request.path_params = {"connection_id": "conn_id"}
+ fastapi_request.body = AsyncMock(return_value=b"{}")
fastapi_request.json =
AsyncMock(side_effect=JSONDecodeError("expecting value", "", 0))
user = Mock()
@@ -1011,6 +1016,67 @@ class TestFastApiSecurity:
assert exc_info.value.status_code == 400
auth_manager.is_authorized_connection.assert_not_called()
+ @pytest.mark.db_test
+ @pytest.mark.parametrize("method", ["POST", "PUT"])
+ @pytest.mark.parametrize(
+ "parse_error",
+ [
+ UnicodeDecodeError("utf-8", b"\xff", 0, 1, "invalid start byte"),
+ ValueError("Exceeds the limit (4300 digits) for integer string
conversion"),
+ RecursionError("maximum recursion depth exceeded while decoding a
JSON array"),
+ ],
+ ids=["invalid-utf8", "oversized-int", "deep-nesting"],
+ )
+ @patch.object(Team, "get_name_if_exists")
+ @patch.object(Connection, "get_team_name")
+ @patch("airflow.api_fastapi.core_api.security.get_auth_manager")
+ async def
test_requires_access_connection_body_parse_failure_other_than_jsondecodeerror(
+ self, mock_get_auth_manager, mock_get_team_name,
mock_get_name_if_exists, parse_error, method
+ ):
+ """``json.loads`` raises these three besides JSONDecodeError; each
must fail closed."""
+ auth_manager = Mock()
+ auth_manager.is_authorized_connection.return_value = False
+ mock_get_auth_manager.return_value = auth_manager
+ mock_get_team_name.return_value = None
+ fastapi_request = Mock()
+ fastapi_request.path_params = {"connection_id": "conn_id"}
+ fastapi_request.body = AsyncMock(return_value=b"<unparsable>")
+ fastapi_request.json = AsyncMock(side_effect=parse_error)
+ user = Mock()
+
+ with conf_vars({("core", "multi_team"): "True"}):
+ with pytest.raises(HTTPException) as exc_info:
+ await requires_access_connection(method)(fastapi_request, user)
+
+ assert exc_info.value.status_code == 400
+ auth_manager.is_authorized_connection.assert_not_called()
+
+ @pytest.mark.db_test
+ @patch.object(Team, "get_name_if_exists")
+ @patch.object(Connection, "get_team_name")
+ @patch("airflow.api_fastapi.core_api.security.get_auth_manager")
+ async def test_requires_access_connection_empty_body_names_no_team(
+ self, mock_get_auth_manager, mock_get_team_name,
mock_get_name_if_exists
+ ):
+ """``POST /connections/defaults`` declares no body model, so a
bodyless request reaches
+ here unparsed: it names no team and must be authorized, not rejected
as unparsable."""
+ auth_manager = Mock()
+ auth_manager.is_authorized_connection.return_value = True
+ mock_get_auth_manager.return_value = auth_manager
+ mock_get_team_name.return_value = None
+ fastapi_request = Mock()
+ fastapi_request.path_params = {}
+ fastapi_request.body = AsyncMock(return_value=b"")
+ fastapi_request.json = AsyncMock(return_value={})
+ user = Mock()
+
+ with conf_vars({("core", "multi_team"): "True"}):
+ await requires_access_connection("POST")(fastapi_request, user)
+
+ fastapi_request.json.assert_not_called()
+ auth_manager.is_authorized_connection.assert_called_once()
+ assert
auth_manager.is_authorized_connection.call_args.kwargs["details"].team_name is
None
+
@pytest.mark.db_test
@pytest.mark.parametrize("method", ["POST", "PUT"])
@pytest.mark.parametrize("bad_team_name", [123, ["x"], {"name": "x"},
True])
@@ -1032,6 +1098,7 @@ class TestFastApiSecurity:
mock_get_team_name.return_value = None
fastapi_request = Mock()
fastapi_request.path_params = {"connection_id": "conn_id"}
+ fastapi_request.body = AsyncMock(return_value=b"{}")
fastapi_request.json = AsyncMock(return_value={"team_name":
bad_team_name})
user = Mock()
@@ -1154,6 +1221,7 @@ class TestFastApiSecurity:
mock_get_name_if_exists.return_value = team_name
fastapi_request = Mock()
fastapi_request.path_params = {"variable_key": "var_key"}
+ fastapi_request.body = AsyncMock(return_value=b"{}")
fastapi_request.json = AsyncMock(return_value={})
user = Mock()
@@ -1180,6 +1248,7 @@ class TestFastApiSecurity:
mock_get_name_if_exists.return_value = "team1"
fastapi_request = Mock()
fastapi_request.path_params = {}
+ fastapi_request.body = AsyncMock(return_value=b"{}")
fastapi_request.json = AsyncMock(return_value={"team_name": "team1"})
user = Mock()
@@ -1208,6 +1277,7 @@ class TestFastApiSecurity:
mock_get_name_if_exists.side_effect = lambda name: name
fastapi_request = Mock()
fastapi_request.path_params = {"variable_key": "var_key"}
+ fastapi_request.body = AsyncMock(return_value=b"{}")
fastapi_request.json = AsyncMock(return_value={"team_name":
"new-team"})
user = Mock()
@@ -1236,6 +1306,7 @@ class TestFastApiSecurity:
mock_get_name_if_exists.return_value = None
fastapi_request = Mock()
fastapi_request.path_params = {}
+ fastapi_request.body = AsyncMock(return_value=b"{}")
fastapi_request.json = AsyncMock(return_value={"team_name":
"nonexistent"})
user = Mock()
@@ -1330,6 +1401,7 @@ class TestFastApiSecurity:
mock_get_name_if_exists.return_value = team_name
fastapi_request = Mock()
fastapi_request.path_params = {"pool_name": "pool"}
+ fastapi_request.body = AsyncMock(return_value=b"{}")
fastapi_request.json = AsyncMock(return_value={})
user = Mock()
@@ -1354,6 +1426,7 @@ class TestFastApiSecurity:
mock_get_name_if_exists.return_value = "team1"
fastapi_request = Mock()
fastapi_request.path_params = {}
+ fastapi_request.body = AsyncMock(return_value=b"{}")
fastapi_request.json = AsyncMock(return_value={"team_name": "team1"})
user = Mock()
@@ -1382,6 +1455,7 @@ class TestFastApiSecurity:
mock_get_name_if_exists.side_effect = lambda name: name
fastapi_request = Mock()
fastapi_request.path_params = {"pool_name": "pool"}
+ fastapi_request.body = AsyncMock(return_value=b"{}")
fastapi_request.json = AsyncMock(return_value={"team_name":
"new-team"})
user = Mock()
@@ -1410,6 +1484,7 @@ class TestFastApiSecurity:
mock_get_name_if_exists.return_value = None
fastapi_request = Mock()
fastapi_request.path_params = {}
+ fastapi_request.body = AsyncMock(return_value=b"{}")
fastapi_request.json = AsyncMock(return_value={"team_name":
"nonexistent"})
user = Mock()
