refactor: cleanups

Author: sweb

airflow/opa-auth-manager/airflow-3/opa_auth_manager/opa_fab_auth_manager.py

@@ -103,14 +103,16 @@ def _init_opa_resources(self) -> None:
         """
         Set up the OPA cache and HTTP session.
 
-        Idempotent so it can be called from both ``init`` (FastAPI api-server)
-        and ``init_flask_resources`` (Flask AppBuilder) — only one of the two
-        runs depending on the Airflow component.
+        Called from both ``init`` (FastAPI api-server) and
+        ``init_flask_resources`` (Flask AppBuilder). In Airflow 3 both run
+        during a single api-server startup but on *different*
+        OpaFabAuthManager instances — ``init_appbuilder`` calls
+        ``create_auth_manager()`` again, which constructs a fresh instance.
+        The api-server instance is reachable via
+        ``request.app.state.auth_manager``; the FAB instance is returned by
+        ``get_auth_manager()``. Both need their own cache and session.
         """
 
-        if getattr(self, "opa_cache", None) is not None:
-            return
-
         Stats.incr(METRIC_NAME_OPA_CACHE_LIMIT_REACHED, count=0)
 
         self.opa_cache = Cache(
@@ -590,11 +592,18 @@ def get_authorized_dag_ids(
         # FabAuthManager's implementation consults the user's FAB DB role
         # permissions and bypasses is_authorized_dag entirely, which makes any
         # user without a FAB role (e.g. the default Public role) see an empty
-        # DAG list even when OPA would allow them. Re-implement the
-        # BaseAuthManager default: list all DAG ids and filter them via
-        # is_authorized_dag → OPA.
+        # DAG list even when OPA would allow them. List all DAG ids and filter
+        # them via is_authorized_dag → OPA directly, without going through
+        # filter_authorized_dag_ids — the Fab base class may add a DB-backed
+        # override of that method in the future.
         dag_ids = {dag.dag_id for dag in session.execute(select(DagModel.dag_id))}
-        return self.filter_authorized_dag_ids(dag_ids=dag_ids, method=method, user=user)
+        return {
+            dag_id
+            for dag_id in dag_ids
+            if self.is_authorized_dag(
+                method=method, details=DagDetails(id=dag_id), user=user
+            )
+        }
 
     @override
     def filter_authorized_menu_items(

airflow/opa-auth-manager/airflow-3/tests/test_opa_fab_auth_manager.py

@@ -389,6 +389,23 @@ def test_filter_authorized_menu_items_routes_through_opa(
         )
         assert denied == []
 
+    def test_filter_authorized_menu_items_denies_unknown(
+        self, auth_manager_with_appbuilder, mock_opa
+    ):
+        # A MenuItem value not handled by _is_menu_item_authorized (e.g. one
+        # introduced in a future Airflow version) must fail closed: denied
+        # without consulting OPA, so a new UI surface isn't silently exposed
+        # before the dispatch table is updated.
+        unknown = Mock(spec=MenuItem)
+        unknown.name = "FUTURE_THING"
+
+        result = auth_manager_with_appbuilder.filter_authorized_menu_items(
+            [unknown], user=_make_user()
+        )
+
+        assert result == []
+        assert mock_opa.calls == []
+
     def test_filter_authorized_menu_items_preserves_order_and_filters(
         self, auth_manager_with_appbuilder, mock_opa
     ):