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

feat(rbac): add customizable related filters #22526

Merged
merged 6 commits into from
Jan 5, 2023

Conversation

villebro
Copy link
Member

@villebro villebro commented Dec 25, 2022

SUMMARY

This is a continuation of #21515, which made it possible to exclude specific users from dropdown lists, to make it possible to add arbitrary filter conditions to both the user and role dropdowns. The primary reason for this is to be able to restrict the available roles in the Dashboard role dropdown when using DASHBOARD_RBAC, as users may unintentionally give too wide access to dashboards by sharing with a too generic role. Similarly, there may be a need to restrict to whom a user should be able to give ownership of an object.

To demonstrate, by adding the following to superset_config.py, the owners dropdown now only contains the entry admin, and the roles dropdown only contains entries starting with the letter "A":

def role_filter(query):
    from superset import security_manager

    role_model = security_manager.role_model
    filters = [role_model.name.like("A%")]
    return query.filter(or_(*filters))


def user_filter(query):
    from superset import security_manager

    user_model = security_manager.user_model
    filters = [user_model.username == "admin"]
    return query.filter(or_(*filters))


EXTRA_RELATED_QUERY_FILTERS = {
    "role": role_filter,
    "user": user_filter,
}

Now users only displays "Admin":

image

Similarly roles only contains "Admin" and "Alpha":

image

TESTING INSTRUCTIONS

  1. Add the above to superset_config.py
  2. Go to dashboard, chart and dataset CRUD views and ensure that the owners and roles dropdowns only contains the expected values.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@codecov
Copy link

codecov bot commented Dec 25, 2022

Codecov Report

Merging #22526 (512cba6) into master (b6d39d1) will decrease coverage by 1.50%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master   #22526      +/-   ##
==========================================
- Coverage   66.91%   65.41%   -1.51%     
==========================================
  Files        1851     1851              
  Lines       70715    70730      +15     
  Branches     7766     7766              
==========================================
- Hits        47320    46266    -1054     
- Misses      21373    22442    +1069     
  Partials     2022     2022              
Flag Coverage Δ
hive ?
mysql ?
postgres 78.03% <100.00%> (+<0.01%) ⬆️
presto ?
python 78.10% <100.00%> (-3.17%) ⬇️
sqlite 76.50% <100.00%> (+0.01%) ⬆️
unit ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
superset/config.py 91.61% <100.00%> (+0.12%) ⬆️
superset/dashboards/api.py 92.57% <100.00%> (ø)
superset/views/filters.py 100.00% <100.00%> (ø)
superset/db_engines/hive.py 0.00% <0.00%> (-85.19%) ⬇️
...set/advanced_data_type/plugins/internet_address.py 16.32% <0.00%> (-79.60%) ⬇️
superset/utils/pandas_postprocessing/boxplot.py 20.51% <0.00%> (-79.49%) ⬇️
superset/charts/post_processing.py 11.76% <0.00%> (-77.95%) ⬇️
...perset/advanced_data_type/plugins/internet_port.py 18.75% <0.00%> (-77.09%) ⬇️
superset/utils/pandas_postprocessing/rolling.py 21.87% <0.00%> (-68.75%) ⬇️
...perset/utils/pandas_postprocessing/contribution.py 34.61% <0.00%> (-65.39%) ⬇️
... and 63 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@EugeneTorap
Copy link
Contributor

LGTM! Thanks for this PR

@villebro villebro requested a review from dpgaspar December 26, 2022 13:07
@villebro villebro merged commit 037deb9 into apache:master Jan 5, 2023
@villebro villebro deleted the villebro/rbac-roles branch January 5, 2023 14:42
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.1.0 and removed 🚢 2.1.3 labels Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/L 🚢 2.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants