gabotorresruiz commented on code in PR #43769:
URL: https://github.com/apache/superset/pull/43769#discussion_r4030097327


##########
superset/charts/api.py:
##########
@@ -2016,3 +2040,272 @@ def restore_version(self, uuid_str: str, 
version_uuid_str: str) -> Response:
         return restore_version_endpoint(
             self, Slice, RestoreChartVersionCommand, uuid_str, version_uuid_str
         )
+
+    @expose("/<pk>/embedded", methods=("GET",))
+    @protect()
+    @safe
+    @permission_name("read")
+    @statsd_metrics
+    @event_logger.log_this_with_context(
+        action=lambda self, *args, **kwargs: 
f"{self.__class__.__name__}.get_embedded",
+        log_to_statsd=False,
+    )
+    def get_embedded(self, pk: int) -> Response:
+        """Get the chart's embedded configuration.
+        ---
+        get:
+          summary: Get the chart's embedded configuration
+          parameters:
+          - in: path
+            schema:
+              type: integer
+            name: pk
+            description: The chart id
+          responses:
+            200:
+              description: Result contains the embedded chart config
+              content:
+                application/json:
+                  schema:
+                    type: object
+                    properties:
+                      result:
+                        $ref: 
'#/components/schemas/EmbeddedChartResponseSchema'
+            401:
+              $ref: '#/components/responses/401'
+            404:
+              $ref: '#/components/responses/404'
+            500:
+              $ref: '#/components/responses/500'
+        """
+        chart = ChartDAO.find_by_id(pk)
+        if not chart:
+            return self.response_404()
+        if not chart.embedded:
+            return self.response(404)
+        embedded: EmbeddedChart = chart.embedded[0]
+        result = self.embedded_response_schema.dump(embedded)
+        return self.response(200, result=result)
+
+    @expose("/<pk>/embedded_context", methods=("GET",))
+    @protect()
+    @safe
+    @permission_name("read")
+    @statsd_metrics
+    @event_logger.log_this_with_context(
+        action=lambda self, *args, **kwargs: (
+            f"{self.__class__.__name__}.get_embedded_context"
+        ),
+        log_to_statsd=False,
+    )
+    def get_embedded_context(self, pk: int) -> Response:
+        """Get a chart together with the dataset needed to render it.
+        ---
+        get:
+          summary: Get a chart and its dataset in one payload
+          description: >-
+            The chart analogue of a dashboard's ``/charts`` and ``/datasets``
+            sub-resources, collapsed into one call because a chart has exactly
+            one of each. Sits under the ``Chart`` read permission, so a
+            standalone embedded chart loads with the same grant its guest token
+            already needs to fetch that chart's data.
+          parameters:
+          - in: path
+            schema:
+              type: integer
+            name: pk
+            description: The chart id
+          responses:
+            200:
+              description: The chart and its dataset
+              content:
+                application/json:
+                  schema:
+                    type: object
+                    properties:
+                      result:
+                        type: object
+                        properties:
+                          slice:
+                            $ref: 
'#/components/schemas/ChartEntityResponseSchema'
+                          dataset:
+                            type: object
+            401:
+              $ref: '#/components/responses/401'
+            403:
+              $ref: '#/components/responses/403'
+            404:
+              $ref: '#/components/responses/404'
+            500:
+              $ref: '#/components/responses/500'
+        """
+        # pylint: disable=import-outside-toplevel
+        from superset.dashboards.api import 
DASHBOARD_DATASET_INACCESSIBLE_FIELDS
+
+        # Resolved through the base filters so ChartFilter's scoping -- 
including
+        # its embedded-guest branch -- decides what is visible here.
+        chart = self.datamodel.get(pk, self._base_filters)
+        if not chart:
+            return self.response_404()
+        try:
+            security_manager.raise_for_access(chart=chart)
+        except SupersetSecurityException:
+            return self.response_403()
+        datasource = chart.datasource
+        if datasource is None:
+            return self.response_404()
+
+        dataset = self.dashboard_dataset_schema.dump(datasource.data)
+        # A dashboard narrows member datasets the caller cannot access on their
+        # own, because it returns many datasets of uneven sensitivity. Here 
there
+        # is exactly one and it belongs to the chart the caller was just
+        # authorized on, so a guest holding a token for that chart keeps the
+        # rendering metadata -- columns, metrics, verbose map -- it needs to 
draw
+        # the chart. ``params`` is withheld even then: it is operator-authored
+        # free-form configuration rather than anything the renderer reads.
+        entitled_guest = security_manager.has_guest_access_to_chart(chart)
+        if not (security_manager.can_access_datasource(datasource) or 
entitled_guest):
+            for key in DASHBOARD_DATASET_INACCESSIBLE_FIELDS:
+                dataset.pop(key, None)
+        elif entitled_guest:

Review Comment:
   This block worries me a bit. For a guest holding this chart's token only 
`params` is dropped, so `columns`, `metrics` and `verbose_map` all go out with 
the response. On my instance that guest received every column of the dataset, 
including one the chart never references, plus each metric's SQL `expression`.
   
   The same guest on the dashboard path receives none of those three: 
`_serialize_dashboard_dataset` (`superset/dashboards/api.py:759`) drops them 
for any caller without datasource access, which is what #42716 put them into 
`DASHBOARD_DATASET_INACCESSIBLE_FIELDS` for.
   
   Are we certain the renderer needs them? An embedded dashboard draws the same 
`Chart` component from the same `datasources` slice (`DashboardPage.tsx:379`, 
fed by `useDashboardDatasets`), and for a guest that payload already arrives 
without those fields. If something does break without them, narrowing `columns` 
and `metrics` to what the chart's own `form_data` references would keep the 
chart drawable without publishing the rest of the dataset alongside it. Either 
way, a test asserting the guest payload carries no column outside the chart's 
`form_data` and no metric `expression` would pin whichever way you land.



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

Reply via email to