Skip to content

Commit

Permalink
Revert "Override the role with perms for give datasources." (#1345)
Browse files Browse the repository at this point in the history
  • Loading branch information
bkyryliuk authored Oct 14, 2016
1 parent 40e7057 commit f0f8478
Show file tree
Hide file tree
Showing 8 changed files with 14 additions and 212 deletions.
1 change: 1 addition & 0 deletions caravel/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@
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
Expand Down
10 changes: 4 additions & 6 deletions caravel/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -807,8 +806,7 @@ def name(self):

@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):
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -1487,8 +1484,9 @@ def link(self):

@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):
Expand Down
8 changes: 0 additions & 8 deletions caravel/source_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,6 @@ def get_datasource(cls, datasource_type, datasource_id, session):
.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):
Expand Down
7 changes: 0 additions & 7 deletions caravel/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,6 @@ def init(caravel):

ADMIN_ONLY_PERMISSIONS = set([
'can_sync_druid_source',
'can_override_role_permissions',
'can_approve',
])

Expand Down Expand Up @@ -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:
Expand Down
47 changes: 0 additions & 47 deletions caravel/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -1062,53 +1062,6 @@ def msg(self):

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/")
Expand Down
134 changes: 2 additions & 132 deletions tests/access_tests.py → tests/access_requests.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,152 +4,23 @@
from __future__ import print_function
from __future__ import unicode_literals

import json
import unittest

from caravel import db, models, sm
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]
Expand Down Expand Up @@ -245,7 +116,6 @@ def create_access_request(ds_type, ds_name, role_name):

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')
Expand Down
13 changes: 2 additions & 11 deletions tests/base_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@


class CaravelTestCase(unittest.TestCase):
requires_examples = True
examples_loaded = True
requires_examples = False
examples_loaded = False

def __init__(self, *args, **kwargs):
if (
Expand Down Expand Up @@ -119,15 +119,6 @@ def get_slice(self, slice_name, session):
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)
Expand Down
6 changes: 5 additions & 1 deletion tests/import_export_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -110,6 +109,10 @@ def get_table(self, table_id):
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(
Expand Down Expand Up @@ -218,6 +221,7 @@ def test_import_2_slices_for_same_table(self):
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)
Expand Down

0 comments on commit f0f8478

Please sign in to comment.