mirror of https://github.com/apache/superset.git
fix: change public role like gamma procedure (#10674)
* fix: change public role like gamma procedure * lint and updating UPDATING with breaking change * fix updating text * add test and support PUBLIC_ROLE_LIKE_GAMMA * fix, cleanup tests * fix, new test * fix, public default * Update superset/config.py Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com> * add simple public welcome page Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com>
This commit is contained in:
parent
c715cad48e
commit
3e374dab07
|
@ -23,6 +23,8 @@ assists people when migrating to a new version.
|
|||
|
||||
## Next
|
||||
|
||||
* [10674](https://github.com/apache/incubator-superset/pull/10674): Breaking change: PUBLIC_ROLE_LIKE_GAMMA was removed is favour of the new PUBLIC_ROLE_LIKE so it can be set it whatever role you want.
|
||||
|
||||
* [10590](https://github.com/apache/incubator-superset/pull/10590): Breaking change: this PR will convert iframe chart into dashboard markdown component, and remove all `iframe`, `separator`, and `markup` slices (and support) from Superset. If you have important data in those slices, please backup manually.
|
||||
|
||||
* [10562](https://github.com/apache/incubator-superset/pull/10562): EMAIL_REPORTS_WEBDRIVER is deprecated use WEBDRIVER_TYPE instead.
|
||||
|
|
|
@ -260,10 +260,10 @@ AUTH_TYPE = AUTH_DB
|
|||
# ---------------------------------------------------
|
||||
# Roles config
|
||||
# ---------------------------------------------------
|
||||
# Grant public role the same set of permissions as for the GAMMA role.
|
||||
# Grant public role the same set of permissions as for a selected builtin role.
|
||||
# This is useful if one wants to enable anonymous users to view
|
||||
# dashboards. Explicit grant on specific datasets is still required.
|
||||
PUBLIC_ROLE_LIKE_GAMMA = False
|
||||
PUBLIC_ROLE_LIKE: Optional[str] = None
|
||||
|
||||
# ---------------------------------------------------
|
||||
# Babel config for translations
|
||||
|
|
|
@ -585,7 +585,7 @@ class SqlaTable( # pylint: disable=too-many-public-methods,too-many-instance-at
|
|||
return security_manager.get_schema_perm(self.database, self.schema)
|
||||
|
||||
def get_perm(self) -> str:
|
||||
return ("[{obj.database}].[{obj.table_name}]" "(id:{obj.id})").format(obj=self)
|
||||
return f"[{self.database}].[{self.table_name}](id:{self.id})"
|
||||
|
||||
@property
|
||||
def name(self) -> str:
|
||||
|
|
|
@ -17,6 +17,7 @@
|
|||
# pylint: disable=too-few-public-methods
|
||||
"""A set of constants and methods to manage permissions and security"""
|
||||
import logging
|
||||
import re
|
||||
from typing import Any, Callable, cast, List, Optional, Set, Tuple, TYPE_CHECKING, Union
|
||||
|
||||
from flask import current_app, g
|
||||
|
@ -172,6 +173,15 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
|
|||
|
||||
ACCESSIBLE_PERMS = {"can_userinfo"}
|
||||
|
||||
data_access_permissions = (
|
||||
"database_access",
|
||||
"schema_access",
|
||||
"datasource_access",
|
||||
"all_datasource_access",
|
||||
"all_database_access",
|
||||
"all_query_access",
|
||||
)
|
||||
|
||||
def get_schema_perm( # pylint: disable=no-self-use
|
||||
self, database: Union["Database", str], schema: Optional[str] = None
|
||||
) -> Optional[str]:
|
||||
|
@ -609,8 +619,15 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
|
|||
self.set_role("granter", self._is_granter_pvm)
|
||||
self.set_role("sql_lab", self._is_sql_lab_pvm)
|
||||
|
||||
# Configure public role
|
||||
if conf["PUBLIC_ROLE_LIKE"]:
|
||||
self.copy_role(conf["PUBLIC_ROLE_LIKE"], self.auth_role_public, merge=True)
|
||||
if conf.get("PUBLIC_ROLE_LIKE_GAMMA", False):
|
||||
self.set_role("Public", self._is_gamma_pvm)
|
||||
logger.warning(
|
||||
"The config `PUBLIC_ROLE_LIKE_GAMMA` is deprecated and will be removed "
|
||||
"in Superset 1.0. Please use `PUBLIC_ROLE_LIKE ` instead."
|
||||
)
|
||||
self.copy_role("Gamma", self.auth_role_public, merge=True)
|
||||
|
||||
self.create_missing_perms()
|
||||
|
||||
|
@ -618,6 +635,58 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
|
|||
self.get_session.commit()
|
||||
self.clean_perms()
|
||||
|
||||
def _get_pvms_from_builtin_role(self, role_name: str) -> List[PermissionView]:
|
||||
"""
|
||||
Gets a list of model PermissionView permissions infered from a builtin role
|
||||
definition
|
||||
"""
|
||||
role_from_permissions_names = self.builtin_roles.get(role_name, [])
|
||||
all_pvms = self.get_session.query(PermissionView).all()
|
||||
role_from_permissions = []
|
||||
for pvm_regex in role_from_permissions_names:
|
||||
view_name_regex = pvm_regex[0]
|
||||
permission_name_regex = pvm_regex[1]
|
||||
for pvm in all_pvms:
|
||||
if re.match(view_name_regex, pvm.view_menu.name) and re.match(
|
||||
permission_name_regex, pvm.permission.name
|
||||
):
|
||||
if pvm not in role_from_permissions:
|
||||
role_from_permissions.append(pvm)
|
||||
return role_from_permissions
|
||||
|
||||
def copy_role(
|
||||
self, role_from_name: str, role_to_name: str, merge: bool = True
|
||||
) -> None:
|
||||
"""
|
||||
Copies permissions from a role to another.
|
||||
|
||||
Note: Supports regex defined builtin roles
|
||||
|
||||
:param role_from_name: The FAB role name from where the permissions are taken
|
||||
:param role_to_name: The FAB role name from where the permissions are copied to
|
||||
:param merge: If merge is true, keep data access permissions
|
||||
if they already exist on the target role
|
||||
"""
|
||||
|
||||
logger.info("Copy/Merge %s to %s", role_from_name, role_to_name)
|
||||
# If it's a builtin role extract permissions from it
|
||||
if role_from_name in self.builtin_roles:
|
||||
role_from_permissions = self._get_pvms_from_builtin_role(role_from_name)
|
||||
else:
|
||||
role_from_permissions = list(self.find_role(role_from_name).permissions)
|
||||
role_to = self.add_role(role_to_name)
|
||||
# If merge, recover existing data access permissions
|
||||
if merge:
|
||||
for permission_view in role_to.permissions:
|
||||
if (
|
||||
permission_view not in role_from_permissions
|
||||
and permission_view.permission.name in self.data_access_permissions
|
||||
):
|
||||
role_from_permissions.append(permission_view)
|
||||
role_to.permissions = role_from_permissions
|
||||
self.get_session.merge(role_to)
|
||||
self.get_session.commit()
|
||||
|
||||
def set_role(
|
||||
self, role_name: str, pvm_check: Callable[[PermissionView], bool]
|
||||
) -> None:
|
||||
|
@ -629,14 +698,15 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
|
|||
"""
|
||||
|
||||
logger.info("Syncing %s perms", role_name)
|
||||
sesh = self.get_session
|
||||
pvms = sesh.query(PermissionView).all()
|
||||
pvms = self.get_session.query(PermissionView).all()
|
||||
pvms = [p for p in pvms if p.permission and p.view_menu]
|
||||
role = self.add_role(role_name)
|
||||
role_pvms = [p for p in pvms if pvm_check(p)]
|
||||
role_pvms = [
|
||||
permission_view for permission_view in pvms if pvm_check(permission_view)
|
||||
]
|
||||
role.permissions = role_pvms
|
||||
sesh.merge(role)
|
||||
sesh.commit()
|
||||
self.get_session.merge(role)
|
||||
self.get_session.commit()
|
||||
|
||||
def _is_admin_only(self, pvm: Model) -> bool:
|
||||
"""
|
||||
|
|
|
@ -0,0 +1,23 @@
|
|||
{#
|
||||
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.
|
||||
#}
|
||||
{% extends "appbuilder/base.html" %}
|
||||
|
||||
{% block content %}
|
||||
<h2><center>Welcome to Apache Superset</center></h2>
|
||||
{% endblock %}
|
|
@ -2529,6 +2529,8 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods
|
|||
def welcome(self) -> FlaskResponse:
|
||||
"""Personalized welcome page"""
|
||||
if not g.user or not g.user.get_id():
|
||||
if conf.get("PUBLIC_ROLE_LIKE_GAMMA", False) or conf["PUBLIC_ROLE_LIKE"]:
|
||||
return self.render_template("superset/public_welcome.html")
|
||||
return redirect(appbuilder.get_url_for_login)
|
||||
|
||||
welcome_dashboard_id = (
|
||||
|
|
|
@ -320,6 +320,9 @@ class TestDashboard(SupersetTestCase):
|
|||
resp = self.get_resp("/api/v1/dashboard/")
|
||||
self.assertNotIn("/superset/dashboard/world_health/", resp)
|
||||
|
||||
# Cleanup
|
||||
self.revoke_public_access_to_table(table)
|
||||
|
||||
def test_dashboard_with_created_by_can_be_accessed_by_public_users(self):
|
||||
self.logout()
|
||||
table = db.session.query(SqlaTable).filter_by(table_name="birth_names").one()
|
||||
|
@ -332,6 +335,8 @@ class TestDashboard(SupersetTestCase):
|
|||
db.session.commit()
|
||||
|
||||
assert "Births" in self.get_resp("/superset/dashboard/births/")
|
||||
# Cleanup
|
||||
self.revoke_public_access_to_table(table)
|
||||
|
||||
def test_only_owners_can_save(self):
|
||||
dash = db.session.query(Dashboard).filter_by(slug="births").first()
|
||||
|
@ -400,6 +405,9 @@ class TestDashboard(SupersetTestCase):
|
|||
self.assertNotIn(f"/superset/dashboard/{hidden_dash_slug}/", resp)
|
||||
self.assertIn(f"/superset/dashboard/{published_dash_slug}/", resp)
|
||||
|
||||
# Cleanup
|
||||
self.revoke_public_access_to_table(table)
|
||||
|
||||
def test_users_can_view_own_dashboard(self):
|
||||
user = security_manager.find_user("gamma")
|
||||
my_dash_slug = f"my_dash_{random()}"
|
||||
|
|
|
@ -20,7 +20,7 @@ import unittest
|
|||
from unittest.mock import Mock, patch
|
||||
|
||||
import prison
|
||||
from flask import g
|
||||
from flask import current_app, g
|
||||
|
||||
import tests.test_app
|
||||
from superset import app, appbuilder, db, security_manager, viz
|
||||
|
@ -531,6 +531,52 @@ class TestRolePermission(SupersetTestCase):
|
|||
) # wb_health_population slice, has access
|
||||
self.assertNotIn("Girl Name Cloud", data) # birth_names slice, no access
|
||||
|
||||
def test_public_sync_role_data_perms(self):
|
||||
"""
|
||||
Security: Tests if the sync role method preserves data access permissions
|
||||
if they already exist on a public role.
|
||||
Also check that non data access permissions are removed
|
||||
"""
|
||||
table = db.session.query(SqlaTable).filter_by(table_name="birth_names").one()
|
||||
self.grant_public_access_to_table(table)
|
||||
public_role = security_manager.get_public_role()
|
||||
unwanted_pvm = security_manager.find_permission_view_menu(
|
||||
"menu_access", "Security"
|
||||
)
|
||||
public_role.permissions.append(unwanted_pvm)
|
||||
db.session.commit()
|
||||
|
||||
security_manager.sync_role_definitions()
|
||||
public_role = security_manager.get_public_role()
|
||||
public_role_resource_names = [
|
||||
permission.view_menu.name for permission in public_role.permissions
|
||||
]
|
||||
|
||||
assert table.get_perm() in public_role_resource_names
|
||||
assert "Security" not in public_role_resource_names
|
||||
|
||||
# Cleanup
|
||||
self.revoke_public_access_to_table(table)
|
||||
|
||||
def test_public_sync_role_builtin_perms(self):
|
||||
"""
|
||||
Security: Tests public role creation based on a builtin role
|
||||
"""
|
||||
current_app.config["PUBLIC_ROLE_LIKE"] = "TestRole"
|
||||
|
||||
security_manager.sync_role_definitions()
|
||||
public_role = security_manager.get_public_role()
|
||||
public_role_resource_names = [
|
||||
[permission.view_menu.name, permission.permission.name]
|
||||
for permission in public_role.permissions
|
||||
]
|
||||
for pvm in current_app.config["FAB_ROLES"]["TestRole"]:
|
||||
assert pvm in public_role_resource_names
|
||||
|
||||
# Cleanup
|
||||
current_app.config["PUBLIC_ROLE_LIKE"] = "Gamma"
|
||||
security_manager.sync_role_definitions()
|
||||
|
||||
def test_sqllab_gamma_user_schema_access_to_sqllab(self):
|
||||
session = db.session
|
||||
|
||||
|
@ -724,7 +770,10 @@ class TestRolePermission(SupersetTestCase):
|
|||
|
||||
def test_gamma_permissions_basic(self):
|
||||
self.assert_can_gamma(get_perm_tuples("Gamma"))
|
||||
self.assert_cannot_alpha(get_perm_tuples("Alpha"))
|
||||
self.assert_cannot_alpha(get_perm_tuples("Gamma"))
|
||||
|
||||
def test_public_permissions_basic(self):
|
||||
self.assert_can_gamma(get_perm_tuples("Public"))
|
||||
|
||||
@unittest.skipUnless(
|
||||
SupersetTestCase.is_module_installed("pydruid"), "pydruid not installed"
|
||||
|
@ -788,6 +837,9 @@ class TestRolePermission(SupersetTestCase):
|
|||
assert_can_all("SliceModelView")
|
||||
assert_can_all("DashboardModelView")
|
||||
|
||||
assert_cannot_write("UserDBModelView")
|
||||
assert_cannot_write("RoleModelView")
|
||||
|
||||
self.assertIn(("can_add_slices", "Superset"), gamma_perm_set)
|
||||
self.assertIn(("can_copy_dash", "Superset"), gamma_perm_set)
|
||||
self.assertIn(("can_created_dashboards", "Superset"), gamma_perm_set)
|
||||
|
|
|
@ -60,7 +60,10 @@ def GET_FEATURE_FLAGS_FUNC(ff):
|
|||
|
||||
TESTING = True
|
||||
WTF_CSRF_ENABLED = False
|
||||
PUBLIC_ROLE_LIKE_GAMMA = True
|
||||
|
||||
FAB_ROLES = {"TestRole": [["Security", "menu_access"], ["List Users", "menu_access"]]}
|
||||
|
||||
PUBLIC_ROLE_LIKE = "Gamma"
|
||||
AUTH_ROLE_PUBLIC = "Public"
|
||||
EMAIL_NOTIFICATIONS = False
|
||||
ENABLE_ROW_LEVEL_SECURITY = True
|
||||
|
|
|
@ -50,7 +50,7 @@ def GET_FEATURE_FLAGS_FUNC(ff):
|
|||
|
||||
TESTING = True
|
||||
WTF_CSRF_ENABLED = False
|
||||
PUBLIC_ROLE_LIKE_GAMMA = True
|
||||
PUBLIC_ROLE_LIKE = "Gamma"
|
||||
AUTH_ROLE_PUBLIC = "Public"
|
||||
EMAIL_NOTIFICATIONS = False
|
||||
|
||||
|
|
Loading…
Reference in New Issue