fix: Allow embedded guest user datasource access with dashboard context #25081
Conversation
| "superset.extensions.feature_flag_manager._feature_flags", | ||
| EMBEDDED_SUPERSET=True, | ||
| ) | ||
| @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") | ||
| @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices_class_scope") |
There was a problem hiding this comment.
This change speeds these tests up significantly
| @@ -1653,6 +1653,7 @@ def run_extra_queries(self) -> None: | |||
| query_obj["orderby"] = [(metric, asc)] | |||
| self.get_query_context_factory().create( | |||
| datasource={"id": self.datasource.id, "type": self.datasource.type}, | |||
| form_data=self.form_data, | |||
There was a problem hiding this comment.
This fixes #25006, form data wasn't being passed through to raise_for_access
| }, | ||
| ) | ||
| } | ||
| ) |
There was a problem hiding this comment.
This lgtm. Not sure if @john-bodley or @Vitor-Avila wanted to look before we merge it.
| ) | ||
| session = db.session | ||
| session.add(dataset) | ||
| session.commit() |
There was a problem hiding this comment.
I suspect the commit is unnecessary. This would mean that lines 240 and 241 become.
db.session.rollback()
There was a problem hiding this comment.
Yeah I guess it's unnecessary for these and we could instead just run these tests within an uncommitted transaction. Do you feel that's preferable? Here I pretty much just copied the pattern from existing fixtures I saw which all committed their changes.
|
Thanks for working on this PR! 🙏 |
michael-s-molina
added
the
v3.0
mistercrunch
added
🍒 3.0.0
🍒 3.0.1
🍒 3.0.2
🍒 3.0.3
🍒 3.0.4
🏷️ bot
|
hello, how/when can we get this update in production? |
If you're running any of the versions tagged right above your comment, you should already have it :) |
|
Thanks @rusackas I just upgraded to the latest release, however all my embeded charts stopped working. I am getting this error: 400 Bad Request: The CSRF session token is missing. Any thoughts on this? |
|
Sorry, not sure what the cause of that would be. |
|
okay, I was able to resolve that but now I have a new problem, the error is: Guest user cannot modify chart payload. Do you know how I can downgrade superset from 3.1.1 to 3.1.0? |
|
@talk2morris that's being tracked here, you might want to ask on that thread if it's not already covered: #27177 (comment) |
BACKGROUND
Recently, #24789 changed the way that datasource access is granted, specifically in the case that a user should have access to a datasource because they have access to a dashboard. Before #24789, datasource access was only granted as a result of dashboard access when
DASHBOARD_RBACwas enabled, or when a guest user viewed an embedded dashboard whileEMBEDDED_SUPERSETwas enabled.The PR made this process more secure, but only enabled it for
DASHBOARD_RBAC, not embedded.This PR is a follow-up that enables this for embedded. Without this PR, charts on embedded dashboards cannot load because guest users have no way to get access to the datasources.
Fixes #25006
cc @john-bodley
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
GIFs below are of a simple host app which renders an embedded superset dashboard
Before:
After:
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION