Skip to content

Conversation

@vincbeck
Copy link
Contributor

Resolves #53936.

Follow-up of #54197.

The way FabAuthManager checks for Dag permission is partially broken in multiple ways:

  1. To check whether user is authorized to list Dags, we are calling get_authorized_dag_ids (which returns the list of Dags the user is authorized to access) and checking whether this list is not empty. Even though this is not strictly wrong, it is not good either. A more robust solution proposed by @pierrejeambrun (see thread here for more information about the solution but also the reason why it is better) is to check whether the user has either access to all Dags or at least one dag permission with RESOURCE_DAG_PREFIX
  2. I remove RESOURCE_DAG_RUN_PREFIX because I think this is very wrong. The purpose of this constant (value is DAG Run:) is to be able to give access to Dag runs of specific Dags. Example: To give read access to user X of Dag runs of Dag test, I can add to the user's permissions (CAN_READ, DAG Run:test). But this is possible without this constant, using the same example, to grant the user access to Dag test runs, you can give them (CAN_READ, Dag:test) and (CAN_READ, RESOURCE_DAG_RUN).

^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@vincbeck vincbeck force-pushed the vincbeck/fab_list_perms branch from 59bcf28 to e360e2c Compare August 26, 2025 14:30
Copy link
Contributor

@o-nikolas o-nikolas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not my area of expertise, but the code seems reasonable.

@vincbeck
Copy link
Contributor Author

Please do not merge before @pierrejeambrun approval, I'd like to have it before merging :)

Copy link
Member

@pierrejeambrun pierrejeambrun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me, thanks Vincent!

Just one question for my own understanding.

@vincbeck vincbeck merged commit 43c31e2 into apache:main Sep 3, 2025
107 checks passed
@vincbeck vincbeck deleted the vincbeck/fab_list_perms branch September 3, 2025 17:14
RoyLee1224 pushed a commit to RoyLee1224/airflow that referenced this pull request Sep 8, 2025
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Sep 30, 2025
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 1, 2025
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 2, 2025
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 3, 2025
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 4, 2025
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 5, 2025
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 5, 2025
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 7, 2025
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 8, 2025
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 9, 2025
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 10, 2025
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 11, 2025
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 12, 2025
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 14, 2025
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 15, 2025
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 17, 2025
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UI - "Runs" and "Task Instances" submenus in the "Dags" Menu only work with "Can Read on Dags" permission

4 participants