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(datasets): security perm simplification #12000

Merged
merged 12 commits into from
Dec 16, 2020

Conversation

dpgaspar
Copy link
Member

@dpgaspar dpgaspar commented Dec 10, 2020

SUMMARY

security permission simplification. Scope Datasets for API and MVC FAB classes.

Existing permissions:

Permission View
can_add TableColumnInlineView
can_delete TableColumnInlineView
can_edit TableColumnInlineView
can_list TableColumnInlineView
can_show TableColumnInlineView
can_add TableModelView
can_delete TableModelView
can_edit TableModelView
can_list TableModelView
can_mulexport TableModelView
muldelete TableModelView
refresh TableModelView
yaml_export TableModelView
can_add SqlMetricInlineView
can_delete SqlMetricInlineView
can_edit SqlMetricInlineView
can_list SqlMetricInlineView
can_show SqlMetricInlineView

Future permissions:

Permission View
can_read Dataset
can_write Dataset

WARNING: Don't merge freely, there are concurrent security converge PR's open, and the parent revision need to be updated once one is merged.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Changes UI
  • Requires DB Migration.
  • Confirm DB Migration upgrade and downgrade tested.
  • Introduces new feature or API
  • Removes existing feature or API

@dpgaspar dpgaspar added the risk:db-migration PRs that require a DB migration label Dec 10, 2020
@codecov-io
Copy link

codecov-io commented Dec 10, 2020

Codecov Report

Merging #12000 (209cb7b) into master (9c8b65d) will decrease coverage by 10.60%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master   #12000       +/-   ##
===========================================
- Coverage   63.72%   53.11%   -10.61%     
===========================================
  Files         958      438      -520     
  Lines       47103    15746    -31357     
  Branches     4609     4077      -532     
===========================================
- Hits        30016     8364    -21652     
+ Misses      16903     7382     -9521     
+ Partials      184        0      -184     
Flag Coverage Δ
cypress 53.11% <0.00%> (?)
javascript ?
python ?

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

Impacted Files Coverage Δ
...ontend/src/views/CRUD/data/dataset/DatasetList.tsx 2.94% <0.00%> (-65.85%) ⬇️
...uperset-frontend/src/dashboard/util/dnd-reorder.js 0.00% <0.00%> (-100.00%) ⬇️
...rset-frontend/src/dashboard/util/getEmptyLayout.js 0.00% <0.00%> (-100.00%) ⬇️
...dashboard/components/resizable/ResizableHandle.jsx 0.00% <0.00%> (-100.00%) ⬇️
.../src/dashboard/util/getFilterScopeFromNodesTree.js 0.00% <0.00%> (-93.48%) ⬇️
...set-frontend/src/views/CRUD/alert/ExecutionLog.tsx 11.76% <0.00%> (-88.24%) ⬇️
...src/dashboard/components/gridComponents/Header.jsx 10.52% <0.00%> (-86.85%) ⬇️
...rc/dashboard/components/gridComponents/Divider.jsx 13.33% <0.00%> (-86.67%) ⬇️
...ontend/src/dashboard/util/getDashboardFilterKey.ts 14.28% <0.00%> (-85.72%) ⬇️
...end/src/SqlLab/components/ExploreResultsButton.jsx 8.00% <0.00%> (-84.00%) ⬇️
... and 865 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9c8b65d...209cb7b. Read the comment docs.

@dpgaspar dpgaspar marked this pull request as ready for review December 11, 2020 11:03
@@ -1086,7 +1100,7 @@ def test_export_dataset_gamma(self):

self.login(username="gamma")
rv = self.client.get(uri)
assert rv.status_code == 401
assert rv.status_code == 404
Copy link
Member Author

Choose a reason for hiding this comment

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

@betodealmeida export and import datasets will not need a separate permission now. import needs can_write on Dataset and export can_read on Dataset

@dpgaspar dpgaspar merged commit 2302adb into apache:master Dec 16, 2020
@dpgaspar dpgaspar deleted the feat/security-converge-datasets branch December 16, 2020 11:49
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.0.0 labels Mar 12, 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 risk:db-migration PRs that require a DB migration size/L 🚢 1.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants