Skip to content

Commit

Permalink
feat(logs): security permissions simplification (#12061)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
kkucharc authored Dec 17, 2020
1 parent 51c3b71 commit 700dee6
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion superset/security/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
4 changes: 3 additions & 1 deletion superset/views/log/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,15 @@
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 = [
Expand Down
4 changes: 3 additions & 1 deletion superset/views/log/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -26,3 +26,5 @@
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
6 changes: 2 additions & 4 deletions tests/security_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -728,13 +728,11 @@ def test_is_admin_only(self):
)
)

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")
)
)

Expand Down

0 comments on commit 700dee6

Please sign in to comment.