willbarrett commented on a change in pull request #11764: URL: https://github.com/apache/incubator-superset/pull/11764#discussion_r529792947
########## File path: superset/migrations/shared/security_converge.py ########## @@ -0,0 +1,260 @@ +# 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. + +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 + +Base = declarative_base() + +PvmType = Tuple[str, str] +PvmMigrationMapType = Dict[PvmType, Tuple[PvmType, ...]] Review comment: Why the tuple? Will these ever be anything other than 1:1? ########## File path: superset/migrations/shared/security_converge.py ########## @@ -0,0 +1,260 @@ +# 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. + +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 + +Base = declarative_base() + +PvmType = Tuple[str, str] +PvmMigrationMapType = Dict[PvmType, Tuple[PvmType, ...]] + +# 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 +) -> None: + """ + Checks if exists and adds new Permissions, Views and PermissionView's + """ + 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 + _add_permission_view(session, new_permission, new_view) + if commit: + session.commit() + + +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 + print(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: + print(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: + print(f"Going to delete view_menu: {old_pvm.view_menu}") + session.delete(old_pvm.view_menu) + + +def migrate_roles( Review comment: I'd feel more comfortable if we wrote tests for these utility functions - they're pretty important and do have a fair amount of logic. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
