-
Notifications
You must be signed in to change notification settings - Fork 16.4k
Fixes loading UI if user does not have read permissions on plugins #54113
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! It would be nice to add the corresponding tests for the ui/config route.
The existed test also need fix as well, thanks.
airflow-core/tests/unit/api_fastapi/core_api/routes/ui/test_config.py::TestGetConfig
pierrejeambrun
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned here #52408 (review)
The simplest way to handle this is to not bring any change in the backend and simply catch the 403 error in the fetch plugins hook in the front-end and return an empty list there.
So the UI can silently proceed as if the user have 0 plugins without crashing.
This means that only users with |
Plugins can register UI menu item, extra views, extra links, etc... To know what the UI should display (extra buttons, link etc...) we need to read plugins information. Indeed only people with The other alternative would be to expose via another endpoint, or the Not sure what would be the correct path forward as I don't see how we can have access to all that plugin information, without having READ access on plugin. Do you have any suggestions? |
|
Unless there is information in the current plugins endpoint that is sensitive and not needed for rendering the plugin then I don't see the need to separate them out. |
|
I don't think this is how we should do things. Closing for now, feel free to re-open if needed. (A frontend fix will be cleaner) |
Fix: Display plugin menu items for all authenticated users
Changes:
plugins_extra_menu_itemsto theui/configendpoint, which aggregates plugin menu items.public/providersendpoint.Benefits:
ui/configendpoint.closes: #51299