Not possible to disable export to json on charts

[Larissa-Rocha]

I'm not sure if it's a bug, but I'm try to disable the "export to .json" feature on the menu of charts by removing the "can slice json on superset" permission and it keeps enabled.
I noticed that here in the export to json Menu Item there is no disabled tag, so maybe thats the expected behavior after all, if that's the case, how can I prevent the user from exporting json data?

How to reproduce the bug

1. Remove the "can slice json on superset" permission to the Admin role (if you're admin user)
2. Go to any chart and click on the .json button on top right
3. See the json data be generated

Expected results

I expected to disable the button .json, likewise when I remove the "can csv on superset" permission.

Actual results

I'm able to see the json data.

Screenshots

Here I removed "can slice json on superset" and "can csv on superset" permissions, the csv option is disabled but the json option keeps enabledd.

Environment

• browser type and version: Google Chrome v. 99.0.4844.84
• superset version: 1.4.1
• python version: python --version
• node.js version: node -v
• any feature flags active:

Checklist

Make sure to follow these steps before submitting your issue - thank you!

• I have checked the superset logs for python stacktraces and included it here as text if there are any.
• I have reproduced the issue with at least the latest released version of superset.
• I have checked the issue tracker for the same issue and I haven't found one similar.

Additional context

[villebro]

Good catch @Larissa-Rocha! The disabled={!canDownloadCSV} should definitely be added to the JSON export item, too (canDownloadCSV is equivalent to "is allowed to download tabular data"). Would you be able to open a PR fixing that?

[Larissa-Rocha]

@villebro thanks for your reply. Yes, I can open a PR

[villebro]

FYI @Larissa-Rocha, I'm leaving this issue open until the fix PR is merged.

[rusackas]

Hoping we can get this one closed soon... Hopefully @mgorsk1 's PR can get merged soon.

[rusackas]

Trying to rekindle the effort on that PR. The UI of this has changed quite a bit since the issue was opened... maybe that has something to do with the test issues on the PR? ¯\_(ツ)_/¯

[edjannoo]

Regarding this issue, the "Export to Excel" feature is similarly affected.
@villebro commented on April 6th 2022 that:

canDownloadCSV is equivalent to "is allowed to download tabular data"

Based on this I think that the same control should be applied to "Export to Excel".

My organisation would also like to control access to the "Copy to Clipboard" feature in SQL Lab - is that something that could also be handled with the same can csv on Superset permission? It feels like copying to clipboard is similar to downloading tabular data. If this is something that the community would accept, I think we may be able to contribute the change.

[rusackas]

@edjannoo a contribution would be more than welcome on this front! You could either open a PR if you have an approach in mind, or you could open Ideas thread in our Github Discussions area and ping us there, if you want to discuss the approach.

@mistercrunch and @villebro are working on a proposal for a new permissions model that may fit well with more granular UI permissions like these. I'm not sure if you'd want to wait for that, or if a resolution is more urgent. We're happy either way :)

[DharaniK]

@rusackas - Where can we find more details about this new permissions model that @villebro and @mistercrunch are working on?

[rusackas]

It's still a rough draft doc and brainstorm... they'll be writing up a SIP once they reach a design that they think will address all the edge cases and requirements they're aware of.

[rusackas]

I take it back... the SIP just went up here:
#28377

[DharaniK]

Thank you @rusackas !

[edjannoo]

@rusackas Thanks for the reply last week and the link to the SIP. We'd like to resolve the issue sooner than waiting for the SIP to progress to implementation and release. With that in mind I've raised a PR referencing this issue with the changes that I think are needed. Please let me know if there's any feedback/changes needed.

[rusackas]

Understandable, and thanks for the PR! We'll review ASAP!