diff --git a/caravel/config.py b/caravel/config.py index 71b754234c..89bcaa76f4 100644 --- a/caravel/config.py +++ b/caravel/config.py @@ -173,6 +173,7 @@ DRUID_DATA_SOURCE_BLACKLIST = [] DEFAULT_MODULE_DS_MAP = {'caravel.models': ['DruidDatasource', 'SqlaTable']} ADDITIONAL_MODULE_DS_MAP = {} + """ 1) http://docs.python-guide.org/en/latest/writing/logging/ 2) https://docs.python.org/2/library/logging.config.html diff --git a/caravel/models.py b/caravel/models.py index 4d0ab5a7e2..00a819989f 100644 --- a/caravel/models.py +++ b/caravel/models.py @@ -582,7 +582,6 @@ class Database(Model, AuditMixinNullable): """An ORM object that stores Database related information""" __tablename__ = 'dbs' - id = Column(Integer, primary_key=True) database_name = Column(String(250), unique=True) sqlalchemy_uri = Column(String(1024)) @@ -807,8 +806,7 @@ class SqlaTable(Model, Queryable, AuditMixinNullable, ImportMixin): @property def full_name(self): - return utils.get_datasource_full_name( - self.database, self.table_name, schema=self.schema) + return "[{obj.database}].[{obj.table_name}]".format(obj=self) @property def dttm_cols(self): @@ -1374,7 +1372,6 @@ class DruidCluster(Model, AuditMixinNullable): """ORM object referencing the Druid clusters""" __tablename__ = 'clusters' - id = Column(Integer, primary_key=True) cluster_name = Column(String(250), unique=True) coordinator_host = Column(String(255)) @@ -1487,8 +1484,9 @@ class DruidDatasource(Model, AuditMixinNullable, Queryable): @property def full_name(self): - return utils.get_datasource_full_name( - self.cluster_name, self.datasource_name) + return ( + "[{obj.cluster_name}]." + "[{obj.datasource_name}]").format(obj=self) @property def time_column_grains(self): diff --git a/caravel/source_registry.py b/caravel/source_registry.py index 669ca176fe..6176c9c0c9 100644 --- a/caravel/source_registry.py +++ b/caravel/source_registry.py @@ -22,14 +22,6 @@ class SourceRegistry(object): .one() ) - @classmethod - def get_all_datasources(cls, session): - datasources = [] - for source_type in SourceRegistry.sources: - datasources.extend( - session.query(SourceRegistry.sources[source_type]).all()) - return datasources - @classmethod def get_datasource_by_name(cls, session, datasource_type, datasource_name, schema, database_name): diff --git a/caravel/utils.py b/caravel/utils.py index c453a6e426..f3f1967304 100644 --- a/caravel/utils.py +++ b/caravel/utils.py @@ -229,7 +229,6 @@ def init(caravel): ADMIN_ONLY_PERMISSIONS = set([ 'can_sync_druid_source', - 'can_override_role_permissions', 'can_approve', ]) @@ -444,12 +443,6 @@ def generic_find_constraint_name(table, columns, referenced, db): return fk.name -def get_datasource_full_name(database_name, datasource_name, schema=None): - if not schema: - return "[{}].[{}]".format(database_name, datasource_name) - return "[{}].[{}].[{}]".format(database_name, schema, datasource_name) - - def validate_json(obj): if obj: try: diff --git a/caravel/views.py b/caravel/views.py index fd4a38721b..19307a0503 100755 --- a/caravel/views.py +++ b/caravel/views.py @@ -1062,53 +1062,6 @@ appbuilder.add_view_no_menu(R) class Caravel(BaseCaravelView): """The base views for Caravel!""" - @has_access - @expose("/override_role_permissions/", methods=['POST']) - def override_role_permissions(self): - """Updates the role with the give datasource permissions. - - Permissions not in the request will be revoked. This endpoint should - be available to admins only. Expects JSON in the format: - { - 'role_name': '{role_name}', - 'database': [{ - 'datasource_type': '{table|druid}', - 'name': '{database_name}', - 'schema': [{ - 'name': '{schema_name}', - 'datasources': ['{datasource name}, {datasource name}'] - }] - }] - } - """ - data = request.get_json(force=True) - role_name = data['role_name'] - databases = data['database'] - - db_ds_names = set() - for dbs in databases: - for schema in dbs['schema']: - for ds_name in schema['datasources']: - db_ds_names.add(utils.get_datasource_full_name( - dbs['name'], ds_name, schema=schema['name'])) - - existing_datasources = SourceRegistry.get_all_datasources(db.session) - datasources = [ - d for d in existing_datasources if d.full_name in db_ds_names] - - role = sm.find_role(role_name) - # remove all permissions - role.permissions = [] - # grant permissions to the list of datasources - for ds_name in datasources: - role.permissions.append( - sm.find_permission_view_menu( - view_menu_name=ds_name.perm, - permission_name='datasource_access') - ) - db.session.commit() - return Response(status=201) - @log_this @has_access @expose("/request_access/") diff --git a/tests/access_tests.py b/tests/access_requests.py similarity index 66% rename from tests/access_tests.py rename to tests/access_requests.py index 65e6a9a667..f443c273ee 100644 --- a/tests/access_tests.py +++ b/tests/access_requests.py @@ -4,7 +4,6 @@ from __future__ import division from __future__ import print_function from __future__ import unicode_literals -import json import unittest from caravel import db, models, sm @@ -12,144 +11,16 @@ from caravel.source_registry import SourceRegistry from .base_tests import CaravelTestCase -ROLE_TABLES_PERM_DATA = { - 'role_name': 'override_me', - 'database': [{ - 'datasource_type': 'table', - 'name': 'main', - 'schema': [{ - 'name': '', - 'datasources': ['birth_names'] - }] - }] -} - -ROLE_ALL_PERM_DATA = { - 'role_name': 'override_me', - 'database': [{ - 'datasource_type': 'table', - 'name': 'main', - 'schema': [{ - 'name': '', - 'datasources': ['birth_names'] - }] - }, { - 'datasource_type': 'druid', - 'name': 'druid_test', - 'schema': [{ - 'name': '', - 'datasources': ['druid_ds_1', 'druid_ds_2'] - }] - } - ] -} class RequestAccessTests(CaravelTestCase): - requires_examples = False - - @classmethod - def setUpClass(cls): - sm.add_role('override_me') - db.session.commit() - - @classmethod - def tearDownClass(cls): - override_me = sm.find_role('override_me') - db.session.delete(override_me) - db.session.commit() - - def setUp(self): - self.login('admin') - - def tearDown(self): - self.logout() - override_me = sm.find_role('override_me') - override_me.permissions = [] - db.session.commit() - db.session.close() - - def test_override_role_permissions_is_admin_only(self): - self.logout() - self.login('alpha') - response = self.client.post( - '/caravel/override_role_permissions/', - data=json.dumps(ROLE_TABLES_PERM_DATA), - content_type='application/json', - follow_redirects=True) - self.assertNotEquals(405, response.status_code) - - def test_override_role_permissions_1_table(self): - response = self.client.post( - '/caravel/override_role_permissions/', - data=json.dumps(ROLE_TABLES_PERM_DATA), - content_type='application/json') - self.assertEquals(201, response.status_code) - - updated_override_me = sm.find_role('override_me') - self.assertEquals(1, len(updated_override_me.permissions)) - birth_names = self.get_table_by_name('birth_names') - self.assertEquals( - birth_names.perm, - updated_override_me.permissions[0].view_menu.name) - self.assertEquals( - 'datasource_access', - updated_override_me.permissions[0].permission.name) - - def test_override_role_permissions_druid_and_table(self): - response = self.client.post( - '/caravel/override_role_permissions/', - data=json.dumps(ROLE_ALL_PERM_DATA), - content_type='application/json') - self.assertEquals(201, response.status_code) - - updated_role = sm.find_role('override_me') - perms = sorted( - updated_role.permissions, key=lambda p: p.view_menu.name) - self.assertEquals(3, len(perms)) - druid_ds_1 = self.get_druid_ds_by_name('druid_ds_1') - self.assertEquals(druid_ds_1.perm, perms[0].view_menu.name) - self.assertEquals('datasource_access', perms[0].permission.name) - - druid_ds_2 = self.get_druid_ds_by_name('druid_ds_2') - self.assertEquals(druid_ds_2.perm, perms[1].view_menu.name) - self.assertEquals( - 'datasource_access', updated_role.permissions[1].permission.name) - - birth_names = self.get_table_by_name('birth_names') - self.assertEquals(birth_names.perm, perms[2].view_menu.name) - self.assertEquals( - 'datasource_access', updated_role.permissions[2].permission.name) - - def test_override_role_permissions_drops_absent_perms(self): - override_me = sm.find_role('override_me') - override_me.permissions.append( - sm.find_permission_view_menu( - view_menu_name=self.get_table_by_name('long_lat').perm, - permission_name='datasource_access') - ) - db.session.flush() - - response = self.client.post( - '/caravel/override_role_permissions/', - data=json.dumps(ROLE_TABLES_PERM_DATA), - content_type='application/json') - self.assertEquals(201, response.status_code) - updated_override_me = sm.find_role('override_me') - self.assertEquals(1, len(updated_override_me.permissions)) - birth_names = self.get_table_by_name('birth_names') - self.assertEquals( - birth_names.perm, - updated_override_me.permissions[0].view_menu.name) - self.assertEquals( - 'datasource_access', - updated_override_me.permissions[0].permission.name) - + requires_examples = True def test_approve(self): session = db.session TEST_ROLE_NAME = 'table_role' sm.add_role(TEST_ROLE_NAME) + self.login('admin') def create_access_request(ds_type, ds_name, role_name): ds_class = SourceRegistry.sources[ds_type] @@ -245,7 +116,6 @@ class RequestAccessTests(CaravelTestCase): def test_request_access(self): session = db.session - self.logout() self.login(username='gamma') gamma_user = sm.find_user(username='gamma') sm.add_role('dummy_role') diff --git a/tests/base_tests.py b/tests/base_tests.py index c841c77cdd..1e22934e27 100644 --- a/tests/base_tests.py +++ b/tests/base_tests.py @@ -20,8 +20,8 @@ cli = imp.load_source('cli', BASE_DIR + "/bin/caravel") class CaravelTestCase(unittest.TestCase): - requires_examples = True - examples_loaded = True + requires_examples = False + examples_loaded = False def __init__(self, *args, **kwargs): if ( @@ -119,15 +119,6 @@ class CaravelTestCase(unittest.TestCase): session.expunge_all() return slc - def get_table_by_name(self, name): - return db.session.query(models.SqlaTable).filter_by( - table_name=name).first() - - def get_druid_ds_by_name(self, name): - return db.session.query(models.DruidDatasource).filter_by( - datasource_name=name).first() - - def get_resp(self, url): """Shortcut to get the parsed results while following redirects""" resp = self.client.get(url, follow_redirects=True) diff --git a/tests/import_export_tests.py b/tests/import_export_tests.py index 33f9bd506d..6a929a6c42 100644 --- a/tests/import_export_tests.py +++ b/tests/import_export_tests.py @@ -20,7 +20,6 @@ class ImportExportTests(CaravelTestCase): def __init__(self, *args, **kwargs): super(ImportExportTests, self).__init__(*args, **kwargs) - db.session.commit() @classmethod def delete_imports(cls): @@ -110,6 +109,10 @@ class ImportExportTests(CaravelTestCase): return db.session.query(models.SqlaTable).filter_by( id=table_id).first() + def get_table_by_name(self, name): + return db.session.query(models.SqlaTable).filter_by( + table_name=name).first() + def assert_dash_equals(self, expected_dash, actual_dash): self.assertEquals(expected_dash.slug, actual_dash.slug) self.assertEquals( @@ -218,6 +221,7 @@ class ImportExportTests(CaravelTestCase): self.assert_slice_equals(slc_1, imported_slc_1) self.assertEquals(imported_slc_1.datasource.perm, imported_slc_1.perm) + self.assertEquals(table_id, imported_slc_2.datasource_id) self.assert_slice_equals(slc_2, imported_slc_2) self.assertEquals(imported_slc_2.datasource.perm, imported_slc_2.perm)