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.

Sign up for GitHub

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

[security] allow security manager provide error message #5500

Merged
merged 1 commit into from
Jul 27, 2018
Merged

[security] allow security manager provide error message #5500

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 12 additions & 0 deletions superset/security.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,18 @@ def datasource_access(self, datasource, user=None):
self.can_access('datasource_access', datasource.perm, user=user)
)

def get_datasource_access_error_msg(self, datasource):
return """This endpoint requires the datasource {}, database or
`all_datasource_access` permission""".format(datasource.name)

def get_datasource_access_link(self, datasource):
from superset import conf
return conf.get('PERMISSION_INSTRUCTIONS_LINK')

def get_table_access_error_msg(self, table_name):
return """You need access to the following tables: {}, all database access or
`all_datasource_access` permission""".format(table_name)

def datasource_access_by_name(
self, database, datasource_name, schema=None):
from superset import db
Expand Down
42 changes: 23 additions & 19 deletions superset/views/core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@
ACCESS_REQUEST_MISSING_ERR = __(
'The access requests seem to have been deleted')
USER_MISSING_ERR = __('The user seems to have been deleted')
DATASOURCE_ACCESS_ERR = __("You don't have access to this datasource")

FORM_DATA_KEY_BLACKLIST = []
if not config.get('ENABLE_JAVASCRIPT_CONTROLS'):
Expand All @@ -84,11 +83,6 @@ def get_database_access_error_msg(database_name):
'`all_datasource_access` permission', name=database_name)


def get_datasource_access_error_msg(datasource_name):
return __('This endpoint requires the datasource %(name)s, database or '
'`all_datasource_access` permission', name=datasource_name)


def json_success(json_msg, status=200):
return Response(json_msg, status=status, mimetype='application/json')

Expand Down Expand Up @@ -1094,8 +1088,9 @@ def generate_json(self, datasource_type, datasource_id, form_data,

if not security_manager.datasource_access(viz_obj.datasource, g.user):
return json_error_response(
DATASOURCE_ACCESS_ERR, status=404, link=config.get(
'PERMISSION_INSTRUCTIONS_LINK'))
security_manager.get_datasource_access_error_msg(viz_obj.datasource),
status=404,
link=security_manager.get_datasource_access_error_msg(viz_obj.datasource))

if csv:
return CsvResponse(
Expand Down Expand Up @@ -1260,9 +1255,11 @@ def explore(self, datasource_type=None, datasource_id=None):
flash(DATASOURCE_MISSING_ERR, 'danger')
return redirect(error_redirect)

if not security_manager.datasource_access(datasource):
if config.get('ENABLE_ACCESS_REQUEST') and (
Copy link
Member

Choose a reason for hiding this comment

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

Why was this check added?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A check is done at a lower point (in explore_json) This check here is strictly for redirecting users who don't have access to the access request page.

not security_manager.datasource_access(datasource)
):
flash(
__(get_datasource_access_error_msg(datasource.name)),
__(security_manager.get_datasource_access_error_msg(datasource)),
'danger')
return redirect(
'superset/request_access/?'
Expand Down Expand Up @@ -1364,7 +1361,8 @@ def filter(self, datasource_type, datasource_id, column):
if not datasource:
return json_error_response(DATASOURCE_MISSING_ERR)
if not security_manager.datasource_access(datasource):
return json_error_response(DATASOURCE_ACCESS_ERR)
return json_error_response(
security_manager.get_datasource_access_error_msg(datasource))

payload = json.dumps(
datasource.values_for_column(
Expand Down Expand Up @@ -2086,7 +2084,7 @@ def dashboard(self, dashboard_id):
for datasource in datasources:
if datasource and not security_manager.datasource_access(datasource):
flash(
__(get_datasource_access_error_msg(datasource.name)),
__(security_manager.get_datasource_access_error_msg(datasource)),
'danger')
return redirect(
'superset/request_access/?'
Expand Down Expand Up @@ -2384,7 +2382,7 @@ def results(self, key):
rejected_tables = security_manager.rejected_datasources(
query.sql, query.database, query.schema)
if rejected_tables:
return json_error_response(get_datasource_access_error_msg(
return json_error_response(security_manager.get_table_access_error_msg(
'{}'.format(rejected_tables)))

return json_success(utils.zlib_decompress_to_string(blob))
Expand Down Expand Up @@ -2426,8 +2424,10 @@ def sql_json(self):

rejected_tables = security_manager.rejected_datasources(sql, mydb, schema)
if rejected_tables:
return json_error_response(get_datasource_access_error_msg(
'{}'.format(rejected_tables)))
return json_error_response(
security_manager.get_datasource_access_error_msg('{}'.format(
rejected_tables)),
link=security_manager.get_table_error_link(rejected_tables))
session.commit()

select_as_cta = request.form.get('select_as_cta') == 'true'
Expand Down Expand Up @@ -2540,7 +2540,8 @@ def csv(self, client_id):
rejected_tables = security_manager.rejected_datasources(
query.sql, query.database, query.schema)
if rejected_tables:
flash(get_datasource_access_error_msg('{}'.format(rejected_tables)))
flash(
security_manager.get_table_access_error_msg('{}'.format(rejected_tables)))
return redirect('/')
blob = None
if results_backend and query.results_key:
Expand Down Expand Up @@ -2582,7 +2583,9 @@ def fetch_datasource_metadata(self):

# Check permission for datasource
if not security_manager.datasource_access(datasource):
return json_error_response(DATASOURCE_ACCESS_ERR)
return json_error_response(
security_manager.get_datasource_access_error_msg(datasource),
link=security_manager.get_datasource_error_link(datasource))
return json_success(json.dumps(datasource.data))

@expose('/queries/<last_updated_ms>')
Expand Down Expand Up @@ -2760,8 +2763,9 @@ def sliceQuery(self, slice_id):
viz_obj = self.get_viz(slice_id)
if not security_manager.datasource_access(viz_obj.datasource):
return json_error_response(
DATASOURCE_ACCESS_ERR, status=401, link=config.get(
'PERMISSION_INSTRUCTIONS_LINK'))
security_manager.get_datasource_access_error_msg(viz_obj.datasource),
status=401,
link=security_manager.get_datasource_error_link(viz_obj.datasource))
return self.get_query_string_response(viz_obj)


Expand Down