apache / superset

Apache Superset is a Data Visualization and Data Exploration Platform
https://superset.apache.org/
Apache License 2.0
62.65k stars 13.83k forks source link

/embedded endpoint has too strict frame policy when Talisman is enabled #27292

Open smalyshev opened 8 months ago

smalyshev commented 8 months ago

Bug description

When trying to embed Superset dashboard, I discovered, that with Talisman enabled, /embedded endpoint produces X-Frame-Options: SAMEORIGIN - which does not allow to embed dashboards in pages of different origin. This seems to be too limiting - while I'm fine with other endpoints having this policy, this particular one is specially designated for embedding, so it makes sense for it do allow wider permissions. I wonder if this would be a better option:


--- a/superset/embedded/view.py
+++ b/superset/embedded/view.py
@@ -27,6 +27,7 @@ from superset.daos.dashboard import EmbeddedDashboardDAO
 from superset.superset_typing import FlaskResponse
 from superset.utils import core as utils
 from superset.views.base import BaseSupersetView, common_bootstrap_payload
+from superset.extensions import talisman

 class EmbeddedView(BaseSupersetView):
@@ -34,6 +35,7 @@ class EmbeddedView(BaseSupersetView):

     route_base = "/embedded"

+    @talisman(frame_options=None)
     @expose("/<uuid>")
     @event_logger.log_this_with_extra_payload
     def embedded(

How to reproduce the bug

  1. Make embeddable dashboard
  2. Try to embed it from another domain
  3. See something like: Refused to display 'https://mysite.embedding/' in a frame because it set 'X-Frame-Options' to 'sameorigin'. in the browser console.

Screenshots/recordings

No response

Superset version

3.0.4

Python version

3.9

Node version

16

Browser

Chrome

Additional context

No response

Checklist

michael-s-molina commented 8 months ago

Hi @smalyshev. According to ChatGPT:

It is possible to override the X-Frame-Options header with a CSP (Content Security Policy) directive. This can be done by using the frame-ancestors directive in the CSP header. By specifying the desired domains in the frame-ancestors directive, you can allow the content to be loaded in an iframe from those domains.

Here is an example of how to override the X-Frame-Options header using the frame-ancestors directive in a CSP header:

content-security-policy: frame-ancestors https://example.com https://anotherdomain.com;

In this example, the content is allowed to be loaded in iframes from https://example.com and https://anotherdomain.com.

You can find more information about CSP and its directives in the Security Docs — Content Security Policy (CSP) article.

To override Superset's CSP policy use TALISMAN_CONFIG.

smalyshev commented 8 months ago

Thank you for the answer. I understand that CSP policy can be amended in TALISMAN_CONFIG, but changing this config would affect all the endpoints. What I am suggesting is leaving the policy as is for all the endpoints - because it's mostly appropriate that parts of the application that are not meant for embedding will not be embeddable - and only changing the policy in one specific endpoint that is meant for embedding. Is this possible to do by modifying TALISMAN_CONFIG?

michael-s-molina commented 8 months ago

Thank you for the answer. I understand that CSP policy can be amended in TALISMAN_CONFIG, but changing this config would affect all the endpoints. What I am suggesting is leaving the policy as is for all the endpoints

To be able to do this, we would need a specific configuration for this endpoint given that Frame-Options: None is not a secure configuration. Considering that in terms of security, one door is sufficient for an attack, I'm not sure about the benefit of restricting other endpoints given that you already opened the door for the embedded endpoint. I'm assuming that when you do that, you'll configure a trusted domain instead of any.

smalyshev commented 8 months ago

I'm not sure what specific attack you're talking about, but in terms of framing I am assuming I do not want my whole superset app to be displayed in a frame on a separate site, but I do want a specific diagram, that I explicitly designated as shareable, and that uses special role & guest token for accessing it, to be accessible in a frame. I think it is different from security standpoint whether we allow it for the whole application or for one specific endpoint that is designed for sharing and embedding. I am not insisting on removing all framing options - making it configurable would be ok too, what I am looking for is to be able to set more relaxed settings for the specifically embeddable endpoint than for the rest of the application, which I don't want to make embeddable as a whole.

michael-s-molina commented 8 months ago

Tagging @dpgaspar @jfrag1 @eschutho @kgabryje who have more context on embedded and might help figuring out a secure solution for your problem.

rusackas commented 3 months ago

Anyone still looking for an answer (or willing to provide one) here? This thread has been silent for long enough that it's at risk of being closed as stale. In the meantime, perhaps @dosu-bot can help.

dosubot[bot] commented 3 months ago

Hello, @rusackas! I'm here to help you out while we wait for a human maintainer. I'm a bot that can assist you with bug-solving, answering questions, and becoming a contributor. Let me know how I can assist you!

To set more relaxed X-Frame-Options settings for the /embedded endpoint in Superset without affecting the rest of the application, you can use the @talisman decorator to override the default settings for this specific view. Here is how you can modify the embedded method in EmbeddedView to achieve this:

from flask_talisman import talisman

class EmbeddedView(BaseSupersetView):
    """The views for embedded resources to be rendered in an iframe"""

    route_base = "/embedded"

    @expose("/<uuid>")
    @event_logger.log_this_with_extra_payload
    @talisman(frame_options_allow_from="*")  # Set more relaxed X-Frame-Options
    def embedded(
        self,
        uuid: str,
        add_extra_log_payload: Callable[..., None] = lambda **kwargs: None,
    ) -> FlaskResponse:
        """
        Server side rendering for the embedded dashboard page
        :param uuid: identifier for embedded dashboard
        :param add_extra_log_payload: added by `log_this_with_manual_updates`, set a
            default value to appease pylint
        """
        if not is_feature_enabled("EMBEDDED_SUPERSET"):
            abort(404)

        embedded = EmbeddedDashboardDAO.find_by_id(uuid)

        if not embedded:
            abort(404)

        assert embedded is not None

        # validate request referrer in allowed domains
        is_referrer_allowed = not embedded.allowed_domains
        for domain in embedded.allowed_domains:
            if same_origin(request.referrer, domain):
                is_referrer_allowed = True
                break

        if not is_referrer_allowed:
            abort(403)

        # Log in as an anonymous user, just for this view.
        # This view needs to be visible to all users,
        # and building the page fails if g.user and/or ctx.user aren't present.
        login_user(AnonymousUserMixin(), force=True)

        add_extra_log_payload(
            embedded_dashboard_id=uuid,
            dashboard_version="v2",
        )

        bootstrap_data = {
            "config": {
                "GUEST_TOKEN_HEADER_NAME": current_app.config["GUEST_TOKEN_HEADER_NAME"]
            },
            "common": common_bootstrap_payload(),
            "embedded": {
                "dashboard_id": embedded.dashboard_id,
            },
        }
        return self.render_template(
            "superset/spa.html",
            entry="embedded",
            bootstrap_data=json.dumps(
                bootstrap_data, default=json.pessimistic_json_iso_dttm_ser
            ),
        )

In this code, the @talisman(frame_options_allow_from="*") decorator is used to set the X-Frame-Options header to allow framing from any origin for the /embedded endpoint. Adjust the frame_options_allow_from parameter as needed to specify the allowed origins [1].

To continue talking to Dosu, mention @dosu.

eschutho commented 3 months ago

I think what you proposed sounds reasonable. For your specific use-case, do you have a limited set of websites that you want to allow to frame your content, because you can also use the csp to limit attacks if you only allow framing to trusted partner sites.

mariomourao commented 2 months ago

Hi, Where should I apply the changes suggested by the @dosubot?

Is there a way to use an embedded dashboard with TALISMAN_ENABLED set to TRUE?"