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

The role role probably shouldn't have menu access to Plugins #39218

Closed
1 of 2 tasks
RocFang opened this issue Apr 24, 2024 · 7 comments · Fixed by #39254
Closed
1 of 2 tasks

The role role probably shouldn't have menu access to Plugins #39218

RocFang opened this issue Apr 24, 2024 · 7 comments · Fixed by #39254
Assignees
Labels

Comments

@RocFang
Copy link

RocFang commented Apr 24, 2024

Apache Airflow version

2.9.0

If "Other Airflow 2 version" selected, which one?

No response

What happened?

The Viewer role does not have access to the Admin menu, which is correct, but it does have menu access to Plugins, which is a submenu of Admin. Is this setting unreasonable?

This is just a small problem because normally because viwer does not have admin's menu access, even if it has Plugins' menu access, it cannot see the Plugins menu on the UI.

But when we want to give it access rights to a specific submenu of the admin menu, we will find that the Plugins menu also appears. This is an unexpected performance.

What you think should happen instead?

The viewer role should not have menu access to Plugins.

How to reproduce

Give the viewer role access rights to the admin menu, and you will find that the plugins menu rights are also available.

Operating System

Red Hat Ennterprise Linux Server 7.9

Versions of Apache Airflow Providers

No response

Deployment

Docker-Compose

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

@RocFang RocFang added area:core kind:bug This is a clearly a bug needs-triage label for new issues that we didn't triage yet labels Apr 24, 2024
Copy link

boring-cyborg bot commented Apr 24, 2024

Thanks for opening your first issue here! Be sure to follow the issue template! If you are willing to raise PR to address this issue please do so, no need to wait for approval.

@raphaelauv
Copy link
Contributor

related : #39135 ?

@RocFang
Copy link
Author

RocFang commented Apr 24, 2024

related : #39135 ?

Should not related with this one.

In https://airflow.apache.org/docs/apache-airflow/2.8.4/security/access-control.html#default-roles , it clearly documented that the viewer have access to menu Plugin:

(permissions.ACTION_CAN_ACCESS_MENU, permissions.RESOURCE_PLUGIN),

Here i use the link of 2.8.4 because this page not available for 2.9.0

@potiuk
Copy link
Member

potiuk commented Apr 25, 2024

Here i use the link of 2.8.4 because this page not available for 2.9.0

The doc links are now a bit wrong but will be fixed in 2.9.1 - and should point to FAB provider where the default roles/permissions are defined. I think whether Viewer should have access to Plugins (read) by default is a good question. I don't think there is any harm in it. We are going to add a bit more documentation on that soon - where we clearly state that that there is NO 100% consistency between "menu" access and "thing" access, and sometimes even some permission when indivudually granted might grant more than is "obvious" and when users decide to deviate from regular defined roles, they shoudl carefully review if the permissions they chose individually are right.

And not all combinations of permissions make sense. For example if we take away "plugin" menu access for a viewer but then someone individually gives the "Admin" right to it - does it make sense to have empty admin menu? No.

And there is no good "logic" we should follow here. The "menu" access is a general access to see menu. So this is not really a "permission" - this is more "visibility". But the "thing the menu points at" is a different permission - and sometimes you should be able to see particular thing when directly directed to it via URL, but still seeing the menu is not obvious. Or the other way.

But yes in this case - plugin menu access for viewer makes no sense at all and we should remove it.

@RocFang
Copy link
Author

RocFang commented Apr 25, 2024

@potiuk Greate expanatation, thank you!

@potiuk potiuk added good first issue and removed needs-triage label for new issues that we didn't triage yet labels Apr 25, 2024
@potiuk
Copy link
Member

potiuk commented Apr 25, 2024

Added a good first issue label if someone would like to tackle this

@csp33
Copy link
Contributor

csp33 commented Apr 25, 2024

I can take it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants