pt
/
superset
mirror of https://github.com/apache/superset.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

feat(dao): admin can remove self from object owners (#15149)

This commit is contained in:
Ville Brofeldt 2021-08-13 12:42:48 +03:00 committed by GitHub
parent 517a678cd7
commit d6f9c48aa1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 138 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
superset/charts/commands/create.py
View File

@ -28,15 +28,15 @@ from superset.charts.commands.exceptions import (
DashboardsNotFoundValidationError,
)
from superset.charts.dao import ChartDAO
from superset.commands.base import BaseCommand
from superset.commands.utils import get_datasource_by_id, populate_owners
from superset.commands.base import BaseCommand, CreateMixin
from superset.commands.utils import get_datasource_by_id
from superset.dao.exceptions import DAOCreateFailedError
from superset.dashboards.dao import DashboardDAO
logger = logging.getLogger(__name__)
class CreateChartCommand(BaseCommand):
class CreateChartCommand(CreateMixin, BaseCommand):
def __init__(self, user: User, data: Dict[str, Any]):
self._actor = user
self._properties = data.copy()
@ -73,7 +73,7 @@ class CreateChartCommand(BaseCommand):
self._properties["dashboards"] = dashboards
try:
owners = populate_owners(self._actor, owner_ids)
owners = self.populate_owners(self._actor, owner_ids)
self._properties["owners"] = owners
except ValidationError as ex:
exceptions.append(ex)

8
superset/charts/commands/update.py
View File

@ -31,8 +31,8 @@ from superset.charts.commands.exceptions import (
DatasourceTypeUpdateRequiredValidationError,
)
from superset.charts.dao import ChartDAO
from superset.commands.base import BaseCommand
from superset.commands.utils import get_datasource_by_id, populate_owners
from superset.commands.base import BaseCommand, UpdateMixin
from superset.commands.utils import get_datasource_by_id
from superset.dao.exceptions import DAOUpdateFailedError
from superset.dashboards.dao import DashboardDAO
from superset.exceptions import SupersetSecurityException
@ -42,7 +42,7 @@ from superset.views.base import check_ownership
logger = logging.getLogger(__name__)
class UpdateChartCommand(BaseCommand):
class UpdateChartCommand(UpdateMixin, BaseCommand):
def __init__(self, user: User, model_id: int, data: Dict[str, Any]):
self._actor = user
self._model_id = model_id
@ -100,7 +100,7 @@ class UpdateChartCommand(BaseCommand):
# Validate/Populate owner
try:
owners = populate_owners(self._actor, owner_ids)
owners = self.populate_owners(self._actor, owner_ids)
self._properties["owners"] = owners
except ValidationError as ex:
exceptions.append(ex)

41
superset/commands/base.py
View File

@ -15,7 +15,11 @@
# specific language governing permissions and limitations
# under the License.
from abc import ABC, abstractmethod
from typing import Any
from typing import Any, List, Optional
from flask_appbuilder.security.sqla.models import User
from superset.commands.utils import populate_owners
class BaseCommand(ABC):
@ -37,3 +41,38 @@ class BaseCommand(ABC):
Will raise exception if validation fails
:raises: CommandException
"""
class CreateMixin:
@staticmethod
def populate_owners(
user: User, owner_ids: Optional[List[int]] = None
) -> List[User]:
"""
Populate list of owners, defaulting to the current user if `owner_ids` is
undefined or empty. If current user is missing in `owner_ids`, current user
is added unless belonging to the Admin role.
:param user: current user
:param owner_ids: list of owners by id's
:raises OwnersNotFoundValidationError: if at least one owner can't be resolved
:returns: Final list of owners
"""
return populate_owners(user, owner_ids, default_to_user=True)
class UpdateMixin:
@staticmethod
def populate_owners(
user: User, owner_ids: Optional[List[int]] = None
) -> List[User]:
"""
Populate list of owners. If current user is missing in `owner_ids`, current user
is added unless belonging to the Admin role.
:param user: current user
:param owner_ids: list of owners by id's
:raises OwnersNotFoundValidationError: if at least one owner can't be resolved
:returns: Final list of owners
"""
return populate_owners(user, owner_ids, default_to_user=False)

20
superset/commands/utils.py
View File

@ -29,17 +29,25 @@ from superset.datasets.commands.exceptions import DatasetNotFoundError
from superset.extensions import db, security_manager
def populate_owners(user: User, owner_ids: Optional[List[int]] = None) -> List[User]:
def populate_owners(
user: User, owner_ids: Optional[List[int]], default_to_user: bool,
) -> List[User]:
"""
Helper function for commands, will fetch all users from owners id's
Can raise ValidationError
:param user: The current user
:param owner_ids: A List of owners by id's
:param user: current user
:param owner_ids: list of owners by id's
:param default_to_user: make user the owner if `owner_ids` is None or empty
:raises OwnersNotFoundValidationError: if at least one owner id can't be resolved
:returns: Final list of owners
"""
owner_ids = owner_ids or []
owners = list()
if not owner_ids:
if not owner_ids and default_to_user:
return [user]
if user.id not in owner_ids:
if user.id not in owner_ids and "admin" not in [
role.name.lower() for role in user.roles
]:
# make sure non-admins can't remove themselves as owner by mistake
owners.append(user)
for owner_id in owner_ids:
owner = security_manager.get_user_by_id(owner_id)

8
superset/dashboards/commands/create.py
View File

@ -21,8 +21,8 @@ from flask_appbuilder.models.sqla import Model
from flask_appbuilder.security.sqla.models import User
from marshmallow import ValidationError
from superset.commands.base import BaseCommand
from superset.commands.utils import populate_owners, populate_roles
from superset.commands.base import BaseCommand, CreateMixin
from superset.commands.utils import populate_roles
from superset.dao.exceptions import DAOCreateFailedError
from superset.dashboards.commands.exceptions import (
DashboardCreateFailedError,
@ -34,7 +34,7 @@ from superset.dashboards.dao import DashboardDAO
logger = logging.getLogger(__name__)
class CreateDashboardCommand(BaseCommand):
class CreateDashboardCommand(CreateMixin, BaseCommand):
def __init__(self, user: User, data: Dict[str, Any]):
self._actor = user
self._properties = data.copy()
@ -60,7 +60,7 @@ class CreateDashboardCommand(BaseCommand):
exceptions.append(DashboardSlugExistsValidationError())
try:
owners = populate_owners(self._actor, owner_ids)
owners = self.populate_owners(self._actor, owner_ids)
self._properties["owners"] = owners
except ValidationError as ex:
exceptions.append(ex)

8
superset/dashboards/commands/update.py
View File

@ -21,8 +21,8 @@ from flask_appbuilder.models.sqla import Model
from flask_appbuilder.security.sqla.models import User
from marshmallow import ValidationError
from superset.commands.base import BaseCommand
from superset.commands.utils import populate_owners, populate_roles
from superset.commands.base import BaseCommand, UpdateMixin
from superset.commands.utils import populate_roles
from superset.dao.exceptions import DAOUpdateFailedError
from superset.dashboards.commands.exceptions import (
DashboardForbiddenError,
@ -39,7 +39,7 @@ from superset.views.base import check_ownership
logger = logging.getLogger(__name__)
class UpdateDashboardCommand(BaseCommand):
class UpdateDashboardCommand(UpdateMixin, BaseCommand):
def __init__(self, user: User, model_id: int, data: Dict[str, Any]):
self._actor = user
self._model_id = model_id
@ -80,7 +80,7 @@ class UpdateDashboardCommand(BaseCommand):
if owners_ids is None:
owners_ids = [owner.id for owner in self._model.owners]
try:
owners = populate_owners(self._actor, owners_ids)
owners = self.populate_owners(self._actor, owners_ids)
self._properties["owners"] = owners
except ValidationError as ex:
exceptions.append(ex)

7
superset/datasets/commands/create.py
View File

@ -22,8 +22,7 @@ from flask_appbuilder.security.sqla.models import User
from marshmallow import ValidationError
from sqlalchemy.exc import SQLAlchemyError
from superset.commands.base import BaseCommand
from superset.commands.utils import populate_owners
from superset.commands.base import BaseCommand, CreateMixin
from superset.dao.exceptions import DAOCreateFailedError
from superset.datasets.commands.exceptions import (
DatabaseNotFoundValidationError,
@ -38,7 +37,7 @@ from superset.extensions import db, security_manager
logger = logging.getLogger(__name__)
class CreateDatasetCommand(BaseCommand):
class CreateDatasetCommand(CreateMixin, BaseCommand):
def __init__(self, user: User, data: Dict[str, Any]):
self._actor = user
self._properties = data.copy()
@ -90,7 +89,7 @@ class CreateDatasetCommand(BaseCommand):
exceptions.append(TableNotFoundValidationError(table_name))
try:
owners = populate_owners(self._actor, owner_ids)
owners = self.populate_owners(self._actor, owner_ids)
self._properties["owners"] = owners
except ValidationError as ex:
exceptions.append(ex)

7
superset/datasets/commands/update.py
View File

@ -22,8 +22,7 @@ from flask_appbuilder.models.sqla import Model
from flask_appbuilder.security.sqla.models import User
from marshmallow import ValidationError
from superset.commands.base import BaseCommand
from superset.commands.utils import populate_owners
from superset.commands.base import BaseCommand, UpdateMixin
from superset.connectors.sqla.models import SqlaTable
from superset.dao.exceptions import DAOUpdateFailedError
from superset.datasets.commands.exceptions import (
@ -47,7 +46,7 @@ from superset.views.base import check_ownership
logger = logging.getLogger(__name__)
class UpdateDatasetCommand(BaseCommand):
class UpdateDatasetCommand(UpdateMixin, BaseCommand):
def __init__(
self,
user: User,
@ -101,7 +100,7 @@ class UpdateDatasetCommand(BaseCommand):
exceptions.append(DatabaseChangeValidationError())
# Validate/Populate owner
try:
owners = populate_owners(self._actor, owner_ids)
owners = self.populate_owners(self._actor, owner_ids)
self._properties["owners"] = owners
except ValidationError as ex:
exceptions.append(ex)

6
superset/reports/commands/create.py
View File

@ -22,7 +22,7 @@ from flask_appbuilder.models.sqla import Model
from flask_appbuilder.security.sqla.models import User
from marshmallow import ValidationError
from superset.commands.utils import populate_owners
from superset.commands.base import CreateMixin
from superset.dao.exceptions import DAOCreateFailedError
from superset.databases.dao import DatabaseDAO
from superset.models.reports import ReportScheduleType
@ -40,7 +40,7 @@ from superset.reports.dao import ReportScheduleDAO
logger = logging.getLogger(__name__)
class CreateReportScheduleCommand(BaseReportScheduleCommand):
class CreateReportScheduleCommand(CreateMixin, BaseReportScheduleCommand):
def __init__(self, user: User, data: Dict[str, Any]):
self._actor = user
self._properties = data.copy()
@ -90,7 +90,7 @@ class CreateReportScheduleCommand(BaseReportScheduleCommand):
)
try:
owners = populate_owners(self._actor, owner_ids)
owners = self.populate_owners(self._actor, owner_ids)
self._properties["owners"] = owners
except ValidationError as ex:
exceptions.append(ex)

6
superset/reports/commands/update.py
View File

@ -22,7 +22,7 @@ from flask_appbuilder.models.sqla import Model
from flask_appbuilder.security.sqla.models import User
from marshmallow import ValidationError
from superset.commands.utils import populate_owners
from superset.commands.base import UpdateMixin
from superset.dao.exceptions import DAOUpdateFailedError
from superset.databases.dao import DatabaseDAO
from superset.exceptions import SupersetSecurityException
@ -42,7 +42,7 @@ from superset.views.base import check_ownership
logger = logging.getLogger(__name__)
class UpdateReportScheduleCommand(BaseReportScheduleCommand):
class UpdateReportScheduleCommand(UpdateMixin, BaseReportScheduleCommand):
def __init__(self, user: User, model_id: int, data: Dict[str, Any]):
self._actor = user
self._model_id = model_id
@ -117,7 +117,7 @@ class UpdateReportScheduleCommand(BaseReportScheduleCommand):
if owner_ids is None:
owner_ids = [owner.id for owner in self._model.owners]
try:
owners = populate_owners(self._actor, owner_ids)
owners = self.populate_owners(self._actor, owner_ids)
self._properties["owners"] = owners
except ValidationError as ex:
exceptions.append(ex)

31
tests/integration_tests/charts/api_tests.py
View File

@ -568,7 +568,7 @@ class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixin):
self.assertEqual(model.created_by, admin)
self.assertEqual(model.slice_name, "title1_changed")
self.assertEqual(model.description, "description1")
self.assertIn(admin, model.owners)
self.assertNotIn(admin, model.owners)
self.assertIn(gamma, model.owners)
self.assertEqual(model.viz_type, "viz_type1")
self.assertEqual(model.params, """{"a": 1}""")
@ -580,20 +580,39 @@ class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixin):
db.session.delete(model)
db.session.commit()
def test_update_chart_new_owner(self):
def test_update_chart_new_owner_not_admin(self):
"""
Chart API: Test update set new owner to current user
Chart API: Test update set new owner implicitly adds logged in owner
"""
gamma = self.get_user("gamma")
alpha = self.get_user("alpha")
chart_id = self.insert_chart("title", [alpha.id], 1).id
chart_data = {"slice_name": "title1_changed", "owners": [gamma.id]}
self.login(username="alpha")
uri = f"api/v1/chart/{chart_id}"
rv = self.put_assert_metric(uri, chart_data, "put")
self.assertEqual(rv.status_code, 200)
model = db.session.query(Slice).get(chart_id)
self.assertIn(alpha, model.owners)
self.assertIn(gamma, model.owners)
db.session.delete(model)
db.session.commit()
def test_update_chart_new_owner_admin(self):
"""
Chart API: Test update set new owner as admin to other than current user
"""
gamma = self.get_user("gamma")
admin = self.get_user("admin")
chart_id = self.insert_chart("title", [gamma.id], 1).id
chart_data = {"slice_name": "title1_changed"}
chart_id = self.insert_chart("title", [admin.id], 1).id
chart_data = {"slice_name": "title1_changed", "owners": [gamma.id]}
self.login(username="admin")
uri = f"api/v1/chart/{chart_id}"
rv = self.put_assert_metric(uri, chart_data, "put")
self.assertEqual(rv.status_code, 200)
model = db.session.query(Slice).get(chart_id)
self.assertIn(admin, model.owners)
self.assertNotIn(admin, model.owners)
self.assertIn(gamma, model.owners)
db.session.delete(model)
db.session.commit()

39
tests/integration_tests/dashboards/api_tests.py
View File

@ -1109,7 +1109,7 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixi
for slice in slices:
self.assertIn(user_alpha1, slice.owners)
self.assertIn(user_alpha2, slice.owners)
self.assertIn(admin, slice.owners)
self.assertNotIn(admin, slice.owners)
# Revert owners on slice
slice.owners = []
db.session.commit()
@ -1149,22 +1149,45 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixi
db.session.delete(model)
db.session.commit()
def test_update_dashboard_new_owner(self):
def test_update_dashboard_new_owner_not_admin(self):
"""
Dashboard API: Test update set new owner to current user
Dashboard API: Test update set new owner implicitly adds logged in owner
"""
gamma_id = self.get_user("gamma").id
gamma = self.get_user("gamma")
alpha = self.get_user("alpha")
dashboard_id = self.insert_dashboard("title1", "slug1", [alpha.id]).id
dashboard_data = {"dashboard_title": "title1_changed", "owners": [gamma.id]}
self.login(username="alpha")
uri = f"api/v1/dashboard/{dashboard_id}"
rv = self.client.put(uri, json=dashboard_data)
self.assertEqual(rv.status_code, 200)
model = db.session.query(Dashboard).get(dashboard_id)
self.assertIn(gamma, model.owners)
self.assertIn(alpha, model.owners)
for slc in model.slices:
self.assertIn(gamma, slc.owners)
self.assertIn(alpha, slc.owners)
db.session.delete(model)
db.session.commit()
def test_update_dashboard_new_owner_admin(self):
"""
Dashboard API: Test update set new owner as admin to other than current user
"""
gamma = self.get_user("gamma")
admin = self.get_user("admin")
dashboard_id = self.insert_dashboard("title1", "slug1", [gamma_id]).id
dashboard_data = {"dashboard_title": "title1_changed"}
dashboard_id = self.insert_dashboard("title1", "slug1", [admin.id]).id
dashboard_data = {"dashboard_title": "title1_changed", "owners": [gamma.id]}
self.login(username="admin")
uri = f"api/v1/dashboard/{dashboard_id}"
rv = self.client.put(uri, json=dashboard_data)
self.assertEqual(rv.status_code, 200)
model = db.session.query(Dashboard).get(dashboard_id)
self.assertIn(admin, model.owners)
self.assertIn(gamma, model.owners)
self.assertNotIn(admin, model.owners)
for slc in model.slices:
self.assertIn(admin, slc.owners)
self.assertIn(gamma, slc.owners)
self.assertNotIn(admin, slc.owners)
db.session.delete(model)
db.session.commit()