kaxil commented on code in PR #72909:
URL: https://github.com/apache/airflow/pull/72909#discussion_r4020244469


##########
airflow-core/src/airflow/api_fastapi/core_api/security.py:
##########
@@ -398,6 +438,34 @@ def depends_permitted_dags_filter(
 ]
 
 
+def readable_dag_bundles_filter_factory() -> Callable[[BaseUser, 
BaseAuthManager], PermittedDagBundleFilter]:
+    """
+    Create a callable for Depends in FastAPI that returns the Dag bundle 
filter for the user.
+
+    Dag bundles need their own factory rather than 
``permitted_dag_filter_factory``: besides the
+    readable Dag ids, the filter needs to know whether the user may see 
bundles that hold no
+    registered Dag, which is a separate authorization decision.
+    """
+
+    def depends_readable_dag_bundles_filter(
+        user: GetUserDep,
+        auth_manager: AuthManagerDep,
+    ) -> PermittedDagBundleFilter:
+        return PermittedDagBundleFilter(
+            auth_manager.get_authorized_dag_ids(user=user, method="GET"),
+            include_dagless_bundles=auth_manager.authorize_view(
+                access_view=AccessView.IMPORT_ERRORS_ALL, user=user

Review Comment:
   You are right, and thank you for reproducing both directions. Fixed in 
8e53572.
   
   The Dag-less bundle names are now resolved before pagination, their teams 
read with the batched `DagBundleModel.get_team_names`, and each one authorized 
with `authorize_view(IMPORT_ERRORS_ALL, team_name=...)`. The survivors go into 
the query as an explicit allow-list, so counting and pagination see the same 
set, which is the shape `get_import_errors` already uses for a file with no 
registered Dag.
   
   Both of your cases are now regression tests in 
`TestDaglessBundleTeamScoping`. I checked they actually bite: reverting the 
filter to the single unscoped check fails three of the four, with exactly the 
two failures you named, over-exposing `team_b` and hiding `team_a`.
   
   The extra cost is one `SELECT name FROM dag_bundle WHERE name NOT IN (SELECT 
bundle_name FROM dag)` per request plus the team lookup, which is the trade you 
called for and matches `/importErrors`. It does land on the polling endpoint, 
which is the same surface as your index comment, so the follow-up there covers 
this query too.
   



-- 
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]

Reply via email to