rawwar commented on code in PR #47532:
URL: https://github.com/apache/airflow/pull/47532#discussion_r1990831152


##########
airflow/api_fastapi/core_api/routes/public/dag_run.py:
##########
@@ -303,18 +327,34 @@ def get_dag_runs(
 
     This endpoint allows specifying `~` as the dag_id to retrieve Dag Runs for 
all DAGs.
     """
+    # if readable_dags_filter.value is None:
+    #     return DAGRunCollectionResponse(dag_runs=[], total_entries=0)
+
     query = select(DagRun)
 
     if dag_id != "~":
         dag: DAG = request.app.state.dag_bag.get_dag(dag_id)
         if not dag:
             raise HTTPException(status.HTTP_404_NOT_FOUND, f"The DAG with 
dag_id: `{dag_id}` was not found")
 
+        # if dag_id not in readable_dags_filter.value:
+        #     raise HTTPException(status.HTTP_403_FORBIDDEN, f"Access to DAG 
with dag_id: `{dag_id}` is forbidden")
+
         query = query.filter(DagRun.dag_id == dag_id)
+    # else:
+    #     query = query.filter(DagRun.dag_id.in_(readable_dags_filter.value))

Review Comment:
   @pierrejeambrun , I thought of two approaches. One in comments, where I 
don't create a separate `ReadableDagRunsFilterDep` , rather reuse 
`ReadableDagsFilterDep`. The other one is to create `ReadableDagRunsFilterDep` 
which is inherited from PermittedDagFilter . Also accepts filter_class as 
below. Pasting changes from security.py for ease of viewing:
   
   ```
   class PermittedDagRunFilter(PermittedDagFilter):
       """A parameter that filters the permitted dag runs for the user."""
   
       def to_orm(self, select: Select) -> Select:
           return select.where(DagRun.dag_id.in_(self.value))
   
   
   def permitted_dag_filter_factory(
       method: ResourceMethod, filter_class=PermittedDagFilter
   ) -> Callable[[Request, BaseUser], PermittedDagFilter]:
       """
       Create a callable for Depends in FastAPI that returns a filter of the 
permitted dags for the user.
       :param method: whether filter readable or writable.
       :return: The callable that can be used as Depends in FastAPI.
       """
       def depends_permitted_dags_filter(
           request: Request,
           user: GetUserDep,
       ) -> PermittedDagFilter:
           auth_manager: BaseAuthManager = request.app.state.auth_manager
           permitted_dags: set[str] = 
auth_manager.get_permitted_dag_ids(user=user, method=method)
           return filter_class(permitted_dags)
   
       return depends_permitted_dags_filter
   
   
   ReadableDagRunsFilterDep = Annotated[
       PermittedDagRunFilter, Depends(permitted_dag_filter_factory("GET", 
PermittedDagRunFilter))
   ]
   EditableDagRunsFilterDep = Annotated[
       PermittedDagRunFilter, Depends(permitted_dag_filter_factory("PUT", 
PermittedDagRunFilter))
   ]
   ```
   
   Which one do you think is good. Maybe if both are not optimal, please 
suggest ideas.



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