diff --git a/superset/config.py b/superset/config.py index fff337dd5c719..b686b988f2ead 100644 --- a/superset/config.py +++ b/superset/config.py @@ -35,7 +35,7 @@ SUPERSET_WEBSERVER_ADDRESS = '0.0.0.0' SUPERSET_WEBSERVER_PORT = 8088 SUPERSET_WEBSERVER_TIMEOUT = 60 - +EMAIL_NOTIFICATIONS = False CUSTOM_SECURITY_MANAGER = None # --------------------------------------------------------- diff --git a/superset/utils.py b/superset/utils.py index cf104fabe26fd..3fcd3502b9c31 100644 --- a/superset/utils.py +++ b/superset/utils.py @@ -63,13 +63,13 @@ class SupersetTemplateException(SupersetException): pass -def can_access(security_manager, permission_name, view_name): +def can_access(sm, permission_name, view_name, user): """Protecting from has_access failing from missing perms/view""" - try: - return security_manager.has_access(permission_name, view_name) - except: - pass - return False + return ( + sm.is_item_public(permission_name, view_name) or + (not user.is_anonymous() and + sm._has_view_access(user, permission_name, view_name)) + ) def flasher(msg, severity=None): @@ -436,7 +436,7 @@ def notify_user_about_perm_udate( subject = __('[Superset] Access to the datasource %(name)s was granted', name=datasource.full_name) send_email_smtp(user.email, subject, msg, config, bcc=granter.email, - dryrun=config.get('EMAIL_NOTIFICATIONS')) + dryrun=not config.get('EMAIL_NOTIFICATIONS')) def send_email_smtp(to, subject, html_content, config, files=None, @@ -478,7 +478,7 @@ def send_email_smtp(to, subject, html_content, config, files=None, Name=basename )) - send_MIME_email(smtp_mail_from, recipients, msg, config, dryrun) + send_MIME_email(smtp_mail_from, recipients, msg, config, dryrun=dryrun) def send_MIME_email(e_from, e_to, mime_msg, config, dryrun=False): diff --git a/superset/views.py b/superset/views.py index 4e5cd7be53935..ec428043571ec 100755 --- a/superset/views.py +++ b/superset/views.py @@ -49,30 +49,34 @@ class BaseSupersetView(BaseView): - def can_access(self, permission_name, view_name): - return utils.can_access(appbuilder.sm, permission_name, view_name) + def can_access(self, permission_name, view_name, user=None): + if not user: + user = g.user + return utils.can_access( + appbuilder.sm, permission_name, view_name, user) - def all_datasource_access(self): + def all_datasource_access(self, user=None): return self.can_access( - "all_datasource_access", "all_datasource_access") + "all_datasource_access", "all_datasource_access", user=user) - def database_access(self, database): + def database_access(self, database, user=None): return ( - self.can_access("all_database_access", "all_database_access") or - self.can_access("database_access", database.perm) + self.can_access( + "all_database_access", "all_database_access", user=user) or + self.can_access("database_access", database.perm, user=user) ) - def schema_access(self, datasource): + def schema_access(self, datasource, user=None): return ( - self.database_access(datasource.database) or - self.all_datasource_access() or - self.can_access("schema_access", datasource.schema_perm) + self.database_access(datasource.database, user=user) or + self.all_datasource_access(user=user) or + self.can_access("schema_access", datasource.schema_perm, user=user) ) - def datasource_access(self, datasource): + def datasource_access(self, datasource, user=None): return ( - self.schema_access(datasource) or - self.can_access("datasource_access", datasource.perm) + self.schema_access(datasource, user=user) or + self.can_access("datasource_access", datasource.perm, user=user) ) def datasource_access_by_name( @@ -82,7 +86,7 @@ def datasource_access_by_name( return True schema_perm = utils.get_schema_perm(database, schema) - if schema and utils.can_access(sm, 'schema_access', schema_perm): + if schema and utils.can_access(sm, 'schema_access', schema_perm, g.user): return True datasources = SourceRegistry.query_datasources_by_name( @@ -1286,6 +1290,14 @@ def request_access(self): @has_access @expose("/approve") def approve(self): + def clean_fulfilled_requests(session): + for r in session.query(DAR).all(): + datasource = SourceRegistry.get_datasource( + r.datasource_type, r.datasource_id, session) + user = sm.get_user_by_id(r.created_by_fk) + if self.datasource_access(datasource, user): + session.delete(r) + session.commit() datasource_type = request.args.get('datasource_type') datasource_id = request.args.get('datasource_id') created_by_username = request.args.get('created_by') @@ -1347,7 +1359,7 @@ def approve(self): g.user, requested_by, role, datasource, 'email/role_extended.txt', app.config) flash(msg, "info") - + clean_fulfilled_requests(session) else: flash(__("You have no permission to approve this request"), "danger") diff --git a/tests/access_tests.py b/tests/access_tests.py index e12408d2f76e1..83eaeb2b93045 100644 --- a/tests/access_tests.py +++ b/tests/access_tests.py @@ -8,7 +8,7 @@ import mock import unittest -from superset import db, models, sm +from superset import db, models, sm, security from superset.source_registry import SourceRegistry from .base_tests import SupersetTestCase @@ -45,6 +45,38 @@ ] } +EXTEND_ROLE_REQUEST = ( + '/superset/approve?datasource_type={}&datasource_id={}&' + 'created_by={}&role_to_extend={}') +GRANT_ROLE_REQUEST = ( + '/superset/approve?datasource_type={}&datasource_id={}&' + 'created_by={}&role_to_grant={}') +TEST_ROLE_1 = 'test_role1' +TEST_ROLE_2 = 'test_role2' +DB_ACCESS_ROLE = 'db_access_role' +SCHEMA_ACCESS_ROLE = 'schema_access_role' + +def create_access_request(session, ds_type, ds_name, role_name, user_name): + ds_class = SourceRegistry.sources[ds_type] + # TODO: generalize datasource names + if ds_type == 'table': + ds = session.query(ds_class).filter( + ds_class.table_name == ds_name).first() + else: + ds = session.query(ds_class).filter( + ds_class.datasource_name == ds_name).first() + ds_perm_view = sm.find_permission_view_menu( + 'datasource_access', ds.perm) + sm.add_permission_role(sm.find_role(role_name), ds_perm_view) + access_request = models.DatasourceAccessRequest( + datasource_id=ds.id, + datasource_type=ds_type, + created_by_fk=sm.find_user(username=user_name).id, + ) + session.add(access_request) + session.commit() + return access_request + class RequestAccessTests(SupersetTestCase): @@ -53,12 +85,20 @@ class RequestAccessTests(SupersetTestCase): @classmethod def setUpClass(cls): sm.add_role('override_me') + sm.add_role(TEST_ROLE_1) + sm.add_role(TEST_ROLE_2) + sm.add_role(DB_ACCESS_ROLE) + sm.add_role(SCHEMA_ACCESS_ROLE) db.session.commit() @classmethod def tearDownClass(cls): override_me = sm.find_role('override_me') db.session.delete(override_me) + db.session.delete(sm.find_role(TEST_ROLE_1)) + db.session.delete(sm.find_role(TEST_ROLE_2)) + db.session.delete(sm.find_role(DB_ACCESS_ROLE)) + db.session.delete(sm.find_role(SCHEMA_ACCESS_ROLE)) db.session.commit() def setUp(self): @@ -147,46 +187,150 @@ def test_override_role_permissions_drops_absent_perms(self): 'datasource_access', updated_override_me.permissions[0].permission.name) + def test_clean_requests_after_role_extend(self): + session = db.session + + # Case 1. Gamma and gamma2 requested test_role1 on energy_usage access + # Gamma already has role test_role1 + # Extend test_role1 with energy_usage access for gamma2 + # Check if access request for gamma at energy_usage was deleted + + # gamma2 and gamma request table_role on energy usage + access_request1 = create_access_request( + session, 'table', 'random_time_series', TEST_ROLE_1, 'gamma2') + ds_1_id = access_request1.datasource_id + access_request2 = create_access_request( + session, 'table', 'random_time_series', TEST_ROLE_1, 'gamma') + access_requests = self.get_access_requests('gamma', 'table', ds_1_id) + self.assertTrue(access_requests) + # gamma gets test_role1 + self.get_resp(GRANT_ROLE_REQUEST.format( + 'table', ds_1_id, 'gamma', TEST_ROLE_1)) + # extend test_role1 with access on energy usage + self.client.get(EXTEND_ROLE_REQUEST.format( + 'table', ds_1_id, 'gamma2', TEST_ROLE_1)) + access_requests = self.get_access_requests('gamma', 'table', ds_1_id) + self.assertFalse(access_requests) + + gamma_user = sm.find_user(username='gamma') + gamma_user.roles.remove(sm.find_role('test_role1')) + + def test_clean_requests_after_alpha_grant(self): + session = db.session + + # Case 2. Two access requests from gamma and gamma2 + # Gamma becomes alpha, gamma2 gets granted + # Check if request by gamma has been deleted + + access_request1 = create_access_request( + session, 'table', 'birth_names', TEST_ROLE_1, 'gamma') + access_request2 = create_access_request( + session, 'table', 'birth_names', TEST_ROLE_2, 'gamma2') + ds_1_id = access_request1.datasource_id + # gamma becomes alpha + alpha_role = sm.find_role('Alpha') + gamma_user = sm.find_user(username='gamma') + gamma_user.roles.append(alpha_role) + session.commit() + access_requests = self.get_access_requests('gamma', 'table', ds_1_id) + self.assertTrue(access_requests) + self.client.get(EXTEND_ROLE_REQUEST.format( + 'table', ds_1_id, 'gamma2', TEST_ROLE_2)) + access_requests = self.get_access_requests('gamma', 'table', ds_1_id) + self.assertFalse(access_requests) + + gamma_user = sm.find_user(username='gamma') + gamma_user.roles.remove(sm.find_role('Alpha')) + session.commit() + + def test_clean_requests_after_db_grant(self): + session = db.session + + # Case 3. Two access requests from gamma and gamma2 + # Gamma gets database access, gamma2 access request granted + # Check if request by gamma has been deleted + + gamma_user = sm.find_user(username='gamma') + access_request1 = create_access_request( + session, 'table', 'long_lat', TEST_ROLE_1, 'gamma') + access_request2 = create_access_request( + session, 'table', 'long_lat', TEST_ROLE_2, 'gamma2') + ds_1_id = access_request1.datasource_id + # gamma gets granted database access + database = session.query(models.Database).first() + + security.merge_perm( + sm, 'database_access', database.perm) + ds_perm_view = sm.find_permission_view_menu( + 'database_access', database.perm) + sm.add_permission_role( + sm.find_role(DB_ACCESS_ROLE) , ds_perm_view) + gamma_user.roles.append(sm.find_role(DB_ACCESS_ROLE)) + session.commit() + access_requests = self.get_access_requests('gamma', 'table', ds_1_id) + self.assertTrue(access_requests) + # gamma2 request gets fulfilled + self.client.get(EXTEND_ROLE_REQUEST.format( + 'table', ds_1_id, 'gamma2', TEST_ROLE_2)) + access_requests = self.get_access_requests('gamma', 'table', ds_1_id) + + self.assertFalse(access_requests) + gamma_user = sm.find_user(username='gamma') + gamma_user.roles.remove(sm.find_role(DB_ACCESS_ROLE)) + session.commit() + + def test_clean_requests_after_schema_grant(self): + session = db.session + + # Case 4. Two access requests from gamma and gamma2 + # Gamma gets schema access, gamma2 access request granted + # Check if request by gamma has been deleted + + gamma_user = sm.find_user(username='gamma') + access_request1 = create_access_request( + session, 'table', 'wb_health_population', TEST_ROLE_1, 'gamma') + access_request2 = create_access_request( + session, 'table', 'wb_health_population', TEST_ROLE_2, 'gamma2') + ds_1_id = access_request1.datasource_id + ds = session.query(models.SqlaTable).filter_by( + table_name='wb_health_population').first() + + + ds.schema = 'temp_schema' + security.merge_perm( + sm, 'schema_access', ds.schema_perm) + schema_perm_view = sm.find_permission_view_menu( + 'schema_access', ds.schema_perm) + sm.add_permission_role( + sm.find_role(SCHEMA_ACCESS_ROLE) , schema_perm_view) + gamma_user.roles.append(sm.find_role(SCHEMA_ACCESS_ROLE)) + session.commit() + # gamma2 request gets fulfilled + self.client.get(EXTEND_ROLE_REQUEST.format( + 'table', ds_1_id, 'gamma2', TEST_ROLE_2)) + access_requests = self.get_access_requests('gamma', 'table', ds_1_id) + self.assertFalse(access_requests) + gamma_user = sm.find_user(username='gamma') + gamma_user.roles.remove(sm.find_role(SCHEMA_ACCESS_ROLE)) + + ds = session.query(models.SqlaTable).filter_by( + table_name='wb_health_population').first() + ds.schema = None + + session.commit() + @mock.patch('superset.utils.send_MIME_email') def test_approve(self, mock_send_mime): session = db.session TEST_ROLE_NAME = 'table_role' sm.add_role(TEST_ROLE_NAME) - def create_access_request(ds_type, ds_name, role_name): - ds_class = SourceRegistry.sources[ds_type] - # TODO: generalize datasource names - if ds_type == 'table': - ds = session.query(ds_class).filter( - ds_class.table_name == ds_name).first() - else: - ds = session.query(ds_class).filter( - ds_class.datasource_name == ds_name).first() - ds_perm_view = sm.find_permission_view_menu( - 'datasource_access', ds.perm) - sm.add_permission_role(sm.find_role(role_name), ds_perm_view) - access_request = models.DatasourceAccessRequest( - datasource_id=ds.id, - datasource_type=ds_type, - created_by_fk=sm.find_user(username='gamma').id, - ) - session.add(access_request) - session.commit() - return access_request - - EXTEND_ROLE_REQUEST = ( - '/superset/approve?datasource_type={}&datasource_id={}&' - 'created_by={}&role_to_extend={}') - GRANT_ROLE_REQUEST = ( - '/superset/approve?datasource_type={}&datasource_id={}&' - 'created_by={}&role_to_grant={}') - # Case 1. Grant new role to the user. access_request1 = create_access_request( - 'table', 'unicode_test', TEST_ROLE_NAME) + session, 'table', 'unicode_test', TEST_ROLE_NAME, 'gamma') ds_1_id = access_request1.datasource_id - self.get_resp(GRANT_ROLE_REQUEST.format( + resp = self.get_resp(GRANT_ROLE_REQUEST.format( 'table', ds_1_id, 'gamma', TEST_ROLE_NAME)) # Test email content. @@ -210,7 +354,8 @@ def create_access_request(ds_type, ds_name, role_name): # Case 2. Extend the role to have access to the table - access_request2 = create_access_request('table', 'long_lat', TEST_ROLE_NAME) + access_request2 = create_access_request( + session, 'table', 'long_lat', TEST_ROLE_NAME, 'gamma') ds_2_id = access_request2.datasource_id long_lat_perm = access_request2.datasource.perm @@ -241,7 +386,8 @@ def create_access_request(ds_type, ds_name, role_name): # Case 3. Grant new role to the user to access the druid datasource. sm.add_role('druid_role') - access_request3 = create_access_request('druid', 'druid_ds_1', 'druid_role') + access_request3 = create_access_request( + session, 'druid', 'druid_ds_1', 'druid_role', 'gamma') self.get_resp(GRANT_ROLE_REQUEST.format( 'druid', access_request3.datasource_id, 'gamma', 'druid_role')) @@ -251,7 +397,8 @@ def create_access_request(ds_type, ds_name, role_name): # Case 4. Extend the role to have access to the druid datasource - access_request4 = create_access_request('druid', 'druid_ds_2', 'druid_role') + access_request4 = create_access_request( + session, 'druid', 'druid_ds_2', 'druid_role', 'gamma') druid_ds_2_perm = access_request4.datasource.perm self.client.get(EXTEND_ROLE_REQUEST.format( diff --git a/tests/base_tests.py b/tests/base_tests.py index 6b38aab169e82..0c20a09bb935b 100644 --- a/tests/base_tests.py +++ b/tests/base_tests.py @@ -65,6 +65,13 @@ def __init__(self, *args, **kwargs): appbuilder.sm.find_role('Gamma'), password='general') + gamma2 = appbuilder.sm.find_user('gamma2') + if not gamma2: + appbuilder.sm.add_user( + 'gamma2', 'gamma2', 'user', 'gamma2@fab.org', + appbuilder.sm.find_role('Gamma'), + password='general') + gamma_sqllab_user = appbuilder.sm.find_user('gamma_sqllab') if not gamma_sqllab_user: appbuilder.sm.add_user( diff --git a/tests/superset_test_config.py b/tests/superset_test_config.py index 1e65d2b0ca357..9f1dd3dd0e9fc 100644 --- a/tests/superset_test_config.py +++ b/tests/superset_test_config.py @@ -21,6 +21,7 @@ WTF_CSRF_ENABLED = False PUBLIC_ROLE_LIKE_GAMMA = True AUTH_ROLE_PUBLIC = 'Public' +EMAIL_NOTIFICATIONS = False class CeleryConfig(object): BROKER_URL = 'sqla+sqlite:///' + SQL_CELERY_DB_FILE_PATH