From 7c32934945258cbb254b1992e88f62cebba0d825 Mon Sep 17 00:00:00 2001 From: Timi Fasubaa Date: Sun, 25 Mar 2018 03:11:52 -0700 Subject: [PATCH] update calls from tests --- superset/__init__.py | 2 +- superset/security.py | 28 ++++++++-------------------- superset/utils.py | 7 ------- superset/views/base.py | 1 - superset/views/core.py | 2 +- tests/access_tests.py | 8 +++----- tests/base_tests.py | 9 ++++----- tests/celery_tests.py | 5 ++--- tests/druid_tests.py | 6 +++--- tests/security_tests.py | 28 ++++++++++++++-------------- 10 files changed, 36 insertions(+), 60 deletions(-) diff --git a/superset/__init__.py b/superset/__init__.py index 88bffd580d1bd..b5dce6d7f604c 100644 --- a/superset/__init__.py +++ b/superset/__init__.py @@ -18,8 +18,8 @@ from flask_wtf.csrf import CSRFProtect from werkzeug.contrib.fixers import ProxyFix +from superset import config, utils from superset.connectors.connector_registry import ConnectorRegistry -from superset import config, utils # noqa from superset.security import SupersetSecurityManager APP_DIR = os.path.dirname(__file__) diff --git a/superset/security.py b/superset/security.py index ac7bedd5c3a3d..a84911965b8a4 100644 --- a/superset/security.py +++ b/superset/security.py @@ -12,7 +12,7 @@ from flask_appbuilder.security.sqla.manager import SecurityManager from sqlalchemy import or_ -from superset import sql_parse, utils +from superset import sql_parse from superset.connectors.connector_registry import ConnectorRegistry READ_ONLY_MODEL_VIEWS = { @@ -79,14 +79,13 @@ class SupersetSecurityManager(SecurityManager): - """docstring for DatasourceSecurityManager""" - def get_schema_perm(self, database, schema): if schema: return '[{}].[{}]'.format(database, schema) def can_access(self, permission_name, view_name, user=None): + """Protecting from has_access failing from missing perms/view""" if not user: user = g.user if user.is_anonymous(): @@ -212,9 +211,7 @@ def accessible_by_user(self, database, datasource_names, schema=None): full_names = {d.full_name for d in user_datasources} return [d for d in datasource_names if d in full_names] - def merge_perm(self, permission_name, view_menu_name): - #copied over # Implementation copied from sm.find_permission_view_menu. # TODO: use sm.find_permission_view_menu once issue # https://github.com/airbnb/superset/issues/1944 is resolved. @@ -230,13 +227,12 @@ def merge_perm(self, permission_name, view_menu_name): def is_user_defined_permission(self, perm): return perm.permission.name in OBJECT_SPEC_PERMISSIONS - def create_custom_permissions(self,): + def create_custom_permissions(self): # Global perms self.merge_perm('all_datasource_access', 'all_datasource_access') self.merge_perm('all_database_access', 'all_database_access') - - def create_missing_perms(self,): + def create_missing_perms(self): """Creates missing perms for datasources, schemas and metrics""" from superset import db from superset.models import core as models @@ -273,11 +269,10 @@ def merge_pv(view_menu, perm): if metric.is_restricted: merge_pv('metric_access', metric.perm) - - def clean_perms(self,): + def clean_perms(self): """FAB leaves faulty permissions that need to be cleaned up""" logging.info('Cleaning faulty perms') - sesh = self.get_session() + sesh = self.get_session pvms = ( sesh.query(ab_models.PermissionView) .filter(or_( @@ -290,7 +285,6 @@ def clean_perms(self,): if deleted_count: logging.info('Deleted {} faulty permissions'.format(deleted_count)) - def sync_role_definitions(self): """Inits the Superset application with security roles and such""" from superset import conf @@ -315,7 +309,7 @@ def sync_role_definitions(self): self.get_session.commit() self.clean_perms() - def get_or_create_main_db(self,): + def get_or_create_main_db(self): from superset import conf, db from superset.models import core as models @@ -336,7 +330,7 @@ def get_or_create_main_db(self,): def set_role(self, role_name, pvm_check): logging.info('Syncing {} perms'.format(role_name)) - sesh = self.get_session() + sesh = self.get_session pvms = sesh.query(ab_models.PermissionView).all() pvms = [p for p in pvms if p.permission and p.view_menu] role = self.add_role(role_name) @@ -355,7 +349,6 @@ def is_admin_only(self, pvm): pvm.permission.name in ADMIN_ONLY_PERMISSIONS ) - def is_alpha_only(self, pvm): if (pvm.view_menu.name in GAMMA_READ_ONLY_MODEL_VIEWS and pvm.permission.name not in READ_ONLY_PERMISSION): @@ -365,26 +358,21 @@ def is_alpha_only(self, pvm): pvm.permission.name in ALPHA_ONLY_PERMISSIONS ) - def is_admin_pvm(self, pvm): return not self.is_user_defined_permission(pvm) - def is_alpha_pvm(self, pvm): return not (self.is_user_defined_permission(pvm) or self.is_admin_only(pvm)) - def is_gamma_pvm(self, pvm): return not (self.is_user_defined_permission(pvm) or self.is_admin_only(pvm) or self.is_alpha_only(pvm)) - def is_sql_lab_pvm(self, pvm): return pvm.view_menu.name in {'SQL Lab'} or pvm.permission.name in { 'can_sql_json', 'can_csv', 'can_search_queries', } - def is_granter_pvm(self, pvm): return pvm.permission.name in { 'can_override_role_permissions', 'can_approve', diff --git a/superset/utils.py b/superset/utils.py index 6bfee510c0132..4c8dee46d7fd5 100644 --- a/superset/utils.py +++ b/superset/utils.py @@ -55,13 +55,6 @@ DTTM_ALIAS = '__timestamp' -def can_access(sm, permission_name, view_name, user): - """Protecting from has_access failing from missing perms/view""" - if user.is_anonymous(): - return sm.is_item_public(permission_name, view_name) - return sm._has_view_access(user, permission_name, view_name) - - def flasher(msg, severity=None): """Flask's flash if available, logging call if not""" try: diff --git a/superset/views/base.py b/superset/views/base.py index 559c8835a5e5d..e69f8cd252a36 100644 --- a/superset/views/base.py +++ b/superset/views/base.py @@ -212,7 +212,6 @@ class SupersetFilter(BaseFilter): """ def get_user_roles(self): - # can this be cached? return get_user_roles() def get_all_permissions(self): diff --git a/superset/views/core.py b/superset/views/core.py index 05b531dea6103..2030349d5dc52 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -33,7 +33,7 @@ from werkzeug.utils import secure_filename from superset import ( - app, appbuilder, cache, db, results_backend, security, sm, sql_lab, utils, + app, appbuilder, cache, db, results_backend, sm, sql_lab, utils, viz, ) from superset.connectors.connector_registry import ConnectorRegistry diff --git a/tests/access_tests.py b/tests/access_tests.py index 39938c9b5534b..8f54dd1f3f47d 100644 --- a/tests/access_tests.py +++ b/tests/access_tests.py @@ -10,7 +10,7 @@ import mock -from superset import app, db, security, sm +from superset import app, db, sm from superset.connectors.connector_registry import ConnectorRegistry from superset.connectors.druid.models import DruidDatasource from superset.connectors.sqla.models import SqlaTable @@ -265,8 +265,7 @@ def test_clean_requests_after_db_grant(self): # gamma gets granted database access database = session.query(models.Database).first() - security.merge_perm( - sm, 'database_access', database.perm) + sm.merge_perm('database_access', database.perm) ds_perm_view = sm.find_permission_view_menu( 'database_access', database.perm) sm.add_permission_role( @@ -302,8 +301,7 @@ def test_clean_requests_after_schema_grant(self): table_name='wb_health_population').first() ds.schema = 'temp_schema' - security.merge_perm( - sm, 'schema_access', ds.schema_perm) + sm.merge_perm('schema_access', ds.schema_perm) schema_perm_view = sm.find_permission_view_menu( 'schema_access', ds.schema_perm) sm.add_permission_role( diff --git a/tests/base_tests.py b/tests/base_tests.py index dcc67988c3172..29acc79776b31 100644 --- a/tests/base_tests.py +++ b/tests/base_tests.py @@ -12,11 +12,10 @@ from flask_appbuilder.security.sqla import models as ab_models -from superset import app, appbuilder, cli, db, security, sm +from superset import app, appbuilder, cli, db, sm from superset.connectors.druid.models import DruidCluster, DruidDatasource from superset.connectors.sqla.models import SqlaTable from superset.models import core as models -from superset.security import sync_role_definitions os.environ['SUPERSET_CONFIG'] = 'tests.superset_test_config' @@ -36,10 +35,10 @@ def __init__(self, *args, **kwargs): logging.info('Loading examples') cli.load_examples(load_test_data=True) logging.info('Done loading examples') - sync_role_definitions() + sm.sync_role_definitions() os.environ['examples_loaded'] = '1' else: - sync_role_definitions() + sm.sync_role_definitions() super(SupersetTestCase, self).__init__(*args, **kwargs) self.client = app.test_client() self.maxDiff = None @@ -48,7 +47,7 @@ def __init__(self, *args, **kwargs): for perm in sm.find_role('Gamma').permissions: sm.add_permission_role(gamma_sqllab_role, perm) db_perm = self.get_main_database(sm.get_session).perm - security.merge_perm(sm, 'database_access', db_perm) + sm.merge_perm('database_access', db_perm) db_pvm = sm.find_permission_view_menu( view_menu_name=db_perm, permission_name='database_access') gamma_sqllab_role.permissions.append(db_pvm) diff --git a/tests/celery_tests.py b/tests/celery_tests.py index 172176ebb53b2..bcb694fdbf993 100644 --- a/tests/celery_tests.py +++ b/tests/celery_tests.py @@ -14,10 +14,9 @@ import pandas as pd from past.builtins import basestring -from superset import app, appbuilder, cli, dataframe, db +from superset import app, appbuilder, cli, dataframe, db, sm from superset.models.helpers import QueryStatus from superset.models.sql_lab import Query -from superset.security import sync_role_definitions from superset.sql_parse import SupersetQuery from .base_tests import SupersetTestCase @@ -98,7 +97,7 @@ def setUpClass(cls): except OSError as e: app.logger.warn(str(e)) - sync_role_definitions() + sm.sync_role_definitions() worker_command = BASE_DIR + '/bin/superset worker' subprocess.Popen( diff --git a/tests/druid_tests.py b/tests/druid_tests.py index e72eb25ee94de..27c8f5a14f997 100644 --- a/tests/druid_tests.py +++ b/tests/druid_tests.py @@ -11,7 +11,7 @@ from mock import Mock, patch -from superset import db, security, sm +from superset import db, sm from superset.connectors.druid.models import ( DruidCluster, DruidColumn, DruidDatasource, DruidMetric, ) @@ -278,8 +278,8 @@ def test_filter_druid_datasource(self): db.session.merge(no_gamma_ds) db.session.commit() - security.merge_perm(sm, 'datasource_access', gamma_ds.perm) - security.merge_perm(sm, 'datasource_access', no_gamma_ds.perm) + sm.merge_perm('datasource_access', gamma_ds.perm) + sm.merge_perm('datasource_access', no_gamma_ds.perm) perm = sm.find_permission_view_menu( 'datasource_access', gamma_ds.get_perm()) diff --git a/tests/security_tests.py b/tests/security_tests.py index e117394a366ce..576b75eb8e092 100644 --- a/tests/security_tests.py +++ b/tests/security_tests.py @@ -4,7 +4,7 @@ from __future__ import print_function from __future__ import unicode_literals -from superset import app, security, sm +from superset import app, sm from .base_tests import SupersetTestCase @@ -103,45 +103,45 @@ def assert_can_admin(self, perm_set): self.assertIn(('can_approve', 'Superset'), perm_set) def test_is_admin_only(self): - self.assertFalse(security.is_admin_only( + self.assertFalse(sm.is_admin_only( sm.find_permission_view_menu('can_show', 'TableModelView'))) - self.assertFalse(security.is_admin_only( + self.assertFalse(sm.is_admin_only( sm.find_permission_view_menu( 'all_datasource_access', 'all_datasource_access'))) - self.assertTrue(security.is_admin_only( + self.assertTrue(sm.is_admin_only( sm.find_permission_view_menu('can_delete', 'DatabaseView'))) if app.config.get('ENABLE_ACCESS_REQUEST'): - self.assertTrue(security.is_admin_only( + self.assertTrue(sm.is_admin_only( sm.find_permission_view_menu( 'can_show', 'AccessRequestsModelView'))) - self.assertTrue(security.is_admin_only( + self.assertTrue(sm.is_admin_only( sm.find_permission_view_menu( 'can_edit', 'UserDBModelView'))) - self.assertTrue(security.is_admin_only( + self.assertTrue(sm.is_admin_only( sm.find_permission_view_menu( 'can_approve', 'Superset'))) - self.assertTrue(security.is_admin_only( + self.assertTrue(sm.is_admin_only( sm.find_permission_view_menu( 'all_database_access', 'all_database_access'))) def test_is_alpha_only(self): - self.assertFalse(security.is_alpha_only( + self.assertFalse(sm.is_alpha_only( sm.find_permission_view_menu('can_show', 'TableModelView'))) - self.assertTrue(security.is_alpha_only( + self.assertTrue(sm.is_alpha_only( sm.find_permission_view_menu('muldelete', 'TableModelView'))) - self.assertTrue(security.is_alpha_only( + self.assertTrue(sm.is_alpha_only( sm.find_permission_view_menu( 'all_datasource_access', 'all_datasource_access'))) - self.assertTrue(security.is_alpha_only( + self.assertTrue(sm.is_alpha_only( sm.find_permission_view_menu('can_edit', 'SqlMetricInlineView'))) - self.assertTrue(security.is_alpha_only( + self.assertTrue(sm.is_alpha_only( sm.find_permission_view_menu( 'can_delete', 'DruidMetricInlineView'))) def test_is_gamma_pvm(self): - self.assertTrue(security.is_gamma_pvm( + self.assertTrue(sm.is_gamma_pvm( sm.find_permission_view_menu('can_show', 'TableModelView'))) def test_gamma_permissions(self):