fix(frontend): fix edit/delete permissions for charts, dashboards, and datasets (#18870) (#32981) #32995

fnardin-maystreet · 2025-04-03T09:41:02Z

SUMMARY

In both Dashboards, Datasets, Charts pages the delete and edit buttons are shown on the only condition that the user has can_write permission even if not the owner. Attempting to delete or edit an object results in a Forbidden error.

This PR fixes the button disabled status in the frontend preventing the user from attempting to delete or edit a resource it has no permission to.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

All three pages will show the delete and edit buttons as disabled if the user is not an owner or an admin:

fixes: #18870
fixes: #32981

TESTING INSTRUCTIONS

Create a dashboard, a chart, and a dataset with an user and try to edit / delete them with a different user.

ADDITIONAL INFORMATION

Has associated issue:
- Gamma user can attempt to Delete dashboards #18870
- User can attempt to delete a dataset even if it's not the owner #32981
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

…sets (apache#18870) (apache#32981)

korbit-ai

I've completed my review and didn't find any issues.

Suppressed issues based on your team's Korbit activity

This issue	Is similar to	Because
lines 412:414: The owner check uses Array.map() followed by includes(), which is inefficient for checking existence. It creates a new array unnecessarily.	Inefficient Array Operations	Similar issues were not addressed in the past

When you react to issues (for example, an upvote or downvote) or you fix them, Korbit will tune future reviews based on these signals.

Files scanned

File Path	Reviewed
superset-frontend/src/pages/DashboardList/index.tsx	✅
superset-frontend/src/pages/ChartList/index.tsx	✅
superset-frontend/src/pages/DatasetList/index.tsx	✅

Explore our documentation to understand the languages and file types we support and the files we ignore.

Need a new review? Comment /korbit-review on this PR and I'll review your latest changes.

Korbit Guide: Usage and Customization

Interacting with Korbit

You can manually ask Korbit to review your PR using the /korbit-review command in a comment at the root of your PR.

You can ask Korbit to generate a new PR description using the /korbit-generate-pr-description command in any comment on your PR.

Too many Korbit comments? I can resolve all my comment threads if you use the /korbit-resolve command in any comment on your PR.

On any given comment that Korbit raises on your pull request, you can have a discussion with Korbit by replying to the comment.

Help train Korbit to improve your reviews by giving a 👍 or 👎 on the comments Korbit posts.

Customizing Korbit

Check out our docs on how you can make Korbit work best for you and your team.

Customize Korbit for your organization through the Korbit Console.

Current Korbit Configuration

General Settings

Setting Value

Review Schedule Automatic excluding drafts

Max Issue Count 10

Automatic PR Descriptions ❌

Issue Categories

Category Enabled

Documentation ✅

Logging ✅

Error Handling ✅

Readability ✅

Design ✅

Performance ✅

Security ✅

Functionality ✅

Feedback and Support

Tell us what you think of Korbit

Schedule a call with our team

Email us @ support@korbit.ai

Note

Korbit Pro is free for open source projects 🎉

Looking to add Korbit to your team? Get started with a free 2 week trial here

github-actions

Congrats on making your first PR and thank you for contributing to Superset! 🎉 ❤️

We hope to see you in our Slack community too! Not signed up? Use our Slack App to self-register.

superset-frontend/src/pages/ChartList/index.tsx

sadpandajoe · 2025-04-03T17:32:51Z

This might be a backend problem and not a front end problem. We used to have a can_delete permission but that was collapsed into can_write. CC: @dpgaspar as he might know what is going on.

fnardin-maystreet · 2025-04-04T10:55:16Z

This might be a backend problem and not a front end problem. We used to have a can_delete permission but that was collapsed into can_write. CC: @dpgaspar as he might know what is going on.

@sadpandajoe Permissions are obtained through the api/v1/dashboard/_info?q=(keys:!(permissions)) endpoint and apply generically to all dashboards. A can_delete permission is not sufficient to determine if a user can delete a resource, as that also depends on the owner of that resource.

superset-frontend/src/components/ListView/ActionsBar.tsx

rusackas · 2025-04-04T15:54:14Z

superset-frontend/src/components/Icons/BaseIcon.tsx

  const iconCss = css`
    color: ${iconColor || theme.colors.grayscale.base};
    font-size: ${iconSize
      ? `${theme.typography.sizes[iconSize] || theme.typography.sizes.m}px`
      : '24px'};
  `;
+  const disabledIconCss = css`
+    opacity: 0.5;
+    cursor: default;


This is fine, but we could use not-allowed if we want to be all fancy :)

updated to be fancy :)

superset-frontend/src/components/Icons/BaseIcon.tsx

rusackas · 2025-04-04T16:00:25Z

superset-frontend/src/pages/ChartList/index.tsx

                      placement="bottom"
                    >
                      <span
                        role="button"
                        tabIndex={0}
                        className="action-button"
-                        onClick={confirmDelete}
+                        onClick={allowEdit ? confirmDelete : undefined}


should we use null rather than undefined? ¯\_(ツ)_/¯

the type is actually onClick?: MouseEventHandler<T> | undefined;

fix: enhance edit/delete permissions for charts, dashboards, and data…

edcfab2

…sets (apache#18870) (apache#32981)

pull-request-size bot added the size/M label Apr 3, 2025

dosubot bot added the change:frontend Requires changing the frontend label Apr 3, 2025

fnardin-maystreet mentioned this pull request Apr 3, 2025

User can attempt to delete a dataset even if it's not the owner #32981

Open

3 tasks

korbit-ai bot reviewed Apr 3, 2025

View reviewed changes

github-actions bot reviewed Apr 3, 2025

View reviewed changes

fix css style

a1a9f5b

pull-request-size bot added size/L and removed size/M labels Apr 3, 2025

rusackas reviewed Apr 3, 2025

View reviewed changes

superset-frontend/src/pages/ChartList/index.tsx Outdated Show resolved Hide resolved

rusackas reviewed Apr 3, 2025

View reviewed changes

superset-frontend/src/pages/ChartList/index.tsx Outdated Show resolved Hide resolved

rusackas mentioned this pull request Apr 4, 2025

Without "can write on DashboardPermalinkRestApi", a click on a anchor (tabs or header) redirect to login page #30004

Open

3 tasks

added new disabled prop to BaseIcon component

46fa6e8

fnardin-maystreet commented Apr 4, 2025

View reviewed changes

superset-frontend/src/components/ListView/ActionsBar.tsx Show resolved Hide resolved

target the icon container instead of the svg element

af198f5

rusackas reviewed Apr 4, 2025

View reviewed changes

superset-frontend/src/components/Icons/BaseIcon.tsx Show resolved Hide resolved

rusackas reviewed Apr 4, 2025

View reviewed changes

fnardin-maystreet added 2 commits April 7, 2025 11:19

fix cursor type

62bd4d4

handle undefined owners list

1e14a52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(frontend): fix edit/delete permissions for charts, dashboards, and datasets (#18870) (#32981) #32995

fix(frontend): fix edit/delete permissions for charts, dashboards, and datasets (#18870) (#32981) #32995

fnardin-maystreet commented Apr 3, 2025 •

edited

Loading

korbit-ai bot left a comment •

edited

Loading

github-actions bot left a comment

sadpandajoe commented Apr 3, 2025

fnardin-maystreet commented Apr 4, 2025

rusackas Apr 4, 2025

fnardin-maystreet Apr 7, 2025

rusackas Apr 4, 2025

fnardin-maystreet Apr 7, 2025

Setting	Value
Review Schedule	Automatic excluding drafts
Max Issue Count	10
Automatic PR Descriptions	❌

Category	Enabled
Documentation	✅
Logging	✅
Error Handling	✅
Readability	✅
Design	✅
Performance	✅
Security	✅
Functionality	✅

fix(frontend): fix edit/delete permissions for charts, dashboards, and datasets (#18870) (#32981) #32995

Are you sure you want to change the base?

fix(frontend): fix edit/delete permissions for charts, dashboards, and datasets (#18870) (#32981) #32995

Conversation

fnardin-maystreet commented Apr 3, 2025 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

korbit-ai bot left a comment • edited Loading

Choose a reason for hiding this comment

Interacting with Korbit

Customizing Korbit

Current Korbit Configuration

Feedback and Support

Looking to add Korbit to your team? Get started with a free 2 week trial here

github-actions bot left a comment

Choose a reason for hiding this comment

sadpandajoe commented Apr 3, 2025

fnardin-maystreet commented Apr 4, 2025

rusackas Apr 4, 2025

Choose a reason for hiding this comment

fnardin-maystreet Apr 7, 2025

Choose a reason for hiding this comment

rusackas Apr 4, 2025

Choose a reason for hiding this comment

fnardin-maystreet Apr 7, 2025

Choose a reason for hiding this comment

fnardin-maystreet commented Apr 3, 2025 •

edited

Loading

korbit-ai bot left a comment •

edited

Loading