vincbeck commented on code in PR #34317:
URL: https://github.com/apache/airflow/pull/34317#discussion_r1347982536


##########
airflow/www/security_manager.py:
##########
@@ -268,125 +271,53 @@ def get_user_roles(user=None):
             user = g.user
         return user.roles
 
-    def get_readable_dags(self, user) -> Iterable[DagModel]:
-        """Gets the DAGs readable by authenticated user."""
-        warnings.warn(
-            "`get_readable_dags` has been deprecated. Please use 
`get_readable_dag_ids` instead.",
-            RemovedInAirflow3Warning,
-            stacklevel=2,
-        )
-        with warnings.catch_warnings():
-            warnings.simplefilter("ignore", RemovedInAirflow3Warning)
-            return self.get_accessible_dags([permissions.ACTION_CAN_READ], 
user)
-
-    def get_editable_dags(self, user) -> Iterable[DagModel]:
-        """Gets the DAGs editable by authenticated user."""
-        warnings.warn(
-            "`get_editable_dags` has been deprecated. Please use 
`get_editable_dag_ids` instead.",
-            RemovedInAirflow3Warning,
-            stacklevel=2,
-        )
-        with warnings.catch_warnings():
-            warnings.simplefilter("ignore", RemovedInAirflow3Warning)
-            return self.get_accessible_dags([permissions.ACTION_CAN_EDIT], 
user)
-
-    @provide_session
-    def get_accessible_dags(
-        self,
-        user_actions: Container[str] | None,
-        user,
-        session: Session = NEW_SESSION,
-    ) -> Iterable[DagModel]:
-        warnings.warn(
-            "`get_accessible_dags` has been deprecated. Please use 
`get_accessible_dag_ids` instead.",
-            RemovedInAirflow3Warning,
-            stacklevel=3,
-        )
-        dag_ids = self.get_accessible_dag_ids(user, user_actions, session)
-        return 
session.scalars(select(DagModel).where(DagModel.dag_id.in_(dag_ids)))
-
-    def get_readable_dag_ids(self, user) -> set[str]:
+    def get_readable_dag_ids(self, user=None) -> set[str]:
         """Gets the DAG IDs readable by authenticated user."""
-        return self.get_accessible_dag_ids(user, [permissions.ACTION_CAN_READ])
+        return self.get_permitted_dag_ids(methods=["GET"], user=user)
 
-    def get_editable_dag_ids(self, user) -> set[str]:
+    def get_editable_dag_ids(self, user=None) -> set[str]:
         """Gets the DAG IDs editable by authenticated user."""
-        return self.get_accessible_dag_ids(user, [permissions.ACTION_CAN_EDIT])
+        return self.get_permitted_dag_ids(methods=["PUT"], user=user)
 
     @provide_session
-    def get_accessible_dag_ids(
+    def get_permitted_dag_ids(
         self,
-        user,
-        user_actions: Container[str] | None = None,
+        *,
+        methods: Container[ResourceMethod] | None = None,
+        user=None,
         session: Session = NEW_SESSION,
     ) -> set[str]:
         """Generic function to get readable or writable DAGs for user."""
-        if not user_actions:
-            user_actions = [permissions.ACTION_CAN_EDIT, 
permissions.ACTION_CAN_READ]
+        if not methods:
+            methods = ["PUT", "GET"]
 
-        if not get_auth_manager().is_logged_in():
-            roles = user.roles
-        else:
-            if (permissions.ACTION_CAN_EDIT in user_actions and 
self.can_edit_all_dags(user)) or (
-                permissions.ACTION_CAN_READ in user_actions and 
self.can_read_all_dags(user)
-            ):
-                return {dag.dag_id for dag in 
session.execute(select(DagModel.dag_id))}
-            user_query = session.scalar(
-                select(User)
-                .options(
-                    joinedload(User.roles)
-                    .subqueryload(Role.permissions)
-                    .options(joinedload(Permission.action), 
joinedload(Permission.resource))
+        dag_ids = {dag.dag_id for dag in 
session.execute(select(DagModel.dag_id))}
+
+        if ("GET" in methods and 
get_auth_manager().is_authorized_dag(method="GET", user=user)) or (
+            "PUT" in methods and 
get_auth_manager().is_authorized_dag(method="PUT", user=user)
+        ):
+            return dag_ids
+
+        return {
+            dag_id
+            for dag_id in dag_ids
+            if (
+                "GET" in methods
+                and get_auth_manager().is_authorized_dag(
+                    method="GET", details=DagDetails(id=dag_id), user=user
+                )
+            )
+            or (
+                "PUT" in methods
+                and get_auth_manager().is_authorized_dag(
+                    method="PUT", details=DagDetails(id=dag_id), user=user
                 )

Review Comment:
   Agree. I created a local function `_is_permitted_dag_id` which, I hope, 
makes it easier to read :)



-- 
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: commits-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to