Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use auth manager is_authorized_ APIs to check user permissions in Rest API #34317

Merged
merged 36 commits into from
Oct 17, 2023

Conversation

vincbeck
Copy link
Contributor

@vincbeck vincbeck commented Sep 12, 2023

As part of #32205.

In #33213, is_authorized_ APIs are introduced. This PR uses these new APIs across Airflow code. I mostly focused on the rest API but I also did it in some other places.


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

@boring-cyborg boring-cyborg bot added area:API Airflow's REST/HTTP API area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues kind:documentation labels Sep 12, 2023
@vincbeck vincbeck added the AIP-56 Extensible user management label Sep 12, 2023
@vincbeck vincbeck changed the title Use auth manager APIs is_authorized_ to check user permissions in Rest API and other places Use auth manager is_authorized_ APIs to check user permissions in Rest API and other places Sep 12, 2023
@vincbeck vincbeck changed the title Use auth manager is_authorized_ APIs to check user permissions in Rest API and other places Use auth manager is_authorized_ APIs to check user permissions in Rest API Sep 12, 2023
@vincbeck vincbeck force-pushed the vincbeck/use_is_authorized branch 3 times, most recently from 7fe1997 to b595387 Compare September 12, 2023 21:42
airflow/www/auth.py Outdated Show resolved Hide resolved
@vincbeck vincbeck force-pushed the vincbeck/use_is_authorized branch 4 times, most recently from 1e1b83d to b6b1619 Compare September 15, 2023 15:50
@vincbeck vincbeck added the use public runners Makes sure that Public runners are used even if commiters creates the PR (useful for testing) label Sep 15, 2023
@vincbeck vincbeck closed this Sep 15, 2023
@vincbeck vincbeck reopened this Sep 15, 2023
@vincbeck vincbeck marked this pull request as draft September 15, 2023 19:18
@vincbeck vincbeck marked this pull request as ready for review September 25, 2023 15:31
DagAccessEntity.DEPENDENCIES: (RESOURCE_DAG_DEPENDENCIES,),
DagAccessEntity.IMPORT_ERRORS: (RESOURCE_IMPORT_ERROR,),
DagAccessEntity.RUN: (RESOURCE_DAG_RUN,),
DagAccessEntity.TASK: (RESOURCE_TASK_INSTANCE,),
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should leave a coomment here - that this is backwards compatibility due to originally wrong/misleading use of TASK_INSTANCE in FAB permissions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds like a good idea :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@potiuk
Copy link
Member

potiuk commented Oct 14, 2023

Re-reviewed, only few comments left. Also possibly #34942 should be merged first.

@vincbeck vincbeck merged commit d72131f into apache:main Oct 17, 2023
42 checks passed
@vincbeck vincbeck deleted the vincbeck/use_is_authorized branch October 17, 2023 16:43
@ephraimbuddy ephraimbuddy added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Oct 27, 2023
@ephraimbuddy ephraimbuddy added this to the Airflow 2.8.0 milestone Oct 27, 2023
ephraimbuddy added a commit to astronomer/airflow that referenced this pull request Nov 1, 2023
This decorator was added in apache#34317, but it seems it's not used anywhere.
Opening this PR to see if I'm missing something
ephraimbuddy added a commit that referenced this pull request Nov 1, 2023
This decorator was added in #34317, but it seems it's not used anywhere.
Opening this PR to see if I'm missing something
romsharon98 pushed a commit to romsharon98/airflow that referenced this pull request Nov 10, 2023
)

This decorator was added in apache#34317, but it seems it's not used anywhere.
Opening this PR to see if I'm missing something
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AIP-56 Extensible user management area:API Airflow's REST/HTTP API area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) kind:documentation use public runners Makes sure that Public runners are used even if commiters creates the PR (useful for testing)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants