iop
/
superset
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix: database permissions on update and delete (avoid orphaned perms) (#20081) 

* fix: database permissions on update and delete (avoid orphaned perms)

* fix event transaction

* fix test

* fix lint

* update datasource access permissions

* add tests

* fix import

* fix tests

* update slice and dataset perms also

* fix lint

* fix tests

* fix lint

* fix lint

* add test for edge case, small refactor

* add test for edge case, small refactor

* improve code

* fix lint
This commit is contained in:
Daniel Vaz Gaspar 2022-08-02 18:28:46 +01:00 committed by GitHub
parent 34ad80c642
commit bfd2a3d79f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 430 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
docs/docs/installation/installing-superset-from-scratch.mdx
View File

@ -64,7 +64,7 @@ We don't recommend using the system installed Python. Instead, first install the
brew install readline pkg-config libffi openssl mysql postgres
```
You should install a recent version of Python (the official docker image uses 3.8.12). We'd recommend using a Python version manager like [pyenv](https://github.com/pyenv/pyenv) (and also [pyenv-virtualenv](https://github.com/pyenv/pyenv-virtualenv)).
You should install a recent version of Python (the official docker image uses 3.8.13). We'd recommend using a Python version manager like [pyenv](https://github.com/pyenv/pyenv) (and also [pyenv-virtualenv](https://github.com/pyenv/pyenv-virtualenv)).
Let's also make sure we have the latest version of `pip` and `setuptools`:

1
superset/databases/commands/create.py
View File

@ -63,7 +63,6 @@ class CreateDatabaseCommand(BaseCommand):
security_manager.add_permission_view_menu(
"schema_access", security_manager.get_schema_perm(database, schema)
)
security_manager.add_permission_view_menu("database_access", database.perm)
db.session.commit()
except DAOCreateFailedError as ex:
db.session.rollback()

22
superset/databases/commands/update.py
View File

@ -44,10 +44,13 @@ class UpdateDatabaseCommand(BaseCommand):
def run(self) -> Model:
self.validate()
if not self._model:
raise DatabaseNotFoundError()
old_database_name = self._model.database_name
try:
database = DatabaseDAO.update(self._model, self._properties, commit=False)
database.set_sqlalchemy_uri(database.sqlalchemy_uri)
security_manager.add_permission_view_menu("database_access", database.perm)
# adding a new database we always want to force refresh schema list
# TODO Improve this simplistic implementation for catching DB conn fails
try:
@ -55,7 +58,24 @@ class UpdateDatabaseCommand(BaseCommand):
except Exception as ex:
db.session.rollback()
raise DatabaseConnectionFailedError() from ex
# Update database schema permissions
new_schemas: List[str] = []
for schema in schemas:
old_view_menu_name = security_manager.get_schema_perm(
old_database_name, schema
)
new_view_menu_name = security_manager.get_schema_perm(
database.database_name, schema
)
schema_pvm = security_manager.find_permission_view_menu(
"schema_access", old_view_menu_name
)
# Update the schema permission if the database name changed
if schema_pvm and old_database_name != database.database_name:
schema_pvm.view_menu.name = new_view_menu_name
else:
new_schemas.append(schema)
for schema in new_schemas:
security_manager.add_permission_view_menu(
"schema_access", security_manager.get_schema_perm(database, schema)
)

3
superset/models/core.py
View File

@ -795,7 +795,8 @@ class Database(
sqla.event.listen(Database, "after_insert", security_manager.set_perm)
sqla.event.listen(Database, "after_update", security_manager.set_perm)
sqla.event.listen(Database, "after_update", security_manager.database_after_update)
sqla.event.listen(Database, "after_delete", security_manager.database_after_delete)
class Log(Model): # pylint: disable=too-few-public-methods

227
superset/security/manager.py
View File

@ -56,7 +56,7 @@ from flask_appbuilder.security.views import (
from flask_appbuilder.widgets import ListWidget
from flask_login import AnonymousUserMixin, LoginManager
from jwt.api_jwt import _jwt_global_obj
from sqlalchemy import and_, or_
from sqlalchemy import and_, inspect, or_
from sqlalchemy.engine.base import Connection
from sqlalchemy.orm import Session
from sqlalchemy.orm.mapper import Mapper
@ -270,6 +270,14 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
return None
@staticmethod
def get_database_perm(database_id: int, database_name: str) -> str:
return f"[{database_name}].(id:{database_id})"
@staticmethod
def get_dataset_perm(dataset_id: int, dataset_name: str, database_name: str) -> str:
return f"[{database_name}].[{dataset_name}](id:{dataset_id})"
def unpack_database_and_schema( # pylint: disable=no-self-use
self, schema_permission: str
) -> DatabaseAndSchema:
@ -933,6 +941,222 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
return pvm.permission.name in {"can_override_role_permissions", "can_approve"}
def database_after_delete(
self,
mapper: Mapper,
connection: Connection,
target: "Database",
) -> None:
self._delete_vm_database_access(
mapper, connection, target.id, target.database_name
)
def _delete_vm_database_access(
self,
mapper: Mapper,
connection: Connection,
database_id: int,
database_name: str,
) -> None:
view_menu_table = self.viewmenu_model.__table__ # pylint: disable=no-member
permission_view_menu_table = (
self.permissionview_model.__table__ # pylint: disable=no-member
)
view_menu_name = self.get_database_perm(database_id, database_name)
# Clean database access permission
db_pvm = self.find_permission_view_menu("database_access", view_menu_name)
if not db_pvm:
logger.warning(
"Could not find previous database permission %s",
view_menu_name,
)
return
connection.execute(
permission_view_menu_table.delete().where(
permission_view_menu_table.c.id == db_pvm.id
)
)
self.on_permission_after_delete(mapper, connection, db_pvm)
connection.execute(
view_menu_table.delete().where(view_menu_table.c.id == db_pvm.view_menu_id)
)
# Clean database schema permissions
schema_pvms = (
self.get_session.query(self.permissionview_model)
.join(self.permission_model)
.join(self.viewmenu_model)
.filter(self.permission_model.name == "schema_access")
.filter(self.viewmenu_model.name.like(f"[{database_name}].[%]"))
.all()
)
for schema_pvm in schema_pvms:
connection.execute(
permission_view_menu_table.delete().where(
permission_view_menu_table.c.id == schema_pvm.id
)
)
self.on_permission_after_delete(mapper, connection, schema_pvm)
connection.execute(
view_menu_table.delete().where(
view_menu_table.c.id == schema_pvm.view_menu_id
)
)
def _update_vm_database_access(
self,
mapper: Mapper,
connection: Connection,
old_database_name: str,
target: "Database",
) -> Optional[ViewMenu]:
view_menu_table = self.viewmenu_model.__table__ # pylint: disable=no-member
new_database_name = target.database_name
old_view_menu_name = self.get_database_perm(target.id, old_database_name)
new_view_menu_name = self.get_database_perm(target.id, new_database_name)
db_pvm = self.find_permission_view_menu("database_access", old_view_menu_name)
if not db_pvm:
logger.warning(
"Could not find previous database permission %s",
old_view_menu_name,
)
return None
new_updated_pvm = self.find_permission_view_menu(
"database_access", new_view_menu_name
)
if new_updated_pvm:
logger.info(
"New permission [%s] already exists, deleting the previous",
new_view_menu_name,
)
self._delete_vm_database_access(
mapper, connection, target.id, old_database_name
)
return None
connection.execute(
view_menu_table.update()
.where(view_menu_table.c.id == db_pvm.view_menu_id)
.values(name=new_view_menu_name)
)
new_db_view_menu = self.find_view_menu(new_view_menu_name)
self.on_view_menu_after_update(mapper, connection, new_db_view_menu)
return new_db_view_menu
def _update_vm_datasources_access( # pylint: disable=too-many-locals
self,
mapper: Mapper,
connection: Connection,
old_database_name: str,
target: "Database",
) -> List[ViewMenu]:
"""
Updates all datasource access permission when a database name changes
:param connection: Current connection (called on SQLAlchemy event listener scope)
:param old_database_name: the old database name
:param target: The new database name
:return: A list of changed view menus (permission resource names)
"""
from superset.connectors.sqla.models import ( # pylint: disable=import-outside-toplevel
SqlaTable,
)
from superset.models.slice import ( # pylint: disable=import-outside-toplevel
Slice,
)
view_menu_table = self.viewmenu_model.__table__ # pylint: disable=no-member
sqlatable_table = SqlaTable.__table__ # pylint: disable=no-member
chart_table = Slice.__table__ # pylint: disable=no-member
new_database_name = target.database_name
datasets = (
self.get_session.query(SqlaTable)
.filter(SqlaTable.database_id == target.id)
.all()
)
updated_view_menus: List[ViewMenu] = []
for dataset in datasets:
old_dataset_vm_name = self.get_dataset_perm(
dataset.id, dataset.table_name, old_database_name
)
new_dataset_vm_name = self.get_dataset_perm(
dataset.id, dataset.table_name, new_database_name
)
new_dataset_view_menu = self.find_view_menu(new_dataset_vm_name)
if new_dataset_view_menu:
continue
connection.execute(
view_menu_table.update()
.where(view_menu_table.c.name == old_dataset_vm_name)
.values(name=new_dataset_vm_name)
)
# Update dataset (SqlaTable perm field)
connection.execute(
sqlatable_table.update()
.where(
sqlatable_table.c.id == dataset.id,
sqlatable_table.c.perm == old_dataset_vm_name,
)
.values(perm=new_dataset_vm_name)
)
# Update charts (Slice perm field)
connection.execute(
chart_table.update()
.where(chart_table.c.perm == old_dataset_vm_name)
.values(perm=new_dataset_vm_name)
)
self.on_view_menu_after_update(mapper, connection, new_dataset_view_menu)
updated_view_menus.append(self.find_view_menu(new_dataset_view_menu))
return updated_view_menus
def database_after_update(
self,
mapper: Mapper,
connection: Connection,
target: "Database",
) -> None:
# Check if database name has changed
state = inspect(target)
history = state.get_history("database_name", True)
if not history.has_changes() or not history.deleted:
return
old_database_name = history.deleted[0]
# update database access permission
self._update_vm_database_access(mapper, connection, old_database_name, target)
# update datasource access
self._update_vm_datasources_access(
mapper, connection, old_database_name, target
)
def on_view_menu_after_update(
self, mapper: Mapper, connection: Connection, target: ViewMenu
) -> None:
"""
Hook that allows for further custom operations when a new ViewMenu
is updated
Since the update may be performed on after_update event. We cannot
update ViewMenus using a session, so any SQLAlchemy events hooked to
`ViewMenu` will not trigger an after_update.
:param mapper: The table mapper
:param connection: The DB-API connection
:param target: The mapped instance being persisted
"""
def on_permission_after_delete(
self, mapper: Mapper, connection: Connection, target: Permission
) -> None:
"""
Hook that allows for further custom operations when a permission
is deleted by sqlalchemy events.
:param mapper: The table mapper
:param connection: The DB-API connection
:param target: The mapped instance being persisted
"""
def on_permission_after_insert(
self, mapper: Mapper, connection: Connection, target: Permission
) -> None:
@ -1005,6 +1229,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
)
target.perm = target_get_perm
# check schema perm for datasets
if (
hasattr(target, "schema_perm")
and target.schema_perm != target.get_schema_perm()

180
tests/integration_tests/security_tests.py
View File

@ -354,6 +354,186 @@ class TestRolePermission(SupersetTestCase):
session.delete(stored_db)
session.commit()
def test_after_update_database__perm_database_access(self):
session = db.session
database = Database(database_name="tmp_database", sqlalchemy_uri="sqlite://")
session.add(database)
session.commit()
stored_db = (
session.query(Database).filter_by(database_name="tmp_database").one()
)
self.assertIsNotNone(
security_manager.find_permission_view_menu(
"database_access", stored_db.perm
)
)
stored_db.database_name = "tmp_database2"
session.commit()
# Assert that the old permission was updated
self.assertIsNone(
security_manager.find_permission_view_menu(
"database_access", f"[tmp_database].(id:{stored_db.id})"
)
)
# Assert that the db permission was updated
self.assertIsNotNone(
security_manager.find_permission_view_menu(
"database_access", f"[tmp_database2].(id:{stored_db.id})"
)
)
session.delete(stored_db)
session.commit()
def test_after_update_database__perm_database_access_exists(self):
session = db.session
# Add a bogus existing permission before the change
database = Database(database_name="tmp_database", sqlalchemy_uri="sqlite://")
session.add(database)
session.commit()
stored_db = (
session.query(Database).filter_by(database_name="tmp_database").one()
)
security_manager.add_permission_view_menu(
"database_access", f"[tmp_database2].(id:{stored_db.id})"
)
self.assertIsNotNone(
security_manager.find_permission_view_menu(
"database_access", stored_db.perm
)
)
stored_db.database_name = "tmp_database2"
session.commit()
# Assert that the old permission was updated
self.assertIsNone(
security_manager.find_permission_view_menu(
"database_access", f"[tmp_database].(id:{stored_db.id})"
)
)
# Assert that the db permission was updated
self.assertIsNotNone(
security_manager.find_permission_view_menu(
"database_access", f"[tmp_database2].(id:{stored_db.id})"
)
)
session.delete(stored_db)
session.commit()
def test_after_update_database__perm_datasource_access(self):
session = db.session
database = Database(database_name="tmp_database", sqlalchemy_uri="sqlite://")
session.add(database)
session.commit()
table1 = SqlaTable(
schema="tmp_schema",
table_name="tmp_table1",
database=database,
)
session.add(table1)
table2 = SqlaTable(
schema="tmp_schema",
table_name="tmp_table2",
database=database,
)
session.add(table2)
session.commit()
slice1 = Slice(
datasource_id=table1.id,
datasource_type=DatasourceType.TABLE,
datasource_name="tmp_table1",
slice_name="tmp_slice1",
)
session.add(slice1)
session.commit()
slice1 = session.query(Slice).filter_by(slice_name="tmp_slice1").one()
table1 = session.query(SqlaTable).filter_by(table_name="tmp_table1").one()
table2 = session.query(SqlaTable).filter_by(table_name="tmp_table2").one()
# assert initial perms
self.assertIsNotNone(
security_manager.find_permission_view_menu(
"datasource_access", f"[tmp_database].[tmp_table1](id:{table1.id})"
)
)
self.assertIsNotNone(
security_manager.find_permission_view_menu(
"datasource_access", f"[tmp_database].[tmp_table2](id:{table2.id})"
)
)
self.assertEqual(slice1.perm, f"[tmp_database].[tmp_table1](id:{table1.id})")
self.assertEqual(table1.perm, f"[tmp_database].[tmp_table1](id:{table1.id})")
self.assertEqual(table2.perm, f"[tmp_database].[tmp_table2](id:{table2.id})")
stored_db = (
session.query(Database).filter_by(database_name="tmp_database").one()
)
stored_db.database_name = "tmp_database2"
session.commit()
# Assert that the old permissions were updated
self.assertIsNone(
security_manager.find_permission_view_menu(
"datasource_access", f"[tmp_database].[tmp_table1](id:{table1.id})"
)
)
self.assertIsNone(
security_manager.find_permission_view_menu(
"datasource_access", f"[tmp_database].[tmp_table2](id:{table2.id})"
)
)
# Assert that the db permission was updated
self.assertIsNotNone(
security_manager.find_permission_view_menu(
"datasource_access", f"[tmp_database2].[tmp_table1](id:{table1.id})"
)
)
self.assertIsNotNone(
security_manager.find_permission_view_menu(
"datasource_access", f"[tmp_database2].[tmp_table2](id:{table2.id})"
)
)
self.assertEqual(slice1.perm, f"[tmp_database2].[tmp_table1](id:{table1.id})")
self.assertEqual(table1.perm, f"[tmp_database2].[tmp_table1](id:{table1.id})")
self.assertEqual(table2.perm, f"[tmp_database2].[tmp_table2](id:{table2.id})")
session.delete(slice1)
session.delete(table1)
session.delete(table2)
session.delete(stored_db)
session.commit()
def test_after_delete_database__perm_database_access(self):
session = db.session
database = Database(database_name="tmp_database", sqlalchemy_uri="sqlite://")
session.add(database)
session.commit()
stored_db = (
session.query(Database).filter_by(database_name="tmp_database").one()
)
self.assertIsNotNone(
security_manager.find_permission_view_menu(
"database_access", stored_db.perm
)
)
session.delete(stored_db)
session.commit()
# Assert that the old permission was updated
self.assertIsNone(
security_manager.find_permission_view_menu(
"database_access", f"[tmp_database].(id:{stored_db.id})"
)
)
def test_hybrid_perm_database(self):
database = Database(database_name="tmp_database3", sqlalchemy_uri="sqlite://")