korbit-ai[bot] commented on code in PR #32289:
URL: https://github.com/apache/superset/pull/32289#discussion_r1959498203
##########
superset/security/manager.py:
##########
@@ -2290,65 +2290,51 @@ def raise_for_access( # noqa: C901
assert datasource
- if not (
- self.can_access_schema(datasource)
- or self.can_access("datasource_access", datasource.perm or "")
- or self.is_owner(datasource)
- or (
- # Grant access to the datasource only if dashboard RBAC is
enabled
- # or the user is an embedded guest user with access to the
dashboard
- # and said datasource is associated with the dashboard
chart in
- # question.
- form_data
- and (dashboard_id := form_data.get("dashboardId"))
- and (
- dashboard_ := self.get_session.query(Dashboard)
- .filter(Dashboard.id == dashboard_id)
- .one_or_none()
- )
- and (
- (is_feature_enabled("DASHBOARD_RBAC") and
dashboard_.roles)
- or (
- is_feature_enabled("EMBEDDED_SUPERSET")
- and self.is_guest_user()
- )
- )
- and (
- (
- # Native filter.
- form_data.get("type") == "NATIVE_FILTER"
- and (native_filter_id :=
form_data.get("native_filter_id"))
- and dashboard_.json_metadata
- and (json_metadata :=
json.loads(dashboard_.json_metadata))
- and any(
+ if datasource or query_context or viz:
+ form_data = None
+
+ if query_context:
+ datasource = query_context.datasource
+ form_data = query_context.form_data
+ elif viz:
+ datasource = viz.datasource
+ form_data = viz.form_data
Review Comment:
### Redundant Data Source Initialization <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The code re-initializes datasource and form_data variables that were already
set outside this block, potentially overwriting valid values.
###### Why this matters
This can lead to data access permission checks being performed with
incorrect or missing datasource/form_data values, potentially granting access
when it should be denied or vice versa.
###### Suggested change ∙ *Feature Preview*
Remove the redundant initialization and checks since they were already
handled before this block:
```python
if not (
can_access_schema
or can_access_datasource_permission
or is_owner
or (dashboard_accessible and self.can_access_dashboard(dashboard_))
):
raise
SupersetSecurityException(self.get_datasource_access_error_object(datasource))
```
</details>
<sub>
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/56881543-9d00-4556-9dcf-495931765480?suggestedFixEnabled=true)
💬 Chat with Korbit by mentioning @korbit-ai.
</sub>
<!--- korbi internal id:a60d86ef-8b88-4966-9f06-ff097c0a8e41 -->
--
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: notifications-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
