You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2023/08/21 13:47:20 UTC

[superset] branch 3.0 updated (9ff1a63c3b -> 6a92ed9bd9)

This is an automated email from the ASF dual-hosted git repository.

michaelsmolina pushed a change to branch 3.0
in repository https://gitbox.apache.org/repos/asf/superset.git


    from 9ff1a63c3b fix: Don't let users see dashboards only because it's favorited (#24991)
     new 88383ded80 chore: Pass the dashboard id when requesting filter values (#25025)
     new fad872fffb fix: Address regression introduced in #24789 (#25008)
     new a5a027d8d1 fix: Dashboard fullscreen is removing custom URL params (#25028)
     new 994fd2301f fix: CTE queries with non-SELECT statements (#25014)
     new 6a92ed9bd9 fix(mssql): avoid trying to return a resultset for DML queries with not resultset (#24999)

The 5 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../src/components/Chart/chartAction.js            |  1 -
 .../FilterBar/FilterControls/FilterValue.tsx       |  5 +-
 .../FiltersConfigForm/FiltersConfigForm.tsx        |  6 +-
 .../dashboard/components/nativeFilters/utils.ts    |  3 +
 .../src/dashboard/util/getDashboardUrl.test.js     | 30 +++++++
 .../src/dashboard/util/getDashboardUrl.ts          | 20 +++--
 superset/db_engine_specs/mssql.py                  |  2 +
 superset/security/manager.py                       | 24 +++++-
 superset/sql_parse.py                              | 55 +++++++++++++
 tests/integration_tests/security_tests.py          | 92 +++++++++++++++++++---
 tests/unit_tests/db_engine_specs/test_mssql.py     | 11 ++-
 tests/unit_tests/sql_parse_tests.py                | 81 +++++++++++++++++++
 12 files changed, 306 insertions(+), 24 deletions(-)


[superset] 01/05: chore: Pass the dashboard id when requesting filter values (#25025)

Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

michaelsmolina pushed a commit to branch 3.0
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 88383ded80cf0a82dd4ed07917140bcb51b95701
Author: Michael S. Molina <70...@users.noreply.github.com>
AuthorDate: Fri Aug 18 13:19:19 2023 -0300

    chore: Pass the dashboard id when requesting filter values (#25025)
    
    (cherry picked from commit 52c7186b56a1591679bdc1dc537f92f545646074)
---
 superset-frontend/src/components/Chart/chartAction.js               | 1 -
 .../nativeFilters/FilterBar/FilterControls/FilterValue.tsx          | 5 ++++-
 .../FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx      | 6 ++++--
 superset-frontend/src/dashboard/components/nativeFilters/utils.ts   | 3 +++
 4 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/superset-frontend/src/components/Chart/chartAction.js b/superset-frontend/src/components/Chart/chartAction.js
index c204d58ebe..d08070fe40 100644
--- a/superset-frontend/src/components/Chart/chartAction.js
+++ b/superset-frontend/src/components/Chart/chartAction.js
@@ -228,7 +228,6 @@ export async function getChartDataRequest({
       credentials: 'include',
     };
   }
-
   const [useLegacyApi, parseMethod] = getQuerySettings(formData);
   if (useLegacyApi) {
     return legacyChartDataRequest(
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx
index b64f1bc8d9..5235edcdc3 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx
@@ -101,6 +101,9 @@ const FilterValue: React.FC<FilterControlProps> = ({
   const dependencies = useFilterDependencies(id, dataMaskSelected);
   const shouldRefresh = useShouldFilterRefresh();
   const [state, setState] = useState<ChartDataResponseResult[]>([]);
+  const dashboardId = useSelector<RootState, number>(
+    state => state.dashboardInfo.id,
+  );
   const [error, setError] = useState<ClientErrorObject>();
   const [formData, setFormData] = useState<Partial<QueryFormData>>({
     inView: false,
@@ -146,6 +149,7 @@ const FilterValue: React.FC<FilterControlProps> = ({
       groupby,
       adhoc_filters,
       time_range,
+      dashboardId,
     });
     const filterOwnState = filter.dataMask?.ownState || {};
     // TODO: We should try to improve our useEffect hooks to depend more on
@@ -170,7 +174,6 @@ const FilterValue: React.FC<FilterControlProps> = ({
       getChartDataRequest({
         formData: newFormData,
         force: false,
-        requestParams: { dashboardId: 0 },
         ownState: filterOwnState,
       })
         .then(({ response, json }) => {
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
index dcfbf3d43d..94b336af92 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
@@ -354,7 +354,9 @@ const FiltersConfigForm = (
   const [activeTabKey, setActiveTabKey] = useState<string>(
     FilterTabs.configuration.key,
   );
-
+  const dashboardId = useSelector<RootState, number>(
+    state => state.dashboardInfo.id,
+  );
   const [undoFormValues, setUndoFormValues] = useState<Record<
     string,
     any
@@ -479,6 +481,7 @@ const FiltersConfigForm = (
       }
       const formData = getFormData({
         datasetId: formFilter?.dataset?.value,
+        dashboardId,
         groupby: formFilter?.column,
         ...formFilter,
       });
@@ -492,7 +495,6 @@ const FiltersConfigForm = (
       getChartDataRequest({
         formData,
         force,
-        requestParams: { dashboardId: 0 },
       })
         .then(({ response, json }) => {
           if (isFeatureEnabled(FeatureFlag.GLOBAL_ASYNC_QUERIES)) {
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/utils.ts
index 7086ac8512..b4284a8a63 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/utils.ts
+++ b/superset-frontend/src/dashboard/components/nativeFilters/utils.ts
@@ -54,7 +54,9 @@ export const getFormData = ({
   time_range,
   granularity_sqla,
   type,
+  dashboardId,
 }: Partial<Filter> & {
+  dashboardId: number;
   datasetId?: number;
   dependencies?: object;
   groupby?: string;
@@ -91,6 +93,7 @@ export const getFormData = ({
     inView: true,
     viz_type: filterType,
     type,
+    dashboardId,
   };
 };
 


[superset] 05/05: fix(mssql): avoid trying to return a resultset for DML queries with not resultset (#24999)

Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

michaelsmolina pushed a commit to branch 3.0
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 6a92ed9bd93d7f0f3dfbcae63af6aea41250ec4e
Author: Yuval Moshe <53...@users.noreply.github.com>
AuthorDate: Mon Aug 21 14:33:26 2023 +0300

    fix(mssql): avoid trying to return a resultset for DML queries with not resultset (#24999)
    
    (cherry picked from commit 66eabc253faf2c27db5aaf5283ab2e00fedaa817)
---
 superset/db_engine_specs/mssql.py              |  2 ++
 tests/unit_tests/db_engine_specs/test_mssql.py | 11 ++++++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/superset/db_engine_specs/mssql.py b/superset/db_engine_specs/mssql.py
index 5d29d36ba8..d5cc86c859 100644
--- a/superset/db_engine_specs/mssql.py
+++ b/superset/db_engine_specs/mssql.py
@@ -141,6 +141,8 @@ class MssqlEngineSpec(BaseEngineSpec):
     def fetch_data(
         cls, cursor: Any, limit: Optional[int] = None
     ) -> list[tuple[Any, ...]]:
+        if not cursor.description:
+            return []
         data = super().fetch_data(cursor, limit)
         # Lists of `pyodbc.Row` need to be unpacked further
         return cls.pyodbc_rows_to_tuples(data)
diff --git a/tests/unit_tests/db_engine_specs/test_mssql.py b/tests/unit_tests/db_engine_specs/test_mssql.py
index 8149e60cd8..56ff887922 100644
--- a/tests/unit_tests/db_engine_specs/test_mssql.py
+++ b/tests/unit_tests/db_engine_specs/test_mssql.py
@@ -157,6 +157,14 @@ def test_extract_error_message() -> None:
     assert expected_message == error_message
 
 
+def test_fetch_data_no_description() -> None:
+    from superset.db_engine_specs.mssql import MssqlEngineSpec
+
+    cursor = mock.MagicMock()
+    cursor.description = []
+    assert MssqlEngineSpec.fetch_data(cursor) == []
+
+
 def test_fetch_data() -> None:
     from superset.db_engine_specs.base import BaseEngineSpec
     from superset.db_engine_specs.mssql import MssqlEngineSpec
@@ -166,9 +174,10 @@ def test_fetch_data() -> None:
         "pyodbc_rows_to_tuples",
         return_value="converted",
     ) as mock_pyodbc_rows_to_tuples:
+        cursor = mock.MagicMock()
         data = [(1, "foo")]
         with mock.patch.object(BaseEngineSpec, "fetch_data", return_value=data):
-            result = MssqlEngineSpec.fetch_data(None, 0)
+            result = MssqlEngineSpec.fetch_data(cursor, 0)
             mock_pyodbc_rows_to_tuples.assert_called_once_with(data)
             assert result == "converted"
 


[superset] 03/05: fix: Dashboard fullscreen is removing custom URL params (#25028)

Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

michaelsmolina pushed a commit to branch 3.0
in repository https://gitbox.apache.org/repos/asf/superset.git

commit a5a027d8d1d3f9c37bc6f6a6f518eefc0d120c98
Author: Michael S. Molina <70...@users.noreply.github.com>
AuthorDate: Fri Aug 18 15:07:31 2023 -0300

    fix: Dashboard fullscreen is removing custom URL params (#25028)
    
    (cherry picked from commit 0be175466641c918564cc592b094c6861d088206)
---
 .../src/dashboard/util/getDashboardUrl.test.js     | 30 ++++++++++++++++++++++
 .../src/dashboard/util/getDashboardUrl.ts          | 20 ++++++++++-----
 2 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/superset-frontend/src/dashboard/util/getDashboardUrl.test.js b/superset-frontend/src/dashboard/util/getDashboardUrl.test.js
index 653c17de89..c75f2c98b0 100644
--- a/superset-frontend/src/dashboard/util/getDashboardUrl.test.js
+++ b/superset-frontend/src/dashboard/util/getDashboardUrl.test.js
@@ -73,6 +73,35 @@ describe('getChartIdsFromLayout', () => {
     );
   });
 
+  it('should encode filters with missing filters', () => {
+    const urlWithStandalone = getDashboardUrl({
+      pathname: 'path',
+      filters: undefined,
+      standalone: DashboardStandaloneMode.HIDE_NAV,
+    });
+    expect(urlWithStandalone).toBe(
+      `path?standalone=${DashboardStandaloneMode.HIDE_NAV}`,
+    );
+  });
+
+  it('should preserve unknown filters', () => {
+    const windowSpy = jest.spyOn(window, 'window', 'get');
+    windowSpy.mockImplementation(() => ({
+      location: {
+        origin: 'https://localhost',
+        search: '?unkown_param=value',
+      },
+    }));
+    const urlWithStandalone = getDashboardUrl({
+      pathname: 'path',
+      standalone: DashboardStandaloneMode.HIDE_NAV,
+    });
+    expect(urlWithStandalone).toBe(
+      `path?unkown_param=value&standalone=${DashboardStandaloneMode.HIDE_NAV}`,
+    );
+    windowSpy.mockRestore();
+  });
+
   it('should process native filters key', () => {
     const windowSpy = jest.spyOn(window, 'window', 'get');
     windowSpy.mockImplementation(() => ({
@@ -89,5 +118,6 @@ describe('getChartIdsFromLayout', () => {
     expect(urlWithNativeFilters).toBe(
       'path?preselect_filters=%7B%7D&native_filters_key=024380498jdkjf-2094838',
     );
+    windowSpy.mockRestore();
   });
 });
diff --git a/superset-frontend/src/dashboard/util/getDashboardUrl.ts b/superset-frontend/src/dashboard/util/getDashboardUrl.ts
index 9c261e721d..bade710936 100644
--- a/superset-frontend/src/dashboard/util/getDashboardUrl.ts
+++ b/superset-frontend/src/dashboard/util/getDashboardUrl.ts
@@ -17,6 +17,7 @@
  * under the License.
  */
 import { JsonObject } from '@superset-ui/core';
+import { isEmpty } from 'lodash';
 import { URL_PARAMS } from 'src/constants';
 import { getUrlParam } from 'src/utils/urlUtils';
 import serializeActiveFilterValues from './serializeActiveFilterValues';
@@ -32,18 +33,23 @@ export default function getDashboardUrl({
   hash: string;
   standalone?: number | null;
 }) {
-  const newSearchParams = new URLSearchParams();
+  const newSearchParams = new URLSearchParams(window.location.search);
 
-  // convert flattened { [id_column]: values } object
-  // to nested filter object
-  newSearchParams.set(
-    URL_PARAMS.preselectFilters.name,
-    JSON.stringify(serializeActiveFilterValues(filters)),
-  );
+  if (!isEmpty(filters)) {
+    // convert flattened { [id_column]: values } object
+    // to nested filter object
+    newSearchParams.set(
+      URL_PARAMS.preselectFilters.name,
+      JSON.stringify(serializeActiveFilterValues(filters)),
+    );
+  }
 
   if (standalone) {
     newSearchParams.set(URL_PARAMS.standalone.name, standalone.toString());
+  } else {
+    newSearchParams.delete(URL_PARAMS.standalone.name);
   }
+
   const dataMaskKey = getUrlParam(URL_PARAMS.nativeFiltersKey);
   if (dataMaskKey) {
     newSearchParams.set(


[superset] 04/05: fix: CTE queries with non-SELECT statements (#25014)

Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

michaelsmolina pushed a commit to branch 3.0
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 994fd2301feb70c8a7e6f4c2b93340541d4fbcbc
Author: Daniel Vaz Gaspar <da...@gmail.com>
AuthorDate: Sat Aug 19 15:49:15 2023 +0100

    fix: CTE queries with non-SELECT statements (#25014)
    
    (cherry picked from commit 357986103b211783455768ad33a4366bec04c578)
---
 superset/sql_parse.py               | 55 +++++++++++++++++++++++++
 tests/unit_tests/sql_parse_tests.py | 81 +++++++++++++++++++++++++++++++++++++
 2 files changed, 136 insertions(+)

diff --git a/superset/sql_parse.py b/superset/sql_parse.py
index a6d4806707..f917be80ab 100644
--- a/superset/sql_parse.py
+++ b/superset/sql_parse.py
@@ -217,9 +217,53 @@ class ParsedQuery:
     def limit(self) -> Optional[int]:
         return self._limit
 
+    def _get_cte_tables(self, parsed: dict[str, Any]) -> list[dict[str, Any]]:
+        if "with" not in parsed:
+            return []
+        return parsed["with"].get("cte_tables", [])
+
+    def _check_cte_is_select(self, oxide_parse: list[dict[str, Any]]) -> bool:
+        """
+        Check if a oxide parsed CTE contains only SELECT statements
+
+        :param oxide_parse: parsed CTE
+        :return: True if CTE is a SELECT statement
+        """
+        for query in oxide_parse:
+            parsed_query = query["Query"]
+            cte_tables = self._get_cte_tables(parsed_query)
+            for cte_table in cte_tables:
+                is_select = all(
+                    key == "Select" for key in cte_table["query"]["body"].keys()
+                )
+                if not is_select:
+                    return False
+        return True
+
     def is_select(self) -> bool:
         # make sure we strip comments; prevents a bug with comments in the CTE
         parsed = sqlparse.parse(self.strip_comments())
+
+        # Check if this is a CTE
+        if parsed[0].is_group and parsed[0][0].ttype == Keyword.CTE:
+            if sqloxide_parse is not None:
+                try:
+                    if not self._check_cte_is_select(
+                        sqloxide_parse(self.strip_comments(), dialect="ansi")
+                    ):
+                        return False
+                except ValueError:
+                    # sqloxide was not able to parse the query, so let's continue with
+                    # sqlparse
+                    pass
+            inner_cte = self.get_inner_cte_expression(parsed[0].tokens) or []
+            # Check if the inner CTE is a not a SELECT
+            if any(token.ttype == DDL for token in inner_cte) or any(
+                token.ttype == DML and token.normalized != "SELECT"
+                for token in inner_cte
+            ):
+                return False
+
         if parsed[0].get_type() == "SELECT":
             return True
 
@@ -241,6 +285,17 @@ class ParsedQuery:
             token.ttype == DML and token.normalized == "SELECT" for token in parsed[0]
         )
 
+    def get_inner_cte_expression(self, tokens: TokenList) -> Optional[TokenList]:
+        for token in tokens:
+            if self._is_identifier(token):
+                for identifier_token in token.tokens:
+                    if (
+                        isinstance(identifier_token, Parenthesis)
+                        and identifier_token.is_group
+                    ):
+                        return identifier_token.tokens
+        return None
+
     def is_valid_ctas(self) -> bool:
         parsed = sqlparse.parse(self.strip_comments())
         return parsed[-1].get_type() == "SELECT"
diff --git a/tests/unit_tests/sql_parse_tests.py b/tests/unit_tests/sql_parse_tests.py
index e00dc3166e..7d8839198c 100644
--- a/tests/unit_tests/sql_parse_tests.py
+++ b/tests/unit_tests/sql_parse_tests.py
@@ -1029,6 +1029,87 @@ FROM foo f"""
     assert sql.is_select()
 
 
+def test_cte_insert_is_not_select() -> None:
+    """
+    Some CTEs with lowercase select are not correctly identified as SELECTS.
+    """
+    sql = ParsedQuery(
+        """WITH foo AS(
+        INSERT INTO foo (id) VALUES (1) RETURNING 1
+        ) select * FROM foo f"""
+    )
+    assert sql.is_select() is False
+
+
+def test_cte_delete_is_not_select() -> None:
+    """
+    Some CTEs with lowercase select are not correctly identified as SELECTS.
+    """
+    sql = ParsedQuery(
+        """WITH foo AS(
+        DELETE FROM foo RETURNING *
+        ) select * FROM foo f"""
+    )
+    assert sql.is_select() is False
+
+
+def test_cte_is_not_select_lowercase() -> None:
+    """
+    Some CTEs with lowercase select are not correctly identified as SELECTS.
+    """
+    sql = ParsedQuery(
+        """WITH foo AS(
+        insert into foo (id) values (1) RETURNING 1
+        ) select * FROM foo f"""
+    )
+    assert sql.is_select() is False
+
+
+def test_cte_with_multiple_selects() -> None:
+    sql = ParsedQuery(
+        "WITH a AS ( select * from foo1 ), b as (select * from foo2) SELECT * FROM a;"
+    )
+    assert sql.is_select()
+
+
+def test_cte_with_multiple_with_non_select() -> None:
+    sql = ParsedQuery(
+        """WITH a AS (
+        select * from foo1
+        ), b as (
+        update foo2 set id=2
+        ) SELECT * FROM a"""
+    )
+    assert sql.is_select() is False
+    sql = ParsedQuery(
+        """WITH a AS (
+         update foo2 set name=2
+         ),
+        b as (
+        select * from foo1
+        ) SELECT * FROM a"""
+    )
+    assert sql.is_select() is False
+    sql = ParsedQuery(
+        """WITH a AS (
+         update foo2 set name=2
+         ),
+        b as (
+        update foo1 set name=2
+        ) SELECT * FROM a"""
+    )
+    assert sql.is_select() is False
+    sql = ParsedQuery(
+        """WITH a AS (
+        INSERT INTO foo (id) VALUES (1)
+        ),
+        b as (
+        select 1
+        ) SELECT * FROM a"""
+    )
+    assert sql.is_select() is False
+
+
 def test_unknown_select() -> None:
     """
     Test that `is_select` works when sqlparse fails to identify the type.


[superset] 02/05: fix: Address regression introduced in #24789 (#25008)

Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

michaelsmolina pushed a commit to branch 3.0
in repository https://gitbox.apache.org/repos/asf/superset.git

commit fad872fffb5f5c3a6f1249f5c8fda6c230f74f52
Author: John Bodley <45...@users.noreply.github.com>
AuthorDate: Fri Aug 18 09:27:34 2023 -0700

    fix: Address regression introduced in #24789 (#25008)
    
    (cherry picked from commit 3f93755be27f1804bb6a08029f6115b8818467cf)
---
 superset/security/manager.py              | 24 +++++++-
 tests/integration_tests/security_tests.py | 92 ++++++++++++++++++++++++++++---
 2 files changed, 104 insertions(+), 12 deletions(-)

diff --git a/superset/security/manager.py b/superset/security/manager.py
index 38bef7f907..a32ef9a1b9 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -1801,7 +1801,8 @@ class SupersetSecurityManager(  # pylint: disable=too-many-public-methods
         # pylint: disable=import-outside-toplevel
         from superset import is_feature_enabled
         from superset.connectors.sqla.models import SqlaTable
-        from superset.daos.dashboard import DashboardDAO
+        from superset.models.dashboard import Dashboard
+        from superset.models.slice import Slice
         from superset.sql_parse import Table
 
         if database and table or query:
@@ -1863,10 +1864,27 @@ class SupersetSecurityManager(  # pylint: disable=too-many-public-methods
                 or self.can_access("datasource_access", datasource.perm or "")
                 or self.is_owner(datasource)
                 or (
+                    # Grant access to the datasource only if dashboard RBAC is enabled
+                    # and said datasource is associated with the dashboard chart in
+                    # question.
                     form_data
+                    and is_feature_enabled("DASHBOARD_RBAC")
                     and (dashboard_id := form_data.get("dashboardId"))
-                    and (dashboard := DashboardDAO.find_by_id(dashboard_id))
-                    and self.can_access_dashboard(dashboard)
+                    and (
+                        dashboard_ := self.get_session.query(Dashboard)
+                        .filter(Dashboard.id == dashboard_id)
+                        .one_or_none()
+                    )
+                    and dashboard_.roles
+                    and (slice_id := form_data.get("slice_id"))
+                    and (
+                        slc := self.get_session.query(Slice)
+                        .filter(Slice.id == slice_id)
+                        .one_or_none()
+                    )
+                    and slc in dashboard_.slices
+                    and slc.datasource == datasource
+                    and self.can_access_dashboard(dashboard_)
                 )
             ):
                 raise SupersetSecurityException(
diff --git a/tests/integration_tests/security_tests.py b/tests/integration_tests/security_tests.py
index 139ad26342..4767e5af0e 100644
--- a/tests/integration_tests/security_tests.py
+++ b/tests/integration_tests/security_tests.py
@@ -1698,6 +1698,7 @@ class TestSecurityManager(SupersetTestCase):
             security_manager.raise_for_access(viz=test_viz)
 
     @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+    @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
     @with_feature_flags(DASHBOARD_RBAC=True)
     @patch("superset.security.manager.g")
     @patch("superset.security.SupersetSecurityManager.is_owner")
@@ -1710,12 +1711,12 @@ class TestSecurityManager(SupersetTestCase):
         mock_is_owner,
         mock_g,
     ):
-        dashboard = self.get_dash_by_slug("births")
+        births = self.get_dash_by_slug("births")
+        girls = self.get_slice("Girls", db.session, expunge_from_session=False)
+        birth_names = girls.datasource
 
-        obj = Mock(
-            datasource=self.get_datasource_mock(),
-            form_data={"dashboardId": dashboard.id},
-        )
+        world_health = self.get_dash_by_slug("world_health")
+        treemap = self.get_slice("Treemap", db.session, expunge_from_session=False)
 
         mock_g.user = security_manager.find_user("gamma")
         mock_is_owner.return_value = False
@@ -1723,15 +1724,88 @@ class TestSecurityManager(SupersetTestCase):
         mock_can_access_schema.return_value = False
 
         for kwarg in ["query_context", "viz"]:
-            dashboard.roles = []
+            births.roles = []
             db.session.flush()
 
+            # No dashboard roles.
             with self.assertRaises(SupersetSecurityException):
-                security_manager.raise_for_access(**{kwarg: obj})
+                security_manager.raise_for_access(
+                    **{
+                        kwarg: Mock(
+                            datasource=birth_names,
+                            form_data={
+                                "dashboardId": births.id,
+                                "slice_id": girls.id,
+                            },
+                        )
+                    }
+                )
 
-            dashboard.roles = [self.get_role("Gamma")]
+            births.roles = [self.get_role("Gamma")]
             db.session.flush()
-            security_manager.raise_for_access(**{kwarg: obj})
+
+            # Undefined dashboard.
+            with self.assertRaises(SupersetSecurityException):
+                security_manager.raise_for_access(
+                    **{
+                        kwarg: Mock(
+                            datasource=birth_names,
+                            form_data={},
+                        )
+                    }
+                )
+
+            # Undefined dashboard chart.
+            with self.assertRaises(SupersetSecurityException):
+                security_manager.raise_for_access(
+                    **{
+                        kwarg: Mock(
+                            datasource=birth_names,
+                            form_data={"dashboardId": births.id},
+                        )
+                    }
+                )
+
+            # Ill-defined dashboard chart.
+            with self.assertRaises(SupersetSecurityException):
+                security_manager.raise_for_access(
+                    **{
+                        kwarg: Mock(
+                            datasource=birth_names,
+                            form_data={
+                                "dashboardId": births.id,
+                                "slice_id": treemap.id,
+                            },
+                        )
+                    }
+                )
+
+            # Dashboard chart not associated with said datasource.
+            with self.assertRaises(SupersetSecurityException):
+                security_manager.raise_for_access(
+                    **{
+                        kwarg: Mock(
+                            datasource=birth_names,
+                            form_data={
+                                "dashboardId": world_health.id,
+                                "slice_id": treemap.id,
+                            },
+                        )
+                    }
+                )
+
+            # Dashboard chart associated with said datasource.
+            security_manager.raise_for_access(
+                **{
+                    kwarg: Mock(
+                        datasource=birth_names,
+                        form_data={
+                            "dashboardId": births.id,
+                            "slice_id": girls.id,
+                        },
+                    )
+                }
+            )
 
         db.session.rollback()