Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Override the role with perms for give datasources." #1345

Merged
merged 1 commit into from
Oct 14, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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