You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/02/02 00:01:06 UTC

[GitHub] [airflow] jhtimmins commented on a change in pull request #13922: Fix read allows dag trigger bug

jhtimmins commented on a change in pull request #13922:
URL: https://github.com/apache/airflow/pull/13922#discussion_r568226639



##########
File path: airflow/migrations/versions/2c6edca13270_resource_based_permissions.py
##########
@@ -161,7 +161,7 @@
         (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
     ],
     ("Airflow", "can_trigger"): [
-        (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG),
+        (permissions.ACTION_CAN_EDIT, permissions.RESOURCE_DAG),

Review comment:
       This will will help anyone who goes straight from 1.X to 2.0.1, which is probably most of the users.
   
   We shouldn't need a migration, since triggering also requires `Dag Run.can_create`, which is only included in `>= User`. `Dag.can_edit` is also included in `>= User`, so either way, someone with default roles won't be able to trigger dags unless they have `User` or higher. 
   
   This might change the behavior of users with custom roles. If someone has `Viewer` + a custom role with `Dag Run.can_create`, or they manually added `Dag Run.can_create` to `Viewer`, then they will lose access until they add `Dag.can_edit` to the custom role. I suspect this is a small group of users.

##########
File path: airflow/migrations/versions/2c6edca13270_resource_based_permissions.py
##########
@@ -161,7 +161,7 @@
         (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
     ],
     ("Airflow", "can_trigger"): [
-        (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG),
+        (permissions.ACTION_CAN_EDIT, permissions.RESOURCE_DAG),

Review comment:
       @ashb This will will help anyone who goes straight from 1.X to 2.0.1, which is probably most of the users.
   
   We shouldn't need a migration, since triggering also requires `Dag Run.can_create`, which is only included in `>= User`. `Dag.can_edit` is also included in `>= User`, so either way, someone with default roles won't be able to trigger dags unless they have `User` or higher. 
   
   This might change the behavior of users with custom roles. If someone has `Viewer` + a custom role with `Dag Run.can_create`, or they manually added `Dag Run.can_create` to `Viewer`, then they will lose access until they add `Dag.can_edit` to the custom role. I suspect this is a small group of users.




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

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