Skip to content

Conversation

@vincbeck
Copy link
Contributor

@vincbeck vincbeck commented Feb 9, 2024

When requires_access_dag is used with access_entity specified, it can lead to some inconsistencies. requires_access_dag should check whether the user is authorized to read/edit at least one DAG (line 155) only when no access_entity is specified.


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

@vincbeck vincbeck requested a review from jhtimmins as a code owner February 9, 2024 21:43
@boring-cyborg boring-cyborg bot added the area:API Airflow's REST/HTTP API label Feb 9, 2024
@vincbeck vincbeck added this to the Airflow 2.8.2 milestone Feb 9, 2024
Copy link
Member

@hussein-awala hussein-awala left a comment

Choose a reason for hiding this comment

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

I just checked and realized that we don't have unit tests for these generic security helpers, it would be great if we could add some tests to (1) ensure they work as expected and (2) validate a bugfix like this one.

(The tests could be added later in a separate PR)

@vincbeck vincbeck merged commit 2adbe88 into apache:main Feb 9, 2024
@vincbeck vincbeck deleted the vincbeck/perms-dags branch February 9, 2024 22:27
@ephraimbuddy ephraimbuddy added the type:bug-fix Changelog: Bug Fixes label Feb 19, 2024
ephraimbuddy pushed a commit that referenced this pull request Feb 20, 2024
ephraimbuddy pushed a commit that referenced this pull request Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:API Airflow's REST/HTTP API type:bug-fix Changelog: Bug Fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants