fix: revert "fix(embedded): adding logic to check dataset used by filters (#24808) #24892
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SUMMARY
This PR reverts #24808. Regrettably @Vitor-Avila after further review I believe the logic outlined in your fix (per the integration test you added) exposes a security vulnerability, i.e., per this check in your test,
will mean said user will have access to the
test_dataset
datasource irrespective of the context. Thus by granting dashboard access to to a guest user you would be in fact (possibly unknowingly) granting full access to all datasources which are referenced within the native dashboard filters.I think the right—context aware—solution is to change the
raise_for_access
caller to include the dashboard context. Note in #24804 I've refactored the dashboard security logic by foldingraise_for_dashboard_access
intoraise_for_access
so the later is (or can be dashboard aware).The TL;DR is your change flexes to accommodate the need for the dashboard filters to function but is over generous in terms of the context in which the permissions are allowed.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
CI.
ADDITIONAL INFORMATION