You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2021/01/28 23:56:36 UTC
[GitHub] [superset] ktmud commented on a change in pull request #12680: feat: dashboard access roles
ktmud commented on a change in pull request #12680:
URL: https://github.com/apache/superset/pull/12680#discussion_r566355651
##########
File path: superset/views/dashboard/mixin.py
##########
@@ -62,6 +63,11 @@ class DashboardMixin: # pylint: disable=too-few-public-methods
"want to alter specific parameters."
),
"owners": _("Owners is a list of users who can alter the dashboard."),
+ "roles": _(
+ "Roles is a list which defines access to the dashboard. "
+ "these roles are always applied in addition to data access leve. "
+ "if no roles then dashboard is available for all roles"
Review comment:
```suggestion
"Roles is a list which defines access to the dashboard. "
"These roles are always applied in addition to restrictions on dataset level access. "
"If no roles defined then the dashboard is available to all roles."
```
##########
File path: tests/dashboards/superset_factory_util.py
##########
@@ -0,0 +1,303 @@
+# 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 logging
+from typing import Any, Callable, List, Optional
+
+from flask_appbuilder import Model
+from flask_appbuilder.security.sqla.models import User
+
+from superset import appbuilder
+from superset.connectors.sqla.models import SqlaTable, sqlatable_user
+from superset.models.core import Database
+from superset.models.dashboard import (
+ Dashboard,
+ dashboard_slices,
+ dashboard_user,
+ DashboardRoles,
+)
+from superset.models.slice import Slice, slice_user
+from tests.dashboards.dashboard_test_utils import random_slug, random_str, random_title
+
+logger = logging.getLogger(__name__)
+
+session = appbuilder.get_session
+
+inserted_dashboards_ids = []
+inserted_databases_ids = []
+inserted_sqltables_ids = []
+inserted_slices_ids = []
Review comment:
I recently added a [`db_insert_temp_object`](https://github.com/apache/superset/blob/2c2ade827f8f9bfb089efab1c7e3a143fa691fe5/tests/base_tests.py#L528) helper, which uses `contextmanager` to automatically delete a temporarily inserted object when the test exists (regardless of test failure or not). I think this is a safer approach as manual deletion called later may fail to execute if some intermediate steps failed.
Do you think it would be worth taking a similar approach for these ids?
Maybe we can extend `db_insert_temp_object` to allow inserting an array of objects at once?
##########
File path: superset/dashboards/filters.py
##########
@@ -107,9 +119,14 @@ def apply(self, query: Query, value: Any) -> Query:
query = query.filter(
or_(
Dashboard.id.in_(owner_ids_query),
- Dashboard.id.in_(published_dash_query),
+ Dashboard.id.in_(datesource_perm_query),
Dashboard.id.in_(users_favorite_dash_query),
+ Dashboard.id.in_(roles_based_query),
)
)
return query
+
+ @staticmethod
+ def has_no_role_based_access() -> bool:
Review comment:
This could probably also be changed to an inline variable as opposed to a static method.
In general, unless a function needs to me exposed as a convenient public API, I'd recommend avoid using static methods, so to keep things local.
##########
File path: superset/dashboards/filters.py
##########
@@ -107,9 +119,14 @@ def apply(self, query: Query, value: Any) -> Query:
query = query.filter(
or_(
Dashboard.id.in_(owner_ids_query),
- Dashboard.id.in_(published_dash_query),
+ Dashboard.id.in_(datesource_perm_query),
Dashboard.id.in_(users_favorite_dash_query),
+ Dashboard.id.in_(roles_based_query),
)
)
return query
+
+ @staticmethod
+ def has_no_role_based_access() -> bool:
Review comment:
nit: we should avoid negation in function methods, to [avoid future double negatives](https://www.gymglish.com/en/gymglish/english-grammar/avoiding-double-negative-constructions), i.e.
```
not self.has_role_based_access()
self.has_role_based_access()
```
Reads nicer than
```
self.has_no_role_based_access()
not self.has_no_role_based_access()
```
##########
File path: tests/base_tests.py
##########
@@ -289,19 +294,39 @@ def get_access_requests(self, username, ds_type, ds_id):
def logout(self):
self.client.get("/logout/", follow_redirects=True)
+ def grant_access_to_dashboard(self, dashboard, role_name):
+ role = security_manager.find_role(role_name)
+ dashboard.roles.append(role)
+ db.session.merge(dashboard)
+ db.session.commit()
Review comment:
These class methods can probably be moved to functions somewhere since they don't rely on `self`.
##########
File path: tests/base_tests.py
##########
@@ -149,7 +150,11 @@ def create_user_with_roles(username: str, roles: List[str]):
db.session.commit()
user_to_create = security_manager.find_user(username)
assert user_to_create
- user_to_create.roles = [security_manager.find_role(r) for r in roles]
+ user_to_create.roles = []
+ for chosen_user_role in roles:
+ if copy_roles:
+ security_manager.copy_role("Gamma", chosen_user_role, False)
Review comment:
The name `copy_roles` is a little confusing considering you are just copying `Gamma` permissions to the roles over to the new roles you are creating. Considering multiple roles can be added to a user, what you want is probably something like this:
```python
def create_user_with_roles(
username: str, roles: List[str], include_gamma_role: bool = True
):
...
user_to_create.roles = (
[security_manager.find_role("Gamma")]
if include_gamma_role else []
) + [security_manager.find_role(r) for r in roles]
```
##########
File path: tests/dashboards/security/security_dataset_tests.py
##########
@@ -0,0 +1,260 @@
+# 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.
+"""Unit tests for Superset"""
+import json
+
+import prison
+import pytest
+from flask import escape
+
+from superset import app, security_manager
+from superset.models import core as models
+from tests.dashboards.base_case import DashboardTestCase
+from tests.dashboards.consts import *
+from tests.dashboards.dashboard_test_utils import *
+from tests.dashboards.superset_factory_util import *
+from tests.fixtures.energy_dashboard import load_energy_table_with_slice
+
+
+# @mark.amit
+class TestDashboardDatasetSecurity(DashboardTestCase):
+ @pytest.fixture
+ def load_dashboard(self):
+ with app.app_context():
+ table = (
+ db.session.query(SqlaTable).filter_by(table_name="energy_usage").one()
+ )
+ # get a slice from the allowed table
+ slice = db.session.query(Slice).filter_by(slice_name="Energy Sankey").one()
+
+ self.grant_public_access_to_table(table)
+
+ pytest.hidden_dash_slug = f"hidden_dash_{random_slug()}"
+ pytest.published_dash_slug = f"published_dash_{random_slug()}"
+
+ # Create a published and hidden dashboard and add them to the database
+ published_dash = Dashboard()
+ published_dash.dashboard_title = "Published Dashboard"
+ published_dash.slug = pytest.published_dash_slug
+ published_dash.slices = [slice]
+ published_dash.published = True
+
+ hidden_dash = Dashboard()
+ hidden_dash.dashboard_title = "Hidden Dashboard"
+ hidden_dash.slug = pytest.hidden_dash_slug
+ hidden_dash.slices = [slice]
+ hidden_dash.published = False
+
+ db.session.merge(published_dash)
+ db.session.merge(hidden_dash)
+ yield db.session.commit()
+
+ self.revoke_public_access_to_table(table)
+ db.session.delete(published_dash)
+ db.session.delete(hidden_dash)
+ db.session.commit()
+
+ def test_dashboard_access__admin_can_access_all(self):
+ # arrange
+ self.login(username=ADMIN_USERNAME)
+ dashboard_title_by_url = {
+ dash.url: dash.dashboard_title for dash in get_all_dashboards()
+ }
+
+ # act
+ responses_by_url = {
+ url: self.client.get(url).data.decode("utf-8")
+ for url in dashboard_title_by_url.keys()
+ }
+
+ # assert
+ for dashboard_url, get_dashboard_response in responses_by_url.items():
+ assert (
+ escape(dashboard_title_by_url[dashboard_url]) in get_dashboard_response
+ )
+
+ def test_get_dashboards__users_are_dashboards_owners(self):
+ # arrange
+ username = "gamma"
+ user = security_manager.find_user(username)
+ my_owned_dashboard = create_dashboard_to_db(
+ dashboard_title="My Dashboard",
+ slug=f"my_dash_{random_slug()}",
+ published=False,
+ owners=[user],
+ )
+
+ not_my_owned_dashboard = create_dashboard_to_db(
+ dashboard_title="Not My Dashboard",
+ slug=f"not_my_dash_{random_slug()}",
+ published=False,
+ )
+
+ self.login(user.username)
+
+ # act
+ get_dashboards_response = self.get_resp(DASHBOARDS_API_URL)
+
+ # assert
+ self.assertIn(my_owned_dashboard.url, get_dashboards_response)
+ self.assertNotIn(not_my_owned_dashboard.url, get_dashboards_response)
+
+ def test_get_dashboards__owners_can_view_empty_dashboard(self):
+ # arrange
+ dash = create_dashboard_to_db("Empty Dashboard", slug="empty_dashboard")
+ dashboard_url = dash.url
+ gamma_user = security_manager.find_user("gamma")
+ self.login(gamma_user.username)
+
+ # act
+ get_dashboards_response = self.get_resp(DASHBOARDS_API_URL)
+
+ # assert
+ self.assertNotIn(dashboard_url, get_dashboards_response)
+
+ def test_get_dashboards__users_can_view_favorites_dashboards(self):
+ # arrange
+ user = security_manager.find_user("gamma")
+ fav_dash_slug = f"my_favorite_dash_{random_slug()}"
+ regular_dash_slug = f"regular_dash_{random_slug()}"
+
+ favorite_dash = Dashboard()
+ favorite_dash.dashboard_title = "My Favorite Dashboard"
+ favorite_dash.slug = fav_dash_slug
+
+ regular_dash = Dashboard()
+ regular_dash.dashboard_title = "A Plain Ol Dashboard"
+ regular_dash.slug = regular_dash_slug
+
+ db.session.merge(favorite_dash)
+ db.session.merge(regular_dash)
+ db.session.commit()
+
+ dash = db.session.query(Dashboard).filter_by(slug=fav_dash_slug).first()
+
+ favorites = models.FavStar()
+ favorites.obj_id = dash.id
+ favorites.class_name = "Dashboard"
+ favorites.user_id = user.id
+
+ db.session.merge(favorites)
+ db.session.commit()
+
+ self.login(user.username)
+
+ # act
+ get_dashboards_response = self.get_resp(DASHBOARDS_API_URL)
+
+ # assert
+ self.assertIn(f"/superset/dashboard/{fav_dash_slug}/", get_dashboards_response)
Review comment:
What's the benefit of testing favorited dashboards separately? Do we expect a risk that favorites somehow wouldn't be accessible?
##########
File path: tests/dashboards/security/security_dataset_tests.py
##########
@@ -0,0 +1,260 @@
+# 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.
+"""Unit tests for Superset"""
+import json
+
+import prison
+import pytest
+from flask import escape
+
+from superset import app, security_manager
+from superset.models import core as models
+from tests.dashboards.base_case import DashboardTestCase
+from tests.dashboards.consts import *
+from tests.dashboards.dashboard_test_utils import *
+from tests.dashboards.superset_factory_util import *
+from tests.fixtures.energy_dashboard import load_energy_table_with_slice
+
+
+# @mark.amit
+class TestDashboardDatasetSecurity(DashboardTestCase):
+ @pytest.fixture
+ def load_dashboard(self):
+ with app.app_context():
+ table = (
+ db.session.query(SqlaTable).filter_by(table_name="energy_usage").one()
+ )
+ # get a slice from the allowed table
+ slice = db.session.query(Slice).filter_by(slice_name="Energy Sankey").one()
+
+ self.grant_public_access_to_table(table)
+
+ pytest.hidden_dash_slug = f"hidden_dash_{random_slug()}"
+ pytest.published_dash_slug = f"published_dash_{random_slug()}"
+
+ # Create a published and hidden dashboard and add them to the database
+ published_dash = Dashboard()
+ published_dash.dashboard_title = "Published Dashboard"
+ published_dash.slug = pytest.published_dash_slug
+ published_dash.slices = [slice]
+ published_dash.published = True
+
+ hidden_dash = Dashboard()
+ hidden_dash.dashboard_title = "Hidden Dashboard"
+ hidden_dash.slug = pytest.hidden_dash_slug
+ hidden_dash.slices = [slice]
+ hidden_dash.published = False
+
+ db.session.merge(published_dash)
+ db.session.merge(hidden_dash)
+ yield db.session.commit()
+
+ self.revoke_public_access_to_table(table)
+ db.session.delete(published_dash)
+ db.session.delete(hidden_dash)
+ db.session.commit()
+
+ def test_dashboard_access__admin_can_access_all(self):
+ # arrange
+ self.login(username=ADMIN_USERNAME)
+ dashboard_title_by_url = {
+ dash.url: dash.dashboard_title for dash in get_all_dashboards()
+ }
+
+ # act
+ responses_by_url = {
+ url: self.client.get(url).data.decode("utf-8")
+ for url in dashboard_title_by_url.keys()
+ }
+
+ # assert
+ for dashboard_url, get_dashboard_response in responses_by_url.items():
+ assert (
+ escape(dashboard_title_by_url[dashboard_url]) in get_dashboard_response
+ )
+
+ def test_get_dashboards__users_are_dashboards_owners(self):
+ # arrange
+ username = "gamma"
+ user = security_manager.find_user(username)
+ my_owned_dashboard = create_dashboard_to_db(
+ dashboard_title="My Dashboard",
+ slug=f"my_dash_{random_slug()}",
+ published=False,
+ owners=[user],
+ )
+
+ not_my_owned_dashboard = create_dashboard_to_db(
+ dashboard_title="Not My Dashboard",
+ slug=f"not_my_dash_{random_slug()}",
+ published=False,
+ )
+
+ self.login(user.username)
+
+ # act
+ get_dashboards_response = self.get_resp(DASHBOARDS_API_URL)
+
+ # assert
+ self.assertIn(my_owned_dashboard.url, get_dashboards_response)
+ self.assertNotIn(not_my_owned_dashboard.url, get_dashboards_response)
+
+ def test_get_dashboards__owners_can_view_empty_dashboard(self):
+ # arrange
+ dash = create_dashboard_to_db("Empty Dashboard", slug="empty_dashboard")
+ dashboard_url = dash.url
+ gamma_user = security_manager.find_user("gamma")
+ self.login(gamma_user.username)
+
+ # act
+ get_dashboards_response = self.get_resp(DASHBOARDS_API_URL)
+
+ # assert
+ self.assertNotIn(dashboard_url, get_dashboards_response)
+
+ def test_get_dashboards__users_can_view_favorites_dashboards(self):
+ # arrange
+ user = security_manager.find_user("gamma")
+ fav_dash_slug = f"my_favorite_dash_{random_slug()}"
+ regular_dash_slug = f"regular_dash_{random_slug()}"
+
+ favorite_dash = Dashboard()
+ favorite_dash.dashboard_title = "My Favorite Dashboard"
+ favorite_dash.slug = fav_dash_slug
+
+ regular_dash = Dashboard()
+ regular_dash.dashboard_title = "A Plain Ol Dashboard"
+ regular_dash.slug = regular_dash_slug
+
+ db.session.merge(favorite_dash)
+ db.session.merge(regular_dash)
+ db.session.commit()
+
+ dash = db.session.query(Dashboard).filter_by(slug=fav_dash_slug).first()
+
+ favorites = models.FavStar()
+ favorites.obj_id = dash.id
+ favorites.class_name = "Dashboard"
+ favorites.user_id = user.id
+
+ db.session.merge(favorites)
+ db.session.commit()
+
+ self.login(user.username)
+
+ # act
+ get_dashboards_response = self.get_resp(DASHBOARDS_API_URL)
+
+ # assert
+ self.assertIn(f"/superset/dashboard/{fav_dash_slug}/", get_dashboards_response)
+
+ def test_get_dashboards__user_can_not_view_unpublished_dash(self):
+ # arrange
+ admin_user = security_manager.find_user(ADMIN_USERNAME)
+ gamma_user = security_manager.find_user(GAMMA_USERNAME)
+ slug = f"admin_owned_unpublished_dash_{random_slug()}"
+
+ admin_and_not_published_dashboard = create_dashboard_to_db(
+ "My Dashboard", slug=slug, owners=[admin_user]
+ )
+
+ self.login(gamma_user.username)
+
+ # act - list dashboards as a gamma user
+ get_dashboards_response_as_gamma = self.get_resp(DASHBOARDS_API_URL)
+
+ # assert
+ self.assertNotIn(
+ admin_and_not_published_dashboard.url, get_dashboards_response_as_gamma
+ )
+
+ @pytest.mark.usefixtures("load_energy_table_with_slice", "load_dashboard")
+ def test_get_dashboards__users_can_view_permitted_dashboard(self):
+ # arrange
+ username = random_str()
+ new_role = f"role_{random_str()}"
+ self.create_user_with_roles(username, [new_role], copy_roles=True)
+ accessed_table = get_sql_table_by_name("energy_usage")
+ self.grant_role_access_to_table(accessed_table, new_role)
+ # get a slice from the allowed table
+ slice_to_add_to_dashboards = get_slice_by_name("Energy Sankey")
+ # Create a published and hidden dashboard and add them to the database
+ first_dash = create_dashboard_to_db(
+ dashboard_title="Published Dashboard",
+ slug=f"first_dash_{random_slug()}",
+ published=True,
+ slices=[slice_to_add_to_dashboards],
+ )
+
+ second_dash = create_dashboard_to_db(
+ dashboard_title="Hidden Dashboard",
+ slug=f"second_dash_{random_slug()}",
+ published=True,
+ slices=[slice_to_add_to_dashboards],
+ )
+
+ try:
+ self.login(username)
+ # act
+ get_dashboards_response = self.get_resp(DASHBOARDS_API_URL)
+
+ # assert
+ self.assertIn(second_dash.url, get_dashboards_response)
+ self.assertIn(first_dash.url, get_dashboards_response)
+ finally:
+ self.revoke_public_access_to_table(accessed_table)
+
+ @staticmethod
+ def __add_dashboard_mode_params(dashboard_url):
Review comment:
Could you shed light on where is this function being used and why was it defined in two places?
##########
File path: tests/dashboards/security/security_dataset_tests.py
##########
@@ -0,0 +1,260 @@
+# 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.
+"""Unit tests for Superset"""
+import json
+
+import prison
+import pytest
+from flask import escape
+
+from superset import app, security_manager
+from superset.models import core as models
+from tests.dashboards.base_case import DashboardTestCase
+from tests.dashboards.consts import *
+from tests.dashboards.dashboard_test_utils import *
+from tests.dashboards.superset_factory_util import *
+from tests.fixtures.energy_dashboard import load_energy_table_with_slice
+
+
+# @mark.amit
Review comment:
Is this for debugging?
##########
File path: tests/dashboards/security/base_case.py
##########
@@ -0,0 +1,88 @@
+# 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 typing import List, Optional
+
+from flask import escape, Response
+
+from superset.models.dashboard import Dashboard
+from tests.dashboards.base_case import DashboardTestCase
+
+DASHBOARD_COUNT_IN_DASHBOARDS_LIST_VIEW_FORMAT = "Record Count:</strong> {count}"
Review comment:
I'm not sure about the value of making this a constant. And why does it have only the closing mark for `<strong>`?
##########
File path: tests/dashboards/security/security_dataset_tests.py
##########
@@ -0,0 +1,260 @@
+# 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.
+"""Unit tests for Superset"""
+import json
+
+import prison
+import pytest
+from flask import escape
+
+from superset import app, security_manager
+from superset.models import core as models
+from tests.dashboards.base_case import DashboardTestCase
+from tests.dashboards.consts import *
+from tests.dashboards.dashboard_test_utils import *
+from tests.dashboards.superset_factory_util import *
+from tests.fixtures.energy_dashboard import load_energy_table_with_slice
+
+
+# @mark.amit
+class TestDashboardDatasetSecurity(DashboardTestCase):
+ @pytest.fixture
+ def load_dashboard(self):
+ with app.app_context():
+ table = (
+ db.session.query(SqlaTable).filter_by(table_name="energy_usage").one()
+ )
+ # get a slice from the allowed table
+ slice = db.session.query(Slice).filter_by(slice_name="Energy Sankey").one()
+
+ self.grant_public_access_to_table(table)
+
+ pytest.hidden_dash_slug = f"hidden_dash_{random_slug()}"
+ pytest.published_dash_slug = f"published_dash_{random_slug()}"
+
+ # Create a published and hidden dashboard and add them to the database
+ published_dash = Dashboard()
+ published_dash.dashboard_title = "Published Dashboard"
+ published_dash.slug = pytest.published_dash_slug
+ published_dash.slices = [slice]
+ published_dash.published = True
+
+ hidden_dash = Dashboard()
+ hidden_dash.dashboard_title = "Hidden Dashboard"
+ hidden_dash.slug = pytest.hidden_dash_slug
+ hidden_dash.slices = [slice]
+ hidden_dash.published = False
+
+ db.session.merge(published_dash)
+ db.session.merge(hidden_dash)
+ yield db.session.commit()
+
+ self.revoke_public_access_to_table(table)
+ db.session.delete(published_dash)
+ db.session.delete(hidden_dash)
+ db.session.commit()
+
+ def test_dashboard_access__admin_can_access_all(self):
+ # arrange
+ self.login(username=ADMIN_USERNAME)
+ dashboard_title_by_url = {
+ dash.url: dash.dashboard_title for dash in get_all_dashboards()
+ }
+
+ # act
+ responses_by_url = {
+ url: self.client.get(url).data.decode("utf-8")
+ for url in dashboard_title_by_url.keys()
+ }
+
+ # assert
+ for dashboard_url, get_dashboard_response in responses_by_url.items():
+ assert (
+ escape(dashboard_title_by_url[dashboard_url]) in get_dashboard_response
+ )
+
+ def test_get_dashboards__users_are_dashboards_owners(self):
+ # arrange
+ username = "gamma"
+ user = security_manager.find_user(username)
+ my_owned_dashboard = create_dashboard_to_db(
+ dashboard_title="My Dashboard",
+ slug=f"my_dash_{random_slug()}",
Review comment:
What's the benefit of adding a slug here? I thought they were optional.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org