villebro commented on code in PR #23586:
URL: https://github.com/apache/superset/pull/23586#discussion_r1194645454
##########
superset/views/core.py:
##########
@@ -1846,25 +1847,33 @@ def dashboard(
if not dashboard:
abort(404)
- if config["ENABLE_ACCESS_REQUEST"]:
- for datasource in dashboard.datasources:
- datasource = DatasourceDAO.get_datasource(
- datasource_type=DatasourceType(datasource.type),
- datasource_id=datasource.id,
- session=db.session(),
+ has_access_ = False
+ for datasource in dashboard.datasources:
+ datasource = DatasourceDAO.get_datasource(
+ datasource_type=DatasourceType(datasource.type),
+ datasource_id=datasource.id,
+ session=db.session(),
+ )
+ if datasource and security_manager.can_access_datasource(
+ datasource=datasource,
+ ):
+ has_access_ = True
+
+ if has_access_ is False and config["ENABLE_ACCESS_REQUEST"]:
+ flash(
+
__(security_manager.get_datasource_access_error_msg(datasource)),
+ "danger",
)
- if datasource and not security_manager.can_access_datasource(
- datasource=datasource
- ):
- flash(
- __(
-
security_manager.get_datasource_access_error_msg(datasource)
- ),
- "danger",
- )
- return redirect(
-
f"/superset/request_access/?dashboard_id={dashboard.id}"
- )
+ return redirect(
+ f"/superset/request_access/?dashboard_id={dashboard.id}"
+ )
+
+ if has_access_:
Review Comment:
Answering both this and the previous question here:
- The logic for reloading the datasets is to check if the user has access to
at least one dataset on the dashboard. AFAIK, this is how regular dashboard
RBAC (not the feature flag!) works - if you have access to at least one dataset
on the dashboard, you are permitted to view the dashboard. Therefore, we break
after confirming that the user is able to access at least one dataset. If we
want to change this logic we need to also change the base filter that chooses
which dashboards to display on the list view.
- The datasets are reloaded in the block above to check individually if the
user has access to them or not. This check was previously only done if
`ENABLE_ACCESS_REQUEST` was enabled - if not, the user would get an obscure
React error (when linking from the dashboard page) or a while page with some
unformatted text indicating that the user doesn't have access to the dashboard
(direct link). Now the user is redirected to the list view with a toast, like
we do for other similar access denied errors.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
