fix: Alpha should not be able to edit datasets that they don't own #19854

hughhhh · 2022-04-26T18:03:04Z

SUMMARY

There is no longer a config value for OLD_API_CHECK_DATASET_OWNERSHIP due to 2.0 release. This is now allowing all users who have write permissions to edit any dataset. Adding this logic will verify if this user is an admin or owner before completing this request.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Screen.Recording.2022-04-26.at.1.01.03.PM.mov

TESTING INSTRUCTIONS

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 · 2022-04-26T20:00:23Z

Codecov Report

Merging #19854 (04925be) into master (e061955) will decrease coverage by 0.15%.
The diff coverage is 71.14%.

❗ Current head 04925be differs from pull request most recent head d33cc8b. Consider uploading reports for the commit d33cc8b to get more accurate results

@@            Coverage Diff             @@
##           master   #19854      +/-   ##
==========================================
- Coverage   66.51%   66.36%   -0.16%     
==========================================
  Files        1690     1714      +24     
  Lines       64614    65031     +417     
  Branches     6655     6717      +62     
==========================================
+ Hits        42978    43157     +179     
- Misses      19936    20167     +231     
- Partials     1700     1707       +7

Flag	Coverage Δ
hive	`?`
mysql	`81.93% <85.52%> (-0.02%)`	⬇️
postgres	`81.98% <85.52%> (-0.02%)`	⬇️
presto	`?`
python	`82.06% <85.52%> (-0.37%)`	⬇️
sqlite	`81.74% <85.52%> (-0.02%)`	⬇️
unit	`?`

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

Impacted Files	Coverage Δ
...-chart-controls/src/sections/advancedAnalytics.tsx	`14.28% <0.00%> (-19.05%)`	⬇️
...rset-ui-chart-controls/src/sections/chartTitle.tsx	`100.00% <ø> (ø)`
...omponents/ColumnConfigControl/ColumnConfigItem.tsx	`0.00% <ø> (ø)`
.../shared-controls/components/RadioButtonControl.tsx	`0.00% <ø> (ø)`
...et-ui-chart-controls/src/shared-controls/index.tsx	`36.19% <ø> (ø)`
...d/packages/superset-ui-chart-controls/src/types.ts	`100.00% <ø> (ø)`
...superset-ui-core/src/query/types/PostProcessing.ts	`100.00% <ø> (ø)`
...legacy-plugin-chart-partition/src/controlPanel.tsx	`33.33% <ø> (ø)`
...gins/legacy-plugin-chart-rose/src/controlPanel.tsx	`100.00% <ø> (ø)`
...gins/legacy-plugin-chart-world-map/src/WorldMap.js	`0.00% <0.00%> (ø)`
... and 150 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 e061955...d33cc8b. Read the comment docs.

ghost · 2022-04-26T20:43:24Z

@hughhhh Looking good! Only thing I'm seeing is the disabled button shouldn't have a hover state (it should remain grey even when the user hovers)

hughhhh · 2022-04-26T22:14:27Z

@jess-dillard

yousoph · 2022-04-27T07:10:23Z

/testenv up

github-actions · 2022-04-27T07:13:01Z

@yousoph Ephemeral environment spinning up at http://34.222.236.159:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

hughhhh · 2022-04-28T00:31:30Z

superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx

