Mock the simple auth manager in permission tests and guard against the base-class patch

.pre-commit-config.yaml

@@ -975,6 +975,14 @@ repos:
         entry: "^\\s*from urllib\\.parse import ((\\|, )(urlparse\\|urlunparse))+$"
         pass_filenames: true
         files: \.py$
+      - id: check-auth-manager-mock-targets
+        name: Patch SimpleAuthManager, not BaseAuthManager, for methods it overrides
+        description: The API test client uses SimpleAuthManager, so patching an overridden BaseAuthManager method has no effect
+        entry: ./scripts/ci/prek/check_auth_manager_mock_targets.py
+        language: python
+        pass_filenames: true
+        files: ^airflow-core/tests/unit/api_fastapi/.*\.py$
+        exclude: ^airflow-core/tests/unit/api_fastapi/auth/managers/simple/
       - id: check-for-inclusive-language
         language: pygrep
         name: Check for language that we do not accept as community

airflow-core/tests/unit/api_fastapi/core_api/routes/ui/test_dependencies.py

@@ -503,7 +503,7 @@ def test_data_dependencies_hides_asset_when_user_has_no_dag_access(self, _, test
         assert response.status_code == 404
 
     @mock.patch(
-        "airflow.api_fastapi.auth.managers.base_auth_manager.BaseAuthManager.get_authorized_assets",
+        "airflow.api_fastapi.auth.managers.simple.simple_auth_manager.SimpleAuthManager.get_authorized_assets",
         autospec=True,
         return_value=set(),
     )
@@ -523,7 +523,7 @@ def test_scheduling_dependencies_hides_assets_the_caller_may_not_read(self, _, t
         assert f"asset:{asset1_id}" not in edge_endpoints
 
     @mock.patch(
-        "airflow.api_fastapi.auth.managers.base_auth_manager.BaseAuthManager.get_authorized_assets",
+        "airflow.api_fastapi.auth.managers.simple.simple_auth_manager.SimpleAuthManager.get_authorized_assets",
         autospec=True,
     )
     def test_scheduling_dependencies_hides_unreadable_gate_leaf(
@@ -554,7 +554,7 @@ def test_scheduling_dependencies_hides_unreadable_gate_leaf(
         assert (f"asset:{asset_b_id}", gate_node["id"]) not in edge_tuples
 
     @mock.patch(
-        "airflow.api_fastapi.auth.managers.base_auth_manager.BaseAuthManager.get_authorized_assets",
+        "airflow.api_fastapi.auth.managers.simple.simple_auth_manager.SimpleAuthManager.get_authorized_assets",
         autospec=True,
         return_value=set(),
     )
@@ -595,7 +595,7 @@ def test_scheduling_dependencies_hides_gate_leaf_without_id(self, dag_maker, tes
         assert {"gate_asset_a", "gate_asset_b"}.isdisjoint(node["label"] for node in result["nodes"])
 
     @mock.patch(
-        "airflow.api_fastapi.auth.managers.base_auth_manager.BaseAuthManager.get_authorized_assets",
+        "airflow.api_fastapi.auth.managers.simple.simple_auth_manager.SimpleAuthManager.get_authorized_assets",
         autospec=True,
         return_value=set(),
     )
@@ -609,7 +609,7 @@ def test_data_dependencies_hides_asset_the_caller_may_not_read(self, _, test_cli
         assert response.status_code == 404
 
     @mock.patch(
-        "airflow.api_fastapi.auth.managers.base_auth_manager.BaseAuthManager.get_authorized_assets",
+        "airflow.api_fastapi.auth.managers.simple.simple_auth_manager.SimpleAuthManager.get_authorized_assets",
         autospec=True,
     )
     def test_data_dependencies_hides_unreadable_sibling_asset(
@@ -642,7 +642,7 @@ def test_data_dependencies_hides_unreadable_sibling_asset(
         assert not any(node_id.startswith("task:gate_upstream_b") for node_id in node_ids)
 
     @mock.patch(
-        "airflow.api_fastapi.auth.managers.base_auth_manager.BaseAuthManager.get_authorized_assets",
+        "airflow.api_fastapi.auth.managers.simple.simple_auth_manager.SimpleAuthManager.get_authorized_assets",
         autospec=True,
     )
     def test_data_dependencies_hides_gate_leaf_without_id(

scripts/ci/prek/check_auth_manager_mock_targets.py

@@ -0,0 +1,88 @@
+#!/usr/bin/env python
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+# /// script
+# requires-python = ">=3.11,<3.12"
+# ///
+"""Check that API tests do not patch ``BaseAuthManager`` methods that ``SimpleAuthManager`` overrides.
+
+The API test client runs with ``SimpleAuthManager``. Patching a ``BaseAuthManager`` method that
+``SimpleAuthManager`` overrides leaves the override in place, so the mock never takes effect and a
+permission test passes or fails for the wrong reason. Patch ``SimpleAuthManager`` instead.
+
+The set of overridden methods is read from the two classes, so it follows the code.
+"""
+
+from __future__ import annotations
+
+import ast
+import re
+import sys
+from pathlib import Path
+
+AIRFLOW_ROOT = Path(__file__).parents[3]
+BASE_AUTH_MANAGER = AIRFLOW_ROOT / "airflow-core/src/airflow/api_fastapi/auth/managers/base_auth_manager.py"
+SIMPLE_AUTH_MANAGER = (
+    AIRFLOW_ROOT / "airflow-core/src/airflow/api_fastapi/auth/managers/simple/simple_auth_manager.py"
+)
+
+PATCH_TARGET = re.compile(r"BaseAuthManager\.(\w+)")
+
+
+def get_methods(path: Path, class_name: str) -> set[str]:
+    tree = ast.parse(path.read_text(), filename=str(path))
+    for node in tree.body:
+        if isinstance(node, ast.ClassDef) and node.name == class_name:
+            return {
+                item.name
+                for item in node.body
+                if isinstance(item, (ast.FunctionDef, ast.AsyncFunctionDef)) and not item.name.startswith("_")
+            }
+    raise ValueError(f"class {class_name} not found in {path}")
+
+
+def get_overridden_methods() -> set[str]:
+    return get_methods(BASE_AUTH_MANAGER, "BaseAuthManager") & get_methods(
+        SIMPLE_AUTH_MANAGER, "SimpleAuthManager"
+    )
+
+
+def find_bad_patch_targets(text: str, overridden: set[str]) -> list[tuple[int, str]]:
+    return [
+        (line_number, match.group(1))
+        for line_number, line in enumerate(text.splitlines(), start=1)
+        for match in PATCH_TARGET.finditer(line)
+        if match.group(1) in overridden
+    ]
+
+
+def main(paths: list[str]) -> int:
+    overridden = get_overridden_methods()
+    errors = []
+    for path in paths:
+        for line_number, method in find_bad_patch_targets(Path(path).read_text(), overridden):
+            errors.append(
+                f"{path}:{line_number}: BaseAuthManager.{method} is overridden by SimpleAuthManager, "
+                f"so patching it has no effect in API tests; patch SimpleAuthManager.{method} instead."
+            )
+    for error in errors:
+        print(error)
+    return 1 if errors else 0
+
+
+if __name__ == "__main__":
+    sys.exit(main(sys.argv[1:]))

scripts/tests/ci/prek/test_check_auth_manager_mock_targets.py

@@ -0,0 +1,88 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+import pytest
+from check_auth_manager_mock_targets import (
+    find_bad_patch_targets,
+    get_methods,
+    get_overridden_methods,
+    main,
+)
+
+BASE = "airflow.api_fastapi.auth.managers.base_auth_manager.BaseAuthManager"
+SIMPLE = "airflow.api_fastapi.auth.managers.simple.simple_auth_manager.SimpleAuthManager"
+
+
+def test_get_methods_skips_private_methods(tmp_path):
+    module = tmp_path / "module.py"
+    module.write_text(
+        "class Manager:\n    def public(self): ...\n    async def run(self): ...\n    def _private(self): ...\n"
+    )
+
+    assert get_methods(module, "Manager") == {"public", "run"}
+
+
+def test_get_methods_raises_for_missing_class(tmp_path):
+    module = tmp_path / "module.py"
+    module.write_text("class Other: ...\n")
+
+    with pytest.raises(ValueError, match="class Manager not found"):
+        get_methods(module, "Manager")
+
+
+def test_overridden_methods_follow_the_real_classes():
+    overridden = get_overridden_methods()
+
+    assert "get_authorized_assets" in overridden
+    assert "get_authorized_dag_ids" not in overridden
+
+
+@pytest.mark.parametrize(
+    ("text", "expected"),
+    [
+        pytest.param(
+            f'@mock.patch("{BASE}.get_authorized_assets")', [(1, "get_authorized_assets")], id="base"
+        ),
+        pytest.param(f'@mock.patch("{SIMPLE}.get_authorized_assets")', [], id="simple"),
+        pytest.param(f'@mock.patch("{BASE}.get_authorized_dag_ids")', [], id="not-overridden"),
+        pytest.param(
+            f'x = 1\n@mock.patch(\n    "{BASE}.is_authorized_dag",\n)',
+            [(3, "is_authorized_dag")],
+            id="multiline",
+        ),
+    ],
+)
+def test_find_bad_patch_targets(text, expected):
+    overridden = {"get_authorized_assets", "is_authorized_dag"}
+
+    assert find_bad_patch_targets(text, overridden) == expected
+
+
+def test_main_reports_bad_target_and_fails(tmp_path, capsys):
+    test_file = tmp_path / "test_routes.py"
+    test_file.write_text(f'@mock.patch("{BASE}.get_authorized_assets")\n')
+
+    assert main([str(test_file)]) == 1
+    assert "patch SimpleAuthManager.get_authorized_assets instead" in capsys.readouterr().out
+
+
+def test_main_passes_good_target(tmp_path):
+    test_file = tmp_path / "test_routes.py"
+    test_file.write_text(f'@mock.patch("{SIMPLE}.get_authorized_assets")\n')
+
+    assert main([str(test_file)]) == 0