[GitHub] [superset] villebro commented on a diff in pull request #23344: feat(embedded): Add 'urlParams' option to pass query parameters to embedded dashboard(for SQL templating)

["villebro (via GitHub)" <gi...@apache.org>]

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: 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