vincbeck commented on code in PR #61400:
URL: https://github.com/apache/airflow/pull/61400#discussion_r2759747272
##########
airflow-core/src/airflow/api_fastapi/auth/managers/base_auth_manager.py:
##########
@@ -244,7 +244,18 @@ def is_authorized_backfill(
:param method: the method to perform
:param user: the user to performing the action
:param details: optional details about the backfill
+
+
+ .. deprecated:: 3.1.8
+ Use ``is_authorized_dag`` on ``DagAccessEntity.RUN`` instead for a
dag level access control.
"""
+ warnings.warn(
+ "You have a plugin that is using a FAB view or Flask Blueprint,
which was used for the Airflow 2 UI,"
Review Comment:
If we decide to keep it, this message should be updated
##########
airflow-core/src/airflow/api_fastapi/auth/managers/models/resource_details.py:
##########
@@ -48,7 +48,12 @@ class DagDetails:
@dataclass
class BackfillDetails:
- """Represents the details of a backfill."""
+ """
+ Represents the details of a backfill.
+
+ .. deprecated:: 3.1.8
+ Use DagAccessEntity.BACKFILL instead for a dag level access control.
Review Comment:
`DagAccessEntity.BACKFILL` does not exist
```suggestion
Use DagAccessEntity.RUN instead for a dag level access control.
```
##########
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:
Do we need to keep it? I do not think removing it is a breaking change
##########
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:
So I assume you do not want to go with moving the backfill api under dag api
(as a nested api)? That way, we would get the dag id from the path
--
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]
