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

Fix issues around Database permissions #7009

Merged
merged 1 commit into from Mar 18, 2019
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 9 additions & 3 deletions superset/security.py
Expand Up @@ -107,8 +107,10 @@ def can_access(self, permission_name, view_name):
return self._has_view_access(user, permission_name, view_name)

def all_datasource_access(self):
return self.can_access(
'all_datasource_access', 'all_datasource_access')
return self.can_access('all_datasource_access', 'all_datasource_access')

def all_database_access(self):
return self.can_access('all_database_access', 'all_database_access')

def database_access(self, database):
return (
Expand Down Expand Up @@ -410,8 +412,12 @@ def set_perm(self, mapper, connection, target): # noqa
.values(perm=target.get_perm()),
)

# add to view menu if not already exists
permission_name = 'datasource_access'
from superset.models.core import Database
if mapper.class_ == Database:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI , superset init would create the proper database_access (one for each database), but the code here (executed on update/insert) would create/update a datasource_access entry for the database

permission_name = 'database_access'

# add to view menu if not already exists
view_menu_name = target.get_perm()
permission = self.find_permission(permission_name)
view_menu = self.find_view_menu(view_menu_name)
Expand Down
12 changes: 11 additions & 1 deletion superset/views/core.py
Expand Up @@ -109,18 +109,27 @@ def is_owner(obj, user):
extend_existing=True)


class DatabaseFilter(SupersetFilter):
def apply(self, query, func): # noqa
if security_manager.all_database_access():
return query
database_perms = self.get_view_menus('database_access')
return query.filter(self.model.perm.in_(database_perms))


class SliceFilter(SupersetFilter):
def apply(self, query, func): # noqa
if security_manager.all_datasource_access():
return query

# TODO(bogdan): add `schema_access` support here
datasource_perms = self.get_view_menus('datasource_access')
database_perms = self.get_view_menus('database_access')
query = (
query.outerjoin(SQLTable, self.model.datasource_id == SQLTable.c.id)
.outerjoin(models.Database, models.Database.id == SQLTable.c.database_id)
.filter(or_(
models.Database.perm.in_(datasource_perms),
models.Database.perm.in_(database_perms),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears to be using the wrong entry.

self.model.perm.in_(datasource_perms),
))
)
Expand Down Expand Up @@ -285,6 +294,7 @@ class DatabaseView(SupersetModelView, DeleteMixin, YamlExportMixin): # noqa
'allow_multi_schema_metadata_fetch': _('Allow Multi Schema Metadata Fetch'),
'backend': _('Backend'),
}
base_filters = [['id', DatabaseFilter, lambda: []]]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this broke stuff on our end, users can't see DBs in SQL Lab. I commented it out and permissions work correctly. (edited to say "can't")

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean "can't see" (you wrote "can see"). This may require adding all_database_access to alpha

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


def pre_add(self, db):
self.check_extra(db)
Expand Down