From 700dee6db6f17624dcecaf87d21fc938f5c9f079 Mon Sep 17 00:00:00 2001 From: Kasia Kucharczyk <2536609+kkucharc@users.noreply.github.com> Date: Thu, 17 Dec 2020 09:50:31 +0100 Subject: [PATCH] feat(logs): security permissions simplification (#12061) * Added migration for logs security converge * Changed class permission name and method permission in LogModelView and LogRestApi * Updated recent revision and filename * Changed name of Log perm in manager. Updated TestRolePermission to have correct menu and permission. * Updated latest migration revision * Updated latest migration revision --- .../4b84f97828aa_security_converge_logs.py | 74 +++++++++++++++++++ superset/security/manager.py | 2 +- superset/views/log/api.py | 4 +- superset/views/log/views.py | 4 +- tests/security_tests.py | 6 +- 5 files changed, 83 insertions(+), 7 deletions(-) create mode 100644 superset/migrations/versions/4b84f97828aa_security_converge_logs.py diff --git a/superset/migrations/versions/4b84f97828aa_security_converge_logs.py b/superset/migrations/versions/4b84f97828aa_security_converge_logs.py new file mode 100644 index 000000000..51862e343 --- /dev/null +++ b/superset/migrations/versions/4b84f97828aa_security_converge_logs.py @@ -0,0 +1,74 @@ +# 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. +"""security converge logs + +Revision ID: 4b84f97828aa +Revises: 45731db65d9c +Create Date: 2020-12-14 13:40:46.492449 + +""" + +from alembic import op +from sqlalchemy.exc import SQLAlchemyError +from sqlalchemy.orm import Session + +from superset.migrations.shared.security_converge import ( + add_pvms, + get_reversed_new_pvms, + get_reversed_pvm_map, + migrate_roles, + Pvm, +) + +revision = "4b84f97828aa" +down_revision = "45731db65d9c" + +NEW_PVMS = {"Log": ("can_read", "can_write",)} +PVM_MAP = { + Pvm("LogModelView", "can_show"): (Pvm("Log", "can_read"),), + Pvm("LogModelView", "can_add",): (Pvm("Log", "can_write"),), + Pvm("LogModelView", "can_list"): (Pvm("Log", "can_read"),), +} + + +def upgrade(): + bind = op.get_bind() + session = Session(bind=bind) + + # Add the new permissions on the migration itself + add_pvms(session, NEW_PVMS) + migrate_roles(session, PVM_MAP) + try: + session.commit() + except SQLAlchemyError as ex: + print(f"An error occurred while upgrading Logs permissions: {ex}") + session.rollback() + + +def downgrade(): + bind = op.get_bind() + session = Session(bind=bind) + + # Add the old permissions on the migration itself + add_pvms(session, get_reversed_new_pvms(PVM_MAP)) + migrate_roles(session, get_reversed_pvm_map(PVM_MAP)) + try: + session.commit() + except SQLAlchemyError as ex: + print(f"An error occurred while downgrading Logs permissions: {ex}") + session.rollback() + pass diff --git a/superset/security/manager.py b/superset/security/manager.py index 047448799..46ccbb1dc 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -133,7 +133,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods "Refresh Druid Metadata", "ResetPasswordView", "RoleModelView", - "LogModelView", + "Log", "Security", "Row Level Security", "Row Level Security Filters", diff --git a/superset/views/log/api.py b/superset/views/log/api.py index 7f14112b1..f4436635d 100644 --- a/superset/views/log/api.py +++ b/superset/views/log/api.py @@ -19,13 +19,15 @@ from flask_appbuilder.models.sqla.interface import SQLAInterface import superset.models.core as models from superset.views.base_api import BaseSupersetModelRestApi +from ...constants import MODEL_API_RW_METHOD_PERMISSION_MAP from . import LogMixin class LogRestApi(LogMixin, BaseSupersetModelRestApi): datamodel = SQLAInterface(models.Log) include_route_methods = {"get_list", "get", "post"} - class_permission_name = "LogModelView" + class_permission_name = "Log" + method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP resource_name = "log" allow_browser_login = True list_columns = [ diff --git a/superset/views/log/views.py b/superset/views/log/views.py index 7139d3485..02205622a 100644 --- a/superset/views/log/views.py +++ b/superset/views/log/views.py @@ -17,7 +17,7 @@ from flask_appbuilder.models.sqla.interface import SQLAInterface import superset.models.core as models -from superset.constants import RouteMethod +from superset.constants import MODEL_VIEW_RW_METHOD_PERMISSION_MAP, RouteMethod from superset.views.base import SupersetModelView from . import LogMixin @@ -26,3 +26,5 @@ from . import LogMixin class LogModelView(LogMixin, SupersetModelView): # pylint: disable=too-many-ancestors datamodel = SQLAInterface(models.Log) include_route_methods = {RouteMethod.LIST, RouteMethod.SHOW} + class_permission_name = "Log" + method_permission_name = MODEL_VIEW_RW_METHOD_PERMISSION_MAP diff --git a/tests/security_tests.py b/tests/security_tests.py index 5625fd963..b92845eda 100644 --- a/tests/security_tests.py +++ b/tests/security_tests.py @@ -728,13 +728,11 @@ class TestRolePermission(SupersetTestCase): ) ) - log_permissions = ["can_list", "can_show"] + log_permissions = ["can_read"] for log_permission in log_permissions: self.assertTrue( security_manager._is_admin_only( - security_manager.find_permission_view_menu( - log_permission, "LogModelView" - ) + security_manager.find_permission_view_menu(log_permission, "Log") ) )