From eb0e4a9998809312fc5b1d69573d1b962050bf8f Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Wed, 23 Mar 2022 10:28:46 -0700 Subject: [PATCH 01/26] embedded dashboard model --- superset/models/embedded_dashboard.py | 43 +++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 superset/models/embedded_dashboard.py diff --git a/superset/models/embedded_dashboard.py b/superset/models/embedded_dashboard.py new file mode 100644 index 0000000000000..0f8d94e786b0f --- /dev/null +++ b/superset/models/embedded_dashboard.py @@ -0,0 +1,43 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from flask_appbuilder import Model +from sqlalchemy import Column, ForeignKey, Integer, Text +from sqlalchemy.orm import backref, relationship +from sqlalchemy_utils import UUIDType + +from superset.models.dashboard import Dashboard +from superset.models.helpers import AuditMixinNullable + + +class EmbeddedDashboard( + Model, AuditMixinNullable +): # pylint: disable=too-few-public-methods + + """ + A configuration of embedding for a dashboard + """ + + __tablename__ = "embedded_dashboards" + + uuid = Column(UUIDType(binary=True)) + allow_domain_list = Column(Text) + dashboard_id = Column(Integer, ForeignKey("dashboards.id"), nullable=False) + dashboard = relationship( + Dashboard, + backref=backref("embedded", cascade="all,delete,delete-orphan"), + foreign_keys=[dashboard_id], + ) From 0732aca2ece750cb867c22be7fa1853e8dd97237 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Fri, 25 Mar 2022 00:39:58 -0700 Subject: [PATCH 02/26] embedded dashboard endpoints --- superset/dashboards/api.py | 149 +++++++++++++++++- superset/dashboards/dao.py | 17 ++ superset/dashboards/schemas.py | 12 ++ superset/models/dashboard.py | 3 + superset/models/embedded_dashboard.py | 27 +++- superset/security/manager.py | 1 + .../integration_tests/dashboards/api_tests.py | 46 ++++++ .../integration_tests/dashboards/dao_tests.py | 13 ++ 8 files changed, 259 insertions(+), 9 deletions(-) diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py index 5e761cdd1f092..4f29bf42363a5 100644 --- a/superset/dashboards/api.py +++ b/superset/dashboards/api.py @@ -15,14 +15,16 @@ # specific language governing permissions and limitations # under the License. # pylint: disable=too-many-lines +import functools import json import logging from datetime import datetime from io import BytesIO -from typing import Any, Optional +from typing import Any, Callable, Optional from zipfile import is_zipfile, ZipFile from flask import g, make_response, redirect, request, Response, send_file, url_for +from flask_appbuilder import permission_name from flask_appbuilder.api import expose, protect, rison, safe from flask_appbuilder.hooks import before_request from flask_appbuilder.models.sqla.interface import SQLAInterface @@ -65,6 +67,8 @@ DashboardGetResponseSchema, DashboardPostSchema, DashboardPutSchema, + EmbeddedDashboardConfigSchema, + EmbeddedDashboardResponseSchema, get_delete_ids_schema, get_export_ids_schema, get_fav_star_ids_schema, @@ -74,6 +78,7 @@ ) from superset.extensions import event_logger from superset.models.dashboard import Dashboard +from superset.models.embedded_dashboard import EmbeddedDashboard from superset.tasks.thumbnails import cache_dashboard_thumbnail from superset.utils.cache import etag_cache from superset.utils.screenshots import DashboardScreenshot @@ -91,6 +96,28 @@ logger = logging.getLogger(__name__) +def with_dashboard( + f: Callable[[BaseSupersetModelRestApi, Dashboard], Response] +) -> Callable[[BaseSupersetModelRestApi, str], Response]: + """ + A decorator that looks up the dashboard by id or slug and passes it to the api. + Route must include an parameter. + Responds with 403 or 404 without calling the route, if necessary. + """ + + def wraps(self: BaseSupersetModelRestApi, id_or_slug: str) -> Response: + # pylint: disable=arguments-differ + try: + dash = DashboardDAO.get_by_id_or_slug(id_or_slug) + return f(self, dash) + except DashboardAccessDeniedError: + return self.response_403() + except DashboardNotFoundError: + return self.response_404() + + return functools.update_wrapper(wraps, f) + + class DashboardRestApi(BaseSupersetModelRestApi): datamodel = SQLAInterface(Dashboard) @@ -108,6 +135,9 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]: "favorite_status", "get_charts", "get_datasets", + "get_embedded", + "set_embedded", + "delete_embedded", "thumbnail", } resource_name = "dashboard" @@ -193,6 +223,8 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]: chart_entity_response_schema = ChartEntityResponseSchema() dashboard_get_response_schema = DashboardGetResponseSchema() dashboard_dataset_schema = DashboardDatasetSchema() + embedded_response_schema = EmbeddedDashboardResponseSchema() + embedded_config_schema = EmbeddedDashboardConfigSchema() base_filters = [["id", DashboardAccessFilter, lambda: []]] @@ -215,6 +247,7 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]: DashboardGetResponseSchema, DashboardDatasetSchema, GetFavStarIdsSchema, + EmbeddedDashboardResponseSchema, ) apispec_parameter_schemas = { "get_delete_ids_schema": get_delete_ids_schema, @@ -1001,3 +1034,117 @@ def import_(self) -> Response: ) command.run() return self.response(200, message="OK") + + @expose("//embedded", methods=["GET"]) + @protect() + @safe + @permission_name("write") + @statsd_metrics + @event_logger.log_this_with_context( + action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.get_embedded", + log_to_statsd=False, # pylint: disable=arguments-renamed + ) + @with_dashboard + def get_embedded(self, dashboard: Dashboard) -> Response: + """Response + Returns the dashboard's embedded configuration + --- + post: + description: >- + Returns the dashboard's embedded configuration + responses: + 200: + description: Result contains the embedded dashboard config + content: + application/json: + schema: + type: object + properties: + result: + $ref: '#/components/schemas/EmbeddedDashboardResponseSchema' + 401: + $ref: '#/components/responses/401' + 500: + $ref: '#/components/responses/500' + """ + if not dashboard.embedded: + return self.response(404) + embedded: EmbeddedDashboard = dashboard.embedded[0] + result = self.embedded_response_schema.dump(embedded) + return self.response(200, result=result) + + @expose("//embedded", methods=["POST", "PUT"]) + @protect() + @safe + @permission_name("manage_embedded") + @statsd_metrics + @event_logger.log_this_with_context( + action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.set_embedded", + log_to_statsd=False, # pylint: disable=arguments-renamed + ) + @with_dashboard + def set_embedded(self, dashboard: Dashboard) -> Response: + """Response + Sets a dashboard's embedded configuration. + --- + post: + description: >- + Sets a dashboard's embedded configuration. + requestBody: + description: The embedded configuration to set + required: true + content: + application/json: + schema: EmbeddedDashboardConfigSchema + responses: + 200: + description: Successfully set the configuration + content: + application/json: + schema: + type: object + properties: + result: + $ref: '#/components/schemas/EmbeddedDashboardResponseSchema' + 401: + $ref: '#/components/responses/401' + 500: + $ref: '#/components/responses/500' + """ + try: + body = self.embedded_config_schema.load(request.json) + embedded = DashboardDAO.upsert_embedded_dashboard( + dashboard, body["allowed_domains"] + ) + result = self.embedded_response_schema.dump(embedded) + return self.response(200, result=result) + except ValidationError as error: + return self.response_400(message=error.messages) + + @expose("//embedded", methods=["DELETE"]) + @protect() + @safe + @permission_name("write") + @statsd_metrics + @event_logger.log_this_with_context( + action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.delete_embedded", + log_to_statsd=False, # pylint: disable=arguments-renamed + ) + @with_dashboard + def delete_embedded(self, dashboard: Dashboard) -> Response: + """Response + Removes a dashboard's embedded configuration. + --- + post: + description: >- + Removes a dashboard's embedded configuration. + responses: + 200: + description: Successfully removed the configuration + 401: + $ref: '#/components/responses/401' + 500: + $ref: '#/components/responses/500' + """ + dashboard.embedded = [] + return self.response(200) diff --git a/superset/dashboards/dao.py b/superset/dashboards/dao.py index ce6e30f8d4beb..e10bf3c021dfe 100644 --- a/superset/dashboards/dao.py +++ b/superset/dashboards/dao.py @@ -28,6 +28,7 @@ from superset.extensions import db from superset.models.core import FavStar, FavStarClassName from superset.models.dashboard import Dashboard +from superset.models.embedded_dashboard import EmbeddedDashboard from superset.models.slice import Slice from superset.utils.dashboard_filter_scopes_converter import copy_filter_scopes @@ -288,3 +289,19 @@ def favorited_ids( ) .all() ] + + @staticmethod + def upsert_embedded_dashboard( + dashboard: Dashboard, allowed_domains: List[str] + ) -> EmbeddedDashboard: + """ + Sets up a dashboard to be embeddable. + Upsert is used to preserve the embedded_dashboard uuid across updates. + """ + embedded: EmbeddedDashboard = dashboard.embedded[ + 0 + ] if dashboard.embedded else EmbeddedDashboard() + embedded.allow_domain_list = ",".join(allowed_domains) + dashboard.embedded = [embedded] + db.session.commit() + return embedded diff --git a/superset/dashboards/schemas.py b/superset/dashboards/schemas.py index 6cb1a3caeee90..13d63ca3056f4 100644 --- a/superset/dashboards/schemas.py +++ b/superset/dashboards/schemas.py @@ -308,3 +308,15 @@ class ImportV1DashboardSchema(Schema): version = fields.String(required=True) is_managed_externally = fields.Boolean(allow_none=True, default=False) external_url = fields.String(allow_none=True) + + +class EmbeddedDashboardConfigSchema(Schema): + allowed_domains = fields.List(fields.String(), required=True) + + +class EmbeddedDashboardResponseSchema(Schema): + uuid = fields.String() + allowed_domains = fields.List(fields.String()) + dashboard_id = fields.String() + changed_on = fields.DateTime() + changed_by = fields.Nested(UserSchema) diff --git a/superset/models/dashboard.py b/superset/models/dashboard.py index 812b0544689e3..48f08e55c073a 100644 --- a/superset/models/dashboard.py +++ b/superset/models/dashboard.py @@ -152,6 +152,9 @@ class Dashboard(Model, AuditMixinNullable, ImportExportMixin): is_managed_externally = Column(Boolean, nullable=False, default=False) external_url = Column(Text, nullable=True) roles = relationship(security_manager.role_model, secondary=DashboardRoles) + embedded = relationship( + "EmbeddedDashboard", back_populates="dashboard", cascade="all, delete-orphan", + ) _filter_sets = relationship( "FilterSet", back_populates="dashboard", cascade="all, delete" ) diff --git a/superset/models/embedded_dashboard.py b/superset/models/embedded_dashboard.py index 0f8d94e786b0f..a0cf1760ec07e 100644 --- a/superset/models/embedded_dashboard.py +++ b/superset/models/embedded_dashboard.py @@ -14,12 +14,14 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +import uuid +from typing import List + from flask_appbuilder import Model from sqlalchemy import Column, ForeignKey, Integer, Text -from sqlalchemy.orm import backref, relationship +from sqlalchemy.orm import relationship from sqlalchemy_utils import UUIDType -from superset.models.dashboard import Dashboard from superset.models.helpers import AuditMixinNullable @@ -28,16 +30,25 @@ class EmbeddedDashboard( ): # pylint: disable=too-few-public-methods """ - A configuration of embedding for a dashboard + A configuration of embedding for a dashboard. + References the dashboard, and contains a config for embedding that dashboard. + This data model allows multiple configurations for a given dashboard, + but at this time the API only allows setting one. """ __tablename__ = "embedded_dashboards" - uuid = Column(UUIDType(binary=True)) - allow_domain_list = Column(Text) + uuid = Column(UUIDType(binary=True), default=uuid.uuid4, primary_key=True) + allow_domain_list = Column(Text) # reference the `allowed_domains` property instead dashboard_id = Column(Integer, ForeignKey("dashboards.id"), nullable=False) dashboard = relationship( - Dashboard, - backref=backref("embedded", cascade="all,delete,delete-orphan"), - foreign_keys=[dashboard_id], + "Dashboard", back_populates="embedded", foreign_keys=[dashboard_id], ) + + @property + def allowed_domains(self) -> List[str]: + """ + A list of domains which are allowed to embed the dashboard. + An empty list means any domain can embed. + """ + return self.allow_domain_list.split(",") if self.allow_domain_list else [] diff --git a/superset/security/manager.py b/superset/security/manager.py index ac764d240549d..e94eb1de58f73 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -189,6 +189,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods "can_update_role", "all_query_access", "can_grant_guest_token", + "can_manage_embedded", } READ_ONLY_PERMISSION = { diff --git a/tests/integration_tests/dashboards/api_tests.py b/tests/integration_tests/dashboards/api_tests.py index 938de31414393..4df28e3ca25d8 100644 --- a/tests/integration_tests/dashboards/api_tests.py +++ b/tests/integration_tests/dashboards/api_tests.py @@ -1688,3 +1688,49 @@ def test_get_filter_related_roles(self): response_roles = [result["text"] for result in response["result"]] assert "Alpha" in response_roles + + @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices") + def test_embedded_dashboards(self): + self.login(username="admin") + uri = "api/v1/dashboard/world_health/embedded" + + # get, assert 404 + resp = self.get_assert_metric(uri, "get_embedded") + self.assertEqual(resp.status_code, 404) + + # post, assert 200 + allowed_domains = ["test.example", "embedded.example"] + resp = self.post_assert_metric( + uri, {"allowed_domains": allowed_domains}, "set_embedded", + ) + self.assertEqual(resp.status_code, 200) + + # get, assert values + resp = self.get_assert_metric(uri, "get_embedded") + self.assertEqual(resp.status_code, 200) + result = json.loads(resp.data.decode("utf-8"))["result"] + self.assertIsNotNone(result["uuid"]) + self.assertNotEqual(result["uuid"], "") + self.assertEqual(result["allowed_domains"], allowed_domains) + + # save uuid for later + original_uuid = result["uuid"] + + # put, assert 200 + resp = self.post_assert_metric(uri, {"allowed_domains": []}, "set_embedded") + self.assertEqual(resp.status_code, 200) + + # get, assert changes upserted + resp = self.get_assert_metric(uri, "get_embedded") + self.assertEqual(resp.status_code, 200) + result = json.loads(resp.data.decode("utf-8"))["result"] + self.assertEqual(result["uuid"], original_uuid) + self.assertEqual(result["allowed_domains"], []) + + # delete, assert 200 + resp = self.delete_assert_metric(uri, "delete_embedded") + self.assertEqual(resp.status_code, 200) + + # get, assert 404 + resp = self.get_assert_metric(uri, "get_embedded") + self.assertEqual(resp.status_code, 404) diff --git a/tests/integration_tests/dashboards/dao_tests.py b/tests/integration_tests/dashboards/dao_tests.py index 4a02cd7107acd..356256301f5f3 100644 --- a/tests/integration_tests/dashboards/dao_tests.py +++ b/tests/integration_tests/dashboards/dao_tests.py @@ -119,3 +119,16 @@ def test_get_dashboard_changed_on(self): DashboardDAO.set_dash_metadata(dashboard, original_data) session.merge(dashboard) session.commit() + + @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices") + def test_upsert_embedded_dashboard(self): + dash = db.session.query(Dashboard).filter_by(slug="world_health").first() + assert not dash.embedded + DashboardDAO.upsert_embedded_dashboard(dash, ["test.example.com"]) + assert dash.embedded + self.assertEqual(dash.embedded[0].allowed_domains, ["test.example.com"]) + original_uuid = dash.embedded[0].uuid + self.assertIsNotNone(original_uuid) + DashboardDAO.upsert_embedded_dashboard(dash, []) + self.assertEqual(dash.embedded[0].allowed_domains, []) + self.assertEqual(dash.embedded[0].uuid, original_uuid) From d718e6f399d715ab03dbed8fe5411af9710b7548 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Fri, 25 Mar 2022 00:44:57 -0700 Subject: [PATCH 03/26] DRY up using the with_dashboard decorator elsewhere --- superset/dashboards/api.py | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py index 4f29bf42363a5..f84ec7b814e72 100644 --- a/superset/dashboards/api.py +++ b/superset/dashboards/api.py @@ -283,7 +283,8 @@ def __repr__(self) -> str: action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.get", log_to_statsd=False, # pylint: disable=arguments-renamed ) - def get(self, id_or_slug: str) -> Response: + @with_dashboard + def get(self, dash: Dashboard) -> Response: """Gets a dashboard --- get: @@ -316,15 +317,8 @@ def get(self, id_or_slug: str) -> Response: 404: $ref: '#/components/responses/404' """ - # pylint: disable=arguments-differ - try: - dash = DashboardDAO.get_by_id_or_slug(id_or_slug) - result = self.dashboard_get_response_schema.dump(dash) - return self.response(200, result=result) - except DashboardAccessDeniedError: - return self.response_403() - except DashboardNotFoundError: - return self.response_404() + result = self.dashboard_get_response_schema.dump(dash) + return self.response(200, result=result) @etag_cache( get_last_modified=lambda _self, id_or_slug: DashboardDAO.get_dashboard_and_datasets_changed_on( # pylint: disable=line-too-long,useless-suppression From 16f08d0dee312d3d2bedee511cf4ec2cf712d483 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Fri, 25 Mar 2022 23:51:29 -0700 Subject: [PATCH 04/26] wip --- .../components/DashboardEmbedControls.tsx | 227 ++++++++++++++++++ .../Header/HeaderActionsDropdown/index.jsx | 12 + .../src/dashboard/components/Header/index.jsx | 19 ++ superset-frontend/src/dashboard/types.ts | 6 + .../src/hooks/apiResources/dashboards.ts | 5 +- superset/dashboards/api.py | 5 +- .../integration_tests/dashboards/api_tests.py | 21 +- 7 files changed, 284 insertions(+), 11 deletions(-) create mode 100644 superset-frontend/src/dashboard/components/DashboardEmbedControls.tsx diff --git a/superset-frontend/src/dashboard/components/DashboardEmbedControls.tsx b/superset-frontend/src/dashboard/components/DashboardEmbedControls.tsx new file mode 100644 index 0000000000000..752d79691eb4d --- /dev/null +++ b/superset-frontend/src/dashboard/components/DashboardEmbedControls.tsx @@ -0,0 +1,227 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import React, { useCallback, useEffect, useState } from 'react'; +import { makeApi, styled, SupersetApiError, t } from '@superset-ui/core'; +import Modal from 'src/components/Modal'; +import Loading from 'src/components/Loading'; +import Button from 'src/components/Button'; +import { EmbeddedDashboard } from '../types'; +import { Input } from 'src/components/Input'; +import { useToasts } from 'src/components/MessageToasts/withToasts'; +import { FormItem } from 'src/components/Form'; +import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls'; + +type Props = { + dashboardId: string; + show: boolean; + onHide: () => void; +}; + +type EmbeddedApiPayload = { allowed_domains: string[] }; + +const stringToList = (stringyList: string): string[] => + stringyList.split(/(?:\s|,)+/).filter(x => x); + +const ButtonRow = styled.div` + display: flex; + flex-direction: row; + justify-content: flex-end; +`; + +export const DashboardEmbedControls = ({ dashboardId, onHide }: Props) => { + const { addInfoToast, addDangerToast } = useToasts(); + const [ready, setReady] = useState(true); // whether we have initialized yet + const [loading, setLoading] = useState(false); // whether we are currently doing an async thing + const [embedded, setEmbedded] = useState(null); // the embedded dashboard config + const [allowedDomains, setAllowedDomains] = useState(''); + + const endpoint = `/api/v1/dashboard/${dashboardId}/embedded`; + // whether saveable changes have been made to the config + const isDirty = + !embedded || + stringToList(allowedDomains).join() !== embedded.allowed_domains.join(); + + const enableEmbedded = useCallback(() => { + setLoading(true); + makeApi({ + method: 'POST', + endpoint, + })({ + allowed_domains: stringToList(allowedDomains), + }) + .then( + ({ result }) => { + setEmbedded(result); + setAllowedDomains(result.allowed_domains.join(', ')); + addInfoToast(t('Changes saved.')); + }, + err => { + console.error(err); + addDangerToast( + t( + t('Sorry, something went wrong. The changes could not be saved.'), + ), + ); + }, + ) + .finally(() => { + setLoading(false); + }); + }, [endpoint, allowedDomains]); + + const disableEmbedded = useCallback(() => { + Modal.confirm({ + title: t('Disable embedding?'), + content: t('This will remove your current embed configuration.'), + okType: 'danger', + onOk: () => { + setLoading(true); + makeApi<{}>({ method: 'DELETE', endpoint })({}) + .then( + () => { + setEmbedded(null); + setAllowedDomains(''); + addInfoToast(t('Embedding deactivated.')); + onHide(); + }, + err => { + console.error(err); + addDangerToast( + t( + 'Sorry, something went wrong. Embedding could not be deactivated.', + ), + ); + }, + ) + .finally(() => { + setLoading(false); + }); + }, + }); + }, [endpoint]); + + useEffect(() => { + setReady(false); + makeApi<{}, { result: EmbeddedDashboard }>({ + method: 'GET', + endpoint, + })({}) + .catch(err => { + if ((err as SupersetApiError).status === 404) { + // 404 just means the dashboard isn't currently embedded + return { result: null }; + } + throw err; + }) + .then(({ result }) => { + setReady(true); + setEmbedded(result); + setAllowedDomains(result ? result.allowed_domains.join(', ') : ''); + }); + }, [dashboardId]); + + if (!ready) { + return ; + } + + return ( + <> +

