Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(saved queries): security perm simplification #11764

Merged
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -70,7 +70,7 @@ const mockqueries = [...new Array(3)].map((_, i) => ({
}));

fetchMock.get(queriesInfoEndpoint, {
permissions: ['can_delete'],
permissions: ['can_write'],
});
fetchMock.get(queriesEndpoint, {
result: mockqueries,
Expand Down
Expand Up @@ -91,8 +91,8 @@ function SavedQueryList({
setSavedQueryCurrentlyPreviewing,
] = useState<SavedQueryObject | null>(null);

const canEdit = hasPerm('can_edit');
const canDelete = hasPerm('can_delete');
const canEdit = hasPerm('can_write');
const canDelete = hasPerm('can_write');

const openNewQuery = () => {
window.open(`${window.location.origin}/superset/sqllab?new=true`);
Expand Down
33 changes: 33 additions & 0 deletions superset/constants.py
Expand Up @@ -63,3 +63,36 @@ class RouteMethod: # pylint: disable=too-few-public-methods
CRUD_SET = {ADD, LIST, EDIT, DELETE, ACTION_POST, SHOW}
RELATED_VIEW_SET = {ADD, LIST, EDIT, DELETE}
REST_MODEL_VIEW_CRUD_SET = {DELETE, GET, GET_LIST, POST, PUT, INFO}


MODEL_VIEW_RW_METHOD_PERMISSION_MAP = {
"add": "write",
"api": "read",
"api_column_add": "write",
"api_column_edit": "write",
"api_create": "write",
"api_delete": "write",
"api_get": "read",
"api_read": "read",
"api_readvalues": "read",
"api_update": "write",
"delete": "write",
"download": "read",
"edit": "write",
"list": "read",
"muldelete": "write",
"show": "read",
}

MODEL_API_RW_METHOD_PERMISSION_MAP = {
"bulk_delete": "write",
"delete": "write",
"distinct": "read",
"export": "read",
"get": "read",
"get_list": "read",
"info": "read",
"post": "write",
"put": "write",
"related": "read",
}
Comment on lines +68 to +98
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add Enums for these, like PermissionType.WRITE?

16 changes: 16 additions & 0 deletions superset/migrations/shared/__init__.py
@@ -0,0 +1,16 @@
# 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.
264 changes: 264 additions & 0 deletions superset/migrations/shared/security_converge.py
@@ -0,0 +1,264 @@
# 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.
import logging
from typing import Dict, List, Tuple

from sqlalchemy import (
Column,
ForeignKey,
Integer,
Sequence,
String,
Table,
UniqueConstraint,
)
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.orm import Load, relationship, Session

logger = logging.getLogger(__name__)

Base = declarative_base()

PvmType = Tuple[str, str]
dpgaspar marked this conversation as resolved.
Show resolved Hide resolved
PvmMigrationMapType = Dict[PvmType, Tuple[PvmType, ...]]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the tuple? Will these ever be anything other than 1:1?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They will, on downgrade we need 1:N because I'm "recovering" for example: "can_read" -> ("can_show", "can_list", ...)


# Partial freeze of the current metadata db schema


class Permission(Base): # type: ignore
__tablename__ = "ab_permission"
id = Column(Integer, Sequence("ab_permission_id_seq"), primary_key=True)
name = Column(String(100), unique=True, nullable=False)

def __repr__(self) -> str:
return f"{self.name}"


class ViewMenu(Base): # type: ignore
__tablename__ = "ab_view_menu"
id = Column(Integer, Sequence("ab_view_menu_id_seq"), primary_key=True)
name = Column(String(250), unique=True, nullable=False)

def __repr__(self) -> str:
return f"{self.name}"

def __eq__(self, other: object) -> bool:
return (isinstance(other, self.__class__)) and (self.name == other.name)

def __neq__(self, other: object) -> bool:
return (isinstance(other, self.__class__)) and self.name != other.name


assoc_permissionview_role = Table(
"ab_permission_view_role",
Base.metadata,
Column("id", Integer, Sequence("ab_permission_view_role_id_seq"), primary_key=True),
Column("permission_view_id", Integer, ForeignKey("ab_permission_view.id")),
Column("role_id", Integer, ForeignKey("ab_role.id")),
UniqueConstraint("permission_view_id", "role_id"),
)


class Role(Base): # type: ignore
__tablename__ = "ab_role"

id = Column(Integer, Sequence("ab_role_id_seq"), primary_key=True)
name = Column(String(64), unique=True, nullable=False)
permissions = relationship(
"PermissionView", secondary=assoc_permissionview_role, backref="role"
)

def __repr__(self) -> str:
return f"{self.name}"


class PermissionView(Base): # type: ignore
__tablename__ = "ab_permission_view"
__table_args__ = (UniqueConstraint("permission_id", "view_menu_id"),)
id = Column(Integer, Sequence("ab_permission_view_id_seq"), primary_key=True)
permission_id = Column(Integer, ForeignKey("ab_permission.id"))
permission = relationship("Permission")
view_menu_id = Column(Integer, ForeignKey("ab_view_menu.id"))
view_menu = relationship("ViewMenu")

def __repr__(self) -> str:
return f"{self.permission} {self.view_menu}"


def _add_view_menu(session: Session, view_name: str) -> ViewMenu:
"""
Check and add the new view menu
"""
new_view = session.query(ViewMenu).filter(ViewMenu.name == view_name).one_or_none()
if not new_view:
new_view = ViewMenu(name=view_name)
session.add(new_view)
return new_view


def _add_permission(session: Session, permission_name: str) -> Permission:
"""
Check and add the new Permission
"""
new_permission = (
session.query(Permission)
.filter(Permission.name == permission_name)
.one_or_none()
)
if not new_permission:
new_permission = Permission(name=permission_name)
session.add(new_permission)
return new_permission


def _add_permission_view(
session: Session, permission: Permission, view_menu: ViewMenu
) -> PermissionView:
"""
Check and add the new Permission View
"""
new_pvm = (
session.query(PermissionView)
.filter(
PermissionView.view_menu_id == view_menu.id,
PermissionView.permission_id == permission.id,
)
.one_or_none()
)
if not new_pvm:
new_pvm = PermissionView(view_menu=view_menu, permission=permission)
session.add(new_pvm)
return new_pvm


def _find_pvm(session: Session, view_name: str, permission_name: str) -> PermissionView:
return (
session.query(PermissionView)
.join(Permission)
.join(ViewMenu)
.filter(ViewMenu.name == view_name, Permission.name == permission_name)
).one_or_none()


def add_pvms(
session: Session, pvm_data: Dict[str, Tuple[str, ...]], commit: bool = False
) -> List[PermissionView]:
"""
Checks if exists and adds new Permissions, Views and PermissionView's
"""
pvms = []
for view_name, permissions in pvm_data.items():
# Check and add the new View
new_view = _add_view_menu(session, view_name)
for permission_name in permissions:
new_permission = _add_permission(session, permission_name)
# Check and add the new PVM
pvms.append(_add_permission_view(session, new_permission, new_view))
if commit:
session.commit()
return pvms


def _delete_old_permissions(
session: Session, pvm_map: Dict[PermissionView, List[PermissionView]]
) -> None:
"""
Delete old permissions:
- Delete the PermissionView
- Deletes the Permission if it's an orphan now
- Deletes the ViewMenu if it's an orphan now
"""
# Delete old permissions
for old_pvm, new_pvms in pvm_map.items():
old_permission_name = old_pvm.permission.name
old_view_name = old_pvm.view_menu.name
logger.info(f"Going to delete pvm: {old_pvm}")
session.delete(old_pvm)
pvms_with_permission = (
session.query(PermissionView)
.join(Permission)
.filter(Permission.name == old_permission_name)
).first()
if not pvms_with_permission:
logger.info(f"Going to delete permission: {old_pvm.permission}")
session.delete(old_pvm.permission)
pvms_with_view_menu = (
session.query(PermissionView)
.join(ViewMenu)
.filter(ViewMenu.name == old_view_name)
).first()
if not pvms_with_view_menu:
logger.info(f"Going to delete view_menu: {old_pvm.view_menu}")
session.delete(old_pvm.view_menu)


def migrate_roles(
dpgaspar marked this conversation as resolved.
Show resolved Hide resolved
session: Session, pvm_key_map: PvmMigrationMapType, commit: bool = False,
) -> None:
"""
Migrates all existing roles that have the permissions to be migrated
"""
# Collect a map of PermissionView objects for migration
pvm_map: Dict[PermissionView, List[PermissionView]] = {}
for old_pvm_key, new_pvms_ in pvm_key_map.items():
old_pvm = _find_pvm(session, old_pvm_key[0], old_pvm_key[1])
dpgaspar marked this conversation as resolved.
Show resolved Hide resolved
if old_pvm:
for new_pvm_key in new_pvms_:
new_pvm = _find_pvm(session, new_pvm_key[0], new_pvm_key[1])
if old_pvm not in pvm_map:
pvm_map[old_pvm] = [new_pvm]
else:
pvm_map[old_pvm].append(new_pvm)

# Replace old permissions by the new ones on all existing roles
roles = session.query(Role).options(Load(Role).joinedload(Role.permissions)).all()
for role in roles:
for old_pvm, new_pvms in pvm_map.items():
if old_pvm in role.permissions:
logger.info(f"Removing {old_pvm} from {role}")
role.permissions.remove(old_pvm)
for new_pvm in new_pvms:
if new_pvm not in role.permissions:
logger.info(f"Add {new_pvm} to {role}")
role.permissions.append(new_pvm)
session.merge(role)

# Delete old permissions
_delete_old_permissions(session, pvm_map)
if commit:
session.commit()


def get_reversed_new_pvms(pvm_map: PvmMigrationMapType) -> Dict[str, Tuple[str, ...]]:
reversed_pvms: Dict[str, Tuple[str, ...]] = {}
for old_pvm, new_pvms in pvm_map.items():
if old_pvm[0] not in reversed_pvms:
reversed_pvms[old_pvm[0]] = (old_pvm[1],)
else:
reversed_pvms[old_pvm[0]] = reversed_pvms[old_pvm[0]] + (old_pvm[1],)
return reversed_pvms


def get_reversed_pvm_map(pvm_map: PvmMigrationMapType) -> PvmMigrationMapType:
reversed_pvm_map: PvmMigrationMapType = {}
for old_pvm, new_pvms in pvm_map.items():
for new_pvm in new_pvms:
if new_pvm not in reversed_pvm_map:
reversed_pvm_map[new_pvm] = (old_pvm,)
else:
reversed_pvm_map[new_pvm] = reversed_pvm_map[new_pvm] + (old_pvm,)
return reversed_pvm_map