You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "imad-hl (via GitHub)" <gi...@apache.org> on 2025/02/17 17:13:49 UTC
[PR] Fix issue 31938 [superset]
imad-hl opened a new pull request, #32289:
URL: https://github.com/apache/superset/pull/32289
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
# fix(security): fix RBAC access check for dashboards to bypass dataset-level checks
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
This PR addresses issue #31938 by fixing the RBAC access checks in the security manager. The changes:
- Ensure that enabling `DASHBOARD_RBAC` and granting a role access to a dashboard will bypass the dataset-level access checks as expected.
- Refactor the logic for clarity and maintainability.
Before this fix, enabling `DASHBOARD_RBAC` did not bypass dataset-level checks when a user had access to a dashboard, resulting in incorrect access control behavior.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
BEFORE : The user, despite having the correct RBAC role, was incorrectly blocked by dataset-level checks.
AFTER: The fix ensures that the userβs RBAC role correctly bypasses dataset-level access checks.
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
1. Enable the `DASHBOARD_RBAC` feature flag.
2. Grant a role access to a dashboard.
3. Verify that the user with the assigned role can access the related datasource or query without being blocked by dataset-level checks.
4. Ensure that the logic works as expected both with and without the `DASHBOARD_RBAC` feature flag enabled.
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [X ] Has associated issue: #31938
- [X ] Required feature flags: 'DASHBOARD_RBAC'
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] 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
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): fix RBAC access check for dashboards to properly bypass dataset-level permission checks [superset]
Posted by "imad-hl (via GitHub)" <gi...@apache.org>.
imad-hl closed pull request #32289: fix(security): fix RBAC access check for dashboards to properly bypass dataset-level permission checks
URL: https://github.com/apache/superset/pull/32289
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): fix RBAC access check for dashboards to properly bypass dataset-level permission checks [superset]
Posted by "tomkins (via GitHub)" <gi...@apache.org>.
tomkins commented on PR #32289:
URL: https://github.com/apache/superset/pull/32289#issuecomment-2698803875
Just tested this against 5.0.0-rc1, and it seems to work. Dashboard RBAC previously came up with all the permission warnings when trying to open one.
Note: just a first time user of Superset, not a review!
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): fix RBAC access check for dashboards to properly bypass dataset-level permission checks [superset]
Posted by "imad-hl (via GitHub)" <gi...@apache.org>.
imad-hl closed pull request #32289: fix(security): fix RBAC access check for dashboards to properly bypass dataset-level permission checks
URL: https://github.com/apache/superset/pull/32289
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): fix RBAC access check for dashboards to properly bypass dataset-level permission checks [superset]
Posted by "korbit-ai[bot] (via GitHub)" <gi...@apache.org>.
korbit-ai[bot] commented on code in PR #32289:
URL: https://github.com/apache/superset/pull/32289#discussion_r1959817729
##########
superset/security/manager.py:
##########
@@ -2289,66 +2289,46 @@ def raise_for_access( # noqa: C901
form_data = viz.form_data
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(
- target.get("datasetId") == datasource.id
- for fltr in json_metadata.get(
- "native_filter_configuration",
- [],
- )
- for target in fltr.get("targets", [])
- if native_filter_id == fltr.get("id")
- )
- )
- or (
- # Chart.
- form_data.get("type") != "NATIVE_FILTER"
- and (slice_id := form_data.get("slice_id"))
- and (
- slc := self.get_session.query(Slice)
- .filter(Slice.id == slice_id)
- .one_or_none()
- )
- and slc in dashboard_.slices
- and slc.datasource == datasource
+
+ can_access_schema = self.can_access_schema(datasource)
+ can_access_datasource_permission = self.can_access("datasource_access", datasource.perm or "")
+ is_owner = self.is_owner(datasource)
+ # 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.
+ dashboard_accessible = False
+ dashboard_ = None
+
+ if form_data and (dashboard_id := form_data.get("dashboardId")):
+ dashboard_ = self.get_session.query(Dashboard).filter(Dashboard.id == dashboard_id).one_or_none()
Review Comment:
### Unclear variable assignment pattern <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The use of walrus operator (:=) combined with underscore suffix naming makes the code less immediately readable.
###### Why this matters
While the walrus operator can be useful, combining it with underscore suffix naming reduces code clarity at first glance.
###### Suggested change β *Feature Preview*
```python
dashboard_id = form_data.get("dashboardId") if form_data else None
if dashboard_id:
dashboard = self.get_session.query(Dashboard).filter(Dashboard.id == dashboard_id).one_or_none()
```
</details>
<sub>
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/f4489a93-a7ba-48c9-aa78-2789f9e0055e?suggestedFixEnabled=true)
π¬ Chat with Korbit by mentioning @korbit-ai.
</sub>
<!--- korbi internal id:eaf7e006-4457-4b18-a6b6-da33e77ff414 -->
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] Fix issue 31938 [superset]
Posted by "korbit-ai[bot] (via GitHub)" <gi...@apache.org>.
korbit-ai[bot] commented on PR #32289:
URL: https://github.com/apache/superset/pull/32289#issuecomment-2663700493
Based on your review schedule, I'll hold off on reviewing this PR until it's marked as ready for review. If you'd like me to take a look now, comment `/korbit-review`.
> Your admin can change your review schedule in the [Korbit Console](https://app.korbit.ai/settings)
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] Fix issue 31938 [superset]
Posted by "imad-hl (via GitHub)" <gi...@apache.org>.
imad-hl commented on PR #32289:
URL: https://github.com/apache/superset/pull/32289#issuecomment-2665259430
/korbit-review
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): fix RBAC access check for dashboards to properly bypass dataset-level permission checks [superset]
Posted by "imad-hl (via GitHub)" <gi...@apache.org>.
imad-hl commented on PR #32289:
URL: https://github.com/apache/superset/pull/32289#issuecomment-2739833577
The issue has been resolved in #32646. Closing this PR.
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): fix RBAC access check for dashboards to properly bypass dataset-level permission checks [superset]
Posted by "imad-hl (via GitHub)" <gi...@apache.org>.
imad-hl commented on PR #32289:
URL: https://github.com/apache/superset/pull/32289#issuecomment-2665813100
/korbit-review
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): fix RBAC access check for dashboards to properly bypass dataset-level permission checks [superset]
Posted by "u35253 (via GitHub)" <gi...@apache.org>.
u35253 commented on PR #32289:
URL: https://github.com/apache/superset/pull/32289#issuecomment-2672431365
Thank you so much for giving DASHBOARD_RBAC this love!
It possible, can this please be tested to see if the change also resolves the issue that "a dataset included in a dataset as a subquery using the built-in `dataset()` Jinja macro does not get granted DASHBOARD_RBAC based accessibility though such access is expected", as commented on previously at?: https://github.com/apache/superset/issues/31938#issuecomment-2635204648
(I looked through the file changes, and was not entirely certain if such DASHBOARD_RBAC-based nested/Jinja-included dataset accessibility is already fixed in this change -- to me, it looked like "maybe?").
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] Fix issue 31938 [superset]
Posted by "korbit-ai[bot] (via GitHub)" <gi...@apache.org>.
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] Fix issue 31938 [superset]
Posted by "korbit-ai[bot] (via GitHub)" <gi...@apache.org>.
korbit-ai[bot] commented on code in PR #32289:
URL: https://github.com/apache/superset/pull/32289#discussion_r1959422900
##########
superset/security/manager.py:
##########
@@ -2290,65 +2290,58 @@
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
+
+ assert datasource
+
+
+ can_access_schema = self.can_access_schema(datasource)
+ can_access_datasource_permission = self.can_access("datasource_access", datasource.perm or "")
+ is_owner = self.is_owner(datasource)
+
+
+ dashboard_accessible = False
+ dashboard_ = None
Review Comment:
### Inconsistent variable grouping spacing <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Double empty lines between related variable declarations creates unnecessary visual separation.
###### Why this matters
Inconsistent spacing makes it harder to visually group related code blocks and understand the logical flow.
###### Suggested change β *Feature Preview*
Use single empty lines to separate related variable groups:
```python
can_access_schema = self.can_access_schema(datasource)
can_access_datasource_permission = self.can_access("datasource_access", datasource.perm or "")
is_owner = self.is_owner(datasource)
dashboard_accessible = False
dashboard_ = None
```
</details>
<sub>
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/5a0a12cb-96ae-4b27-8d6f-ba9d0d5f7be7?suggestedFixEnabled=true)
π¬ Chat with Korbit by mentioning @korbit-ai.
</sub>
<!--- korbi internal id:a06f8ef4-65cd-4a6c-bd74-3da474315b90 -->
##########
superset/security/manager.py:
##########
@@ -2290,65 +2290,58 @@
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
+
+ assert datasource
+
+
+ can_access_schema = self.can_access_schema(datasource)
+ can_access_datasource_permission = self.can_access("datasource_access", datasource.perm or "")
+ is_owner = self.is_owner(datasource)
+
+
+ dashboard_accessible = False
+ dashboard_ = None
+
+ if form_data and (dashboard_id := form_data.get("dashboardId")):
+ try:
+ dashboard_ = self.get_session.query(Dashboard).filter(Dashboard.id == dashboard_id).one_or_none()
+ except Exception as e:
+ logging.error(f"Error fetching dashboard {dashboard_id}: {e}")
Review Comment:
### Unsafe Dashboard Fetch Exception Handling <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The exception handling for dashboard fetching logs the error but continues execution, potentially with an invalid dashboard_ value.
###### Why this matters
If there's a database error while fetching the dashboard, the code continues with dashboard_ = None, which could lead to incorrect access control decisions.
###### Suggested change β *Feature Preview*
Add a re-raise after logging to ensure access is denied on database errors:
```python
try:
dashboard_ = self.get_session.query(Dashboard).filter(Dashboard.id == dashboard_id).one_or_none()
except Exception as e:
logging.error(f"Error fetching dashboard {dashboard_id}: {e}")
raise SupersetSecurityException(
self.get_dashboard_access_error_object(None)
)
```
</details>
<sub>
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/03e092e1-8574-48ca-96dc-f6c59b04eadc?suggestedFixEnabled=true)
π¬ Chat with Korbit by mentioning @korbit-ai.
</sub>
<!--- korbi internal id:bebb2931-24cf-4cb4-92d2-09c2f3f3d7f9 -->
##########
superset/security/manager.py:
##########
@@ -2290,65 +2290,58 @@ 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
+
+ assert datasource
+
+
+ can_access_schema = self.can_access_schema(datasource)
+ can_access_datasource_permission = self.can_access("datasource_access", datasource.perm or "")
+ is_owner = self.is_owner(datasource)
+
+
+ dashboard_accessible = False
+ dashboard_ = None
+
+ if form_data and (dashboard_id := form_data.get("dashboardId")):
+ try:
+ dashboard_ = self.get_session.query(Dashboard).filter(Dashboard.id == dashboard_id).one_or_none()
+ except Exception as e:
+ logging.error(f"Error fetching dashboard {dashboard_id}: {e}")
Review Comment:
### Insufficient error logging context <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The log message lacks context about the method and operation being performed. The exception traceback is not captured.
###### Why this matters
Without method context and stack trace, it will be difficult to debug production issues when dashboard fetching fails. The error log may not provide enough information to identify the root cause.
###### Suggested change β *Feature Preview*
```python
except Exception as e:
logging.error(
"Error in raise_for_access when fetching dashboard %s: %s",
dashboard_id,
str(e),
exc_info=True
)
```
</details>
<sub>
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/32b57884-9478-4c2f-9c3b-f064eb87f714?suggestedFixEnabled=true)
π¬ Chat with Korbit by mentioning @korbit-ai.
</sub>
<!--- korbi internal id:613f9558-a275-48a3-a1fc-ef78faff2c60 -->
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): fix RBAC access check for dashboards to properly bypass dataset-level permission checks [superset]
Posted by "imad-hl (via GitHub)" <gi...@apache.org>.
imad-hl closed pull request #32289: fix(security): fix RBAC access check for dashboards to properly bypass dataset-level permission checks
URL: https://github.com/apache/superset/pull/32289
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org