+ {embedded ? ( + <> + {t( + 'This dashboard is ready to embed. In your application, pass the following id to the SDK:', + )} +
+ {embedded.uuid} + + ) : ( + t( + 'Configure this dashboard to embed it into an external web application.', + ) + )} +

+

+ {t('For further instructions, consult the')}{' '} + + {t('Superset Embedded SDK documentation.')} + +

+

Settings

+ + + setAllowedDomains(event.target.value)} + /> + + + {embedded ? ( + <> + + + + ) : ( + + )} + + + ); +}; + +export const DashboardEmbedModal = (props: Props) => { + const { show, onHide } = props; + + return ( + + + + ); +}; diff --git a/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx b/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx index 9375c684af90a..1eece0745e2e9 100644 --- a/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx +++ b/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx @@ -64,6 +64,7 @@ const propTypes = { expandedSlices: PropTypes.object.isRequired, onSave: PropTypes.func.isRequired, showPropertiesModal: PropTypes.func.isRequired, + manageEmbedded: PropTypes.func.isRequired, refreshLimit: PropTypes.number, refreshWarning: PropTypes.string, lastModifiedTime: PropTypes.number.isRequired, @@ -88,6 +89,7 @@ const MENU_KEYS = { EDIT_CSS: 'edit-css', DOWNLOAD_AS_IMAGE: 'download-as-image', TOGGLE_FULLSCREEN: 'toggle-fullscreen', + MANAGE_EMBEDDED: 'manage-embedded', }; const DropdownButton = styled.div` @@ -182,6 +184,10 @@ class HeaderActionsDropdown extends React.PureComponent { window.location.replace(url); break; } + case MENU_KEYS.MANAGE_EMBEDDED: { + this.props.manageEmbedded(); + break; + } default: break; } @@ -313,6 +319,12 @@ class HeaderActionsDropdown extends React.PureComponent { )} + {!editMode && isFeatureEnabled(FeatureFlag.EMBEDDED_SUPERSET) && ( + + {t('Embed dashboard')} + + )} + {!editMode && ( {t('Download as image')} diff --git a/superset-frontend/src/dashboard/components/Header/index.jsx b/superset-frontend/src/dashboard/components/Header/index.jsx index 7fd1afc82eb3b..218b6f0c85d78 100644 --- a/superset-frontend/src/dashboard/components/Header/index.jsx +++ b/superset-frontend/src/dashboard/components/Header/index.jsx @@ -53,6 +53,8 @@ import setPeriodicRunner, { } from 'src/dashboard/util/setPeriodicRunner'; import { options as PeriodicRefreshOptions } from 'src/dashboard/components/RefreshIntervalModal'; import { FILTER_BOX_MIGRATION_STATES } from 'src/explore/constants'; +import Modal from 'src/components/Modal'; +import { DashboardEmbedModal } from '../DashboardEmbedControls'; const propTypes = { addSuccessToast: PropTypes.func.isRequired, @@ -420,6 +422,14 @@ class Header extends React.PureComponent { this.setState({ showingReportModal: false }); } + showEmbedModal = () => { + this.setState({ showingEmbedModal: true }); + }; + + hideEmbedModal = () => { + this.setState({ showingEmbedModal: false }); + }; + renderReportModal() { const attachedReportExists = !!Object.keys(this.props.reports).length; return attachedReportExists ? ( @@ -658,6 +668,14 @@ class Header extends React.PureComponent { /> )} + {isFeatureEnabled(FeatureFlag.EMBEDDED_SUPERSET) && ( + + )} + // that are necessary for rendering the given dashboard export const useDashboardDatasets = (idOrSlug: string | number) => useApiV1Resource(`/api/v1/dashboard/${idOrSlug}/datasets`); + +export const useEmbeddedDashboard = (idOrSlug: string | number) => + useApiV1Resource(`/api/v1/dashboard/${idOrSlug}/embedded`); diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py index f84ec7b814e72..198b5f0692d48 100644 --- a/superset/dashboards/api.py +++ b/superset/dashboards/api.py @@ -1032,7 +1032,7 @@ def import_(self) -> Response: @expose("//embedded", methods=["GET"]) @protect() @safe - @permission_name("write") + @permission_name("read") @statsd_metrics @event_logger.log_this_with_context( action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.get_embedded", @@ -1070,7 +1070,6 @@ def get_embedded(self, dashboard: Dashboard) -> Response: @expose("//embedded", methods=["POST", "PUT"]) @protect() @safe - @permission_name("manage_embedded") @statsd_metrics @event_logger.log_this_with_context( action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.set_embedded", @@ -1118,7 +1117,7 @@ def set_embedded(self, dashboard: Dashboard) -> Response: @expose("//embedded", methods=["DELETE"]) @protect() @safe - @permission_name("write") + @permission_name("set_embedded") @statsd_metrics @event_logger.log_this_with_context( action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.delete_embedded", diff --git a/tests/integration_tests/dashboards/api_tests.py b/tests/integration_tests/dashboards/api_tests.py index 4df28e3ca25d8..f72ed85d67068 100644 --- a/tests/integration_tests/dashboards/api_tests.py +++ b/tests/integration_tests/dashboards/api_tests.py @@ -1694,18 +1694,22 @@ def test_embedded_dashboards(self): self.login(username="admin") uri = "api/v1/dashboard/world_health/embedded" - # get, assert 404 + # initial get should return 404 resp = self.get_assert_metric(uri, "get_embedded") self.assertEqual(resp.status_code, 404) - # post, assert 200 + # post succeeds and returns value allowed_domains = ["test.example", "embedded.example"] resp = self.post_assert_metric( uri, {"allowed_domains": allowed_domains}, "set_embedded", ) self.assertEqual(resp.status_code, 200) + result = json.loads(resp.data.decode("utf-8"))["result"] + self.assertIsNotNone(result["uuid"]) + self.assertNotEqual(result["uuid"], "") + self.assertEqual(result["allowed_domains"], allowed_domains) - # get, assert values + # get returns value resp = self.get_assert_metric(uri, "get_embedded") self.assertEqual(resp.status_code, 200) result = json.loads(resp.data.decode("utf-8"))["result"] @@ -1716,21 +1720,24 @@ def test_embedded_dashboards(self): # save uuid for later original_uuid = result["uuid"] - # put, assert 200 + # put succeeds and returns value resp = self.post_assert_metric(uri, {"allowed_domains": []}, "set_embedded") self.assertEqual(resp.status_code, 200) + self.assertIsNotNone(result["uuid"]) + self.assertNotEqual(result["uuid"], "") + self.assertEqual(result["allowed_domains"], allowed_domains) - # get, assert changes upserted + # get returns changed value resp = self.get_assert_metric(uri, "get_embedded") self.assertEqual(resp.status_code, 200) result = json.loads(resp.data.decode("utf-8"))["result"] self.assertEqual(result["uuid"], original_uuid) self.assertEqual(result["allowed_domains"], []) - # delete, assert 200 + # delete succeeds resp = self.delete_assert_metric(uri, "delete_embedded") self.assertEqual(resp.status_code, 200) - # get, assert 404 + # get returns 404 resp = self.get_assert_metric(uri, "get_embedded") self.assertEqual(resp.status_code, 404) From 5f431b55df39ebcf51fc75857bb03fbae542a067 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Mon, 28 Mar 2022 11:45:27 -0700 Subject: [PATCH 05/26] check feature flags and permissions --- .../components/Header/HeaderActionsDropdown/index.jsx | 4 +++- superset-frontend/src/dashboard/components/Header/index.jsx | 5 +++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx b/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx index 1eece0745e2e9..619e10ea22e80 100644 --- a/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx +++ b/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx @@ -59,6 +59,7 @@ const propTypes = { userCanEdit: PropTypes.bool.isRequired, userCanShare: PropTypes.bool.isRequired, userCanSave: PropTypes.bool.isRequired, + userCanCurate: PropTypes.bool.isRequired, isLoading: PropTypes.bool.isRequired, layout: PropTypes.object.isRequired, expandedSlices: PropTypes.object.isRequired, @@ -210,6 +211,7 @@ class HeaderActionsDropdown extends React.PureComponent { userCanEdit, userCanShare, userCanSave, + userCanCurate, isLoading, refreshLimit, refreshWarning, @@ -319,7 +321,7 @@ class HeaderActionsDropdown extends React.PureComponent { )} - {!editMode && isFeatureEnabled(FeatureFlag.EMBEDDED_SUPERSET) && ( + {!editMode && userCanCurate && ( {t('Embed dashboard')} diff --git a/superset-frontend/src/dashboard/components/Header/index.jsx b/superset-frontend/src/dashboard/components/Header/index.jsx index 218b6f0c85d78..0fd49d354cd13 100644 --- a/superset-frontend/src/dashboard/components/Header/index.jsx +++ b/superset-frontend/src/dashboard/components/Header/index.jsx @@ -55,6 +55,7 @@ import { options as PeriodicRefreshOptions } from 'src/dashboard/components/Refr import { FILTER_BOX_MIGRATION_STATES } from 'src/explore/constants'; import Modal from 'src/components/Modal'; import { DashboardEmbedModal } from '../DashboardEmbedControls'; +import findPermission from 'src/dashboard/util/findPermission'; const propTypes = { addSuccessToast: PropTypes.func.isRequired, @@ -508,6 +509,9 @@ class Header extends React.PureComponent { const userCanSaveAs = dashboardInfo.dash_save_perm && filterboxMigrationState !== FILTER_BOX_MIGRATION_STATES.REVIEWING; + const userCanCurate = + isFeatureEnabled(FeatureFlag.EMBEDDED_SUPERSET) && + findPermission('can_set_embedded', 'Dashboard', user.roles); const shouldShowReport = !editMode && this.canAddReports(); const refreshLimit = dashboardInfo.common?.conf?.SUPERSET_DASHBOARD_PERIODICAL_REFRESH_LIMIT; @@ -701,6 +705,7 @@ class Header extends React.PureComponent { userCanEdit={userCanEdit} userCanShare={userCanShare} userCanSave={userCanSaveAs} + userCanCurate={userCanCurate} isLoading={isLoading} showPropertiesModal={this.showPropertiesModal} manageEmbedded={this.showEmbedModal} From 451e47d88171a98a755464c410e8d2b74e1e422b Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Mon, 28 Mar 2022 17:30:37 -0700 Subject: [PATCH 06/26] wip --- .../dashboard/containers/DashboardPage.tsx | 8 ++++-- .../dashboard/containers/DashboardRoute.tsx | 28 +++++++++++++++++++ superset-frontend/src/embedded/index.tsx | 6 ++-- superset-frontend/src/preamble.ts | 3 ++ superset-frontend/src/views/routes.tsx | 6 ++-- superset/security/manager.py | 2 +- superset/views/dashboard/views.py | 20 ++++++++++--- 7 files changed, 60 insertions(+), 13 deletions(-) create mode 100644 superset-frontend/src/dashboard/containers/DashboardRoute.tsx diff --git a/superset-frontend/src/dashboard/containers/DashboardPage.tsx b/superset-frontend/src/dashboard/containers/DashboardPage.tsx index be0ba9177278c..97a4dc2283d65 100644 --- a/superset-frontend/src/dashboard/containers/DashboardPage.tsx +++ b/superset-frontend/src/dashboard/containers/DashboardPage.tsx @@ -27,7 +27,6 @@ import { } from '@superset-ui/core'; import { useDispatch, useSelector } from 'react-redux'; import { Global } from '@emotion/react'; -import { useParams } from 'react-router-dom'; import { useToasts } from 'src/components/MessageToasts/withToasts'; import Loading from 'src/components/Loading'; import FilterBoxMigrationModal from 'src/dashboard/components/FilterBoxMigrationModal'; @@ -79,14 +78,17 @@ const DashboardContainer = React.lazy( const originalDocumentTitle = document.title; -const DashboardPage: FC = () => { +type PageProps = { + idOrSlug: string; +}; + +export const DashboardPage: FC = ({ idOrSlug }: PageProps) => { const dispatch = useDispatch(); const theme = useTheme(); const user = useSelector( state => state.user, ); const { addDangerToast } = useToasts(); - const { idOrSlug } = useParams<{ idOrSlug: string }>(); const { result: dashboard, error: dashboardApiError } = useDashboard(idOrSlug); const { result: charts, error: chartsApiError } = diff --git a/superset-frontend/src/dashboard/containers/DashboardRoute.tsx b/superset-frontend/src/dashboard/containers/DashboardRoute.tsx new file mode 100644 index 0000000000000..a382a28d4586f --- /dev/null +++ b/superset-frontend/src/dashboard/containers/DashboardRoute.tsx @@ -0,0 +1,28 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import React, { FC } from 'react'; +import { useParams } from 'react-router-dom'; +import { DashboardPage } from './DashboardPage'; + +const DashboardRoute: FC = () => { + const { idOrSlug } = useParams<{ idOrSlug: string }>(); + return ; +}; + +export default DashboardRoute; diff --git a/superset-frontend/src/embedded/index.tsx b/superset-frontend/src/embedded/index.tsx index 3cd89a88be225..819502e2aa802 100644 --- a/superset-frontend/src/embedded/index.tsx +++ b/superset-frontend/src/embedded/index.tsx @@ -47,11 +47,13 @@ const LazyDashboardPage = lazy( const EmbeddedApp = () => ( - + }> - + diff --git a/superset-frontend/src/preamble.ts b/superset-frontend/src/preamble.ts index 7bd6dbe29b365..8d89104bf2098 100644 --- a/superset-frontend/src/preamble.ts +++ b/superset-frontend/src/preamble.ts @@ -37,6 +37,9 @@ export let bootstrapData: { user?: User | undefined; common?: any; config?: any; + embedded?: { + dashboard_id: string; + }; } = {}; // Configure translation if (typeof window !== 'undefined') { diff --git a/superset-frontend/src/views/routes.tsx b/superset-frontend/src/views/routes.tsx index 255dc99da7796..525860ecef066 100644 --- a/superset-frontend/src/views/routes.tsx +++ b/superset-frontend/src/views/routes.tsx @@ -57,10 +57,10 @@ const DashboardList = lazy( /* webpackChunkName: "DashboardList" */ 'src/views/CRUD/dashboard/DashboardList' ), ); -const DashboardPage = lazy( +const DashboardRoute = lazy( () => import( - /* webpackChunkName: "DashboardPage" */ 'src/dashboard/containers/DashboardPage' + /* webpackChunkName: "DashboardRoute" */ 'src/dashboard/containers/DashboardRoute' ), ); const DatabaseList = lazy( @@ -112,7 +112,7 @@ export const routes: Routes = [ }, { path: '/superset/dashboard/:idOrSlug/', - Component: DashboardPage, + Component: DashboardRoute, }, { path: '/chart/list/', diff --git a/superset/security/manager.py b/superset/security/manager.py index e94eb1de58f73..37801ac76e977 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -189,7 +189,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods "can_update_role", "all_query_access", "can_grant_guest_token", - "can_manage_embedded", + "can_set_embedded", } READ_ONLY_PERMISSION = { diff --git a/superset/views/dashboard/views.py b/superset/views/dashboard/views.py index 471d168e1ec64..7cbaf626e2dde 100644 --- a/superset/views/dashboard/views.py +++ b/superset/views/dashboard/views.py @@ -29,6 +29,7 @@ from superset import db, event_logger, is_feature_enabled, security_manager from superset.constants import MODEL_VIEW_RW_METHOD_PERMISSION_MAP, RouteMethod from superset.models.dashboard import Dashboard as DashboardModel +from superset.models.embedded_dashboard import EmbeddedDashboard from superset.superset_typing import FlaskResponse from superset.utils import core as utils from superset.views.base import ( @@ -132,22 +133,32 @@ def new(self) -> FlaskResponse: # pylint: disable=no-self-use db.session.commit() return redirect(f"/superset/dashboard/{new_dashboard.id}/?edit=true") - @expose("//embedded") + +class EmbeddedView(BaseSupersetView): + """ The views for embedded resources to be rendered in an iframe """ + + route_base = "/embedded" + + @expose("/") @event_logger.log_this_with_extra_payload def embedded( self, - dashboard_id_or_slug: str, + uuid: str, add_extra_log_payload: Callable[..., None] = lambda **kwargs: None, ) -> FlaskResponse: """ Server side rendering for a dashboard - :param dashboard_id_or_slug: identifier for dashboard. used in the decorators + :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"): return Response(status=404) + embedded: EmbeddedDashboard = db.session.get(EmbeddedDashboard, uuid) + if not embedded: + return Response(status=404) + # 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. @@ -155,11 +166,12 @@ def embedded( login_manager.reload_user(AnonymousUserMixin()) add_extra_log_payload( - dashboard_id=dashboard_id_or_slug, dashboard_version="v2", + embedded_dashboard_id=uuid, dashboard_version="v2", ) bootstrap_data = { "common": common_bootstrap_payload(), + "embedded": {"dashboard_id": embedded.dashboard_id,}, } return self.render_template( From bbdee14327a2f5960213b2f87bf5339e2a4e0acc Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Mon, 28 Mar 2022 22:18:13 -0700 Subject: [PATCH 07/26] sdk --- superset-embedded-sdk/src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-embedded-sdk/src/index.ts b/superset-embedded-sdk/src/index.ts index 34932bd6250d7..32b02641e00d2 100644 --- a/superset-embedded-sdk/src/index.ts +++ b/superset-embedded-sdk/src/index.ts @@ -131,7 +131,7 @@ export async function embedDashboard({ resolve(new Switchboard({ port: ourPort, name: 'superset-embedded-sdk', debug })); }); - iframe.src = `${supersetDomain}/dashboard/${id}/embedded${dashboardConfig}`; + iframe.src = `${supersetDomain}/embedded/${id}${dashboardConfig}`; mountPoint.replaceChildren(iframe); log('placed the iframe') }); From 8051d9934642316a6b1d73e0190580a7521b43ab Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Mon, 28 Mar 2022 22:19:11 -0700 Subject: [PATCH 08/26] urls --- superset/initialization/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index 2b970b718fadf..737eecd4cf5cb 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -166,6 +166,7 @@ def init_views(self) -> None: Dashboard, DashboardModelView, DashboardModelViewAsync, + EmbeddedView, ) from superset.views.database.views import ( ColumnarToDatabaseView, @@ -292,6 +293,7 @@ def init_views(self) -> None: appbuilder.add_view_no_menu(Dashboard) appbuilder.add_view_no_menu(DashboardModelViewAsync) appbuilder.add_view_no_menu(Datasource) + appbuilder.add_view_no_menu(EmbeddedView) appbuilder.add_view_no_menu(KV) appbuilder.add_view_no_menu(R) appbuilder.add_view_no_menu(SavedQueryView) From 703a0766071c1965cc735d3f3dc855167d8450cb Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Tue, 29 Mar 2022 04:29:02 -0700 Subject: [PATCH 09/26] dao option for id column --- superset/common/query_context_processor.py | 2 +- superset/dao/base.py | 28 ++- superset/dashboards/commands/export.py | 7 +- superset/databases/dao.py | 3 +- superset/sqllab/command.py | 2 +- tests/integration_tests/embedded/__init__.py | 0 tests/integration_tests/embedded/dao_tests.py | 0 tox.ini | 175 ------------------ yarn.lock | 2 - 9 files changed, 26 insertions(+), 193 deletions(-) create mode 100644 tests/integration_tests/embedded/__init__.py create mode 100644 tests/integration_tests/embedded/dao_tests.py delete mode 100644 tox.ini delete mode 100644 yarn.lock diff --git a/superset/common/query_context_processor.py b/superset/common/query_context_processor.py index 7954f86cdba0c..1645afd7425cd 100644 --- a/superset/common/query_context_processor.py +++ b/superset/common/query_context_processor.py @@ -461,9 +461,9 @@ def get_viz_annotation_data( annotation_layer: Dict[str, Any], force: bool ) -> Dict[str, Any]: chart = ChartDAO.find_by_id(annotation_layer["value"]) - form_data = chart.form_data.copy() if not chart: raise QueryObjectValidationError(_("The chart does not exist")) + form_data = chart.form_data.copy() try: viz_obj = get_viz( datasource_type=chart.datasource.type, diff --git a/superset/dao/base.py b/superset/dao/base.py index ebd6a890886a8..607967e3041e2 100644 --- a/superset/dao/base.py +++ b/superset/dao/base.py @@ -15,12 +15,12 @@ # specific language governing permissions and limitations # under the License. # pylint: disable=isinstance-second-argument-not-valid-type -from typing import Any, Dict, List, Optional, Type +from typing import Any, Dict, List, Optional, Type, Union from flask_appbuilder.models.filters import BaseFilter from flask_appbuilder.models.sqla import Model from flask_appbuilder.models.sqla.interface import SQLAInterface -from sqlalchemy.exc import SQLAlchemyError +from sqlalchemy.exc import SQLAlchemyError, StatementError from sqlalchemy.orm import Session from superset.dao.exceptions import ( @@ -46,9 +46,12 @@ class BaseDAO: """ Child classes can register base filtering to be aplied to all filter methods """ + id_column_name = "id" @classmethod - def find_by_id(cls, model_id: int, session: Session = None) -> Model: + def find_by_id( + cls, model_id: Union[str, int], session: Session = None + ) -> Optional[Model]: """ Find a model by id, if defined applies `base_filter` """ @@ -57,23 +60,28 @@ def find_by_id(cls, model_id: int, session: Session = None) -> Model: if cls.base_filter: data_model = SQLAInterface(cls.model_cls, session) query = cls.base_filter( # pylint: disable=not-callable - "id", data_model + cls.id_column_name, data_model ).apply(query, None) - return query.filter_by(id=model_id).one_or_none() + id_filter = {cls.id_column_name: model_id} + try: + return query.filter_by(**id_filter).one_or_none() + except StatementError: + # can happen if int is passed instead of a string or similar + return None @classmethod - def find_by_ids(cls, model_ids: List[int]) -> List[Model]: + def find_by_ids(cls, model_ids: Union[List[str], List[int]]) -> List[Model]: """ Find a List of models by a list of ids, if defined applies `base_filter` """ - id_col = getattr(cls.model_cls, "id", None) + id_col = getattr(cls.model_cls, cls.id_column_name, None) if id_col is None: return [] query = db.session.query(cls.model_cls).filter(id_col.in_(model_ids)) if cls.base_filter: data_model = SQLAInterface(cls.model_cls, db.session) query = cls.base_filter( # pylint: disable=not-callable - "id", data_model + cls.id_column_name, data_model ).apply(query, None) return query.all() @@ -86,7 +94,7 @@ def find_all(cls) -> List[Model]: if cls.base_filter: data_model = SQLAInterface(cls.model_cls, db.session) query = cls.base_filter( # pylint: disable=not-callable - "id", data_model + cls.id_column_name, data_model ).apply(query, None) return query.all() @@ -99,7 +107,7 @@ def find_one_or_none(cls, **filter_by: Any) -> Optional[Model]: if cls.base_filter: data_model = SQLAInterface(cls.model_cls, db.session) query = cls.base_filter( # pylint: disable=not-callable - "id", data_model + cls.id_column_name, data_model ).apply(query, None) return query.filter_by(**filter_by).one_or_none() diff --git a/superset/dashboards/commands/export.py b/superset/dashboards/commands/export.py index 87408bab37706..c7aa8b6e5c65b 100644 --- a/superset/dashboards/commands/export.py +++ b/superset/dashboards/commands/export.py @@ -140,9 +140,10 @@ def _export( dataset_id = target.pop("datasetId", None) if dataset_id is not None: dataset = DatasetDAO.find_by_id(dataset_id) - target["datasetUuid"] = str(dataset.uuid) - if export_related: - yield from ExportDatasetsCommand([dataset_id]).run() + if dataset: + target["datasetUuid"] = str(dataset.uuid) + if export_related: + yield from ExportDatasetsCommand([dataset_id]).run() # the mapping between dashboard -> charts is inferred from the position # attribute, so if it's not present we need to add a default config diff --git a/superset/databases/dao.py b/superset/databases/dao.py index 3d2cdf6d4ffa0..c32de4621eb4f 100644 --- a/superset/databases/dao.py +++ b/superset/databases/dao.py @@ -67,7 +67,8 @@ def build_db_for_connection_test( @classmethod def get_related_objects(cls, database_id: int) -> Dict[str, Any]: - datasets = cls.find_by_id(database_id).tables + database: Any = cls.find_by_id(database_id) + datasets = database.tables dataset_ids = [dataset.id for dataset in datasets] charts = ( diff --git a/superset/sqllab/command.py b/superset/sqllab/command.py index 1c2674b060f19..58c3ea5013304 100644 --- a/superset/sqllab/command.py +++ b/superset/sqllab/command.py @@ -138,7 +138,7 @@ def _run_sql_json_exec_from_scratch(self) -> SqlJsonExecutionStatus: raise ex def _get_the_query_db(self) -> Database: - mydb = self._database_dao.find_by_id(self._execution_context.database_id) + mydb: Any = self._database_dao.find_by_id(self._execution_context.database_id) self._validate_query_db(mydb) return mydb diff --git a/tests/integration_tests/embedded/__init__.py b/tests/integration_tests/embedded/__init__.py new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/tests/integration_tests/embedded/dao_tests.py b/tests/integration_tests/embedded/dao_tests.py new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/tox.ini b/tox.ini deleted file mode 100644 index 774aa681b6396..0000000000000 --- a/tox.ini +++ /dev/null @@ -1,175 +0,0 @@ -# -# Licensed to the Apache Software Foundation (ASF) under one or more -# contributor license agreements. See the NOTICE file distributed with -# this work for additional information regarding copyright ownership. -# The ASF licenses this file to You under the Apache License, Version 2.0 -# (the "License"); you may not use this file except in compliance with -# the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -# - -# Remember to start celery workers to run celery tests, e.g. -# celery --app=superset.tasks.celery_app:app worker -Ofair -c 2 -[testenv] -basepython = python3.8 -ignore_basepython_conflict = true -commands = - superset db upgrade - superset init - # use -s to be able to use break pointers. - # no args or tests/* can be passed as an argument to run all tests - pytest -s {posargs} -deps = - -rrequirements/testing.txt -setenv = - PYTHONPATH = {toxinidir} - SUPERSET_TESTENV = true - SUPERSET_CONFIG = tests.integration_tests.superset_test_config - SUPERSET_HOME = {envtmpdir} - mysql: SUPERSET__SQLALCHEMY_DATABASE_URI = mysql://mysqluser:mysqluserpassword@localhost/superset?charset=utf8 - postgres: SUPERSET__SQLALCHEMY_DATABASE_URI = postgresql+psycopg2://superset:superset@localhost/test - sqlite: SUPERSET__SQLALCHEMY_DATABASE_URI = sqlite:////{envtmpdir}/superset.db - mysql-presto: SUPERSET__SQLALCHEMY_DATABASE_URI = mysql://mysqluser:mysqluserpassword@localhost/superset?charset=utf8 - # docker run -p 8080:8080 --name presto starburstdata/presto - mysql-presto: SUPERSET__SQLALCHEMY_EXAMPLES_URI = presto://localhost:8080/memory/default - # based on https://github.com/big-data-europe/docker-hadoop - # clone the repo & run docker-compose up -d to test locally - mysql-hive: SUPERSET__SQLALCHEMY_DATABASE_URI = mysql://mysqluser:mysqluserpassword@localhost/superset?charset=utf8 - mysql-hive: SUPERSET__SQLALCHEMY_EXAMPLES_URI = hive://localhost:10000/default - # make sure that directory is accessible by docker - hive: UPLOAD_FOLDER = /tmp/.superset/app/static/uploads/ -usedevelop = true -allowlist_externals = - npm - pkill - -[testenv:cypress] -setenv = - PYTHONPATH = {toxinidir} - SUPERSET_TESTENV = true - SUPERSET_CONFIG = tests.integration_tests.superset_test_config - SUPERSET_HOME = {envtmpdir} -commands = - npm install -g npm@'>=6.5.0' - pip install -e {toxinidir}/ - {toxinidir}/superset-frontend/cypress_build.sh -commands_post = - pkill -if "python {envbindir}/flask" - -[testenv:cypress-dashboard] -setenv = - PYTHONPATH = {toxinidir} - SUPERSET_TESTENV = true - SUPERSET_CONFIG = tests.integration_tests.superset_test_config - SUPERSET_HOME = {envtmpdir} -commands = - npm install -g npm@'>=6.5.0' - pip install -e {toxinidir}/ - {toxinidir}/superset-frontend/cypress_build.sh dashboard -commands_post = - pkill -if "python {envbindir}/flask" - -[testenv:cypress-explore] -setenv = - PYTHONPATH = {toxinidir} - SUPERSET_TESTENV = true - SUPERSET_CONFIG = tests.integration_tests.superset_test_config - SUPERSET_HOME = {envtmpdir} -commands = - npm install -g npm@'>=6.5.0' - pip install -e {toxinidir}/ - {toxinidir}/superset-frontend/cypress_build.sh explore -commands_post = - pkill -if "python {envbindir}/flask" - -[testenv:cypress-sqllab] -setenv = - PYTHONPATH = {toxinidir} - SUPERSET_TESTENV = true - SUPERSET_CONFIG = tests.integration_tests.superset_test_config - SUPERSET_HOME = {envtmpdir} -commands = - npm install -g npm@'>=6.5.0' - pip install -e {toxinidir}/ - {toxinidir}/superset-frontend/cypress_build.sh sqllab -commands_post = - pkill -if "python {envbindir}/flask" - -[testenv:cypress-sqllab-backend-persist] -setenv = - PYTHONPATH = {toxinidir} - SUPERSET_TESTENV = true - SUPERSET_CONFIG = tests.integration_tests.superset_test_config - SUPERSET_HOME = {envtmpdir} -commands = - npm install -g npm@'>=6.5.0' - pip install -e {toxinidir}/ - {toxinidir}/superset-frontend/cypress_build.sh sqllab -commands_post = - pkill -if "python {envbindir}/flask" - -[testenv:eslint] -changedir = {toxinidir}/superset-frontend -commands = - npm run lint -deps = - -[testenv:fossa] -commands = - {toxinidir}/scripts/fossa.sh -deps = -passenv = * - -[testenv:javascript] -commands = - npm install -g npm@'>=6.5.0' - {toxinidir}/superset-frontend/js_build.sh -deps = - -[testenv:license-check] -commands = - {toxinidir}/scripts/check_license.sh -passenv = * -whitelist_externals = - {toxinidir}/scripts/check_license.sh -deps = - -[testenv:pre-commit] -commands = - pre-commit run --all-files -deps = - -rrequirements/integration.txt -skip_install = true - -[testenv:pylint] -commands = - pylint superset -deps = - -rrequirements/testing.txt - -[testenv:thumbnails] -setenv = - SUPERSET_CONFIG = tests.integration_tests.superset_test_config_thumbnails -deps = - -rrequirements/testing.txt - -[tox] -envlist = - cypress-dashboard - cypress-explore - cypress-sqllab - cypress-sqllab-backend-persist - eslint - fossa - javascript - license-check - pre-commit - pylint -skipsdist = true diff --git a/yarn.lock b/yarn.lock deleted file mode 100644 index 4a5801883d149..0000000000000 --- a/yarn.lock +++ /dev/null @@ -1,2 +0,0 @@ -# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. -# yarn lockfile v1 From 7e5cb376f4cce80394b919ab8a0c6719e9e145de Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Tue, 29 Mar 2022 04:39:31 -0700 Subject: [PATCH 10/26] got it working --- superset/dashboards/api.py | 5 +- superset/dashboards/dao.py | 16 -- superset/dashboards/filters.py | 9 +- superset/embedded/__init__.py | 16 ++ superset/embedded/dao.py | 63 +++++++ superset/embedded/view.py | 77 ++++++++ superset/initialization/__init__.py | 2 +- superset/models/embedded_dashboard.py | 4 + superset/security/manager.py | 17 +- superset/views/dashboard/views.py | 49 ----- .../integration_tests/dashboards/dao_tests.py | 13 -- tests/integration_tests/embedded/__init__.py | 16 ++ tests/integration_tests/embedded/dao_tests.py | 51 +++++ .../security/guest_token_security_tests.py | 104 +++++------ tox.ini | 175 ++++++++++++++++++ yarn.lock | 2 + 16 files changed, 466 insertions(+), 153 deletions(-) create mode 100644 superset/embedded/__init__.py create mode 100644 superset/embedded/dao.py create mode 100644 superset/embedded/view.py create mode 100644 tox.ini create mode 100644 yarn.lock diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py index 198b5f0692d48..dc9cd47819a94 100644 --- a/superset/dashboards/api.py +++ b/superset/dashboards/api.py @@ -76,6 +76,7 @@ openapi_spec_methods_override, thumbnail_query_schema, ) +from superset.embedded.dao import EmbeddedDAO from superset.extensions import event_logger from superset.models.dashboard import Dashboard from superset.models.embedded_dashboard import EmbeddedDashboard @@ -1106,9 +1107,7 @@ def set_embedded(self, dashboard: Dashboard) -> Response: """ try: body = self.embedded_config_schema.load(request.json) - embedded = DashboardDAO.upsert_embedded_dashboard( - dashboard, body["allowed_domains"] - ) + embedded = EmbeddedDAO.upsert(dashboard, body["allowed_domains"]) result = self.embedded_response_schema.dump(embedded) return self.response(200, result=result) except ValidationError as error: diff --git a/superset/dashboards/dao.py b/superset/dashboards/dao.py index e10bf3c021dfe..90ffcccc0d998 100644 --- a/superset/dashboards/dao.py +++ b/superset/dashboards/dao.py @@ -289,19 +289,3 @@ def favorited_ids( ) .all() ] - - @staticmethod - def upsert_embedded_dashboard( - dashboard: Dashboard, allowed_domains: List[str] - ) -> EmbeddedDashboard: - """ - Sets up a dashboard to be embeddable. - Upsert is used to preserve the embedded_dashboard uuid across updates. - """ - embedded: EmbeddedDashboard = dashboard.embedded[ - 0 - ] if dashboard.embedded else EmbeddedDashboard() - embedded.allow_domain_list = ",".join(allowed_domains) - dashboard.embedded = [embedded] - db.session.commit() - return embedded diff --git a/superset/dashboards/filters.py b/superset/dashboards/filters.py index e398af97b744a..e253f6f4896ad 100644 --- a/superset/dashboards/filters.py +++ b/superset/dashboards/filters.py @@ -25,6 +25,7 @@ from superset import db, is_feature_enabled, security_manager from superset.models.core import FavStar from superset.models.dashboard import Dashboard +from superset.models.embedded_dashboard import EmbeddedDashboard from superset.models.slice import Slice from superset.security.guest_token import GuestTokenResourceType, GuestUser from superset.views.base import BaseFilter, is_user_admin @@ -133,14 +134,18 @@ def apply(self, query: Query, value: Any) -> Query: if is_feature_enabled("EMBEDDED_SUPERSET") and security_manager.is_guest_user( g.user ): + guest_user: GuestUser = g.user embedded_dashboard_ids = [ r["id"] for r in guest_user.resources if r["type"] == GuestTokenResourceType.DASHBOARD.value ] - if len(embedded_dashboard_ids) != 0: - feature_flagged_filters.append(Dashboard.id.in_(embedded_dashboard_ids)) + feature_flagged_filters.append( + Dashboard.embedded.any( + EmbeddedDashboard.uuid.in_(embedded_dashboard_ids) + ) + ) query = query.filter( or_( diff --git a/superset/embedded/__init__.py b/superset/embedded/__init__.py new file mode 100644 index 0000000000000..13a83393a9124 --- /dev/null +++ b/superset/embedded/__init__.py @@ -0,0 +1,16 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. diff --git a/superset/embedded/dao.py b/superset/embedded/dao.py new file mode 100644 index 0000000000000..d78cfd53aac0a --- /dev/null +++ b/superset/embedded/dao.py @@ -0,0 +1,63 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +import json +import logging +from datetime import datetime +from typing import Any, Dict, List, Optional, Union + +from sqlalchemy.exc import SQLAlchemyError + +from superset import security_manager +from superset.dao.base import BaseDAO +from superset.dashboards.commands.exceptions import DashboardNotFoundError +from superset.dashboards.filters import DashboardAccessFilter +from superset.extensions import db +from superset.models.core import FavStar, FavStarClassName +from superset.models.dashboard import Dashboard +from superset.models.embedded_dashboard import EmbeddedDashboard +from superset.models.slice import Slice +from superset.utils.dashboard_filter_scopes_converter import copy_filter_scopes + +logger = logging.getLogger(__name__) + + +class EmbeddedDAO(BaseDAO): + model_cls = EmbeddedDashboard + # There isn't really a regular scenario where we would rather get Embedded by id + id_column_name = "uuid" + + @staticmethod + def upsert(dashboard: Dashboard, allowed_domains: List[str]) -> EmbeddedDashboard: + """ + Sets up a dashboard to be embeddable. + Upsert is used to preserve the embedded_dashboard uuid across updates. + """ + embedded: EmbeddedDashboard = dashboard.embedded[ + 0 + ] if dashboard.embedded else EmbeddedDashboard() + embedded.allow_domain_list = ",".join(allowed_domains) + dashboard.embedded = [embedded] + db.session.commit() + return embedded + + @classmethod + def create(cls, properties: Dict[str, Any], commit: bool = True) -> Any: + """ + Use EmbeddedDAO.upsert() instead. + At least, until we are ok with more than one embedded instance per dashboard. + """ + raise NotImplementedError("Use EmbeddedDAO.upsert() instead.") diff --git a/superset/embedded/view.py b/superset/embedded/view.py new file mode 100644 index 0000000000000..56086bc36e02d --- /dev/null +++ b/superset/embedded/view.py @@ -0,0 +1,77 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +import json +from typing import Callable + +from flask import abort, Response +from flask_appbuilder import expose +from flask_login import AnonymousUserMixin, LoginManager + +from superset import event_logger, is_feature_enabled, security_manager +from superset.embedded.dao import EmbeddedDAO +from superset.superset_typing import FlaskResponse +from superset.utils import core as utils +from superset.views.base import BaseSupersetView, common_bootstrap_payload + + +class EmbeddedView(BaseSupersetView): + """ The views for embedded resources to be rendered in an iframe """ + + route_base = "/embedded" + + @expose("/") + @event_logger.log_this_with_extra_payload + def embedded( + self, + uuid: str, + add_extra_log_payload: Callable[..., None] = lambda **kwargs: None, + ) -> FlaskResponse: + """ + Server side rendering for a dashboard + :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 = EmbeddedDAO.find_by_id(uuid) + if not embedded: + abort(404) + + # 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_manager: LoginManager = security_manager.lm + login_manager.reload_user(AnonymousUserMixin()) + + add_extra_log_payload( + embedded_dashboard_id=uuid, dashboard_version="v2", + ) + + bootstrap_data = { + "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=utils.pessimistic_json_iso_dttm_ser + ), + ) diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index 737eecd4cf5cb..5b11f0930a782 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -141,6 +141,7 @@ def init_views(self) -> None: from superset.datasets.api import DatasetRestApi from superset.datasets.columns.api import DatasetColumnsRestApi from superset.datasets.metrics.api import DatasetMetricRestApi + from superset.embedded.view import EmbeddedView from superset.explore.form_data.api import ExploreFormDataRestApi from superset.explore.permalink.api import ExplorePermalinkRestApi from superset.importexport.api import ImportExportRestApi @@ -166,7 +167,6 @@ def init_views(self) -> None: Dashboard, DashboardModelView, DashboardModelViewAsync, - EmbeddedView, ) from superset.views.database.views import ( ColumnarToDatabaseView, diff --git a/superset/models/embedded_dashboard.py b/superset/models/embedded_dashboard.py index a0cf1760ec07e..f6500057f3e32 100644 --- a/superset/models/embedded_dashboard.py +++ b/superset/models/embedded_dashboard.py @@ -31,7 +31,11 @@ class EmbeddedDashboard( """ A configuration of embedding for a dashboard. + Currently, the only embeddable resource is the Dashboard. + If we add new embeddable resource types, this model should probably be renamed. + References the dashboard, and contains a config for embedding that dashboard. + This data model allows multiple configurations for a given dashboard, but at this time the API only allows setting one. """ diff --git a/superset/security/manager.py b/superset/security/manager.py index 37801ac76e977..31da0955afb52 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -1269,10 +1269,8 @@ def has_rbac_access() -> bool: for dashboard_role in dashboard.roles ) - if self.is_guest_user(): - can_access = self.has_guest_access( - GuestTokenResourceType.DASHBOARD, dashboard.id - ) + if self.is_guest_user() and dashboard.embedded: + can_access = self.has_guest_access(dashboard) else: can_access = ( is_user_admin() @@ -1410,15 +1408,14 @@ def get_current_guest_user_if_guest(self) -> Optional[GuestUser]: return g.user return None - def has_guest_access( - self, resource_type: GuestTokenResourceType, resource_id: Union[str, int] - ) -> bool: + def has_guest_access(self, dashboard: "Dashboard") -> bool: user = self.get_current_guest_user_if_guest() - if not user: + if not user or not dashboard.embedded: return False - strid = str(resource_id) for resource in user.resources: - if resource["type"] == resource_type.value and str(resource["id"]) == strid: + if resource["type"] == GuestTokenResourceType.DASHBOARD.value and str( + resource["id"] + ) == str(dashboard.embedded[0].uuid): return True return False diff --git a/superset/views/dashboard/views.py b/superset/views/dashboard/views.py index 7cbaf626e2dde..5ec79bee39ce5 100644 --- a/superset/views/dashboard/views.py +++ b/superset/views/dashboard/views.py @@ -134,55 +134,6 @@ def new(self) -> FlaskResponse: # pylint: disable=no-self-use return redirect(f"/superset/dashboard/{new_dashboard.id}/?edit=true") -class EmbeddedView(BaseSupersetView): - """ The views for embedded resources to be rendered in an iframe """ - - route_base = "/embedded" - - @expose("/") - @event_logger.log_this_with_extra_payload - def embedded( - self, - uuid: str, - add_extra_log_payload: Callable[..., None] = lambda **kwargs: None, - ) -> FlaskResponse: - """ - Server side rendering for a dashboard - :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"): - return Response(status=404) - - embedded: EmbeddedDashboard = db.session.get(EmbeddedDashboard, uuid) - if not embedded: - return Response(status=404) - - # 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_manager: LoginManager = security_manager.lm - login_manager.reload_user(AnonymousUserMixin()) - - add_extra_log_payload( - embedded_dashboard_id=uuid, dashboard_version="v2", - ) - - bootstrap_data = { - "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=utils.pessimistic_json_iso_dttm_ser - ), - ) - - class DashboardModelViewAsync(DashboardModelView): # pylint: disable=too-many-ancestors route_base = "/dashboardasync" class_permission_name = "Dashboard" diff --git a/tests/integration_tests/dashboards/dao_tests.py b/tests/integration_tests/dashboards/dao_tests.py index 356256301f5f3..4a02cd7107acd 100644 --- a/tests/integration_tests/dashboards/dao_tests.py +++ b/tests/integration_tests/dashboards/dao_tests.py @@ -119,16 +119,3 @@ def test_get_dashboard_changed_on(self): DashboardDAO.set_dash_metadata(dashboard, original_data) session.merge(dashboard) session.commit() - - @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices") - def test_upsert_embedded_dashboard(self): - dash = db.session.query(Dashboard).filter_by(slug="world_health").first() - assert not dash.embedded - DashboardDAO.upsert_embedded_dashboard(dash, ["test.example.com"]) - assert dash.embedded - self.assertEqual(dash.embedded[0].allowed_domains, ["test.example.com"]) - original_uuid = dash.embedded[0].uuid - self.assertIsNotNone(original_uuid) - DashboardDAO.upsert_embedded_dashboard(dash, []) - self.assertEqual(dash.embedded[0].allowed_domains, []) - self.assertEqual(dash.embedded[0].uuid, original_uuid) diff --git a/tests/integration_tests/embedded/__init__.py b/tests/integration_tests/embedded/__init__.py index e69de29bb2d1d..13a83393a9124 100644 --- a/tests/integration_tests/embedded/__init__.py +++ b/tests/integration_tests/embedded/__init__.py @@ -0,0 +1,16 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. diff --git a/tests/integration_tests/embedded/dao_tests.py b/tests/integration_tests/embedded/dao_tests.py index e69de29bb2d1d..757a0b51131b3 100644 --- a/tests/integration_tests/embedded/dao_tests.py +++ b/tests/integration_tests/embedded/dao_tests.py @@ -0,0 +1,51 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# isort:skip_file +import pytest + +import tests.integration_tests.test_app # pylint: disable=unused-import +from superset import db +from superset.embedded.dao import EmbeddedDAO +from superset.models.dashboard import Dashboard +from tests.integration_tests.base_tests import SupersetTestCase +from tests.integration_tests.fixtures.world_bank_dashboard import ( + load_world_bank_dashboard_with_slices, + load_world_bank_data, +) + + +class TestEmbeddedDAO(SupersetTestCase): + @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices") + def test_upsert(self): + dash = db.session.query(Dashboard).filter_by(slug="world_health").first() + assert not dash.embedded + EmbeddedDAO.upsert(dash, ["test.example.com"]) + assert dash.embedded + self.assertEqual(dash.embedded[0].allowed_domains, ["test.example.com"]) + original_uuid = dash.embedded[0].uuid + self.assertIsNotNone(original_uuid) + EmbeddedDAO.upsert(dash, []) + self.assertEqual(dash.embedded[0].allowed_domains, []) + self.assertEqual(dash.embedded[0].uuid, original_uuid) + + @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices") + def test_get_by_uuid(self): + dash = db.session.query(Dashboard).filter_by(slug="world_health").first() + uuid = str(EmbeddedDAO.upsert(dash, ["test.example.com"]).uuid) + db.session.expire_all() + embedded = EmbeddedDAO.find_by_id(uuid) + self.assertNotNone(embedded) diff --git a/tests/integration_tests/security/guest_token_security_tests.py b/tests/integration_tests/security/guest_token_security_tests.py index 9ca34198dbdf2..732bb9ff4a6aa 100644 --- a/tests/integration_tests/security/guest_token_security_tests.py +++ b/tests/integration_tests/security/guest_token_security_tests.py @@ -22,6 +22,7 @@ from superset import db, security_manager from superset.dashboards.commands.exceptions import DashboardAccessDeniedError +from superset.embedded.dao import EmbeddedDAO from superset.exceptions import SupersetSecurityException from superset.models.dashboard import Dashboard from superset.security.guest_token import GuestTokenResourceType @@ -37,14 +38,9 @@ "superset.extensions.feature_flag_manager._feature_flags", EMBEDDED_SUPERSET=True, ) class TestGuestUserSecurity(SupersetTestCase): - # This test doesn't use a dashboard fixture, the next test does. - # That way tests are faster. - - resource_id = 42 - def authorized_guest(self): return security_manager.get_guest_user_from_token( - {"user": {}, "resources": [{"type": "dashboard", "id": self.resource_id}]} + {"user": {}, "resources": [{"type": "dashboard", "id": "some-uuid"}]} ) def test_is_guest_user__regular_user(self): @@ -82,87 +78,77 @@ def test_get_guest_user__guest_user(self): guest_user = security_manager.get_current_guest_user_if_guest() self.assertEqual(guest_user, g.user) + def test_get_guest_user_roles_explicit(self): + guest = self.authorized_guest() + roles = security_manager.get_user_roles(guest) + self.assertEqual(guest.roles, roles) + + def test_get_guest_user_roles_implicit(self): + guest = self.authorized_guest() + g.user = guest + + roles = security_manager.get_user_roles() + self.assertEqual(guest.roles, roles) + + +@mock.patch.dict( + "superset.extensions.feature_flag_manager._feature_flags", EMBEDDED_SUPERSET=True, +) +@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") +class TestGuestUserDashboardAccess(SupersetTestCase): + def setUp(self) -> None: + self.dash = db.session.query(Dashboard).filter_by(slug="births").first() + self.embedded = EmbeddedDAO.upsert(self.dash, []) + self.authorized_guest = security_manager.get_guest_user_from_token( + { + "user": {}, + "resources": [{"type": "dashboard", "id": str(self.embedded.uuid)}], + } + ) + self.unauthorized_guest = security_manager.get_guest_user_from_token( + {"user": {}, "resources": [{"type": "dashboard", "id": "not-a-uuid"}]} + ) + def test_has_guest_access__regular_user(self): g.user = security_manager.find_user("admin") - has_guest_access = security_manager.has_guest_access( - GuestTokenResourceType.DASHBOARD, self.resource_id - ) + has_guest_access = security_manager.has_guest_access(self.dash) self.assertFalse(has_guest_access) def test_has_guest_access__anonymous_user(self): g.user = security_manager.get_anonymous_user() - has_guest_access = security_manager.has_guest_access( - GuestTokenResourceType.DASHBOARD, self.resource_id - ) + has_guest_access = security_manager.has_guest_access(self.dash) self.assertFalse(has_guest_access) def test_has_guest_access__authorized_guest_user(self): - g.user = self.authorized_guest() - has_guest_access = security_manager.has_guest_access( - GuestTokenResourceType.DASHBOARD, self.resource_id - ) + g.user = self.authorized_guest + has_guest_access = security_manager.has_guest_access(self.dash) self.assertTrue(has_guest_access) def test_has_guest_access__authorized_guest_user__non_zero_resource_index(self): - guest = self.authorized_guest() + # set up a user who has authorized access, plus another resource + guest = self.authorized_guest guest.resources = [ - {"type": "dashboard", "id": self.resource_id - 1} + {"type": "dashboard", "id": "not-a-real-id"} ] + guest.resources g.user = guest - has_guest_access = security_manager.has_guest_access( - GuestTokenResourceType.DASHBOARD, self.resource_id - ) + has_guest_access = security_manager.has_guest_access(self.dash) self.assertTrue(has_guest_access) def test_has_guest_access__unauthorized_guest_user__different_resource_id(self): g.user = security_manager.get_guest_user_from_token( - { - "user": {}, - "resources": [{"type": "dashboard", "id": self.resource_id - 1}], - } - ) - has_guest_access = security_manager.has_guest_access( - GuestTokenResourceType.DASHBOARD, self.resource_id + {"user": {}, "resources": [{"type": "dashboard", "id": "not-a-real-id"}],} ) + has_guest_access = security_manager.has_guest_access(self.dash) self.assertFalse(has_guest_access) def test_has_guest_access__unauthorized_guest_user__different_resource_type(self): g.user = security_manager.get_guest_user_from_token( - {"user": {}, "resources": [{"type": "dirt", "id": self.resource_id}]} - ) - has_guest_access = security_manager.has_guest_access( - GuestTokenResourceType.DASHBOARD, self.resource_id + {"user": {}, "resources": [{"type": "dirt", "id": self.embedded.id}]} ) + has_guest_access = security_manager.has_guest_access(self.dash) self.assertFalse(has_guest_access) - def test_get_guest_user_roles_explicit(self): - guest = self.authorized_guest() - roles = security_manager.get_user_roles(guest) - self.assertEqual(guest.roles, roles) - - def test_get_guest_user_roles_implicit(self): - guest = self.authorized_guest() - g.user = guest - - roles = security_manager.get_user_roles() - self.assertEqual(guest.roles, roles) - - -@mock.patch.dict( - "superset.extensions.feature_flag_manager._feature_flags", EMBEDDED_SUPERSET=True, -) -@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") -class TestGuestUserDashboardAccess(SupersetTestCase): - def setUp(self) -> None: - self.dash = db.session.query(Dashboard).filter_by(slug="births").first() - self.authorized_guest = security_manager.get_guest_user_from_token( - {"user": {}, "resources": [{"type": "dashboard", "id": self.dash.id}]} - ) - self.unauthorized_guest = security_manager.get_guest_user_from_token( - {"user": {}, "resources": [{"type": "dashboard", "id": self.dash.id + 1}]} - ) - def test_chart_raise_for_access_as_guest(self): chart = self.dash.slices[0] g.user = self.authorized_guest diff --git a/tox.ini b/tox.ini new file mode 100644 index 0000000000000..774aa681b6396 --- /dev/null +++ b/tox.ini @@ -0,0 +1,175 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +# Remember to start celery workers to run celery tests, e.g. +# celery --app=superset.tasks.celery_app:app worker -Ofair -c 2 +[testenv] +basepython = python3.8 +ignore_basepython_conflict = true +commands = + superset db upgrade + superset init + # use -s to be able to use break pointers. + # no args or tests/* can be passed as an argument to run all tests + pytest -s {posargs} +deps = + -rrequirements/testing.txt +setenv = + PYTHONPATH = {toxinidir} + SUPERSET_TESTENV = true + SUPERSET_CONFIG = tests.integration_tests.superset_test_config + SUPERSET_HOME = {envtmpdir} + mysql: SUPERSET__SQLALCHEMY_DATABASE_URI = mysql://mysqluser:mysqluserpassword@localhost/superset?charset=utf8 + postgres: SUPERSET__SQLALCHEMY_DATABASE_URI = postgresql+psycopg2://superset:superset@localhost/test + sqlite: SUPERSET__SQLALCHEMY_DATABASE_URI = sqlite:////{envtmpdir}/superset.db + mysql-presto: SUPERSET__SQLALCHEMY_DATABASE_URI = mysql://mysqluser:mysqluserpassword@localhost/superset?charset=utf8 + # docker run -p 8080:8080 --name presto starburstdata/presto + mysql-presto: SUPERSET__SQLALCHEMY_EXAMPLES_URI = presto://localhost:8080/memory/default + # based on https://github.com/big-data-europe/docker-hadoop + # clone the repo & run docker-compose up -d to test locally + mysql-hive: SUPERSET__SQLALCHEMY_DATABASE_URI = mysql://mysqluser:mysqluserpassword@localhost/superset?charset=utf8 + mysql-hive: SUPERSET__SQLALCHEMY_EXAMPLES_URI = hive://localhost:10000/default + # make sure that directory is accessible by docker + hive: UPLOAD_FOLDER = /tmp/.superset/app/static/uploads/ +usedevelop = true +allowlist_externals = + npm + pkill + +[testenv:cypress] +setenv = + PYTHONPATH = {toxinidir} + SUPERSET_TESTENV = true + SUPERSET_CONFIG = tests.integration_tests.superset_test_config + SUPERSET_HOME = {envtmpdir} +commands = + npm install -g npm@'>=6.5.0' + pip install -e {toxinidir}/ + {toxinidir}/superset-frontend/cypress_build.sh +commands_post = + pkill -if "python {envbindir}/flask" + +[testenv:cypress-dashboard] +setenv = + PYTHONPATH = {toxinidir} + SUPERSET_TESTENV = true + SUPERSET_CONFIG = tests.integration_tests.superset_test_config + SUPERSET_HOME = {envtmpdir} +commands = + npm install -g npm@'>=6.5.0' + pip install -e {toxinidir}/ + {toxinidir}/superset-frontend/cypress_build.sh dashboard +commands_post = + pkill -if "python {envbindir}/flask" + +[testenv:cypress-explore] +setenv = + PYTHONPATH = {toxinidir} + SUPERSET_TESTENV = true + SUPERSET_CONFIG = tests.integration_tests.superset_test_config + SUPERSET_HOME = {envtmpdir} +commands = + npm install -g npm@'>=6.5.0' + pip install -e {toxinidir}/ + {toxinidir}/superset-frontend/cypress_build.sh explore +commands_post = + pkill -if "python {envbindir}/flask" + +[testenv:cypress-sqllab] +setenv = + PYTHONPATH = {toxinidir} + SUPERSET_TESTENV = true + SUPERSET_CONFIG = tests.integration_tests.superset_test_config + SUPERSET_HOME = {envtmpdir} +commands = + npm install -g npm@'>=6.5.0' + pip install -e {toxinidir}/ + {toxinidir}/superset-frontend/cypress_build.sh sqllab +commands_post = + pkill -if "python {envbindir}/flask" + +[testenv:cypress-sqllab-backend-persist] +setenv = + PYTHONPATH = {toxinidir} + SUPERSET_TESTENV = true + SUPERSET_CONFIG = tests.integration_tests.superset_test_config + SUPERSET_HOME = {envtmpdir} +commands = + npm install -g npm@'>=6.5.0' + pip install -e {toxinidir}/ + {toxinidir}/superset-frontend/cypress_build.sh sqllab +commands_post = + pkill -if "python {envbindir}/flask" + +[testenv:eslint] +changedir = {toxinidir}/superset-frontend +commands = + npm run lint +deps = + +[testenv:fossa] +commands = + {toxinidir}/scripts/fossa.sh +deps = +passenv = * + +[testenv:javascript] +commands = + npm install -g npm@'>=6.5.0' + {toxinidir}/superset-frontend/js_build.sh +deps = + +[testenv:license-check] +commands = + {toxinidir}/scripts/check_license.sh +passenv = * +whitelist_externals = + {toxinidir}/scripts/check_license.sh +deps = + +[testenv:pre-commit] +commands = + pre-commit run --all-files +deps = + -rrequirements/integration.txt +skip_install = true + +[testenv:pylint] +commands = + pylint superset +deps = + -rrequirements/testing.txt + +[testenv:thumbnails] +setenv = + SUPERSET_CONFIG = tests.integration_tests.superset_test_config_thumbnails +deps = + -rrequirements/testing.txt + +[tox] +envlist = + cypress-dashboard + cypress-explore + cypress-sqllab + cypress-sqllab-backend-persist + eslint + fossa + javascript + license-check + pre-commit + pylint +skipsdist = true diff --git a/yarn.lock b/yarn.lock new file mode 100644 index 0000000000000..4a5801883d149 --- /dev/null +++ b/yarn.lock @@ -0,0 +1,2 @@ +# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. +# yarn lockfile v1 From 4250165ebdeadda70ba657728cda2a8366a28deb Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian <1858430+suddjian@users.noreply.github.com> Date: Tue, 29 Mar 2022 10:57:15 -0700 Subject: [PATCH 11/26] Update superset/embedded/view.py --- superset/embedded/view.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/embedded/view.py b/superset/embedded/view.py index 56086bc36e02d..87e2acebc4059 100644 --- a/superset/embedded/view.py +++ b/superset/embedded/view.py @@ -41,7 +41,7 @@ def embedded( add_extra_log_payload: Callable[..., None] = lambda **kwargs: None, ) -> FlaskResponse: """ - Server side rendering for a dashboard + 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 From c50e920555b21a89b5a5f088dcdcda388f66eef4 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Tue, 29 Mar 2022 11:01:24 -0700 Subject: [PATCH 12/26] use the curator check --- superset-frontend/src/dashboard/components/Header/index.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/dashboard/components/Header/index.jsx b/superset-frontend/src/dashboard/components/Header/index.jsx index 0fd49d354cd13..7345d04f47cdd 100644 --- a/superset-frontend/src/dashboard/components/Header/index.jsx +++ b/superset-frontend/src/dashboard/components/Header/index.jsx @@ -672,7 +672,7 @@ class Header extends React.PureComponent { /> )} - {isFeatureEnabled(FeatureFlag.EMBEDDED_SUPERSET) && ( + {userCanCurate && ( Date: Tue, 29 Mar 2022 13:45:51 -0700 Subject: [PATCH 13/26] put back old endpoint, for now --- superset/views/dashboard/views.py | 38 +++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/superset/views/dashboard/views.py b/superset/views/dashboard/views.py index 5ec79bee39ce5..9af133a8d7ed9 100644 --- a/superset/views/dashboard/views.py +++ b/superset/views/dashboard/views.py @@ -133,6 +133,44 @@ def new(self) -> FlaskResponse: # pylint: disable=no-self-use db.session.commit() return redirect(f"/superset/dashboard/{new_dashboard.id}/?edit=true") + @expose("//embedded") + @event_logger.log_this_with_extra_payload + def embedded( + self, + dashboard_id_or_slug: str, + add_extra_log_payload: Callable[..., None] = lambda **kwargs: None, + ) -> FlaskResponse: + """ + Server side rendering for a dashboard + :param dashboard_id_or_slug: identifier for dashboard. used in the decorators + :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"): + return Response(status=404) + + # 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_manager: LoginManager = security_manager.lm + login_manager.reload_user(AnonymousUserMixin()) + + add_extra_log_payload( + dashboard_id=dashboard_id_or_slug, dashboard_version="v2", + ) + + bootstrap_data = { + "common": common_bootstrap_payload(), + } + + return self.render_template( + "superset/spa.html", + entry="embedded", + bootstrap_data=json.dumps( + bootstrap_data, default=utils.pessimistic_json_iso_dttm_ser + ), + ) + class DashboardModelViewAsync(DashboardModelView): # pylint: disable=too-many-ancestors route_base = "/dashboardasync" From 6d0336160f71d2059a789ddc3aca065470d90c39 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Tue, 29 Mar 2022 14:20:11 -0700 Subject: [PATCH 14/26] allow access by either embedded.uuid or dashboard.id --- superset/security/manager.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/superset/security/manager.py b/superset/security/manager.py index 31da0955afb52..85ede02074a80 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -1414,8 +1414,8 @@ def has_guest_access(self, dashboard: "Dashboard") -> bool: return False for resource in user.resources: - if resource["type"] == GuestTokenResourceType.DASHBOARD.value and str( - resource["id"] - ) == str(dashboard.embedded[0].uuid): + strid = str(resource["id"]) + # TODO once the uuid is deployed, only check uuid here (no downtime plz) + if strid == str(dashboard.id) or strid == str(dashboard.embedded[0].uuid): return True return False From fd543a4b76fd5fea51b015bd2ef1a7542276eb03 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Tue, 29 Mar 2022 16:11:43 -0700 Subject: [PATCH 15/26] keep the old endpoint around, for the time being --- superset-frontend/src/embedded/index.tsx | 29 ++++++++++++------------ superset/dashboards/filters.py | 19 ++++++++++++++-- superset/security/manager.py | 14 +++++++++--- superset/views/dashboard/views.py | 1 + 4 files changed, 44 insertions(+), 19 deletions(-) diff --git a/superset-frontend/src/embedded/index.tsx b/superset-frontend/src/embedded/index.tsx index 819502e2aa802..5dc43c6b42295 100644 --- a/superset-frontend/src/embedded/index.tsx +++ b/superset-frontend/src/embedded/index.tsx @@ -45,20 +45,21 @@ const LazyDashboardPage = lazy( ), ); +const EmbeddedRoute = () => ( + }> + + + + + + + +); + const EmbeddedApp = () => ( - - }> - - - - - - - - + + ); @@ -66,9 +67,9 @@ const appMountPoint = document.getElementById('app')!; const MESSAGE_TYPE = '__embedded_comms__'; -if (!window.parent) { +if (!window.parent || window.parent === window) { appMountPoint.innerHTML = - 'This page is intended to be embedded in an iframe, but no window.parent was found.'; + 'This page is intended to be embedded in an iframe, but it looks like that is not the case.'; } // if the page is embedded in an origin that hasn't diff --git a/superset/dashboards/filters.py b/superset/dashboards/filters.py index e253f6f4896ad..58e0b9ee8983a 100644 --- a/superset/dashboards/filters.py +++ b/superset/dashboards/filters.py @@ -14,7 +14,8 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -from typing import Any, Optional +import uuid +from typing import Any, Optional, Union from flask import g from flask_appbuilder.security.sqla.models import Role @@ -60,6 +61,14 @@ class DashboardFavoriteFilter( # pylint: disable=too-few-public-methods model = Dashboard +def is_uuid(value: Union[str, int]) -> bool: + try: + uuid.UUID(str(value)) + return True + except ValueError: + return False + + class DashboardAccessFilter(BaseFilter): # pylint: disable=too-few-public-methods """ List dashboards with the following criteria: @@ -141,12 +150,18 @@ def apply(self, query: Query, value: Any) -> Query: for r in guest_user.resources if r["type"] == GuestTokenResourceType.DASHBOARD.value ] - feature_flagged_filters.append( + + # TODO (embedded): only use uuid filter once uuids are rolled out + condition = ( Dashboard.embedded.any( EmbeddedDashboard.uuid.in_(embedded_dashboard_ids) ) + if any(is_uuid(id_) for id_ in embedded_dashboard_ids) + else Dashboard.id.in_(embedded_dashboard_ids) ) + feature_flagged_filters.append(condition) + query = query.filter( or_( Dashboard.id.in_(owner_ids_query), diff --git a/superset/security/manager.py b/superset/security/manager.py index 85ede02074a80..2341f0c03e4da 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -1410,12 +1410,20 @@ def get_current_guest_user_if_guest(self) -> Optional[GuestUser]: def has_guest_access(self, dashboard: "Dashboard") -> bool: user = self.get_current_guest_user_if_guest() - if not user or not dashboard.embedded: + if not user: + return False + + # TODO (embedded): remove this check once uuids are rolled out + for resource in user.resources: + strid = str(resource["id"]) + if str(resource["id"]) == str(dashboard.id): + return True + + if not dashboard.embedded: return False for resource in user.resources: strid = str(resource["id"]) - # TODO once the uuid is deployed, only check uuid here (no downtime plz) - if strid == str(dashboard.id) or strid == str(dashboard.embedded[0].uuid): + if strid == str(dashboard.embedded[0].uuid): return True return False diff --git a/superset/views/dashboard/views.py b/superset/views/dashboard/views.py index 9af133a8d7ed9..548aa3b0e225d 100644 --- a/superset/views/dashboard/views.py +++ b/superset/views/dashboard/views.py @@ -161,6 +161,7 @@ def embedded( bootstrap_data = { "common": common_bootstrap_payload(), + "embedded": {"dashboard_id": dashboard_id_or_slug}, } return self.render_template( From d704ecdff85b31f4f08f86c4ce556cd6ec3ffb58 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Tue, 29 Mar 2022 19:14:29 -0700 Subject: [PATCH 16/26] openapi --- superset/dashboards/api.py | 60 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 57 insertions(+), 3 deletions(-) diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py index dc9cd47819a94..08b1d03e5ef48 100644 --- a/superset/dashboards/api.py +++ b/superset/dashboards/api.py @@ -1044,9 +1044,15 @@ def get_embedded(self, dashboard: Dashboard) -> Response: """Response Returns the dashboard's embedded configuration --- - post: + get: description: >- Returns the dashboard's embedded configuration + parameters: + - in: path + schema: + type: string + name: id_or_slug + description: The dashboard id or slug responses: 200: description: Result contains the embedded dashboard config @@ -1084,6 +1090,41 @@ def set_embedded(self, dashboard: Dashboard) -> Response: post: description: >- Sets a dashboard's embedded configuration. + parameters: + - in: path + schema: + type: string + name: id_or_slug + description: The dashboard id or slug + requestBody: + description: The embedded configuration to set + required: true + content: + application/json: + schema: EmbeddedDashboardConfigSchema + responses: + 200: + description: Successfully set the configuration + content: + application/json: + schema: + type: object + properties: + result: + $ref: '#/components/schemas/EmbeddedDashboardResponseSchema' + 401: + $ref: '#/components/responses/401' + 500: + $ref: '#/components/responses/500' + put: + description: >- + Sets a dashboard's embedded configuration. + parameters: + - in: path + schema: + type: string + name: id_or_slug + description: The dashboard id or slug requestBody: description: The embedded configuration to set required: true @@ -1127,16 +1168,29 @@ def delete_embedded(self, dashboard: Dashboard) -> Response: """Response Removes a dashboard's embedded configuration. --- - post: + delete: description: >- Removes a dashboard's embedded configuration. + parameters: + - in: path + schema: + type: string + name: id_or_slug + description: The dashboard id or slug responses: 200: description: Successfully removed the configuration + content: + application/json: + schema: + type: object + properties: + message: + type: string 401: $ref: '#/components/responses/401' 500: $ref: '#/components/responses/500' """ dashboard.embedded = [] - return self.response(200) + return self.response(200, message="OK") From 5d7546a69ab4561678bdb8cf902bd1547dfff805 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Tue, 29 Mar 2022 19:27:39 -0700 Subject: [PATCH 17/26] lint --- superset/dashboards/api.py | 16 ++++++++-------- superset/dashboards/dao.py | 1 - superset/embedded/dao.py | 18 ++++-------------- superset/embedded/view.py | 11 +++++++---- superset/models/embedded_dashboard.py | 9 ++++----- superset/security/manager.py | 1 - superset/views/dashboard/views.py | 1 - 7 files changed, 23 insertions(+), 34 deletions(-) diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py index 08b1d03e5ef48..7fdb9bf55c0f6 100644 --- a/superset/dashboards/api.py +++ b/superset/dashboards/api.py @@ -282,9 +282,9 @@ def __repr__(self) -> str: @statsd_metrics @event_logger.log_this_with_context( action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.get", - log_to_statsd=False, # pylint: disable=arguments-renamed + log_to_statsd=False, ) - @with_dashboard + @with_dashboard # pylint: disable=arguments-renamed def get(self, dash: Dashboard) -> Response: """Gets a dashboard --- @@ -1037,9 +1037,9 @@ def import_(self) -> Response: @statsd_metrics @event_logger.log_this_with_context( action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.get_embedded", - log_to_statsd=False, # pylint: disable=arguments-renamed + log_to_statsd=False, ) - @with_dashboard + @with_dashboard # pylint: disable=arguments-renamed def get_embedded(self, dashboard: Dashboard) -> Response: """Response Returns the dashboard's embedded configuration @@ -1080,9 +1080,9 @@ def get_embedded(self, dashboard: Dashboard) -> Response: @statsd_metrics @event_logger.log_this_with_context( action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.set_embedded", - log_to_statsd=False, # pylint: disable=arguments-renamed + log_to_statsd=False, ) - @with_dashboard + @with_dashboard # pylint: disable=arguments-renamed def set_embedded(self, dashboard: Dashboard) -> Response: """Response Sets a dashboard's embedded configuration. @@ -1161,9 +1161,9 @@ def set_embedded(self, dashboard: Dashboard) -> Response: @statsd_metrics @event_logger.log_this_with_context( action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.delete_embedded", - log_to_statsd=False, # pylint: disable=arguments-renamed + log_to_statsd=False, ) - @with_dashboard + @with_dashboard # pylint: disable=arguments-renamed def delete_embedded(self, dashboard: Dashboard) -> Response: """Response Removes a dashboard's embedded configuration. diff --git a/superset/dashboards/dao.py b/superset/dashboards/dao.py index 90ffcccc0d998..ce6e30f8d4beb 100644 --- a/superset/dashboards/dao.py +++ b/superset/dashboards/dao.py @@ -28,7 +28,6 @@ from superset.extensions import db from superset.models.core import FavStar, FavStarClassName from superset.models.dashboard import Dashboard -from superset.models.embedded_dashboard import EmbeddedDashboard from superset.models.slice import Slice from superset.utils.dashboard_filter_scopes_converter import copy_filter_scopes diff --git a/superset/embedded/dao.py b/superset/embedded/dao.py index d78cfd53aac0a..957a7242a77d3 100644 --- a/superset/embedded/dao.py +++ b/superset/embedded/dao.py @@ -14,23 +14,13 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -import json import logging -from datetime import datetime -from typing import Any, Dict, List, Optional, Union +from typing import Any, Dict, List -from sqlalchemy.exc import SQLAlchemyError - -from superset import security_manager from superset.dao.base import BaseDAO -from superset.dashboards.commands.exceptions import DashboardNotFoundError -from superset.dashboards.filters import DashboardAccessFilter from superset.extensions import db -from superset.models.core import FavStar, FavStarClassName from superset.models.dashboard import Dashboard from superset.models.embedded_dashboard import EmbeddedDashboard -from superset.models.slice import Slice -from superset.utils.dashboard_filter_scopes_converter import copy_filter_scopes logger = logging.getLogger(__name__) @@ -46,9 +36,9 @@ def upsert(dashboard: Dashboard, allowed_domains: List[str]) -> EmbeddedDashboar Sets up a dashboard to be embeddable. Upsert is used to preserve the embedded_dashboard uuid across updates. """ - embedded: EmbeddedDashboard = dashboard.embedded[ - 0 - ] if dashboard.embedded else EmbeddedDashboard() + embedded: EmbeddedDashboard = ( + dashboard.embedded[0] if dashboard.embedded else EmbeddedDashboard() + ) embedded.allow_domain_list = ",".join(allowed_domains) dashboard.embedded = [embedded] db.session.commit() diff --git a/superset/embedded/view.py b/superset/embedded/view.py index 87e2acebc4059..487850b728851 100644 --- a/superset/embedded/view.py +++ b/superset/embedded/view.py @@ -17,7 +17,7 @@ import json from typing import Callable -from flask import abort, Response +from flask import abort from flask_appbuilder import expose from flask_login import AnonymousUserMixin, LoginManager @@ -29,7 +29,7 @@ class EmbeddedView(BaseSupersetView): - """ The views for embedded resources to be rendered in an iframe """ + """The views for embedded resources to be rendered in an iframe""" route_base = "/embedded" @@ -60,12 +60,15 @@ def embedded( login_manager.reload_user(AnonymousUserMixin()) add_extra_log_payload( - embedded_dashboard_id=uuid, dashboard_version="v2", + embedded_dashboard_id=uuid, + dashboard_version="v2", ) bootstrap_data = { "common": common_bootstrap_payload(), - "embedded": {"dashboard_id": embedded.dashboard_id,}, + "embedded": { + "dashboard_id": embedded.dashboard_id, + }, } return self.render_template( diff --git a/superset/models/embedded_dashboard.py b/superset/models/embedded_dashboard.py index f6500057f3e32..7718bc886f97a 100644 --- a/superset/models/embedded_dashboard.py +++ b/superset/models/embedded_dashboard.py @@ -25,10 +25,7 @@ from superset.models.helpers import AuditMixinNullable -class EmbeddedDashboard( - Model, AuditMixinNullable -): # pylint: disable=too-few-public-methods - +class EmbeddedDashboard(Model, AuditMixinNullable): """ A configuration of embedding for a dashboard. Currently, the only embeddable resource is the Dashboard. @@ -46,7 +43,9 @@ class EmbeddedDashboard( allow_domain_list = Column(Text) # reference the `allowed_domains` property instead dashboard_id = Column(Integer, ForeignKey("dashboards.id"), nullable=False) dashboard = relationship( - "Dashboard", back_populates="embedded", foreign_keys=[dashboard_id], + "Dashboard", + back_populates="embedded", + foreign_keys=[dashboard_id], ) @property diff --git a/superset/security/manager.py b/superset/security/manager.py index 03e0f7cad613b..0135019a6a646 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -68,7 +68,6 @@ from superset.security.guest_token import ( GuestToken, GuestTokenResources, - GuestTokenResourceType, GuestTokenRlsRule, GuestTokenUser, GuestUser, diff --git a/superset/views/dashboard/views.py b/superset/views/dashboard/views.py index 0881fa9d46582..256bb4c95c025 100644 --- a/superset/views/dashboard/views.py +++ b/superset/views/dashboard/views.py @@ -29,7 +29,6 @@ from superset import db, event_logger, is_feature_enabled, security_manager from superset.constants import MODEL_VIEW_RW_METHOD_PERMISSION_MAP, RouteMethod from superset.models.dashboard import Dashboard as DashboardModel -from superset.models.embedded_dashboard import EmbeddedDashboard from superset.superset_typing import FlaskResponse from superset.utils import core as utils from superset.views.base import ( From 2180db2251563b3649c257b5db25206cdb96b6ae Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Tue, 29 Mar 2022 19:30:12 -0700 Subject: [PATCH 18/26] lint --- .../src/dashboard/components/DashboardEmbedControls.tsx | 5 +++-- superset-frontend/src/dashboard/components/Header/index.jsx | 3 +-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/superset-frontend/src/dashboard/components/DashboardEmbedControls.tsx b/superset-frontend/src/dashboard/components/DashboardEmbedControls.tsx index 752d79691eb4d..d20cc2460090b 100644 --- a/superset-frontend/src/dashboard/components/DashboardEmbedControls.tsx +++ b/superset-frontend/src/dashboard/components/DashboardEmbedControls.tsx @@ -18,14 +18,14 @@ */ import React, { useCallback, useEffect, useState } from 'react'; import { makeApi, styled, SupersetApiError, t } from '@superset-ui/core'; +import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls'; import Modal from 'src/components/Modal'; import Loading from 'src/components/Loading'; import Button from 'src/components/Button'; -import { EmbeddedDashboard } from '../types'; import { Input } from 'src/components/Input'; import { useToasts } from 'src/components/MessageToasts/withToasts'; import { FormItem } from 'src/components/Form'; -import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls'; +import { EmbeddedDashboard } from '../types'; type Props = { dashboardId: string; @@ -162,6 +162,7 @@ export const DashboardEmbedControls = ({ dashboardId, onHide }: Props) => { {t('Superset Embedded SDK documentation.')} diff --git a/superset-frontend/src/dashboard/components/Header/index.jsx b/superset-frontend/src/dashboard/components/Header/index.jsx index 7345d04f47cdd..ad668daf79a53 100644 --- a/superset-frontend/src/dashboard/components/Header/index.jsx +++ b/superset-frontend/src/dashboard/components/Header/index.jsx @@ -52,10 +52,9 @@ import setPeriodicRunner, { stopPeriodicRender, } from 'src/dashboard/util/setPeriodicRunner'; import { options as PeriodicRefreshOptions } from 'src/dashboard/components/RefreshIntervalModal'; +import findPermission from 'src/dashboard/util/findPermission'; import { FILTER_BOX_MIGRATION_STATES } from 'src/explore/constants'; -import Modal from 'src/components/Modal'; import { DashboardEmbedModal } from '../DashboardEmbedControls'; -import findPermission from 'src/dashboard/util/findPermission'; const propTypes = { addSuccessToast: PropTypes.func.isRequired, From 4bbbd72961c7be8633d9a6647909f88e068dd69a Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Tue, 29 Mar 2022 19:55:12 -0700 Subject: [PATCH 19/26] lint --- superset/dashboards/api.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py index 7fdb9bf55c0f6..d97b5f78e3c04 100644 --- a/superset/dashboards/api.py +++ b/superset/dashboards/api.py @@ -107,7 +107,6 @@ def with_dashboard( """ def wraps(self: BaseSupersetModelRestApi, id_or_slug: str) -> Response: - # pylint: disable=arguments-differ try: dash = DashboardDAO.get_by_id_or_slug(id_or_slug) return f(self, dash) @@ -284,7 +283,8 @@ def __repr__(self) -> str: action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.get", log_to_statsd=False, ) - @with_dashboard # pylint: disable=arguments-renamed + @with_dashboard + # pylint: disable=arguments-renamed, arguments-differ def get(self, dash: Dashboard) -> Response: """Gets a dashboard --- @@ -1039,7 +1039,7 @@ def import_(self) -> Response: action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.get_embedded", log_to_statsd=False, ) - @with_dashboard # pylint: disable=arguments-renamed + @with_dashboard def get_embedded(self, dashboard: Dashboard) -> Response: """Response Returns the dashboard's embedded configuration @@ -1082,7 +1082,7 @@ def get_embedded(self, dashboard: Dashboard) -> Response: action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.set_embedded", log_to_statsd=False, ) - @with_dashboard # pylint: disable=arguments-renamed + @with_dashboard def set_embedded(self, dashboard: Dashboard) -> Response: """Response Sets a dashboard's embedded configuration. @@ -1163,7 +1163,7 @@ def set_embedded(self, dashboard: Dashboard) -> Response: action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.delete_embedded", log_to_statsd=False, ) - @with_dashboard # pylint: disable=arguments-renamed + @with_dashboard def delete_embedded(self, dashboard: Dashboard) -> Response: """Response Removes a dashboard's embedded configuration. From 86ff579e09de08d27031c7e8d26b77803461228f Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Tue, 29 Mar 2022 19:57:41 -0700 Subject: [PATCH 20/26] test stuff --- tests/integration_tests/security_tests.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/integration_tests/security_tests.py b/tests/integration_tests/security_tests.py index d8f608ecfc2b3..0b3a8b1d82d88 100644 --- a/tests/integration_tests/security_tests.py +++ b/tests/integration_tests/security_tests.py @@ -888,7 +888,9 @@ def test_views_are_secured(self): ["AuthDBView", "login"], ["AuthDBView", "logout"], ["CurrentUserRestApi", "get_me"], + # TODO (embedded) remove Dashboard:embedded after uuids have been shipped ["Dashboard", "embedded"], + ["EmbeddedView", "embedded"], ["R", "index"], ["Superset", "log"], ["Superset", "theme"], From b05ca22025cfc7a34da2aa15669f4cc95e186ae0 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Tue, 29 Mar 2022 21:34:10 -0700 Subject: [PATCH 21/26] lint, test --- superset/models/dashboard.py | 4 +++- tests/integration_tests/dashboards/api_tests.py | 13 +++++++++++-- .../security/guest_token_security_tests.py | 6 +++++- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/superset/models/dashboard.py b/superset/models/dashboard.py index 3f506c1e50043..50619829c289e 100644 --- a/superset/models/dashboard.py +++ b/superset/models/dashboard.py @@ -153,7 +153,9 @@ class Dashboard(Model, AuditMixinNullable, ImportExportMixin): external_url = Column(Text, nullable=True) roles = relationship(security_manager.role_model, secondary=DashboardRoles) embedded = relationship( - "EmbeddedDashboard", back_populates="dashboard", cascade="all, delete-orphan", + "EmbeddedDashboard", + back_populates="dashboard", + cascade="all, delete-orphan", ) _filter_sets = relationship( "FilterSet", back_populates="dashboard", cascade="all, delete" diff --git a/tests/integration_tests/dashboards/api_tests.py b/tests/integration_tests/dashboards/api_tests.py index d77fe22db21f7..a179fa7c7e453 100644 --- a/tests/integration_tests/dashboards/api_tests.py +++ b/tests/integration_tests/dashboards/api_tests.py @@ -388,7 +388,14 @@ def test_info_security_database(self): rv = self.get_assert_metric(uri, "info") data = json.loads(rv.data.decode("utf-8")) assert rv.status_code == 200 - assert set(data["permissions"]) == {"can_read", "can_write", "can_export"} + assert set(data["permissions"]) == { + "can_read", + "can_write", + "can_export", + "can_get_embedded", + "can_delete_embedded", + "can_set_embedded", + } @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices") def test_get_dashboard_not_found(self): @@ -1723,7 +1730,9 @@ def test_embedded_dashboards(self): # post succeeds and returns value allowed_domains = ["test.example", "embedded.example"] resp = self.post_assert_metric( - uri, {"allowed_domains": allowed_domains}, "set_embedded", + uri, + {"allowed_domains": allowed_domains}, + "set_embedded", ) self.assertEqual(resp.status_code, 200) result = json.loads(resp.data.decode("utf-8"))["result"] diff --git a/tests/integration_tests/security/guest_token_security_tests.py b/tests/integration_tests/security/guest_token_security_tests.py index b57e2d34f88b6..b70013a1113c1 100644 --- a/tests/integration_tests/security/guest_token_security_tests.py +++ b/tests/integration_tests/security/guest_token_security_tests.py @@ -91,6 +91,7 @@ def test_get_guest_user_roles_implicit(self): roles = security_manager.get_user_roles() self.assertEqual(guest.roles, roles) + @mock.patch.dict( "superset.extensions.feature_flag_manager._feature_flags", EMBEDDED_SUPERSET=True, @@ -138,7 +139,10 @@ def test_has_guest_access__authorized_guest_user__non_zero_resource_index(self): def test_has_guest_access__unauthorized_guest_user__different_resource_id(self): g.user = security_manager.get_guest_user_from_token( - {"user": {}, "resources": [{"type": "dashboard", "id": "not-a-real-id"}],} + { + "user": {}, + "resources": [{"type": "dashboard", "id": "not-a-real-id"}], + } ) has_guest_access = security_manager.has_guest_access(self.dash) self.assertFalse(has_guest_access) From 4d713bf96cbe422ec87d53b01f3f1b1c25d08a16 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Tue, 29 Mar 2022 22:44:13 -0700 Subject: [PATCH 22/26] typo --- tests/integration_tests/embedded/dao_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration_tests/embedded/dao_tests.py b/tests/integration_tests/embedded/dao_tests.py index 757a0b51131b3..8160144a25cbc 100644 --- a/tests/integration_tests/embedded/dao_tests.py +++ b/tests/integration_tests/embedded/dao_tests.py @@ -48,4 +48,4 @@ def test_get_by_uuid(self): uuid = str(EmbeddedDAO.upsert(dash, ["test.example.com"]).uuid) db.session.expire_all() embedded = EmbeddedDAO.find_by_id(uuid) - self.assertNotNone(embedded) + self.assertIsNotNone(embedded) From 4237402beb7469609bf9ee449558b4ea81210b00 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian <1858430+suddjian@users.noreply.github.com> Date: Tue, 29 Mar 2022 22:52:33 -0700 Subject: [PATCH 23/26] Update superset-frontend/src/embedded/index.tsx --- superset-frontend/src/embedded/index.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/superset-frontend/src/embedded/index.tsx b/superset-frontend/src/embedded/index.tsx index 5dc43c6b42295..48fd93bd72ef0 100644 --- a/superset-frontend/src/embedded/index.tsx +++ b/superset-frontend/src/embedded/index.tsx @@ -58,6 +58,7 @@ const EmbeddedRoute = () => ( const EmbeddedApp = () => ( + // todo (embedded) remove this line after uuids are deployed From 067b0b4a8851fcc8a83e940cf31f8a64ab6093cb Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian <1858430+suddjian@users.noreply.github.com> Date: Tue, 29 Mar 2022 22:53:33 -0700 Subject: [PATCH 24/26] Update superset-frontend/src/embedded/index.tsx --- superset-frontend/src/embedded/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/embedded/index.tsx b/superset-frontend/src/embedded/index.tsx index 48fd93bd72ef0..afea2fd8bb94b 100644 --- a/superset-frontend/src/embedded/index.tsx +++ b/superset-frontend/src/embedded/index.tsx @@ -58,7 +58,7 @@ const EmbeddedRoute = () => ( const EmbeddedApp = () => ( - // todo (embedded) remove this line after uuids are deployed + {/* todo (embedded) remove this line after uuids are deployed */} From 7edb89f911d854b04a5a7ca6e883f1dc00e741b6 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Tue, 29 Mar 2022 23:21:58 -0700 Subject: [PATCH 25/26] fix tests --- superset/security/manager.py | 15 ++++++++++----- .../security/guest_token_security_tests.py | 9 +++++++-- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/superset/security/manager.py b/superset/security/manager.py index 0135019a6a646..ab21dbd54eed4 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -68,6 +68,7 @@ from superset.security.guest_token import ( GuestToken, GuestTokenResources, + GuestTokenResourceType, GuestTokenRlsRule, GuestTokenUser, GuestUser, @@ -1413,17 +1414,21 @@ def has_guest_access(self, dashboard: "Dashboard") -> bool: if not user: return False + dashboards = [ + r + for r in user.resources + if r["type"] == GuestTokenResourceType.DASHBOARD.value + ] + # TODO (embedded): remove this check once uuids are rolled out - for resource in user.resources: - strid = str(resource["id"]) + for resource in dashboards: if str(resource["id"]) == str(dashboard.id): return True if not dashboard.embedded: return False - for resource in user.resources: - strid = str(resource["id"]) - if strid == str(dashboard.embedded[0].uuid): + for resource in dashboards: + if str(resource["id"]) == str(dashboard.embedded[0].uuid): return True return False diff --git a/tests/integration_tests/security/guest_token_security_tests.py b/tests/integration_tests/security/guest_token_security_tests.py index b70013a1113c1..78bd8bde86f51 100644 --- a/tests/integration_tests/security/guest_token_security_tests.py +++ b/tests/integration_tests/security/guest_token_security_tests.py @@ -108,7 +108,12 @@ def setUp(self) -> None: } ) self.unauthorized_guest = security_manager.get_guest_user_from_token( - {"user": {}, "resources": [{"type": "dashboard", "id": "not-a-uuid"}]} + { + "user": {}, + "resources": [ + {"type": "dashboard", "id": "06383667-3e02-4e5e-843f-44e9c5896b6c"} + ], + } ) def test_has_guest_access__regular_user(self): @@ -149,7 +154,7 @@ def test_has_guest_access__unauthorized_guest_user__different_resource_id(self): def test_has_guest_access__unauthorized_guest_user__different_resource_type(self): g.user = security_manager.get_guest_user_from_token( - {"user": {}, "resources": [{"type": "dirt", "id": self.embedded.id}]} + {"user": {}, "resources": [{"type": "dirt", "id": self.embedded.uuid}]} ) has_guest_access = security_manager.has_guest_access(self.dash) self.assertFalse(has_guest_access) From 62b16f8f55ab88d8c55045aad9086df0eaf5d05f Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Wed, 30 Mar 2022 01:18:12 -0700 Subject: [PATCH 26/26] bump sdk --- superset-embedded-sdk/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-embedded-sdk/package.json b/superset-embedded-sdk/package.json index 88642e72327f3..49debb4ad321f 100644 --- a/superset-embedded-sdk/package.json +++ b/superset-embedded-sdk/package.json @@ -1,6 +1,6 @@ { "name": "@superset-ui/embedded-sdk", - "version": "0.1.0-alpha.6", + "version": "0.1.0-alpha.7", "description": "SDK for embedding resources from Superset into your own application", "access": "public", "keywords": [