pierrejeambrun commented on code in PR #61400:
URL: https://github.com/apache/airflow/pull/61400#discussion_r2759737351


##########
airflow-core/src/airflow/api_fastapi/core_api/security.py:
##########
@@ -263,17 +270,33 @@ def depends_permitted_dags_filter(
 ]
 
 
-def requires_access_backfill(method: ResourceMethod) -> Callable[[Request, 
BaseUser], None]:
-    def inner(
+def requires_access_backfill(method: ResourceMethod) -> Callable[[Request, 
BaseUser, Session], None]:
+    """Wrap ``requires_access_dag`` and extract the dag_id from the 
backfill_id."""
+
+    async def inner(
         request: Request,
         user: GetUserDep,
+        session: SessionDep,
     ) -> None:
-        backfill_id: NonNegativeInt | None = 
request.path_params.get("backfill_id")
-
-        _requires_access(
-            is_authorized_callback=lambda: 
get_auth_manager().is_authorized_backfill(
-                method=method, details=BackfillDetails(id=backfill_id), 
user=user
-            ),
+        dag_id = None
+
+        # Try to retrieve the dag_id from the backfill_id path param
+        backfill_id = request.path_params.get("backfill_id")
+        if backfill_id is not None:
+            backfill = session.scalars(select(Backfill).where(Backfill.id == 
backfill_id)).one_or_none()
+            dag_id = backfill.dag_id if backfill else None
+
+        # Try to retrieve the dag_id from the request body (POST backfill)

Review Comment:
   Fixes a bug, we should check access control based on the 'dag_id' in the 
payload when we try to create a backfill.



##########
airflow-core/src/airflow/api_fastapi/auth/managers/base_auth_manager.py:
##########
@@ -230,7 +231,6 @@ def is_authorized_dag(
         :param details: optional details about the DAG
         """
 
-    @abstractmethod

Review Comment:
   Remove the abstractmethod and implement a body, so we are not forcing custom 
authmanager to implement this anymore since it is deprecated.
   
   By default will return False and log a deprecation warning.



##########
airflow-core/docs/core-concepts/auth-manager/index.rst:
##########
@@ -136,7 +136,6 @@ These authorization methods are:
   Also, ``is_authorized_dag`` is called for any entity related to Dags (e.g. 
task instances, Dag runs, ...). This information is passed in ``access_entity``.
   Example: ``auth_manager.is_authorized_dag(method="GET", 
access_entity=DagAccessEntity.Run, details=DagDetails(id="dag-1"))`` asks
   whether the user has permission to read the Dag runs of the Dag "dag-1".
-* ``is_authorized_backfill``: Return whether the user is authorized to access 
Airflow backfills. Some details about the backfill can be provided (e.g. the 
backfill ID).

Review Comment:
   Remove the `is_authorized_backfill` from the public interface as this is 
deprecated. 



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