villebro commented on code in PR #23344:
URL: https://github.com/apache/superset/pull/23344#discussion_r1159313784
##########
superset-embedded-sdk/src/index.ts:
##########
@@ -116,6 +117,8 @@ export async function embedDashboard({
+ filterConfigKeys
.map(key => DASHBOARD_UI_FILTER_CONFIG_URL_PARAM_KEY[key] + '=' +
filterConfig[key]).join('&')
: ""
+ let extraUrlParams = new URLSearchParams(dashboardUiConfig?.urlParams ||
window.location.search).toString()
Review Comment:
@jileeon, IIUC it would still make it possible to pass arbitrary query
params from the embedded dashboard to Superset. I think it would be nice to
have a scheme for allowlisting or mapping specific query params, so that admins
have control over which params are allowed.
How would you feel about the following: we add a parameter `allow_embedded`
to the jinja `url_param` function signature that defaults to `False`, so that
the person responsible for the chart/dataset that contains jinja has control of
either allowing or disallowing embedded query params through? Example:
```python
{{
url_param("gender", allow_embedded=True) # Would allow embedded query
params through
}}
```
vs.
```python
{{
url_param("gender") # Would default to not allowing them through
}}
```
This would ensure we don't introduce a breaking security change, and would
open up support for allowing arbitrary url params.
--
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]
