You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "nicolasge (via GitHub)" <gi...@apache.org> on 2024/10/09 03:43:31 UTC
[I] Dataset DEL/POST API should also check the access_control at DAG level if defined [airflow]
nicolasge opened a new issue, #42846:
URL: https://github.com/apache/airflow/issues/42846
### Apache Airflow version
2.10.2
### If "Other Airflow 2 version" selected, which one?
_No response_
### What happened?
Let's say we have a DAG called DAG_A with several tasks and one of the task will trigger a dataset update.
- In the same DAG, we have added an additional access_control to achieve DAG level access control and defined only users belong to Role_A can create DAG run on this DAG.
- the downstream DAGs also have DAG level access control defined, let's call it Role_B and Role_C
Right now, one user with a role which "can create on Datasets" will have the permission to trigger an event for this dataset, even this user doesn't have any role with dag run permissions to the DAG_A or DAG_A's downstream DAGs
### What you think should happen instead?
To support DAG level access control, in order to trigger a dataset update event, besides the "can create on Datasets" permission, the user should also:
- As the upstream or datasets generator, user should have permission to create dag_run on the DAGs which actually generate the dataset event if no human intervention. Because upstream DAG owners can always re-run their DAG to create a new event
- As the downstream of the dataset, user will need to have permission to create dag_run on ALL the downtreams DAGs.
So in this case, in order to call the API to create a dataset event, beside a role with permission to "can create on Datasets", this user need to be in Role_A( if he/she is the upstream owner), or both Role_B and Role_C (If he/she is the downstream owner )
### How to reproduce
create 3 users with 3 roles:
- User C with role_C: "can create on Datasets"
- User A with role_A: can create dag runs on "DAG_A"
- User B with role_B: can create dag runs on "DAG_B"
Create 2 DAGs with dag level access control defined in DAG:
- DAG_A: only Role_A can create dag runs, and define a dataset outlet
- DAG_B: only Role_B can create dag runs, and schedule based on dataset defined in DAG_A
Then use user C to call the Airflow API
### Operating System
Debian 12
### Versions of Apache Airflow Providers
_No response_
### Deployment
Official Apache Airflow Helm Chart
### Deployment details
_No response_
### Anything else?
_No response_
### Are you willing to submit PR?
- [ ] Yes I am willing to submit a PR!
### Code of Conduct
- [X] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md)
--
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-unsubscribe@airflow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Dataset DEL/POST API should also check the access_control at DAG level if defined [airflow]
Posted by "darshil929 (via GitHub)" <gi...@apache.org>.
darshil929 commented on issue #42846:
URL: https://github.com/apache/airflow/issues/42846#issuecomment-4826940747
I will work on this
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Dataset DEL/POST API should also check the access_control at DAG level if defined [airflow]
Posted by "darshil929 (via GitHub)" <gi...@apache.org>.
darshil929 commented on issue #42846:
URL: https://github.com/apache/airflow/issues/42846#issuecomment-4847121753
Thanks for the detailed explanation, that clears it up. The multi-team framing is the piece I was missing: the consumer-side control exists for team isolation, and triggering via an asset is intentionally gated by "can update the asset" rather than per-Dag access_control. The point that another team may not even know the Dag exists makes it obvious why a per-Dag trigger permission wouldn't fit.
Happy to defer to that, I'll close the PR. Thanks both for taking the time to walk through it.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Dataset DEL/POST API should also check the access_control at DAG level if defined [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on issue #42846:
URL: https://github.com/apache/airflow/issues/42846#issuecomment-3489814766
We don’t have a DELETE API in Airflow 3, but yeah I agree POST should check this. PR welcomed.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Dataset DEL/POST API should also check the access_control at DAG level if defined [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on issue #42846:
URL: https://github.com/apache/airflow/issues/42846#issuecomment-4846986309
I 100% agree with Vincent. This is as designed. And also the way how Multi-team is designed around that. By default anyone can create any asset and that's fine. And when you create Dag to be triggered by it, be default it is for the whole team. There we introduced "consumer" side control - but only because "Teams" should be isolated, we still allow anyone who can update asset to trigger the Dag in the same team by default. And Dag Author can add optionally "other teams can trigger the Dag" - but also that is not based on "can you trigger Dag" - because practically speaking the other Team does not even **KNOW** that particular Dag exists - so there is even **NO WAY** to give the other team permissions to allow them trigger the Dags from the other team.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Dataset DEL/POST API should also check the access_control at DAG level if defined [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on issue #42846:
URL: https://github.com/apache/airflow/issues/42846#issuecomment-4893272675
Closing the issue as well
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Dataset DEL/POST API should also check the access_control at DAG level if defined [airflow]
Posted by "darshil929 (via GitHub)" <gi...@apache.org>.
darshil929 commented on issue #42846:
URL: https://github.com/apache/airflow/issues/42846#issuecomment-4846799574
Yeah, that's fair, and I think you're right about the consumer side. If you schedule a Dag on an asset, you're basically signing up to be triggered whenever that asset updates, so having the consumer's access_control override that doesn't really make sense.
Where I was coming from is more the producer side. Normally, to emit one of these events you'd have to run a producing Dag, which means you need permission on that Dag. The API lets anyone with the global "create on Assets" permission emit the event directly and skip that, and that's the part that felt off to me.
It did come up earlier in the thread as something POST should check (https://github.com/apache/airflow/issues/42846#issuecomment-3489814766), so there might be a couple of different views on this. Happy to go with whatever you all land on.
If the producer side seems worth tightening, I can trim the PR down to just that check and drop the consumer part. And if you'd rather call the whole thing working as intended, I'm fine closing it out. Just let me know which way you'd prefer.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Dataset DEL/POST API should also check the access_control at DAG level if defined [airflow]
Posted by "pakapoo (via GitHub)" <gi...@apache.org>.
pakapoo commented on issue #42846:
URL: https://github.com/apache/airflow/issues/42846#issuecomment-3714050088
Hi,
I noticed the issue has been open for a while and would like to pick this up! Although I'm a first-time contributor, I've researched the logic and plan to implement the check as suggested:
A user can trigger an Asset event if they are authorized for EITHER:
* Any producer DAG (upstream)
* OR all consumer DAGs (downstream)
I'll start working on this in my Breeze environment and will open a PR for review soon (for Airflow 3). Thanks.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Dataset DEL/POST API should also check the access_control at DAG level if defined [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on issue #42846:
URL: https://github.com/apache/airflow/issues/42846#issuecomment-4846881401
> Where I was coming from is more the producer side. Normally, to emit one of these events you'd have to run a producing Dag, which means you need permission on that Dag. The API lets anyone with the global "create on Assets" permission emit the event directly and skip that, and that's the part that felt off to me.
To me that's intended, both should be decoupled. By giving permissions to someone to update an asset, you indirectly allow this same persona to trigger indirectly any Dag that are scheduled bas on that asset. At least that's my understanding, very happy to hear others opinion :)
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Dataset DEL/POST API should also check the access_control at DAG level if defined [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck closed issue #42846: Dataset DEL/POST API should also check the access_control at DAG level if defined
URL: https://github.com/apache/airflow/issues/42846
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Dataset DEL/POST API should also check the access_control at DAG level if defined [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on issue #42846:
URL: https://github.com/apache/airflow/issues/42846#issuecomment-4844440612
I disagree, by configuring your Dag to be scheduled on asset update, then you indirectly allow anyone having permission to update this asset to trigger this Dag. To be this is not a bug, this is by design. Very happy to achat about it of some people disagree wth my point of view.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org