From 2e0059d19555f867cf1efae6c01b1e314d6932f8 Mon Sep 17 00:00:00 2001 From: Pierre Jeambrun Date: Tue, 19 May 2026 22:47:50 +0200 Subject: [PATCH] [v3-2-test] Apply requires_access_event_log to GET /eventLogs list endpoint (#67185) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Apply requires_access_event_log to GET /eventLogs list endpoint * Fail closed on non-integer event_log_id; fix list-endpoint test mock requires_access_event_log silently swallowed ValueError on non-integer event_log_id and fell through to the generic AUDIT_LOG check. Raise HTTPException(400) instead — matches the fail-closed pattern used by requires_access_backfill. Also fix test_requires_access_event_log_no_path_param_uses_generic_check: the test mocked request.path_params = {} but left request.query_params as an auto-created Mock attribute, whose .get("dag_id") returned a Mock (truthy non-None). requires_access_dag then resolved dag_id to that Mock and called is_authorized_dag with the wrong DagDetails. Mock both path_params and query_params as empty dicts. --------- (cherry picked from commit b28681f154f3e7d4f90055e59a640e5a6ffc3615) Co-authored-by: Pierre Jeambrun Co-authored-by: Rahul Vats <43964496+vatsrahul1001@users.noreply.github.com> Co-authored-by: vatsrahul1001 --- .../core_api/routes/public/event_logs.py | 4 +- .../airflow/api_fastapi/core_api/security.py | 14 +++--- .../api_fastapi/core_api/test_security.py | 48 +++++++++++++++++++ 3 files changed, 57 insertions(+), 9 deletions(-) diff --git a/airflow-core/src/airflow/api_fastapi/core_api/routes/public/event_logs.py b/airflow-core/src/airflow/api_fastapi/core_api/routes/public/event_logs.py index c4fb56b5bb52e..5088fb6f6b9c9 100644 --- a/airflow-core/src/airflow/api_fastapi/core_api/routes/public/event_logs.py +++ b/airflow-core/src/airflow/api_fastapi/core_api/routes/public/event_logs.py @@ -46,9 +46,7 @@ ) from airflow.api_fastapi.core_api.openapi.exceptions import create_openapi_http_exception_doc from airflow.api_fastapi.core_api.security import ( - DagAccessEntity, ReadableEventLogsFilterDep, - requires_access_dag, requires_access_event_log, ) from airflow.models import Log @@ -75,7 +73,7 @@ def get_event_log( @event_logs_router.get( "", - dependencies=[Depends(requires_access_dag("GET", DagAccessEntity.AUDIT_LOG))], + dependencies=[Depends(requires_access_event_log("GET"))], ) def get_event_logs( limit: QueryLimit, 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 3ef0df031af2f..39f2b6ec2688e 100644 --- a/airflow-core/src/airflow/api_fastapi/core_api/security.py +++ b/airflow-core/src/airflow/api_fastapi/core_api/security.py @@ -352,12 +352,14 @@ async def inner( dag_id = None event_log_id_raw = request.path_params.get("event_log_id") - try: - event_log_id = int(event_log_id_raw) if event_log_id_raw is not None else None - except ValueError: - event_log_id = None - - if event_log_id is not None: + if event_log_id_raw is not None: + try: + event_log_id = int(event_log_id_raw) + except ValueError: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="'event_log_id' must be an integer", + ) dag_id = session.scalar(select(Log.dag_id).where(Log.id == event_log_id)) requires_access_dag(method, DagAccessEntity.AUDIT_LOG, dag_id)( 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 0d4a0fd4cec74..6a01636eeb982 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 @@ -469,6 +469,54 @@ async def test_requires_access_event_log_row_not_found(self, mock_get_auth_manag ) mock_get_team_name.assert_not_called() + @pytest.mark.db_test + @pytest.mark.parametrize("bad_event_log_id", ["abc", "1.5", "1,2", ""]) + @patch("airflow.api_fastapi.core_api.security.requires_access_dag") + async def test_requires_access_event_log_non_integer_id_returns_400( + self, mock_requires_access_dag, bad_event_log_id + ): + """Non-integer event_log_id in the path must be rejected with 400 before authz.""" + request = Mock() + request.path_params = {"event_log_id": bad_event_log_id} + user = Mock() + session = Mock() + + with pytest.raises(HTTPException) as exc_info: + await requires_access_event_log("GET")(request, user, session) + + assert exc_info.value.status_code == 400 + assert "event_log_id" in exc_info.value.detail + mock_requires_access_dag.assert_not_called() + session.scalar.assert_not_called() + + @pytest.mark.db_test + @patch.object(DagModel, "get_team_name") + @patch("airflow.api_fastapi.core_api.security.get_auth_manager") + async def test_requires_access_event_log_no_path_param_uses_generic_check( + self, mock_get_auth_manager, mock_get_team_name + ): + """When called on the list endpoint (no event_log_id), the generic AUDIT_LOG check applies.""" + auth_manager = Mock() + auth_manager.is_authorized_dag.return_value = True + mock_get_auth_manager.return_value = auth_manager + + session = Mock() + request = Mock() + request.path_params = {} + request.query_params = {} + user = Mock() + + await requires_access_event_log("GET")(request, user, session) + + auth_manager.is_authorized_dag.assert_called_once_with( + method="GET", + access_entity=DagAccessEntity.AUDIT_LOG, + details=DagDetails(id=None, team_name=None), + user=user, + ) + session.scalar.assert_not_called() + mock_get_team_name.assert_not_called() + @pytest.mark.parametrize( ("url", "expected_is_safe"), [