revert #9329

[graceguo-supercat]

CATEGORY

Choose one

• Revert

SUMMARY

Airbnb users can not use chart edit modal because of following error:

We reported this issue last week, but it is still in master branch so we have to revert it again in our release branch.

@dpgaspar If you have suggested fix, I can test it in airbnb environment.

@mistercrunch @robdiciuccio

ADDITIONAL INFORMATION

• Has associated issue: [charts] Refactor API using SIP-35 #9329
• Changes UI
• Requires DB Migration.
• Confirm DB Migration upgrade and downgrade tested.
• Introduces new feature or API
• Removes existing feature or API

REVIEWERS

[suddjian]

Can you point to where this issue was reported, for context?

edit: nevermind I see, the discussion was on the original PR.

[dpgaspar]

Having an hard time to reproduce the issue. Are user's getting this when coming from dashboards? can user's view the charts on the chart list?

Note: HTTP 404, means that the chart does not exist or it's being filtered by the already in-place filter.
Do user's have datasource access or schema access or all datasource access to the datasource related to the chart?

Following logic, can't be sure, are you certain that the revert solves the issue?

[dpgaspar]

I'm able to reproduce it the following way:

• Give users access to dashboards that contain charts they are able to access by having database access permission, and datasource permission.
• Low priv user Favorites the dashboard
• Remove the datasource access permission from the user/role.
• Users are not able to view these charts on the charts list view, but they can viz them on the dashboard
• Users use "Explore Chart" on the dashboard, and then click "Edit properties" and get HTTP 404.

This is caused by user's having access to viz charts by having the database access permission, but the chart filter does not include the database access filter, just schema, datasource or all datasources:

here: https://github.com/apache/incubator-superset/blob/master/superset/charts/filters.py
and here: https://github.com/apache/incubator-superset/blob/master/superset/views/chart/filters.py

Can you confirm this reasoning?

Possible path forward could be to add the database access to this filter, and assume this is the new security filter in-place for charts

Other path: could be the front end disables the edit properties if the backend returns 404

[graceguo-supercat]

The error is from chart view: open edit chart property modal will see the Error message. (edit dashboard property works good no 404).

This error message is showing just for all charts.

@dpgaspar I feel your investigate is right, all users have datasource_access permission, but no schema_access.

[dpgaspar]

I don't mean the dashboard edit, I mean accessing the chart from the dashboard and then access "Edit chart property". Can you confirm?

datasource_access shoud be enough, but to be practical can you confirm the revert solves the issue?

[graceguo-supercat]

@dpgaspar I feel this issue may only happens in airbnb environment. So we will add configuration internally, but keep open source codebase as is.

[dpgaspar]

@graceguo-supercat sounds good, I'll be around for any further help on this you my need. Thks