Pushkal-Gupta opened a new pull request, #74615:
URL: https://github.com/apache/airflow/pull/74615
With `core.multi_team` turned on, `POST /connections/defaults` always fails
with a 400 - even for a perfectly valid call. Three kinds of malformed body
also come back as a 500 instead of a 400. Both have the same cause.
### What's going on
In multi-team mode, authorization needs to know which team a request is
about, so `_collect_teams_to_check` reads `team_name` out of the request body
on POST and PUT. It only catches `JSONDecodeError` around that read.
That looked safe, and #66504 said why when it added the guard: every
POST/PUT route in core_api declares a Pydantic body model, so FastAPI parses
the body and returns a 422 long before the auth dependency ever runs.
One route doesn't declare a body model, though:
```python
def create_default_connections(
session: SessionDep,
):
```
FastAPI only parses a body when the route asks for one. So for
`/connections/defaults`, nothing has touched the body by the time the
dependency runs — that read is the first and only parse. The route has been
around since #45363 (January 2025), sixteen months before the guard landed.
Two things follow, both only when multi-team is on.
**The endpoint stops working.** It takes no body by design, but the guard
insists on a parseable one, so a correct call gets `400 Request body is not
valid JSON` instead of `204`. Switch multi-team off and the same call returns
204, which is why this slipped by.
**Three malformed bodies become 500s.** `json.loads` doesn't only raise
`JSONDecodeError` - it also raises `UnicodeDecodeError`, a plain `ValueError`
when a number exceeds `int_max_str_digits`, and `RecursionError` on deeply
nested input. None of those were caught.
| `POST /connections/defaults` | Before | After |
|---|---|---|
| no body (a valid call) | 400 | 204 |
| invalid UTF-8 bytes | 500 | 400 |
| number over `int_max_str_digits` | 500 | 400 |
| deeply nested JSON | 500 | 400 |
| malformed JSON | 400 | 400 |
### How much is affected
Only this one route. Every other POST/PUT route behind
`requires_access_pool` / `requires_access_connection` /
`requires_access_variable` declares a body model, so FastAPI gets there first
and answers 422 or 400 before the dependency runs. I fired all five payloads at
both kinds of route to check.
### The fix
Two small changes: skip the read when the body is empty, since a request
with no body isn't naming a team anyway, and catch any parse failure rather
than just `JSONDecodeError`.
Neither idea is new here. `requires_access_backfill`, a few hundred lines up
in the same file, already does exactly this and even names the three extra
exception types in its comment. `parse_login_body` in the simple auth manager
does the same. `_collect_teams_to_check` just never caught up.
Fail-closed behaviour is unchanged. A non-empty body that can't be read is
still rejected with a 400 before any authorization decision. An empty body
reaches the authorization check with `team_name=None` - the same thing a GET
presents - and routes that genuinely need a body still get FastAPI's 422,
exactly as before. Nothing becomes authorized that wasn't already.
Worth saying plainly: both broken cases *denied* the request, so there was
no authorization bypass here. It's a wrong status code and a leaked traceback,
not a vulnerability, so it's raised as an ordinary bug.
### Tests
Three new ones:
- the three exception types × POST/PUT, checking we return 400 and never
reach the authorization callback
- an empty body, checking we don't try to parse it and that authorization
runs with `team_name=None`
- a route-level test that `POST /connections/defaults` returns 204 in
multi-team mode - the actual reported symptom
All eight cases fail without the change to `security.py`; I checked by
stashing just that file and re-running. The fourteen existing
`_collect_teams_to_check` tests mocked `request.json` but not `request.body`,
so they now mock both and
still assert what they always did.
mypy and ruff are clean, and the full `airflow-core/tests/unit/api_fastapi/`
suite passes at 4395.
No newsfragment — this is a bug fix, not a breaking or user-facing change.
One thing for a committer to weigh in on: the alternative would be to stop
routing `POST /connections/defaults` through team scoping altogether, since
creating default connections has no team dimension. I fixed the helper instead
so it holds up for any future bodyless route, but I'm happy to go the other way
if you'd prefer.
This affects `v3-2-test` too, where #66504 was backported as #67182.
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes — Claude Code (Opus 5)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