@@ -344,6 +365,11 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({
      },
      {
        Cell: ({ row: { original } }: any) => {
+          // Verify owner or isAdmin
+          const allowEdit =
+            original.owners.map((o: any) => o.id).includes(user.userId) ||


add Owners to for loop

yousoph · 2022-04-29T16:45:06Z

/testenv up

github-actions · 2022-04-29T16:49:10Z

@yousoph Ephemeral environment spinning up at http://35.165.228.74:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

yousoph · 2022-04-29T17:48:32Z

Ephemeral looks good to me (and design + QA took a look as well) :)

jinghua-qa · 2022-04-29T17:49:05Z

tested in the ephermal env, primary contributor can not edit/delete the dataset they did not own. LGTM!

github-actions · 2022-04-29T21:21:26Z

Ephemeral environment shutdown and build artifacts deleted.

…pache#19854) * fix api for checking owners * fix styles for disabling * fix styles for disabling * fix lint * fix lint * add owners key * plzz * remove * update test * add tooltip * add type * fix test * fix user reference * lit * fix test * work (cherry picked from commit 8b15b68)

sadpandajoe · 2022-05-02T17:18:20Z

🏷️ preset:2022.17

…pache#19854) * fix api for checking owners * fix styles for disabling * fix styles for disabling * fix lint * fix lint * add owners key * plzz * remove * update test * add tooltip * add type * fix test * fix user reference * lit * fix test * work

…19854) * fix api for checking owners * fix styles for disabling * fix styles for disabling * fix lint * fix lint * add owners key * plzz * remove * update test * add tooltip * add type * fix test * fix user reference * lit * fix test * work (cherry picked from commit 8b15b68)

…pache#19854) * fix api for checking owners * fix styles for disabling * fix styles for disabling * fix lint * fix lint * add owners key * plzz * remove * update test * add tooltip * add type * fix test * fix user reference * lit * fix test * work

fix api for checking owners

ce3cb10

pull-request-size bot added the size/M label Apr 26, 2022

fix styles for disabling

3c500fe

hughhhh changed the title ~~fix: api for checking owners~~ fix: Alpha should not be able to edit datasets that they don't own Apr 26, 2022

hughhhh force-pushed the fix-write-ds-alphs branch 2 times, most recently from 48a4cd1 to c1f4ff1 Compare April 26, 2022 22:14

hughhhh force-pushed the fix-write-ds-alphs branch from c1f4ff1 to eae6a1a Compare April 27, 2022 03:52

pull-request-size bot added size/L and removed size/M labels Apr 27, 2022

hughhhh force-pushed the fix-write-ds-alphs branch 2 times, most recently from 252d70c to 6663bc3 Compare April 27, 2022 03:54

fix styles for disabling

6d5e269

hughhhh force-pushed the fix-write-ds-alphs branch from 6663bc3 to 6d5e269 Compare April 27, 2022 03:59

pull-request-size bot added size/M and removed size/L labels Apr 27, 2022

hughhhh added 3 commits April 27, 2022 19:04

fix lint

8ddacbd

fix lint

158417f

add owners key

2815838

hughhhh force-pushed the fix-write-ds-alphs branch from 930f335 to 2815838 Compare April 28, 2022 00:23

pull-request-size bot added size/L and removed size/M labels Apr 28, 2022

hughhhh commented Apr 28, 2022

View reviewed changes

hughhhh added 2 commits April 28, 2022 01:13

plzz

df81e2b

remove

d5d6719

pull-request-size bot removed the size/L label Apr 28, 2022

fix user reference

c409a23

hughhhh force-pushed the fix-write-ds-alphs branch from 9ee57c1 to c409a23 Compare April 29, 2022 02:26

lit

206e288

hughhhh force-pushed the fix-write-ds-alphs branch from 72a96cf to 206e288 Compare April 29, 2022 18:00

hughhhh added 2 commits April 29, 2022 19:42

fix test

ccf1aca

work

d33cc8b

hughhhh force-pushed the fix-write-ds-alphs branch from 039c751 to d33cc8b Compare April 29, 2022 20:33

hughhhh merged commit 8b15b68 into apache:master Apr 29, 2022

superset-github-bot bot added the preset:2022.17 label May 2, 2022

dpgaspar added v1.5 v2.0 labels May 20, 2022

michael-s-molina removed the v2.0 label Jun 16, 2022

john-bodley mentioned this pull request Jul 6, 2022

fix: Add migration to add created_by_fk as explicit owner for charts and datasets #20617

Merged

9 tasks

michael-s-molina removed the v1.5 label Jan 3, 2023

mistercrunch added 🍒 1.5.1 🍒 1.5.2 🍒 1.5.3 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.0.0 labels Mar 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Alpha should not be able to edit datasets that they don't own #19854

fix: Alpha should not be able to edit datasets that they don't own #19854

hughhhh commented Apr 26, 2022 •

edited

codecov bot commented Apr 26, 2022 •

edited

ghost commented Apr 26, 2022

hughhhh commented Apr 26, 2022 •

edited

yousoph commented Apr 27, 2022

github-actions bot commented Apr 27, 2022

hughhhh Apr 28, 2022

yousoph commented Apr 29, 2022

github-actions bot commented Apr 29, 2022

yousoph commented Apr 29, 2022

jinghua-qa commented Apr 29, 2022

github-actions bot commented Apr 29, 2022

sadpandajoe commented May 2, 2022

fix: Alpha should not be able to edit datasets that they don't own #19854

fix: Alpha should not be able to edit datasets that they don't own #19854

Conversation

hughhhh commented Apr 26, 2022 • edited

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

codecov bot commented Apr 26, 2022 • edited

Codecov Report

ghost commented Apr 26, 2022

hughhhh commented Apr 26, 2022 • edited

yousoph commented Apr 27, 2022

github-actions bot commented Apr 27, 2022

hughhhh Apr 28, 2022

Choose a reason for hiding this comment

yousoph commented Apr 29, 2022

github-actions bot commented Apr 29, 2022

yousoph commented Apr 29, 2022

jinghua-qa commented Apr 29, 2022

github-actions bot commented Apr 29, 2022

sadpandajoe commented May 2, 2022

hughhhh commented Apr 26, 2022 •

edited

codecov bot commented Apr 26, 2022 •

edited

hughhhh commented Apr 26, 2022 •

edited