From e286106e5ee401e95e70f9d74a300d286f873b86 Mon Sep 17 00:00:00 2001 From: Archer_Tsai Date: Wed, 6 May 2020 10:20:52 +0800 Subject: [PATCH 01/16] WIP: first version --- ...23bf17ffa782_add_table_permission_model.py | 59 ++++++++++++++++ superset/models/table_permission.py | 70 +++++++++++++++++++ 2 files changed, 129 insertions(+) create mode 100644 superset/migrations/versions/23bf17ffa782_add_table_permission_model.py create mode 100644 superset/models/table_permission.py diff --git a/superset/migrations/versions/23bf17ffa782_add_table_permission_model.py b/superset/migrations/versions/23bf17ffa782_add_table_permission_model.py new file mode 100644 index 0000000000000..54b8114f9c61b --- /dev/null +++ b/superset/migrations/versions/23bf17ffa782_add_table_permission_model.py @@ -0,0 +1,59 @@ +# 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. +"""Add AICS table permission control model + +Revision ID: 23bf17ffa782 +Revises: 5ad4d4f30245 +Create Date: 2020-05-04 12:06:19.643832 + +""" + +# revision identifiers, used by Alembic. +revision = '23bf17ffa782' +down_revision = '5ad4d4f30245' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.create_table('table_permission', + sa.Column('created_on', sa.DateTime(), nullable=True), + sa.Column('changed_on', sa.DateTime(), nullable=True), + sa.Column('id', sa.Integer(), nullable=False), + sa.Column('user_id', sa.Integer(), nullable=True), + sa.Column('table_id', sa.Integer(), nullable=True), + sa.Column('apply_date', sa.Date(), nullable=False), + sa.Column('expire_date', sa.Date(), nullable=False), + sa.Column('force_treminate_date', sa.DateTime(), nullable=True), + sa.Column('is_active', sa.Boolean(), nullable=False), + sa.Column('created_by_fk', sa.Integer(), nullable=True), + sa.Column('changed_by_fk', sa.Integer(), nullable=True), + sa.ForeignKeyConstraint(['changed_by_fk'], ['ab_user.id'], ), + sa.ForeignKeyConstraint(['created_by_fk'], ['ab_user.id'], ), + sa.ForeignKeyConstraint(['table_id'], ['tables.id'], ), + sa.ForeignKeyConstraint(['user_id'], ['ab_user.id'], ), + sa.PrimaryKeyConstraint('id') + ) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_table('table_permission') + # ### end Alembic commands ### diff --git a/superset/models/table_permission.py b/superset/models/table_permission.py new file mode 100644 index 0000000000000..a302b11908599 --- /dev/null +++ b/superset/models/table_permission.py @@ -0,0 +1,70 @@ +# 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 datetime import datetime + +from flask_appbuilder import Model +from sqlalchemy import Column, ForeignKey, Integer, String, Date, DateTime, Boolean +from sqlalchemy.orm import relationship + +from superset import security_manager +from superset.models.helpers import AuditMixinNullable + + +class TablePermission(Model, AuditMixinNullable): + + """ + Custom attributes attached to the user. + + Extending the user attribute is tricky due to its dependency on the + authentication typew an circular dependencies in Superset. Instead, we use + a custom model for adding attributes. + + """ + + __tablename__ = "table_permission" + id = Column(Integer, primary_key=True) # pylint: disable=invalid-name + user_id = Column(Integer, ForeignKey("ab_user.id")) + user = relationship( + security_manager.user_model, backref="table_permissions", foreign_keys=[user_id] + ) + + table_id = Column(Integer, ForeignKey("tables.id")) + apply_date = Column(Date, nullable=False, default=datetime.now()) + expire_date = Column(Date, nullable=False) + force_treminate_date = Column(DateTime) + is_active = Column(Boolean, default=True, nullable=False) + + @property + def username(self): + return self.user.username + + @username.setter + def username(self, value): + pass + + @property + def table_names_with_id(self): + return self._new_key + + @table_names_with_id.setter + def table_names_with_id(self, value): + self._new_key = value + + @property + def changed_by_name(self): + return self.changed_by_ From a142fb8c3818bbf3a052dd4220537d4de13c25ca Mon Sep 17 00:00:00 2001 From: Archer_Tsai Date: Wed, 6 May 2020 10:22:23 +0800 Subject: [PATCH 02/16] WIP: Use PermissionView.id instead of Tables.id --- ...23bf17ffa782_add_table_permission_model.py | 4 +- superset/models/__init__.py | 2 +- superset/models/table_permission.py | 8 ++- superset/security.py | 7 ++ superset/views/aics_privacy_control/views.py | 68 ++++++++++++++++++- 5 files changed, 81 insertions(+), 8 deletions(-) diff --git a/superset/migrations/versions/23bf17ffa782_add_table_permission_model.py b/superset/migrations/versions/23bf17ffa782_add_table_permission_model.py index 54b8114f9c61b..4773c36458416 100644 --- a/superset/migrations/versions/23bf17ffa782_add_table_permission_model.py +++ b/superset/migrations/versions/23bf17ffa782_add_table_permission_model.py @@ -37,7 +37,7 @@ def upgrade(): sa.Column('changed_on', sa.DateTime(), nullable=True), sa.Column('id', sa.Integer(), nullable=False), sa.Column('user_id', sa.Integer(), nullable=True), - sa.Column('table_id', sa.Integer(), nullable=True), + sa.Column('table_perm_id', sa.Integer(), nullable=True), sa.Column('apply_date', sa.Date(), nullable=False), sa.Column('expire_date', sa.Date(), nullable=False), sa.Column('force_treminate_date', sa.DateTime(), nullable=True), @@ -46,7 +46,7 @@ def upgrade(): sa.Column('changed_by_fk', sa.Integer(), nullable=True), sa.ForeignKeyConstraint(['changed_by_fk'], ['ab_user.id'], ), sa.ForeignKeyConstraint(['created_by_fk'], ['ab_user.id'], ), - sa.ForeignKeyConstraint(['table_id'], ['tables.id'], ), + sa.ForeignKeyConstraint(['table_perm_id'], ['ab_permission_view.id'], ), sa.ForeignKeyConstraint(['user_id'], ['ab_user.id'], ), sa.PrimaryKeyConstraint('id') ) diff --git a/superset/models/__init__.py b/superset/models/__init__.py index 17e7d188dc11a..7e4df3bbfe65f 100644 --- a/superset/models/__init__.py +++ b/superset/models/__init__.py @@ -14,4 +14,4 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -from . import core, schedules, sql_lab, user_attributes +from . import core, schedules, sql_lab, user_attributes, table_permission diff --git a/superset/models/table_permission.py b/superset/models/table_permission.py index a302b11908599..e19c09abc3c36 100644 --- a/superset/models/table_permission.py +++ b/superset/models/table_permission.py @@ -40,10 +40,14 @@ class TablePermission(Model, AuditMixinNullable): id = Column(Integer, primary_key=True) # pylint: disable=invalid-name user_id = Column(Integer, ForeignKey("ab_user.id")) user = relationship( - security_manager.user_model, backref="table_permissions", foreign_keys=[user_id] + security_manager.user_model, backref="user_table_perm", foreign_keys=[user_id] ) - table_id = Column(Integer, ForeignKey("tables.id")) + # table_id = Column(Integer, ForeignKey("tables.id")) + table_perm_id = Column(Integer, ForeignKey("ab_permission_view.id")) + table_perm = relationship( + security_manager.permissionview_model, backref="table_perm", foreign_keys=[table_perm_id] + ) apply_date = Column(Date, nullable=False, default=datetime.now()) expire_date = Column(Date, nullable=False) force_treminate_date = Column(DateTime) diff --git a/superset/security.py b/superset/security.py index 151591971b905..f54cf7810b5f8 100644 --- a/superset/security.py +++ b/superset/security.py @@ -315,7 +315,14 @@ def _datasource_access_by_name( db.session, database, table_name, schema=schema ) + all_datasources = ConnectorRegistry.get_all_datasources(db.session) + + for datasource in all_datasources: + print(f'datasource.perm: {datasource.perm}') + + print(f'-------------------------') for datasource in datasources: + print(f'datasource.perm: {datasource.perm}') if self.can_access("datasource_access", datasource.perm): return True diff --git a/superset/views/aics_privacy_control/views.py b/superset/views/aics_privacy_control/views.py index a16a3273b6bca..d5a4f35fe2d50 100644 --- a/superset/views/aics_privacy_control/views.py +++ b/superset/views/aics_privacy_control/views.py @@ -1,15 +1,16 @@ from uuid import uuid4 from sqlalchemy.sql import exists from flask_appbuilder.models.sqla.interface import SQLAInterface -from flask_appbuilder.fieldwidgets import BS3TextFieldWidget +from flask_appbuilder.fieldwidgets import BS3TextFieldWidget, Select2Widget from flask_appbuilder.security.sqla import models as ab_models from flask_babel import gettext as __, lazy_gettext as _ -from superset import appbuilder, db, event_logger +from superset import appbuilder, db, event_logger, security_manager from superset.views.base import ( DeleteMixin, SupersetModelView, ) from superset.models.user_attributes import UserAttribute +from superset.models.table_permission import TablePermission from wtforms.ext.sqlalchemy.fields import QuerySelectField from wtforms import TextField, SelectField from wtforms.validators import DataRequired @@ -29,6 +30,18 @@ def get_user_options(): User = ab_models.User return db.session.query(User).filter(User.active == True).filter(~ exists().where(UserAttribute.user_id == User.id)) +def get_table_perm_list(): + '''Get table list + Used in the QuerySelectField of add form of TablePermission + ''' + # get id of "datasource access" + # get all permission_id == "datasource access" from ab_permission_view + # store the id into table + perm_datasource_access = security_manager.find_permission('datasource_access') + print(perm_datasource_access.name) + pv_model = security_manager.permissionview_model + return db.session.query(pv_model).filter(pv_model.permission_id == perm_datasource_access.id) + class AccessKeyModelView(SupersetModelView, DeleteMixin): datamodel = SQLAInterface(UserAttribute) @@ -59,7 +72,8 @@ class AccessKeyModelView(SupersetModelView, DeleteMixin): add_form_extra_fields = { 'user': QuerySelectField('User', query_factory=get_user_options, - get_label='username'), + get_label='username', + widget=Select2Widget()), } description_columns = { @@ -88,17 +102,56 @@ class AccessKeyModelView(SupersetModelView, DeleteMixin): "changed_by_name", ] + @event_logger.log_this def pre_add(self, obj): obj.user_id = obj.user.id + pass # Replace access_key with the new one for write back to DB + @event_logger.log_this def pre_update(self, obj): obj.access_key = obj.new_access_key + @event_logger.log_this + def pre_delete(self, obj): + print(f'delete access key of user: {obj.username}') + # This function is used to generate new access key and fill in edit form def prefill_form(self, form, pk): form.new_access_key.data = str(uuid4()) +class TablePermissionModelView(SupersetModelView, DeleteMixin): + datamodel = SQLAInterface(TablePermission) + list_columns = [ + "username", + "avail_table_list", + "table_id", + "expire_date", + "is_active", + ] + order_columns = ["user_id"] + base_order = ("changed_on", "desc") + label_columns = { + "username": _("User"), + "avail_table_list": _("Table Permissions"), + "created_on": _("Created On"), + "changed_on": _("Changed On"), + "changed_by_name": _("Changed By"), + } + add_columns = [ + "user", + "tables", + "table_perm", + "expire_date", + "is_active", + ] + + add_form_extra_fields = { + 'tables': QuerySelectField('Tables', + query_factory=get_table_perm_list, + widget=Select2Widget()), + } + appbuilder.add_separator("Security") appbuilder.add_view( @@ -109,3 +162,12 @@ def prefill_form(self, form, pk): category_label=__("Security"), icon="fa-key", ) + +appbuilder.add_view( + TablePermissionModelView, + "Manage Table Permission", + label=__("Manage Table Permission"), + category="Security", + category_label=__("Security"), + icon="fa-list", +) From 64cc8e1b6c422374642913c6372c5afaedd7f012 Mon Sep 17 00:00:00 2001 From: Archer_Tsai Date: Wed, 6 May 2020 14:00:49 +0800 Subject: [PATCH 03/16] WIP: 2nd version of table permission --- ...Add AICS table permission control model.py | 92 +++++++++++++ superset/models/table_permission.py | 130 +++++++++++++----- superset/views/aics_privacy_control/views.py | 24 ++-- 3 files changed, 201 insertions(+), 45 deletions(-) create mode 100644 superset/migrations/versions/9beb6e4a3988_Add AICS table permission control model.py diff --git a/superset/migrations/versions/9beb6e4a3988_Add AICS table permission control model.py b/superset/migrations/versions/9beb6e4a3988_Add AICS table permission control model.py new file mode 100644 index 0000000000000..2371b3fc1a70a --- /dev/null +++ b/superset/migrations/versions/9beb6e4a3988_Add AICS table permission control model.py @@ -0,0 +1,92 @@ +# 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. +"""Add AICS table permission control model + +Revision ID: 9beb6e4a3988 +Revises: 23bf17ffa782 +Create Date: 2020-05-06 11:19:33.930627 + +""" + +# revision identifiers, used by Alembic. +revision = '9beb6e4a3988' +down_revision = '23bf17ffa782' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.create_table('aics_table_permission', + sa.Column('created_on', sa.DateTime(), nullable=True), + sa.Column('changed_on', sa.DateTime(), nullable=True), + sa.Column('id', sa.Integer(), nullable=False), + sa.Column('user_id', sa.Integer(), nullable=True), + sa.Column('created_by_fk', sa.Integer(), nullable=True), + sa.Column('changed_by_fk', sa.Integer(), nullable=True), + sa.ForeignKeyConstraint(['changed_by_fk'], ['ab_user.id'], ), + sa.ForeignKeyConstraint(['created_by_fk'], ['ab_user.id'], ), + sa.ForeignKeyConstraint(['user_id'], ['ab_user.id'], ), + sa.PrimaryKeyConstraint('id') + ) + op.create_table('aics_tableperm_permissionview', + sa.Column('created_on', sa.DateTime(), nullable=True), + sa.Column('changed_on', sa.DateTime(), nullable=True), + sa.Column('id', sa.Integer(), nullable=False), + sa.Column('tableperm_id', sa.Integer(), nullable=True), + sa.Column('permissionview_id', sa.Integer(), nullable=True), + sa.Column('apply_date', sa.Date(), nullable=False), + sa.Column('expire_date', sa.Date(), nullable=False), + sa.Column('force_treminate_date', sa.DateTime(), nullable=True), + sa.Column('is_active', sa.Boolean(), nullable=False), + sa.Column('created_by_fk', sa.Integer(), nullable=True), + sa.Column('changed_by_fk', sa.Integer(), nullable=True), + sa.ForeignKeyConstraint(['changed_by_fk'], ['ab_user.id'], ), + sa.ForeignKeyConstraint(['created_by_fk'], ['ab_user.id'], ), + sa.ForeignKeyConstraint(['permissionview_id'], ['ab_permission_view.id'], ), + sa.ForeignKeyConstraint(['tableperm_id'], ['aics_table_permission.id'], ), + sa.PrimaryKeyConstraint('id') + ) + op.drop_table('table_permission') + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.create_table('table_permission', + sa.Column('created_on', sa.DATETIME(), nullable=True), + sa.Column('changed_on', sa.DATETIME(), nullable=True), + sa.Column('id', sa.INTEGER(), nullable=False), + sa.Column('user_id', sa.INTEGER(), nullable=True), + sa.Column('table_perm_id', sa.INTEGER(), nullable=True), + sa.Column('apply_date', sa.DATE(), nullable=False), + sa.Column('expire_date', sa.DATE(), nullable=False), + sa.Column('force_treminate_date', sa.DATETIME(), nullable=True), + sa.Column('is_active', sa.BOOLEAN(), nullable=False), + sa.Column('created_by_fk', sa.INTEGER(), nullable=True), + sa.Column('changed_by_fk', sa.INTEGER(), nullable=True), + sa.CheckConstraint('is_active IN (0, 1)'), + sa.ForeignKeyConstraint(['changed_by_fk'], ['ab_user.id'], ), + sa.ForeignKeyConstraint(['created_by_fk'], ['ab_user.id'], ), + sa.ForeignKeyConstraint(['table_perm_id'], ['ab_permission_view.id'], ), + sa.ForeignKeyConstraint(['user_id'], ['ab_user.id'], ), + sa.PrimaryKeyConstraint('id') + ) + op.drop_table('aics_tableperm_permissionview') + op.drop_table('aics_table_permission') + # ### end Alembic commands ### diff --git a/superset/models/table_permission.py b/superset/models/table_permission.py index e19c09abc3c36..523bbd6691d39 100644 --- a/superset/models/table_permission.py +++ b/superset/models/table_permission.py @@ -15,60 +15,124 @@ # specific language governing permissions and limitations # under the License. import logging -from datetime import datetime +import re +from datetime import datetime, timedelta from flask_appbuilder import Model -from sqlalchemy import Column, ForeignKey, Integer, String, Date, DateTime, Boolean +from sqlalchemy import ( + Table, + Column, + ForeignKey, + Integer, + String, + Date, + DateTime, + Boolean, + Sequence, +) from sqlalchemy.orm import relationship from superset import security_manager from superset.models.helpers import AuditMixinNullable -class TablePermission(Model, AuditMixinNullable): +# class TablePermission(Model, AuditMixinNullable): - """ - Custom attributes attached to the user. +# """ +# Custom attributes attached to the user. + +# Extending the user attribute is tricky due to its dependency on the +# authentication typew an circular dependencies in Superset. Instead, we use +# a custom model for adding attributes. + +# """ + +# __tablename__ = "table_permission" +# id = Column(Integer, primary_key=True) # pylint: disable=invalid-name +# user_id = Column(Integer, ForeignKey("ab_user.id")) +# user = relationship( +# security_manager.user_model, backref="user_table_perm", foreign_keys=[user_id] +# ) + +# # table_id = Column(Integer, ForeignKey("tables.id")) +# table_perm_id = Column(Integer, ForeignKey("ab_permission_view.id")) +# table_perm = relationship( +# security_manager.permissionview_model, backref="table_perm", foreign_keys=[table_perm_id] +# ) +# apply_date = Column(Date, nullable=False, default=datetime.now()) +# expire_date = Column(Date, nullable=False) +# force_treminate_date = Column(DateTime) +# is_active = Column(Boolean, default=True, nullable=False) + +# @property +# def username(self): +# return self.user.username + +# @username.setter +# def username(self, value): +# pass - Extending the user attribute is tricky due to its dependency on the - authentication typew an circular dependencies in Superset. Instead, we use - a custom model for adding attributes. +# @property +# def table_names_with_id(self): +# return self._new_key + +# @table_names_with_id.setter +# def table_names_with_id(self, value): +# self._new_key = value + +# @property +# def changed_by_name(self): +# return self.changed_by_ + + +class TablePermission2PermissionView(Model, AuditMixinNullable): """ - __tablename__ = "table_permission" - id = Column(Integer, primary_key=True) # pylint: disable=invalid-name - user_id = Column(Integer, ForeignKey("ab_user.id")) - user = relationship( - security_manager.user_model, backref="user_table_perm", foreign_keys=[user_id] - ) + """ - # table_id = Column(Integer, ForeignKey("tables.id")) - table_perm_id = Column(Integer, ForeignKey("ab_permission_view.id")) - table_perm = relationship( - security_manager.permissionview_model, backref="table_perm", foreign_keys=[table_perm_id] + __tablename__ = "aics_tableperm_permissionview" + id = Column(Integer, Sequence("aics_tableperm_permissionview_id_seq"), primary_key=True) + tableperm_id = Column(Integer, ForeignKey("aics_table_permission.id")) + permissionview_id = Column(Integer, ForeignKey("ab_permission_view.id")) + permissionview = relationship( + security_manager.permissionview_model, backref="tableperm_permissionview", foreign_keys=[permissionview_id] ) + apply_date = Column(Date, nullable=False, default=datetime.now()) - expire_date = Column(Date, nullable=False) + expire_date = Column(Date, nullable=False, default=datetime.now()+timedelta(days=6*365/12)) force_treminate_date = Column(DateTime) is_active = Column(Boolean, default=True, nullable=False) - @property - def username(self): - return self.user.username + def __repr__(self): + if self.is_active: + return re.sub(r'.* ', '', self.permissionview)+str(self.expire_date) + else: + return '' - @username.setter - def username(self, value): - pass +class TablePermission(Model, AuditMixinNullable): - @property - def table_names_with_id(self): - return self._new_key + """ - @table_names_with_id.setter - def table_names_with_id(self, value): - self._new_key = value + """ + + __tablename__ = "aics_table_permission" + id = Column(Integer, Sequence("aics_table_permission_id_seq"), primary_key=True) # pylint: disable=invalid-name + user_id = Column(Integer, ForeignKey("ab_user.id")) + user = relationship( + security_manager.user_model, backref="user_table_perm", foreign_keys=[user_id] + ) + + table_permissions = relationship( + security_manager.permissionview_model, secondary=TablePermission2PermissionView.__table__, backref="table_perm" + ) @property - def changed_by_name(self): - return self.changed_by_ + def expire_date(self): + return self._exp_date + + @expire_date.setter + def expire_date(self, exp_date): + self._exp_date = exp_date + # if not + self.table_permissions.expire_date = exp_date \ No newline at end of file diff --git a/superset/views/aics_privacy_control/views.py b/superset/views/aics_privacy_control/views.py index d5a4f35fe2d50..cf0d5a4ecdf6a 100644 --- a/superset/views/aics_privacy_control/views.py +++ b/superset/views/aics_privacy_control/views.py @@ -123,16 +123,14 @@ def prefill_form(self, form, pk): class TablePermissionModelView(SupersetModelView, DeleteMixin): datamodel = SQLAInterface(TablePermission) list_columns = [ - "username", - "avail_table_list", - "table_id", - "expire_date", - "is_active", + "user", + "table_permissions", ] - order_columns = ["user_id"] - base_order = ("changed_on", "desc") + order_columns = ["user"] + # base_order = ("changed_on", "desc") label_columns = { - "username": _("User"), + "user": _("User"), + "table_permissions": _("Table Permissions"), "avail_table_list": _("Table Permissions"), "created_on": _("Created On"), "changed_on": _("Changed On"), @@ -140,16 +138,18 @@ class TablePermissionModelView(SupersetModelView, DeleteMixin): } add_columns = [ "user", - "tables", - "table_perm", - "expire_date", - "is_active", + "table_permissions", + # "tables", + # "table_perm", + # "expire_date", + # "is_active", ] add_form_extra_fields = { 'tables': QuerySelectField('Tables', query_factory=get_table_perm_list, widget=Select2Widget()), + # 'expire_date': } appbuilder.add_separator("Security") From 617f395e2b2c64d0d9d4e6b18dd028eb0454e5f5 Mon Sep 17 00:00:00 2001 From: Archer_Tsai Date: Wed, 6 May 2020 15:25:44 +0800 Subject: [PATCH 04/16] WIP: Refine table structure --- ...dd_aics_table_permission_control_model.py} | 22 +++-- ...Add AICS table permission control model.py | 92 ------------------- superset/models/table_permission.py | 72 +++++++++------ superset/views/aics_privacy_control/views.py | 14 +-- 4 files changed, 65 insertions(+), 135 deletions(-) rename superset/migrations/versions/{23bf17ffa782_add_table_permission_model.py => 9847d62ead51_add_aics_table_permission_control_model.py} (73%) delete mode 100644 superset/migrations/versions/9beb6e4a3988_Add AICS table permission control model.py diff --git a/superset/migrations/versions/23bf17ffa782_add_table_permission_model.py b/superset/migrations/versions/9847d62ead51_add_aics_table_permission_control_model.py similarity index 73% rename from superset/migrations/versions/23bf17ffa782_add_table_permission_model.py rename to superset/migrations/versions/9847d62ead51_add_aics_table_permission_control_model.py index 4773c36458416..418e54985053f 100644 --- a/superset/migrations/versions/23bf17ffa782_add_table_permission_model.py +++ b/superset/migrations/versions/9847d62ead51_add_aics_table_permission_control_model.py @@ -16,14 +16,14 @@ # under the License. """Add AICS table permission control model -Revision ID: 23bf17ffa782 +Revision ID: 9847d62ead51 Revises: 5ad4d4f30245 -Create Date: 2020-05-04 12:06:19.643832 +Create Date: 2020-05-06 15:23:35.200160 """ # revision identifiers, used by Alembic. -revision = '23bf17ffa782' +revision = '9847d62ead51' down_revision = '5ad4d4f30245' from alembic import op @@ -32,12 +32,11 @@ def upgrade(): # ### commands auto generated by Alembic - please adjust! ### - op.create_table('table_permission', + op.create_table('aics_table_permission', sa.Column('created_on', sa.DateTime(), nullable=True), sa.Column('changed_on', sa.DateTime(), nullable=True), sa.Column('id', sa.Integer(), nullable=False), sa.Column('user_id', sa.Integer(), nullable=True), - sa.Column('table_perm_id', sa.Integer(), nullable=True), sa.Column('apply_date', sa.Date(), nullable=False), sa.Column('expire_date', sa.Date(), nullable=False), sa.Column('force_treminate_date', sa.DateTime(), nullable=True), @@ -46,14 +45,23 @@ def upgrade(): sa.Column('changed_by_fk', sa.Integer(), nullable=True), sa.ForeignKeyConstraint(['changed_by_fk'], ['ab_user.id'], ), sa.ForeignKeyConstraint(['created_by_fk'], ['ab_user.id'], ), - sa.ForeignKeyConstraint(['table_perm_id'], ['ab_permission_view.id'], ), sa.ForeignKeyConstraint(['user_id'], ['ab_user.id'], ), sa.PrimaryKeyConstraint('id') ) + op.create_table('aics_tableperm_permissionview', + sa.Column('id', sa.Integer(), nullable=False), + sa.Column('tableperm_id', sa.Integer(), nullable=True), + sa.Column('permissionview_id', sa.Integer(), nullable=True), + sa.ForeignKeyConstraint(['permissionview_id'], ['ab_permission_view.id'], ), + sa.ForeignKeyConstraint(['tableperm_id'], ['aics_table_permission.id'], ), + sa.PrimaryKeyConstraint('id'), + sa.UniqueConstraint('tableperm_id', 'permissionview_id') + ) # ### end Alembic commands ### def downgrade(): # ### commands auto generated by Alembic - please adjust! ### - op.drop_table('table_permission') + op.drop_table('aics_tableperm_permissionview') + op.drop_table('aics_table_permission') # ### end Alembic commands ### diff --git a/superset/migrations/versions/9beb6e4a3988_Add AICS table permission control model.py b/superset/migrations/versions/9beb6e4a3988_Add AICS table permission control model.py deleted file mode 100644 index 2371b3fc1a70a..0000000000000 --- a/superset/migrations/versions/9beb6e4a3988_Add AICS table permission control model.py +++ /dev/null @@ -1,92 +0,0 @@ -# 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. -"""Add AICS table permission control model - -Revision ID: 9beb6e4a3988 -Revises: 23bf17ffa782 -Create Date: 2020-05-06 11:19:33.930627 - -""" - -# revision identifiers, used by Alembic. -revision = '9beb6e4a3988' -down_revision = '23bf17ffa782' - -from alembic import op -import sqlalchemy as sa - - -def upgrade(): - # ### commands auto generated by Alembic - please adjust! ### - op.create_table('aics_table_permission', - sa.Column('created_on', sa.DateTime(), nullable=True), - sa.Column('changed_on', sa.DateTime(), nullable=True), - sa.Column('id', sa.Integer(), nullable=False), - sa.Column('user_id', sa.Integer(), nullable=True), - sa.Column('created_by_fk', sa.Integer(), nullable=True), - sa.Column('changed_by_fk', sa.Integer(), nullable=True), - sa.ForeignKeyConstraint(['changed_by_fk'], ['ab_user.id'], ), - sa.ForeignKeyConstraint(['created_by_fk'], ['ab_user.id'], ), - sa.ForeignKeyConstraint(['user_id'], ['ab_user.id'], ), - sa.PrimaryKeyConstraint('id') - ) - op.create_table('aics_tableperm_permissionview', - sa.Column('created_on', sa.DateTime(), nullable=True), - sa.Column('changed_on', sa.DateTime(), nullable=True), - sa.Column('id', sa.Integer(), nullable=False), - sa.Column('tableperm_id', sa.Integer(), nullable=True), - sa.Column('permissionview_id', sa.Integer(), nullable=True), - sa.Column('apply_date', sa.Date(), nullable=False), - sa.Column('expire_date', sa.Date(), nullable=False), - sa.Column('force_treminate_date', sa.DateTime(), nullable=True), - sa.Column('is_active', sa.Boolean(), nullable=False), - sa.Column('created_by_fk', sa.Integer(), nullable=True), - sa.Column('changed_by_fk', sa.Integer(), nullable=True), - sa.ForeignKeyConstraint(['changed_by_fk'], ['ab_user.id'], ), - sa.ForeignKeyConstraint(['created_by_fk'], ['ab_user.id'], ), - sa.ForeignKeyConstraint(['permissionview_id'], ['ab_permission_view.id'], ), - sa.ForeignKeyConstraint(['tableperm_id'], ['aics_table_permission.id'], ), - sa.PrimaryKeyConstraint('id') - ) - op.drop_table('table_permission') - # ### end Alembic commands ### - - -def downgrade(): - # ### commands auto generated by Alembic - please adjust! ### - op.create_table('table_permission', - sa.Column('created_on', sa.DATETIME(), nullable=True), - sa.Column('changed_on', sa.DATETIME(), nullable=True), - sa.Column('id', sa.INTEGER(), nullable=False), - sa.Column('user_id', sa.INTEGER(), nullable=True), - sa.Column('table_perm_id', sa.INTEGER(), nullable=True), - sa.Column('apply_date', sa.DATE(), nullable=False), - sa.Column('expire_date', sa.DATE(), nullable=False), - sa.Column('force_treminate_date', sa.DATETIME(), nullable=True), - sa.Column('is_active', sa.BOOLEAN(), nullable=False), - sa.Column('created_by_fk', sa.INTEGER(), nullable=True), - sa.Column('changed_by_fk', sa.INTEGER(), nullable=True), - sa.CheckConstraint('is_active IN (0, 1)'), - sa.ForeignKeyConstraint(['changed_by_fk'], ['ab_user.id'], ), - sa.ForeignKeyConstraint(['created_by_fk'], ['ab_user.id'], ), - sa.ForeignKeyConstraint(['table_perm_id'], ['ab_permission_view.id'], ), - sa.ForeignKeyConstraint(['user_id'], ['ab_user.id'], ), - sa.PrimaryKeyConstraint('id') - ) - op.drop_table('aics_tableperm_permissionview') - op.drop_table('aics_table_permission') - # ### end Alembic commands ### diff --git a/superset/models/table_permission.py b/superset/models/table_permission.py index 523bbd6691d39..08e16e29c6aa6 100644 --- a/superset/models/table_permission.py +++ b/superset/models/table_permission.py @@ -29,6 +29,7 @@ DateTime, Boolean, Sequence, + UniqueConstraint, ) from sqlalchemy.orm import relationship @@ -85,30 +86,34 @@ # return self.changed_by_ -class TablePermission2PermissionView(Model, AuditMixinNullable): +# class TablePermission2PermissionView(Model, AuditMixinNullable): - """ +# """ - """ +# """ - __tablename__ = "aics_tableperm_permissionview" - id = Column(Integer, Sequence("aics_tableperm_permissionview_id_seq"), primary_key=True) - tableperm_id = Column(Integer, ForeignKey("aics_table_permission.id")) - permissionview_id = Column(Integer, ForeignKey("ab_permission_view.id")) - permissionview = relationship( - security_manager.permissionview_model, backref="tableperm_permissionview", foreign_keys=[permissionview_id] - ) - - apply_date = Column(Date, nullable=False, default=datetime.now()) - expire_date = Column(Date, nullable=False, default=datetime.now()+timedelta(days=6*365/12)) - force_treminate_date = Column(DateTime) - is_active = Column(Boolean, default=True, nullable=False) +# __tablename__ = "aics_tableperm_permissionview" +# id = Column(Integer, Sequence("aics_tableperm_permissionview_id_seq"), primary_key=True) +# tableperm_id = Column(Integer, ForeignKey("aics_table_permission.id")) +# permissionview_id = Column(Integer, ForeignKey("ab_permission_view.id")) +# permissionview = relationship( +# security_manager.permissionview_model, backref="tableperm_permissionview", foreign_keys=[permissionview_id] +# ) - def __repr__(self): - if self.is_active: - return re.sub(r'.* ', '', self.permissionview)+str(self.expire_date) - else: - return '' +# def __repr__(self): +# if self.is_active: +# return re.sub(r'.* ', '', self.permissionview)+str(self.expire_date) +# else: +# return '' + +assoc_tableperm_permissionview = Table( + "aics_tableperm_permissionview", + Model.metadata, + Column('id', Integer, Sequence("aics_tableperm_permissionview_id_seq"), primary_key=True) , + Column('tableperm_id', Integer, ForeignKey("aics_table_permission.id")), + Column('permissionview_id', Integer, ForeignKey("ab_permission_view.id")), + UniqueConstraint("tableperm_id", "permissionview_id"), +) class TablePermission(Model, AuditMixinNullable): @@ -123,16 +128,23 @@ class TablePermission(Model, AuditMixinNullable): security_manager.user_model, backref="user_table_perm", foreign_keys=[user_id] ) + apply_date = Column(Date, nullable=False, default=datetime.now()) + expire_date = Column(Date, nullable=False, default=datetime.now()+timedelta(days=6*365/12)) + force_treminate_date = Column(DateTime) + is_active = Column(Boolean, default=True, nullable=False) + table_permissions = relationship( - security_manager.permissionview_model, secondary=TablePermission2PermissionView.__table__, backref="table_perm" + security_manager.permissionview_model, secondary=assoc_tableperm_permissionview, backref="table_perm" ) - @property - def expire_date(self): - return self._exp_date - - @expire_date.setter - def expire_date(self, exp_date): - self._exp_date = exp_date - # if not - self.table_permissions.expire_date = exp_date \ No newline at end of file + # @property + # def expire_date(self): + # return self._exp_date + + # @expire_date.setter + # def expire_date(self, exp_date): + # self._exp_date = exp_date + # print(str(len(self.table_permissions))) + # for permission in self.table_permissions: + # print(permission) + # permission.expire_date = exp_date \ No newline at end of file diff --git a/superset/views/aics_privacy_control/views.py b/superset/views/aics_privacy_control/views.py index cf0d5a4ecdf6a..921b0a5a232d5 100644 --- a/superset/views/aics_privacy_control/views.py +++ b/superset/views/aics_privacy_control/views.py @@ -1,7 +1,7 @@ from uuid import uuid4 from sqlalchemy.sql import exists from flask_appbuilder.models.sqla.interface import SQLAInterface -from flask_appbuilder.fieldwidgets import BS3TextFieldWidget, Select2Widget +from flask_appbuilder.fieldwidgets import BS3TextFieldWidget, Select2Widget, Select2ManyWidget, DatePickerWidget from flask_appbuilder.security.sqla import models as ab_models from flask_babel import gettext as __, lazy_gettext as _ from superset import appbuilder, db, event_logger, security_manager @@ -11,8 +11,8 @@ ) from superset.models.user_attributes import UserAttribute from superset.models.table_permission import TablePermission -from wtforms.ext.sqlalchemy.fields import QuerySelectField -from wtforms import TextField, SelectField +from wtforms.ext.sqlalchemy.fields import QuerySelectField, QuerySelectMultipleField +from wtforms import TextField, SelectField, DateField from wtforms.validators import DataRequired class BS3TextFieldROWidget(BS3TextFieldWidget): @@ -146,10 +146,12 @@ class TablePermissionModelView(SupersetModelView, DeleteMixin): ] add_form_extra_fields = { - 'tables': QuerySelectField('Tables', + 'tables': QuerySelectMultipleField('Tables', query_factory=get_table_perm_list, - widget=Select2Widget()), - # 'expire_date': + widget=Select2ManyWidget()), + # 'expire_date': DateField('Expire Date', + # widget=DatePickerWidget() + # ), } appbuilder.add_separator("Security") From e1167f43a4f624eb6c47450cd612b9c183e76746 Mon Sep 17 00:00:00 2001 From: Archer_Tsai Date: Wed, 6 May 2020 18:29:08 +0800 Subject: [PATCH 05/16] WIP: refine table definition and fixed typo --- ...51_add_aics_table_permission_control_model.py | 4 ++-- superset/models/table_permission.py | 16 ++++++++++++---- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/superset/migrations/versions/9847d62ead51_add_aics_table_permission_control_model.py b/superset/migrations/versions/9847d62ead51_add_aics_table_permission_control_model.py index 418e54985053f..49308fcde5ca9 100644 --- a/superset/migrations/versions/9847d62ead51_add_aics_table_permission_control_model.py +++ b/superset/migrations/versions/9847d62ead51_add_aics_table_permission_control_model.py @@ -39,8 +39,8 @@ def upgrade(): sa.Column('user_id', sa.Integer(), nullable=True), sa.Column('apply_date', sa.Date(), nullable=False), sa.Column('expire_date', sa.Date(), nullable=False), - sa.Column('force_treminate_date', sa.DateTime(), nullable=True), - sa.Column('is_active', sa.Boolean(), nullable=False), + sa.Column('force_terminate_date', sa.DateTime(), nullable=True), + sa.Column('is_active', sa.Boolean()), sa.Column('created_by_fk', sa.Integer(), nullable=True), sa.Column('changed_by_fk', sa.Integer(), nullable=True), sa.ForeignKeyConstraint(['changed_by_fk'], ['ab_user.id'], ), diff --git a/superset/models/table_permission.py b/superset/models/table_permission.py index 08e16e29c6aa6..a052b36686d4c 100644 --- a/superset/models/table_permission.py +++ b/superset/models/table_permission.py @@ -128,15 +128,23 @@ class TablePermission(Model, AuditMixinNullable): security_manager.user_model, backref="user_table_perm", foreign_keys=[user_id] ) - apply_date = Column(Date, nullable=False, default=datetime.now()) - expire_date = Column(Date, nullable=False, default=datetime.now()+timedelta(days=6*365/12)) - force_treminate_date = Column(DateTime) - is_active = Column(Boolean, default=True, nullable=False) + apply_date = Column(Date, nullable=False, default=datetime.now().date()) + expire_date = Column(Date, nullable=False, default=datetime.now().date()+timedelta(days=6*365/12)) + force_terminate_date = Column(DateTime) + is_active = Column(Boolean, default=True) table_permissions = relationship( security_manager.permissionview_model, secondary=assoc_tableperm_permissionview, backref="table_perm" ) + @property + def username(self): + return self.user.username + + @username.setter + def username(self, value): + pass + # @property # def expire_date(self): # return self._exp_date From dc2444d70c333a10c5b19d3637cd630497fe0eb1 Mon Sep 17 00:00:00 2001 From: Archer_Tsai Date: Wed, 6 May 2020 18:29:44 +0800 Subject: [PATCH 06/16] WIP: Refine table permission view --- superset/views/aics_privacy_control/views.py | 174 +++++++++++++------ 1 file changed, 117 insertions(+), 57 deletions(-) diff --git a/superset/views/aics_privacy_control/views.py b/superset/views/aics_privacy_control/views.py index 921b0a5a232d5..c7df4e76f6dd9 100644 --- a/superset/views/aics_privacy_control/views.py +++ b/superset/views/aics_privacy_control/views.py @@ -1,28 +1,46 @@ from uuid import uuid4 +from datetime import datetime + from sqlalchemy.sql import exists from flask_appbuilder.models.sqla.interface import SQLAInterface -from flask_appbuilder.fieldwidgets import BS3TextFieldWidget, Select2Widget, Select2ManyWidget, DatePickerWidget +from flask_appbuilder.fieldwidgets import ( + BS3TextFieldWidget, + Select2Widget, + Select2ManyWidget, + DatePickerWidget +) from flask_appbuilder.security.sqla import models as ab_models +from flask_appbuilder.models.sqla.filters import FilterEqual from flask_babel import gettext as __, lazy_gettext as _ from superset import appbuilder, db, event_logger, security_manager from superset.views.base import ( DeleteMixin, SupersetModelView, ) +from superset.connectors.connector_registry import ConnectorRegistry from superset.models.user_attributes import UserAttribute from superset.models.table_permission import TablePermission from wtforms.ext.sqlalchemy.fields import QuerySelectField, QuerySelectMultipleField -from wtforms import TextField, SelectField, DateField +from wtforms import TextField, SelectField, SelectMultipleField, DateField from wtforms.validators import DataRequired class BS3TextFieldROWidget(BS3TextFieldWidget): - '''Inherent BS3TextFieldWidget and create a read only version + '''Inherit BS3TextFieldWidget and create a read only version ref: https://github.com/dpgaspar/Flask-AppBuilder/blob/master/docs/advanced.rst#forms---readonly-fields ''' def __call__(self, field, **kwargs): kwargs['readonly'] = 'true' return super(BS3TextFieldROWidget, self).__call__(field, **kwargs) + +class Select2ManyROWidget(Select2ManyWidget): + '''Inherit Select2ManyWidget and create a read only version + ref: https://github.com/dpgaspar/Flask-AppBuilder/blob/master/docs/advanced.rst#forms---readonly-fields + ''' + def __call__(self, field, **kwargs): + kwargs['readonly'] = 'true' + return super(Select2ManyWidget, self).__call__(field, **kwargs) + def get_user_options(): '''Get user list and filter out the ones already has a access key Used in the QuerySelectField of add form @@ -34,39 +52,52 @@ def get_table_perm_list(): '''Get table list Used in the QuerySelectField of add form of TablePermission ''' - # get id of "datasource access" - # get all permission_id == "datasource access" from ab_permission_view + # get id of 'datasource access' + # get all permission_id == 'datasource access' from ab_permission_view # store the id into table perm_datasource_access = security_manager.find_permission('datasource_access') print(perm_datasource_access.name) pv_model = security_manager.permissionview_model - return db.session.query(pv_model).filter(pv_model.permission_id == perm_datasource_access.id) + + all_datasources = ConnectorRegistry.get_all_datasources(db.session) + # all_datasources_perm = [(datasource.perm) for datasource in all_datasources] + view_menu_ids = [] + for datasource in all_datasources: + view_menu_ids.append(security_manager.find_view_menu(datasource.perm).id) + + print(f'view_menu_ids count: {len(view_menu_ids)} {len(all_datasources)}') + + all_permissions = db.session.query(pv_model).filter(pv_model.permission == perm_datasource_access).filter(pv_model.view_menu_id.in_(view_menu_ids)) + print(all_permissions[0]) + print(all_permissions[0].__dict__) + + return all_permissions class AccessKeyModelView(SupersetModelView, DeleteMixin): datamodel = SQLAInterface(UserAttribute) list_columns = [ - "username", - "access_key", - "created_on", - "changed_on", - "changed_by_name", + 'username', + 'access_key', + 'created_on', + 'changed_on', + 'changed_by_name', ] - order_columns = ["user_id"] - base_order = ("changed_on", "desc") + order_columns = ['user_id'] + base_order = ('changed_on', 'desc') label_columns = { - "username": _("User"), - "access_key": _("Access Key"), - "created_on": _("Created On"), - "changed_on": _("Changed On"), - "changed_by_name": _("Changed By"), + 'username': _('User'), + 'access_key': _('Access Key'), + 'created_on': _('Created On'), + 'changed_on': _('Changed On'), + 'changed_by_name': _('Changed By'), } add_columns = [ - "user", - "access_key", + 'user', + 'access_key', ] add_form_extra_fields = { @@ -84,28 +115,27 @@ class AccessKeyModelView(SupersetModelView, DeleteMixin): 'username': TextField('User Name', widget=BS3TextFieldROWidget()), 'access_key': TextField('Original Access Key', widget=BS3TextFieldROWidget()), 'new_access_key': TextField('New Access Key', - description=('Not editable, click "Save" to replace the access key with this new key.'), + description=('Not editable, click \'Save\' to replace the access key with this new key.'), widget=BS3TextFieldROWidget()), } edit_columns = [ - "username", - "access_key", - "new_access_key", + 'username', + 'access_key', + 'new_access_key', ] show_columns = [ - "username", - "access_key", - "created_on", - "changed_on", - "changed_by_name", + 'username', + 'access_key', + 'created_on', + 'changed_on', + 'changed_by_name', ] @event_logger.log_this def pre_add(self, obj): obj.user_id = obj.user.id - pass # Replace access_key with the new one for write back to DB @event_logger.log_this @@ -123,26 +153,49 @@ def prefill_form(self, form, pk): class TablePermissionModelView(SupersetModelView, DeleteMixin): datamodel = SQLAInterface(TablePermission) list_columns = [ - "user", - "table_permissions", + 'username', + 'table_permissions', + 'expire_date', + 'is_active', ] - order_columns = ["user"] - # base_order = ("changed_on", "desc") + + order_columns = ['username', 'expire_date', 'is_active'] + base_order = ('user_id', 'desc') + base_filters = [['is_active', FilterEqual, True]] + + label_columns = { - "user": _("User"), - "table_permissions": _("Table Permissions"), - "avail_table_list": _("Table Permissions"), - "created_on": _("Created On"), - "changed_on": _("Changed On"), - "changed_by_name": _("Changed By"), + 'user': _('User'), + 'username': _('User'), + 'table_permissions': _('Table Permissions'), + 'avail_table_list': _('Table Permissions'), + 'created_on': _('Created On'), + 'changed_on': _('Changed On'), + 'changed_by_name': _('Changed By'), } + + edit_columns = [ + 'username', + 'table_permissions', + 'expire_date', + 'force_terminate_date', + 'is_active', + ] + + edit_form_extra_fields = { + 'username': TextField('User Name', widget=BS3TextFieldROWidget()), + 'expire_date': TextField('Expire Date', widget=BS3TextFieldROWidget()), + # 'table_permissions': SelectMultipleField('Table Permissions', widget=Select2ManyROWidget()), + 'table_permissions': TextField('Table Permissions', widget=BS3TextFieldROWidget()), + } + add_columns = [ - "user", - "table_permissions", - # "tables", - # "table_perm", - # "expire_date", - # "is_active", + 'user', + # 'table_permissions', + 'tables', + # 'table_perm', + 'expire_date', + # 'is_active', ] add_form_extra_fields = { @@ -154,22 +207,29 @@ class TablePermissionModelView(SupersetModelView, DeleteMixin): # ), } -appbuilder.add_separator("Security") + def pre_add(self, obj): + obj.table_permissions = obj.tables + + def pre_update(self, obj): + if obj.is_active == False: + obj.force_treminate_date = datetime.now() + +appbuilder.add_separator('Security') appbuilder.add_view( AccessKeyModelView, - "Manage Access Keys", - label=__("Manage Access Keys"), - category="Security", - category_label=__("Security"), - icon="fa-key", + 'Manage Access Keys', + label=__('Manage Access Keys'), + category='Security', + category_label=__('Security'), + icon='fa-key', ) appbuilder.add_view( TablePermissionModelView, - "Manage Table Permission", - label=__("Manage Table Permission"), - category="Security", - category_label=__("Security"), - icon="fa-list", + 'Manage Table Permission', + label=__('Manage Table Permission'), + category='Security', + category_label=__('Security'), + icon='fa-list', ) From 80123cf620f9715cc334e8760fe2662ef5817685 Mon Sep 17 00:00:00 2001 From: Archer_Tsai Date: Wed, 6 May 2020 19:59:17 +0800 Subject: [PATCH 07/16] WIP: finished table permission check --- superset/models/table_permission.py | 2 +- superset/security.py | 33 +++++++++++++++++++++++------ 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/superset/models/table_permission.py b/superset/models/table_permission.py index a052b36686d4c..bd86a265b3f35 100644 --- a/superset/models/table_permission.py +++ b/superset/models/table_permission.py @@ -125,7 +125,7 @@ class TablePermission(Model, AuditMixinNullable): id = Column(Integer, Sequence("aics_table_permission_id_seq"), primary_key=True) # pylint: disable=invalid-name user_id = Column(Integer, ForeignKey("ab_user.id")) user = relationship( - security_manager.user_model, backref="user_table_perm", foreign_keys=[user_id] + security_manager.user_model, backref="table_permissions", foreign_keys=[user_id] ) apply_date = Column(Date, nullable=False, default=datetime.now().date()) diff --git a/superset/security.py b/superset/security.py index f54cf7810b5f8..2c40326c12eb4 100644 --- a/superset/security.py +++ b/superset/security.py @@ -288,6 +288,26 @@ def get_table_access_link(self, tables: List[str]) -> Optional[str]: return conf.get("PERMISSION_INSTRUCTIONS_LINK") + def _has_aics_table_permission(self, user: object, permission_name: str, view_name: str): + from superset import db + from superset.models.table_permission import TablePermission + + if user.is_anonymous: + print('user is anomynous') + return False + + perm = self.find_permission_view_menu( permission_name, view_name) + print(user) + print(f'{permission_name} on {view_name}') + print(perm) + table_permissions = db.session.query(TablePermission).filter(TablePermission.user_id == user.id, TablePermission.is_active == True) + for table_perm in table_permissions: + for pv in table_perm.table_permissions: + print(pv) + if pv == perm: + return True + + def _datasource_access_by_name( self, database: "Database", table_name: str, schema: str = None ) -> bool: @@ -315,17 +335,16 @@ def _datasource_access_by_name( db.session, database, table_name, schema=schema ) - all_datasources = ConnectorRegistry.get_all_datasources(db.session) - - for datasource in all_datasources: - print(f'datasource.perm: {datasource.perm}') - - print(f'-------------------------') for datasource in datasources: - print(f'datasource.perm: {datasource.perm}') + # Check permission using AppBuilder Roles if self.can_access("datasource_access", datasource.perm): return True + print('Check AICS Permission') + # Check AICS table permissions + if self._has_aics_table_permission(g.user, "datasource_access", datasource.perm): + return True + return False def _get_schema_and_table( From 24f7b45fd712d212c72ece44b26a145e9892e855 Mon Sep 17 00:00:00 2001 From: Archer_Tsai Date: Wed, 6 May 2020 22:17:32 +0800 Subject: [PATCH 08/16] WIP: Finished auto-revoke Rest API, code cleaning and refactoring are required --- superset/config.py | 2 +- superset/utils/log.py | 18 +++++++++++++----- superset/views/aics_privacy_control/views.py | 1 - 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/superset/config.py b/superset/config.py index 89679ea828d40..b6be6f289952d 100644 --- a/superset/config.py +++ b/superset/config.py @@ -117,7 +117,7 @@ def _try_json_readfile(filepath): WTF_CSRF_ENABLED = True # Add endpoints that need to be exempt from CSRF protection -WTF_CSRF_EXEMPT_LIST = ["superset.views.core.log", "superset.views.core.sql_csv_api"] +WTF_CSRF_EXEMPT_LIST = ["superset.views.core.log", "superset.views.core.sql_csv_api", "superset.views.core.revoke_expired_perm"] # Whether to run the web server in debug mode or not DEBUG = os.environ.get("FLASK_ENV") == "development" diff --git a/superset/utils/log.py b/superset/utils/log.py index ece1082e85abe..e338ae7c7e628 100644 --- a/superset/utils/log.py +++ b/superset/utils/log.py @@ -91,7 +91,8 @@ def wrapper(*args, **kwargs): database=extra_info.get("database"), schema=extra_info.get("schema"), sql=extra_info.get("sql"), - err_msg=extra_info.get("err_msg") + err_msg=extra_info.get("err_msg"), + details=extra_info.get("details") ) return value @@ -159,6 +160,7 @@ def log(self, user_id, action, *args, **kwargs): database = kwargs.get("database") schema = kwargs.get("schema") sql = kwargs.get("sql") + details = kwargs.get("details") err_msg = kwargs.get("err_msg") success = "true" if err_msg == None else "false" @@ -178,12 +180,18 @@ def log(self, user_id, action, *args, **kwargs): user_id=user_id, ) logs.append(log) - self.appinsights( - {'level': 'info', 'success': success, 'state':'finish', + json_log = {'level': 'info', 'success': success, 'state':'finish', 'function': action, 'json': json_string, 'duration': duration_ms, 'referrer': referrer, 'user_id': user_id, - 'database': database, 'schema': schema, 'sql': sql, 'err_msg': err_msg - }) + 'database': database, 'schema': schema, 'sql': sql + } + if err_msg != None: + json_log['err_msg'] = err_msg + + if details != None: + json_log['details'] = details + + self.appinsights(json_log) sesh = current_app.appbuilder.get_session sesh.bulk_save_objects(logs) diff --git a/superset/views/aics_privacy_control/views.py b/superset/views/aics_privacy_control/views.py index c7df4e76f6dd9..468e8c6810fac 100644 --- a/superset/views/aics_privacy_control/views.py +++ b/superset/views/aics_privacy_control/views.py @@ -161,7 +161,6 @@ class TablePermissionModelView(SupersetModelView, DeleteMixin): order_columns = ['username', 'expire_date', 'is_active'] base_order = ('user_id', 'desc') - base_filters = [['is_active', FilterEqual, True]] label_columns = { From e936b1787158bfa53a933f9b74fa24d46dc1cd62 Mon Sep 17 00:00:00 2001 From: Archer_Tsai Date: Wed, 6 May 2020 22:21:23 +0800 Subject: [PATCH 09/16] WIP: Add missing file of last commit --- superset/views/core.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/superset/views/core.py b/superset/views/core.py index be95e7283c79f..762307d5613f6 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -78,6 +78,7 @@ from superset.jinja_context import get_template_processor from superset.models.sql_lab import Query from superset.models.user_attributes import UserAttribute +from superset.models.table_permission import TablePermission from superset.sql_parse import ParsedQuery from superset.sql_validators import get_validator_by_name from superset.utils import core as utils, dashboard_import_export @@ -2981,6 +2982,31 @@ def csv(self, client_id): return self._streaming_csv(query, client_id) + @api + @expose("/revoke_expired_perm", methods=["GET"]) + @event_logger.log_this + def revoke_expired_perm(self): + session = db.session() + expired_perms = session.query(TablePermission).filter( + TablePermission.is_active == True, + TablePermission.expire_date < datetime.now().date() + ) + revoke_msg = {'revoke-perm':[]} + for perm in expired_perms: + perm_info = {'user': perm.username, 'perms':[]} + for table in perm.table_permissions: + perm_info['perms'].append(str(table)) + print(f"revoke {str(table)}") + revoke_msg['revoke-perm'].append(perm_info) + perm.is_active = False + perm.changed_by_fk = perm.changed_by_fk + session.commit() + session.close() + + extra_info = {'details': revoke_msg} + + return "OK", extra_info + @api @handle_api_exception @has_access From 7939d0e06910195b3b0b0a613cde69ab30220227 Mon Sep 17 00:00:00 2001 From: Archer_Tsai Date: Thu, 7 May 2020 14:03:19 +0800 Subject: [PATCH 10/16] WIP: Refine GUI and forbid re-activate revoked permissions and clean up code --- superset/models/table_permission.py | 119 ++++++------------- superset/views/aics_privacy_control/views.py | 62 +++++++--- superset/views/core.py | 3 +- 3 files changed, 80 insertions(+), 104 deletions(-) diff --git a/superset/models/table_permission.py b/superset/models/table_permission.py index bd86a265b3f35..e9a850e9752cf 100644 --- a/superset/models/table_permission.py +++ b/superset/models/table_permission.py @@ -14,7 +14,6 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -import logging import re from datetime import datetime, timedelta @@ -24,7 +23,6 @@ Column, ForeignKey, Integer, - String, Date, DateTime, Boolean, @@ -36,76 +34,6 @@ from superset import security_manager from superset.models.helpers import AuditMixinNullable - -# class TablePermission(Model, AuditMixinNullable): - -# """ -# Custom attributes attached to the user. - -# Extending the user attribute is tricky due to its dependency on the -# authentication typew an circular dependencies in Superset. Instead, we use -# a custom model for adding attributes. - -# """ - -# __tablename__ = "table_permission" -# id = Column(Integer, primary_key=True) # pylint: disable=invalid-name -# user_id = Column(Integer, ForeignKey("ab_user.id")) -# user = relationship( -# security_manager.user_model, backref="user_table_perm", foreign_keys=[user_id] -# ) - -# # table_id = Column(Integer, ForeignKey("tables.id")) -# table_perm_id = Column(Integer, ForeignKey("ab_permission_view.id")) -# table_perm = relationship( -# security_manager.permissionview_model, backref="table_perm", foreign_keys=[table_perm_id] -# ) -# apply_date = Column(Date, nullable=False, default=datetime.now()) -# expire_date = Column(Date, nullable=False) -# force_treminate_date = Column(DateTime) -# is_active = Column(Boolean, default=True, nullable=False) - -# @property -# def username(self): -# return self.user.username - -# @username.setter -# def username(self, value): -# pass - -# @property -# def table_names_with_id(self): -# return self._new_key - -# @table_names_with_id.setter -# def table_names_with_id(self, value): -# self._new_key = value - -# @property -# def changed_by_name(self): -# return self.changed_by_ - - -# class TablePermission2PermissionView(Model, AuditMixinNullable): - -# """ - -# """ - -# __tablename__ = "aics_tableperm_permissionview" -# id = Column(Integer, Sequence("aics_tableperm_permissionview_id_seq"), primary_key=True) -# tableperm_id = Column(Integer, ForeignKey("aics_table_permission.id")) -# permissionview_id = Column(Integer, ForeignKey("ab_permission_view.id")) -# permissionview = relationship( -# security_manager.permissionview_model, backref="tableperm_permissionview", foreign_keys=[permissionview_id] -# ) - -# def __repr__(self): -# if self.is_active: -# return re.sub(r'.* ', '', self.permissionview)+str(self.expire_date) -# else: -# return '' - assoc_tableperm_permissionview = Table( "aics_tableperm_permissionview", Model.metadata, @@ -116,9 +44,12 @@ ) class TablePermission(Model, AuditMixinNullable): - """ + Customized table permission control table + Superset 0.35 leverage Flask AppBuilder Role/Permission for permission control + However, it makes permission auto-revocation impossible. + So we build this permission control table and with auto-revocation mechanism """ __tablename__ = "aics_table_permission" @@ -145,14 +76,34 @@ def username(self): def username(self, value): pass - # @property - # def expire_date(self): - # return self._exp_date - - # @expire_date.setter - # def expire_date(self, exp_date): - # self._exp_date = exp_date - # print(str(len(self.table_permissions))) - # for permission in self.table_permissions: - # print(permission) - # permission.expire_date = exp_date \ No newline at end of file + @property + def exp_or_terminate_date(self): + if self.force_terminate_date != None: + return f'{str(self.force_terminate_date.date())}(Forced)' + return self.expire_date + + @exp_or_terminate_date.setter + def exp_or_terminate_date(self, value): + pass + + @property + def table_permission_list(self): + table_perm = [str(perm.view_menu) for perm in self.table_permissions] + return ', '.join(table_perm) + + @table_permission_list.setter + def table_permission_list(self, value): + pass + + @property + def status(self): + if self.is_active: + return 'Active' + elif self.force_terminate_date != None: + return 'Force Revoked' + else: + return 'Expired' + + @status.setter + def status(self, value): + pass diff --git a/superset/views/aics_privacy_control/views.py b/superset/views/aics_privacy_control/views.py index 468e8c6810fac..a6f48fa24bb53 100644 --- a/superset/views/aics_privacy_control/views.py +++ b/superset/views/aics_privacy_control/views.py @@ -12,6 +12,9 @@ from flask_appbuilder.security.sqla import models as ab_models from flask_appbuilder.models.sqla.filters import FilterEqual from flask_babel import gettext as __, lazy_gettext as _ +from flask import Markup + +from superset.exceptions import SupersetException from superset import appbuilder, db, event_logger, security_manager from superset.views.base import ( DeleteMixin, @@ -21,7 +24,14 @@ from superset.models.user_attributes import UserAttribute from superset.models.table_permission import TablePermission from wtforms.ext.sqlalchemy.fields import QuerySelectField, QuerySelectMultipleField -from wtforms import TextField, SelectField, SelectMultipleField, DateField +from wtforms import ( + BooleanField, + DateField, + HiddenField, + SelectField, + SelectMultipleField, + TextField +) from wtforms.validators import DataRequired class BS3TextFieldROWidget(BS3TextFieldWidget): @@ -154,8 +164,8 @@ class TablePermissionModelView(SupersetModelView, DeleteMixin): datamodel = SQLAInterface(TablePermission) list_columns = [ 'username', - 'table_permissions', - 'expire_date', + 'table_permission_list', + 'exp_or_terminate_date', 'is_active', ] @@ -164,9 +174,9 @@ class TablePermissionModelView(SupersetModelView, DeleteMixin): label_columns = { - 'user': _('User'), + 'exp_or_terminate_date': _('Expire/Terminate Date'), 'username': _('User'), - 'table_permissions': _('Table Permissions'), + 'table_permission_list': _('Table Permissions'), 'avail_table_list': _('Table Permissions'), 'created_on': _('Created On'), 'changed_on': _('Changed On'), @@ -175,43 +185,57 @@ class TablePermissionModelView(SupersetModelView, DeleteMixin): edit_columns = [ 'username', - 'table_permissions', - 'expire_date', - 'force_terminate_date', - 'is_active', + 'table_permission_list', + 'exp_or_terminate_date', + 'status', + 'force_revoke', ] edit_form_extra_fields = { 'username': TextField('User Name', widget=BS3TextFieldROWidget()), - 'expire_date': TextField('Expire Date', widget=BS3TextFieldROWidget()), + 'table_permission_list': TextField('Table Permissions', widget=BS3TextFieldROWidget()), + 'exp_or_terminate_date': TextField('Expire/Terminate Date', widget=BS3TextFieldROWidget()), + 'status': TextField('Status', widget=BS3TextFieldROWidget()), + 'force_revoke': BooleanField('Force Revoke'), + # 'table_permissions': SelectMultipleField('Table Permissions'), # 'table_permissions': SelectMultipleField('Table Permissions', widget=Select2ManyROWidget()), - 'table_permissions': TextField('Table Permissions', widget=BS3TextFieldROWidget()), + # 'table_permissions': TextField('Table Permissions', widget=BS3TextFieldROWidget()), } add_columns = [ 'user', - # 'table_permissions', 'tables', - # 'table_perm', 'expire_date', - # 'is_active', ] add_form_extra_fields = { 'tables': QuerySelectMultipleField('Tables', query_factory=get_table_perm_list, widget=Select2ManyWidget()), - # 'expire_date': DateField('Expire Date', - # widget=DatePickerWidget() - # ), } def pre_add(self, obj): obj.table_permissions = obj.tables def pre_update(self, obj): - if obj.is_active == False: - obj.force_treminate_date = datetime.now() + # Not allow re-activate permission + if obj.status != 'Active': + print("Modification on expired/force terminated permission is not allowed") + raise SupersetException( + Markup( + "Modification on expired/force terminated permission is not allowed" + ) + ) + + if obj.force_revoke == True: + obj.pop('force_revoke') + obj.is_active == False + obj.force_terminate_date = datetime.now() + else: + raise SupersetException( + Markup("Nothing Changed") + ) + appbuilder.add_separator('Security') diff --git a/superset/views/core.py b/superset/views/core.py index 762307d5613f6..0c10c228d158b 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -2996,7 +2996,8 @@ def revoke_expired_perm(self): perm_info = {'user': perm.username, 'perms':[]} for table in perm.table_permissions: perm_info['perms'].append(str(table)) - print(f"revoke {str(table)}") + logging.info(f"auto revoke {str(table)}") + revoke_msg['revoke-perm'].append(perm_info) perm.is_active = False perm.changed_by_fk = perm.changed_by_fk From 8e4b4414594931177f378fb212ca2188afe7e3cb Mon Sep 17 00:00:00 2001 From: Archer_Tsai Date: Thu, 7 May 2020 15:33:31 +0800 Subject: [PATCH 11/16] WIP: Add log of permission change and Forbid delete active permissions --- superset/utils/log.py | 8 ++--- superset/views/aics_privacy_control/views.py | 37 ++++++++++++++++++-- superset/views/core.py | 2 +- 3 files changed, 39 insertions(+), 8 deletions(-) diff --git a/superset/utils/log.py b/superset/utils/log.py index e338ae7c7e628..610186d1454a7 100644 --- a/superset/utils/log.py +++ b/superset/utils/log.py @@ -92,7 +92,7 @@ def wrapper(*args, **kwargs): schema=extra_info.get("schema"), sql=extra_info.get("sql"), err_msg=extra_info.get("err_msg"), - details=extra_info.get("details") + log_msg=extra_info.get("log_msg") ) return value @@ -160,7 +160,7 @@ def log(self, user_id, action, *args, **kwargs): database = kwargs.get("database") schema = kwargs.get("schema") sql = kwargs.get("sql") - details = kwargs.get("details") + log_msg = kwargs.get("log_msg") err_msg = kwargs.get("err_msg") success = "true" if err_msg == None else "false" @@ -188,8 +188,8 @@ def log(self, user_id, action, *args, **kwargs): if err_msg != None: json_log['err_msg'] = err_msg - if details != None: - json_log['details'] = details + if log_msg != None: + json_log['log_msg'] = log_msg self.appinsights(json_log) diff --git a/superset/views/aics_privacy_control/views.py b/superset/views/aics_privacy_control/views.py index a6f48fa24bb53..8d5b2611f3472 100644 --- a/superset/views/aics_privacy_control/views.py +++ b/superset/views/aics_privacy_control/views.py @@ -146,15 +146,27 @@ class AccessKeyModelView(SupersetModelView, DeleteMixin): @event_logger.log_this def pre_add(self, obj): obj.user_id = obj.user.id + extra_info = { + 'log_msg': f'create access key {obj.access_key} for {obj.username}' + } + return obj, extra_info # Replace access_key with the new one for write back to DB @event_logger.log_this def pre_update(self, obj): obj.access_key = obj.new_access_key + extra_info = { + 'log_msg': f'renew access key {obj.access_key} of {obj.username}' + } + return obj, extra_info @event_logger.log_this def pre_delete(self, obj): print(f'delete access key of user: {obj.username}') + extra_info = { + 'log_msg': f'revoke access key {obj.access_key} of {obj.username}' + } + return obj, extra_info # This function is used to generate new access key and fill in edit form def prefill_form(self, form, pk): @@ -197,9 +209,6 @@ class TablePermissionModelView(SupersetModelView, DeleteMixin): 'exp_or_terminate_date': TextField('Expire/Terminate Date', widget=BS3TextFieldROWidget()), 'status': TextField('Status', widget=BS3TextFieldROWidget()), 'force_revoke': BooleanField('Force Revoke'), - # 'table_permissions': SelectMultipleField('Table Permissions'), - # 'table_permissions': SelectMultipleField('Table Permissions', widget=Select2ManyROWidget()), - # 'table_permissions': TextField('Table Permissions', widget=BS3TextFieldROWidget()), } add_columns = [ @@ -214,9 +223,15 @@ class TablePermissionModelView(SupersetModelView, DeleteMixin): widget=Select2ManyWidget()), } + @event_logger.log_this def pre_add(self, obj): obj.table_permissions = obj.tables + extra_info = { + 'log_msg': f'grant permissions of tables: {obj.table_permission_list} to {obj.user} til {obj.expire_date}' + } + return obj, extra_info + @event_logger.log_this def pre_update(self, obj): # Not allow re-activate permission if obj.status != 'Active': @@ -231,11 +246,27 @@ def pre_update(self, obj): obj.pop('force_revoke') obj.is_active == False obj.force_terminate_date = datetime.now() + extra_info = { + 'log_msg': f'force revoke permissions of tables: {obj.table_permission_list} of {obj.user}' + } + return obj, extra_info else: raise SupersetException( Markup("Nothing Changed") ) + @event_logger.log_this + def pre_delete(self, obj): + if obj.is_active: + raise SupersetException( + Markup("Delete active permission is prohibited. Revoke permission before delete it.") + ) + + extra_info = { + 'log_msg': f'delete expired/revoked permissions of tables: {obj.table_permission_list} of {obj.user}' + } + return obj, extra_info + appbuilder.add_separator('Security') diff --git a/superset/views/core.py b/superset/views/core.py index 0c10c228d158b..60cda57389a47 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -3004,7 +3004,7 @@ def revoke_expired_perm(self): session.commit() session.close() - extra_info = {'details': revoke_msg} + extra_info = {'log_msg': revoke_msg} return "OK", extra_info From fb63f0aa32cd40ed6043a396292bded189fed200 Mon Sep 17 00:00:00 2001 From: Archer_Tsai Date: Thu, 7 May 2020 16:12:19 +0800 Subject: [PATCH 12/16] WIP: Unify GUI appearance of user name --- superset/models/table_permission.py | 11 +++---- superset/models/user_attributes.py | 12 +++----- superset/views/aics_privacy_control/views.py | 31 ++++++++++---------- 3 files changed, 25 insertions(+), 29 deletions(-) diff --git a/superset/models/table_permission.py b/superset/models/table_permission.py index e9a850e9752cf..4473062c4433b 100644 --- a/superset/models/table_permission.py +++ b/superset/models/table_permission.py @@ -69,17 +69,18 @@ class TablePermission(Model, AuditMixinNullable): ) @property - def username(self): - return self.user.username + def detail_name(self): + # return f'{self.user.get_full_name()}(id:{self.user.id})' + return f'{self.user.get_full_name()} ({self.user.username})' - @username.setter - def username(self, value): + @detail_name.setter + def detail_name(self, value): pass @property def exp_or_terminate_date(self): if self.force_terminate_date != None: - return f'{str(self.force_terminate_date.date())}(Forced)' + return f'{str(self.force_terminate_date.date())}(F)' return self.expire_date @exp_or_terminate_date.setter diff --git a/superset/models/user_attributes.py b/superset/models/user_attributes.py index b5ba4d656fd36..67ab3b23dfa85 100644 --- a/superset/models/user_attributes.py +++ b/superset/models/user_attributes.py @@ -52,12 +52,12 @@ class UserAttribute(Model, AuditMixinNullable): # This is related to SQLA. Use @property from python or @hybrid_property from SQLA are both OK here. # Ref: https://docs.sqlalchemy.org/en/13/orm/extensions/hybrid.html#defining-expression-behavior-distinct-from-attribute-behavior @property - def username(self): - return self.user.username + def detail_name(self): + return f'{self.user.get_full_name()} ({self.user.username})' # As we need to show username in add/edit views, we need to define a setter to deal with the assignment of the property - @username.setter - def username(self, value): + @detail_name.setter + def detail_name(self, value): pass @property @@ -67,7 +67,3 @@ def new_access_key(self): @new_access_key.setter def new_access_key(self, value): self._new_key = value - - @property - def changed_by_name(self): - return self.changed_by_ diff --git a/superset/views/aics_privacy_control/views.py b/superset/views/aics_privacy_control/views.py index 8d5b2611f3472..5e9a74335d99c 100644 --- a/superset/views/aics_privacy_control/views.py +++ b/superset/views/aics_privacy_control/views.py @@ -27,7 +27,6 @@ from wtforms import ( BooleanField, DateField, - HiddenField, SelectField, SelectMultipleField, TextField @@ -87,7 +86,7 @@ class AccessKeyModelView(SupersetModelView, DeleteMixin): datamodel = SQLAInterface(UserAttribute) list_columns = [ - 'username', + 'detail_name', 'access_key', 'created_on', 'changed_on', @@ -98,7 +97,7 @@ class AccessKeyModelView(SupersetModelView, DeleteMixin): base_order = ('changed_on', 'desc') label_columns = { - 'username': _('User'), + 'detail_name': _('User'), 'access_key': _('Access Key'), 'created_on': _('Created On'), 'changed_on': _('Changed On'), @@ -113,7 +112,6 @@ class AccessKeyModelView(SupersetModelView, DeleteMixin): add_form_extra_fields = { 'user': QuerySelectField('User', query_factory=get_user_options, - get_label='username', widget=Select2Widget()), } @@ -122,7 +120,7 @@ class AccessKeyModelView(SupersetModelView, DeleteMixin): } edit_form_extra_fields = { - 'username': TextField('User Name', widget=BS3TextFieldROWidget()), + 'detail_name': TextField('User Name', widget=BS3TextFieldROWidget()), 'access_key': TextField('Original Access Key', widget=BS3TextFieldROWidget()), 'new_access_key': TextField('New Access Key', description=('Not editable, click \'Save\' to replace the access key with this new key.'), @@ -130,13 +128,13 @@ class AccessKeyModelView(SupersetModelView, DeleteMixin): } edit_columns = [ - 'username', + 'detail_name', 'access_key', 'new_access_key', ] show_columns = [ - 'username', + 'detail_name', 'access_key', 'created_on', 'changed_on', @@ -147,7 +145,7 @@ class AccessKeyModelView(SupersetModelView, DeleteMixin): def pre_add(self, obj): obj.user_id = obj.user.id extra_info = { - 'log_msg': f'create access key {obj.access_key} for {obj.username}' + 'log_msg': f'create access key {obj.access_key} for {obj.detail_name}' } return obj, extra_info @@ -156,15 +154,15 @@ def pre_add(self, obj): def pre_update(self, obj): obj.access_key = obj.new_access_key extra_info = { - 'log_msg': f'renew access key {obj.access_key} of {obj.username}' + 'log_msg': f'renew access key {obj.access_key} of {obj.detail_name}' } return obj, extra_info @event_logger.log_this def pre_delete(self, obj): - print(f'delete access key of user: {obj.username}') + print(f'delete access key of user: {obj.detail_name}') extra_info = { - 'log_msg': f'revoke access key {obj.access_key} of {obj.username}' + 'log_msg': f'revoke access key {obj.access_key} of {obj.detail_name}' } return obj, extra_info @@ -175,19 +173,20 @@ def prefill_form(self, form, pk): class TablePermissionModelView(SupersetModelView, DeleteMixin): datamodel = SQLAInterface(TablePermission) list_columns = [ - 'username', + 'detail_name', 'table_permission_list', 'exp_or_terminate_date', 'is_active', + 'changed_by_name', ] - order_columns = ['username', 'expire_date', 'is_active'] + order_columns = ['detail_name', 'expire_date', 'is_active'] base_order = ('user_id', 'desc') label_columns = { 'exp_or_terminate_date': _('Expire/Terminate Date'), - 'username': _('User'), + 'detail_name': _('User'), 'table_permission_list': _('Table Permissions'), 'avail_table_list': _('Table Permissions'), 'created_on': _('Created On'), @@ -196,7 +195,7 @@ class TablePermissionModelView(SupersetModelView, DeleteMixin): } edit_columns = [ - 'username', + 'detail_name', 'table_permission_list', 'exp_or_terminate_date', 'status', @@ -204,7 +203,7 @@ class TablePermissionModelView(SupersetModelView, DeleteMixin): ] edit_form_extra_fields = { - 'username': TextField('User Name', widget=BS3TextFieldROWidget()), + 'detail_name': TextField('User Name', widget=BS3TextFieldROWidget()), 'table_permission_list': TextField('Table Permissions', widget=BS3TextFieldROWidget()), 'exp_or_terminate_date': TextField('Expire/Terminate Date', widget=BS3TextFieldROWidget()), 'status': TextField('Status', widget=BS3TextFieldROWidget()), From 38da6aa49b5688f7e63a8111b777f49ce3f8b270 Mon Sep 17 00:00:00 2001 From: Archer_Tsai Date: Thu, 7 May 2020 16:28:16 +0800 Subject: [PATCH 13/16] WIP: Fixed a bug of force revoke and clean up code --- superset/views/aics_privacy_control/views.py | 66 ++++++++------------ 1 file changed, 26 insertions(+), 40 deletions(-) diff --git a/superset/views/aics_privacy_control/views.py b/superset/views/aics_privacy_control/views.py index 5e9a74335d99c..6d68671594afa 100644 --- a/superset/views/aics_privacy_control/views.py +++ b/superset/views/aics_privacy_control/views.py @@ -6,16 +6,19 @@ from flask_appbuilder.fieldwidgets import ( BS3TextFieldWidget, Select2Widget, - Select2ManyWidget, - DatePickerWidget + Select2ManyWidget ) from flask_appbuilder.security.sqla import models as ab_models -from flask_appbuilder.models.sqla.filters import FilterEqual from flask_babel import gettext as __, lazy_gettext as _ from flask import Markup from superset.exceptions import SupersetException -from superset import appbuilder, db, event_logger, security_manager +from superset import ( + appbuilder, + db, + event_logger, + security_manager +) from superset.views.base import ( DeleteMixin, SupersetModelView, @@ -24,13 +27,7 @@ from superset.models.user_attributes import UserAttribute from superset.models.table_permission import TablePermission from wtforms.ext.sqlalchemy.fields import QuerySelectField, QuerySelectMultipleField -from wtforms import ( - BooleanField, - DateField, - SelectField, - SelectMultipleField, - TextField -) +from wtforms import BooleanField, TextField from wtforms.validators import DataRequired class BS3TextFieldROWidget(BS3TextFieldWidget): @@ -41,15 +38,6 @@ def __call__(self, field, **kwargs): kwargs['readonly'] = 'true' return super(BS3TextFieldROWidget, self).__call__(field, **kwargs) - -class Select2ManyROWidget(Select2ManyWidget): - '''Inherit Select2ManyWidget and create a read only version - ref: https://github.com/dpgaspar/Flask-AppBuilder/blob/master/docs/advanced.rst#forms---readonly-fields - ''' - def __call__(self, field, **kwargs): - kwargs['readonly'] = 'true' - return super(Select2ManyWidget, self).__call__(field, **kwargs) - def get_user_options(): '''Get user list and filter out the ones already has a access key Used in the QuerySelectField of add form @@ -61,25 +49,23 @@ def get_table_perm_list(): '''Get table list Used in the QuerySelectField of add form of TablePermission ''' - # get id of 'datasource access' - # get all permission_id == 'datasource access' from ab_permission_view - # store the id into table - perm_datasource_access = security_manager.find_permission('datasource_access') - print(perm_datasource_access.name) pv_model = security_manager.permissionview_model + # get permission id of 'datasource_access' + perm_datasource_access = security_manager.find_permission('datasource_access') + + # get all datasources all_datasources = ConnectorRegistry.get_all_datasources(db.session) - # all_datasources_perm = [(datasource.perm) for datasource in all_datasources] view_menu_ids = [] for datasource in all_datasources: view_menu_ids.append(security_manager.find_view_menu(datasource.perm).id) - print(f'view_menu_ids count: {len(view_menu_ids)} {len(all_datasources)}') - - all_permissions = db.session.query(pv_model).filter(pv_model.permission == perm_datasource_access).filter(pv_model.view_menu_id.in_(view_menu_ids)) - print(all_permissions[0]) - print(all_permissions[0].__dict__) - + # get all permission related to 'datasource_access' and the datasources + all_permissions = db.session.query(pv_model).filter( + pv_model.permission == perm_datasource_access + ).filter( + pv_model.view_menu_id.in_(view_menu_ids) + ) return all_permissions class AccessKeyModelView(SupersetModelView, DeleteMixin): @@ -160,7 +146,6 @@ def pre_update(self, obj): @event_logger.log_this def pre_delete(self, obj): - print(f'delete access key of user: {obj.detail_name}') extra_info = { 'log_msg': f'revoke access key {obj.access_key} of {obj.detail_name}' } @@ -180,7 +165,7 @@ class TablePermissionModelView(SupersetModelView, DeleteMixin): 'changed_by_name', ] - order_columns = ['detail_name', 'expire_date', 'is_active'] + order_columns = ['expire_date', 'is_active'] base_order = ('user_id', 'desc') @@ -226,7 +211,8 @@ class TablePermissionModelView(SupersetModelView, DeleteMixin): def pre_add(self, obj): obj.table_permissions = obj.tables extra_info = { - 'log_msg': f'grant permissions of tables: {obj.table_permission_list} to {obj.user} til {obj.expire_date}' + 'log_msg': 'grant permissions of tables: ' + + f'{obj.table_permission_list} to {obj.user}({obj.user.username}) til {obj.expire_date}' } return obj, extra_info @@ -234,7 +220,6 @@ def pre_add(self, obj): def pre_update(self, obj): # Not allow re-activate permission if obj.status != 'Active': - print("Modification on expired/force terminated permission is not allowed") raise SupersetException( Markup( "Modification on expired/force terminated permission is not allowed" @@ -242,11 +227,11 @@ def pre_update(self, obj): ) if obj.force_revoke == True: - obj.pop('force_revoke') - obj.is_active == False + obj.is_active = False obj.force_terminate_date = datetime.now() extra_info = { - 'log_msg': f'force revoke permissions of tables: {obj.table_permission_list} of {obj.user}' + 'log_msg': f'force revoke permissions of tables: ' + + f'{obj.table_permission_list} of {obj.user}({obj.user.username})' } return obj, extra_info else: @@ -262,7 +247,8 @@ def pre_delete(self, obj): ) extra_info = { - 'log_msg': f'delete expired/revoked permissions of tables: {obj.table_permission_list} of {obj.user}' + 'log_msg': f'delete expired/revoked permissions of tables: ' + + f'{obj.table_permission_list} of {obj.user}({obj.user.username})' } return obj, extra_info From 70e36b6ac6d7d7d3af8763d9550b75db74b52829 Mon Sep 17 00:00:00 2001 From: Archer_Tsai Date: Thu, 7 May 2020 18:53:10 +0800 Subject: [PATCH 14/16] Refactor and add some comments --- superset/models/table_permission.py | 6 ++-- superset/security.py | 30 ++++++++++++-------- superset/views/aics_privacy_control/views.py | 12 ++++++++ superset/views/core.py | 6 ++++ 4 files changed, 40 insertions(+), 14 deletions(-) diff --git a/superset/models/table_permission.py b/superset/models/table_permission.py index 4473062c4433b..5304efbffea8a 100644 --- a/superset/models/table_permission.py +++ b/superset/models/table_permission.py @@ -34,6 +34,8 @@ from superset import security_manager from superset.models.helpers import AuditMixinNullable +# Table for many-to-many relation between users and tables +# Ref: https://docs.sqlalchemy.org/en/13/orm/basic_relationships.html#relationships-many-to-many assoc_tableperm_permissionview = Table( "aics_tableperm_permissionview", Model.metadata, @@ -68,9 +70,9 @@ class TablePermission(Model, AuditMixinNullable): security_manager.permissionview_model, secondary=assoc_tableperm_permissionview, backref="table_perm" ) + # Following properties are used for display fields that are not expected to be changed @property def detail_name(self): - # return f'{self.user.get_full_name()}(id:{self.user.id})' return f'{self.user.get_full_name()} ({self.user.username})' @detail_name.setter @@ -80,7 +82,7 @@ def detail_name(self, value): @property def exp_or_terminate_date(self): if self.force_terminate_date != None: - return f'{str(self.force_terminate_date.date())}(F)' + return f'{str(self.force_terminate_date.date())}(T)' return self.expire_date @exp_or_terminate_date.setter diff --git a/superset/security.py b/superset/security.py index 2c40326c12eb4..7d27fa1caec36 100644 --- a/superset/security.py +++ b/superset/security.py @@ -17,6 +17,7 @@ # pylint: disable=C,R,W """A set of constants and methods to manage permissions and security""" import logging +from datetime import datetime from typing import Callable, List, Optional, Set, Tuple, TYPE_CHECKING, Union from flask import current_app, g @@ -289,23 +290,29 @@ def get_table_access_link(self, tables: List[str]) -> Optional[str]: return conf.get("PERMISSION_INSTRUCTIONS_LINK") def _has_aics_table_permission(self, user: object, permission_name: str, view_name: str): + """ + Customize table permission check + """ + from superset import db from superset.models.table_permission import TablePermission + # Anonymous user is not allowed if user.is_anonymous: - print('user is anomynous') return False - + + # get permission of specified permission name and view name perm = self.find_permission_view_menu( permission_name, view_name) - print(user) - print(f'{permission_name} on {view_name}') - print(perm) - table_permissions = db.session.query(TablePermission).filter(TablePermission.user_id == user.id, TablePermission.is_active == True) - for table_perm in table_permissions: - for pv in table_perm.table_permissions: - print(pv) - if pv == perm: - return True + + user_table_permissions = db.session.query(TablePermission).filter( + TablePermission.user_id == user.id, + TablePermission.is_active == True, + TablePermission.expire_date > datetime.now().date() + ) + + for table_perm in user_table_permissions: + if perm in table_perm.table_permissions: + return True def _datasource_access_by_name( @@ -340,7 +347,6 @@ def _datasource_access_by_name( if self.can_access("datasource_access", datasource.perm): return True - print('Check AICS Permission') # Check AICS table permissions if self._has_aics_table_permission(g.user, "datasource_access", datasource.perm): return True diff --git a/superset/views/aics_privacy_control/views.py b/superset/views/aics_privacy_control/views.py index 6d68671594afa..66e41419655c4 100644 --- a/superset/views/aics_privacy_control/views.py +++ b/superset/views/aics_privacy_control/views.py @@ -71,6 +71,11 @@ def get_table_perm_list(): class AccessKeyModelView(SupersetModelView, DeleteMixin): datamodel = SQLAInterface(UserAttribute) + list_title = _("Access Keys") + show_title = _("Show Access Key Details") + add_title = _("Create Access Key") + edit_title = _("Renew Access Key") + list_columns = [ 'detail_name', 'access_key', @@ -157,6 +162,12 @@ def prefill_form(self, form, pk): class TablePermissionModelView(SupersetModelView, DeleteMixin): datamodel = SQLAInterface(TablePermission) + + list_title = _("Table Permissions") + show_title = _("Table Permission Details") + add_title = _("Grant Table Permissions") + edit_title = _("Revoke Table Permissions") + list_columns = [ 'detail_name', 'table_permission_list', @@ -174,6 +185,7 @@ class TablePermissionModelView(SupersetModelView, DeleteMixin): 'detail_name': _('User'), 'table_permission_list': _('Table Permissions'), 'avail_table_list': _('Table Permissions'), + 'is_active': _('Active'), 'created_on': _('Created On'), 'changed_on': _('Changed On'), 'changed_by_name': _('Changed By'), diff --git a/superset/views/core.py b/superset/views/core.py index 60cda57389a47..a1b28c043cacd 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -2986,11 +2986,16 @@ def csv(self, client_id): @expose("/revoke_expired_perm", methods=["GET"]) @event_logger.log_this def revoke_expired_perm(self): + """Revoke expired permissions, triggered by HTTP GET requests from other scheduler""" session = db.session() + + # Get all expired permissions expired_perms = session.query(TablePermission).filter( TablePermission.is_active == True, TablePermission.expire_date < datetime.now().date() ) + + # Revoke permissions and prepare log msg revoke_msg = {'revoke-perm':[]} for perm in expired_perms: perm_info = {'user': perm.username, 'perms':[]} @@ -3001,6 +3006,7 @@ def revoke_expired_perm(self): revoke_msg['revoke-perm'].append(perm_info) perm.is_active = False perm.changed_by_fk = perm.changed_by_fk + session.commit() session.close() From cc1dec5a9d67592f3e9fd10b8d411e34e21f8871 Mon Sep 17 00:00:00 2001 From: Archer_Tsai Date: Fri, 8 May 2020 11:11:19 +0800 Subject: [PATCH 15/16] Fixed a bug caused by wrong variable name --- superset/models/table_permission.py | 6 +++--- superset/models/user_attributes.py | 6 +++--- superset/views/core.py | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/superset/models/table_permission.py b/superset/models/table_permission.py index 5304efbffea8a..a6f552cc6d12f 100644 --- a/superset/models/table_permission.py +++ b/superset/models/table_permission.py @@ -72,11 +72,11 @@ class TablePermission(Model, AuditMixinNullable): # Following properties are used for display fields that are not expected to be changed @property - def detail_name(self): + def username_detail(self): return f'{self.user.get_full_name()} ({self.user.username})' - @detail_name.setter - def detail_name(self, value): + @username_detail.setter + def username_detail(self, value): pass @property diff --git a/superset/models/user_attributes.py b/superset/models/user_attributes.py index 67ab3b23dfa85..275f573334c43 100644 --- a/superset/models/user_attributes.py +++ b/superset/models/user_attributes.py @@ -52,12 +52,12 @@ class UserAttribute(Model, AuditMixinNullable): # This is related to SQLA. Use @property from python or @hybrid_property from SQLA are both OK here. # Ref: https://docs.sqlalchemy.org/en/13/orm/extensions/hybrid.html#defining-expression-behavior-distinct-from-attribute-behavior @property - def detail_name(self): + def username_detail(self): return f'{self.user.get_full_name()} ({self.user.username})' # As we need to show username in add/edit views, we need to define a setter to deal with the assignment of the property - @detail_name.setter - def detail_name(self, value): + @username_detail.setter + def username_detail(self, value): pass @property diff --git a/superset/views/core.py b/superset/views/core.py index a1b28c043cacd..df46c487727bd 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -2998,11 +2998,11 @@ def revoke_expired_perm(self): # Revoke permissions and prepare log msg revoke_msg = {'revoke-perm':[]} for perm in expired_perms: - perm_info = {'user': perm.username, 'perms':[]} + perm_info = {'user': perm.username_detail, 'permissions':[]} for table in perm.table_permissions: - perm_info['perms'].append(str(table)) - logging.info(f"auto revoke {str(table)}") + perm_info['permissions'].append(re.sub(r'.* ', '', str(table))) + logging.info(f"Auto revoke: {str(perm_info)}") revoke_msg['revoke-perm'].append(perm_info) perm.is_active = False perm.changed_by_fk = perm.changed_by_fk From 55727ddc726fe337d46ec4bdc62a0254a52bc421 Mon Sep 17 00:00:00 2001 From: Archer_Tsai Date: Fri, 8 May 2020 16:03:59 +0800 Subject: [PATCH 16/16] Add missing changes in previous commit --- superset/views/aics_privacy_control/views.py | 24 ++++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/superset/views/aics_privacy_control/views.py b/superset/views/aics_privacy_control/views.py index 66e41419655c4..7ffe7ad9696e3 100644 --- a/superset/views/aics_privacy_control/views.py +++ b/superset/views/aics_privacy_control/views.py @@ -77,7 +77,7 @@ class AccessKeyModelView(SupersetModelView, DeleteMixin): edit_title = _("Renew Access Key") list_columns = [ - 'detail_name', + 'username_detail', 'access_key', 'created_on', 'changed_on', @@ -88,7 +88,7 @@ class AccessKeyModelView(SupersetModelView, DeleteMixin): base_order = ('changed_on', 'desc') label_columns = { - 'detail_name': _('User'), + 'username_detail': _('User'), 'access_key': _('Access Key'), 'created_on': _('Created On'), 'changed_on': _('Changed On'), @@ -111,7 +111,7 @@ class AccessKeyModelView(SupersetModelView, DeleteMixin): } edit_form_extra_fields = { - 'detail_name': TextField('User Name', widget=BS3TextFieldROWidget()), + 'username_detail': TextField('User Name', widget=BS3TextFieldROWidget()), 'access_key': TextField('Original Access Key', widget=BS3TextFieldROWidget()), 'new_access_key': TextField('New Access Key', description=('Not editable, click \'Save\' to replace the access key with this new key.'), @@ -119,13 +119,13 @@ class AccessKeyModelView(SupersetModelView, DeleteMixin): } edit_columns = [ - 'detail_name', + 'username_detail', 'access_key', 'new_access_key', ] show_columns = [ - 'detail_name', + 'username_detail', 'access_key', 'created_on', 'changed_on', @@ -136,7 +136,7 @@ class AccessKeyModelView(SupersetModelView, DeleteMixin): def pre_add(self, obj): obj.user_id = obj.user.id extra_info = { - 'log_msg': f'create access key {obj.access_key} for {obj.detail_name}' + 'log_msg': f'create access key {obj.access_key} for {obj.username_detail}' } return obj, extra_info @@ -145,14 +145,14 @@ def pre_add(self, obj): def pre_update(self, obj): obj.access_key = obj.new_access_key extra_info = { - 'log_msg': f'renew access key {obj.access_key} of {obj.detail_name}' + 'log_msg': f'renew access key {obj.access_key} of {obj.username_detail}' } return obj, extra_info @event_logger.log_this def pre_delete(self, obj): extra_info = { - 'log_msg': f'revoke access key {obj.access_key} of {obj.detail_name}' + 'log_msg': f'revoke access key {obj.access_key} of {obj.username_detail}' } return obj, extra_info @@ -169,7 +169,7 @@ class TablePermissionModelView(SupersetModelView, DeleteMixin): edit_title = _("Revoke Table Permissions") list_columns = [ - 'detail_name', + 'username_detail', 'table_permission_list', 'exp_or_terminate_date', 'is_active', @@ -182,7 +182,7 @@ class TablePermissionModelView(SupersetModelView, DeleteMixin): label_columns = { 'exp_or_terminate_date': _('Expire/Terminate Date'), - 'detail_name': _('User'), + 'username_detail': _('User'), 'table_permission_list': _('Table Permissions'), 'avail_table_list': _('Table Permissions'), 'is_active': _('Active'), @@ -192,7 +192,7 @@ class TablePermissionModelView(SupersetModelView, DeleteMixin): } edit_columns = [ - 'detail_name', + 'username_detail', 'table_permission_list', 'exp_or_terminate_date', 'status', @@ -200,7 +200,7 @@ class TablePermissionModelView(SupersetModelView, DeleteMixin): ] edit_form_extra_fields = { - 'detail_name': TextField('User Name', widget=BS3TextFieldROWidget()), + 'username_detail': TextField('User Name', widget=BS3TextFieldROWidget()), 'table_permission_list': TextField('Table Permissions', widget=BS3TextFieldROWidget()), 'exp_or_terminate_date': TextField('Expire/Terminate Date', widget=BS3TextFieldROWidget()), 'status': TextField('Status', widget=BS3TextFieldROWidget()),