Skip to content

Commit

Permalink
Change has_access and require_access signatures (#3611)
Browse files Browse the repository at this point in the history
* change has_access and require_access signatures to work with the objects that require access, instead of their groups

* rename `object` to `obj`

* support both objects and group dicts in `has_access` and `require_access`

* simplify permission tests once `has_access` accepts groups
  • Loading branch information
Omer Lachish authored and arikfr committed Mar 28, 2019
1 parent 1871287 commit ec4f77c
Show file tree
Hide file tree
Showing 10 changed files with 25 additions and 23 deletions.
8 changes: 4 additions & 4 deletions redash/handlers/alerts.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
class AlertResource(BaseResource):
def get(self, alert_id):
alert = get_object_or_404(models.Alert.get_by_id_and_org, alert_id, self.current_org)
require_access(alert.groups, self.current_user, view_only)
require_access(alert, self.current_user, view_only)
self.record_event({
'action': 'view',
'object_id': alert.id,
Expand Down Expand Up @@ -53,7 +53,7 @@ def post(self):

query = models.Query.get_by_id_and_org(req['query_id'],
self.current_org)
require_access(query.groups, self.current_user, view_only)
require_access(query, self.current_user, view_only)

alert = models.Alert(
name=req['name'],
Expand Down Expand Up @@ -89,7 +89,7 @@ def post(self, alert_id):
req = request.get_json(True)

alert = models.Alert.get_by_id_and_org(alert_id, self.current_org)
require_access(alert.groups, self.current_user, view_only)
require_access(alert, self.current_user, view_only)
kwargs = {'alert': alert, 'user': self.current_user}

if 'destination_id' in req:
Expand All @@ -113,7 +113,7 @@ def post(self, alert_id):
def get(self, alert_id):
alert_id = int(alert_id)
alert = models.Alert.get_by_id_and_org(alert_id, self.current_org)
require_access(alert.groups, self.current_user, view_only)
require_access(alert, self.current_user, view_only)

subscriptions = models.AlertSubscription.all(alert_id)
return [s.to_dict() for s in subscriptions]
Expand Down
2 changes: 1 addition & 1 deletion redash/handlers/data_sources.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ def post(self):
class DataSourceSchemaResource(BaseResource):
def get(self, data_source_id):
data_source = get_object_or_404(models.DataSource.get_by_id_and_org, data_source_id, self.current_org)
require_access(data_source.groups, self.current_user, view_only)
require_access(data_source, self.current_user, view_only)
refresh = request.args.get('refresh') is not None

response = {}
Expand Down
4 changes: 2 additions & 2 deletions redash/handlers/favorites.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
class QueryFavoriteResource(BaseResource):
def post(self, query_id):
query = get_object_or_404(models.Query.get_by_id_and_org, query_id, self.current_org)
require_access(query.groups, self.current_user, view_only)
require_access(query, self.current_user, view_only)

fav = models.Favorite(org_id=self.current_org.id, object=query, user=self.current_user)
models.db.session.add(fav)
Expand All @@ -31,7 +31,7 @@ def post(self, query_id):

def delete(self, query_id):
query = get_object_or_404(models.Query.get_by_id_and_org, query_id, self.current_org)
require_access(query.groups, self.current_user, view_only)
require_access(query, self.current_user, view_only)

models.Favorite.query.filter(
models.Favorite.object_id == query_id,
Expand Down
8 changes: 4 additions & 4 deletions redash/handlers/queries.py
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ def post(self):
"""
query_def = request.get_json(force=True)
data_source = models.DataSource.get_by_id_and_org(query_def.pop('data_source_id'), self.current_org)
require_access(data_source.groups, self.current_user, not_view_only)
require_access(data_source, self.current_user, not_view_only)
require_access_to_dropdown_queries(self.current_user, query_def)

for field in ['id', 'created_at', 'api_key', 'visualizations', 'latest_query_data', 'last_modified_by']:
Expand Down Expand Up @@ -356,7 +356,7 @@ def get(self, query_id):
Responds with the :ref:`query <query-response-label>` contents.
"""
q = get_object_or_404(models.Query.get_by_id_and_org, query_id, self.current_org)
require_access(q.groups, self.current_user, view_only)
require_access(q, self.current_user, view_only)

result = QuerySerializer(q, with_visualizations=True).serialize()
result['can_edit'] = can_modify(q, self.current_user)
Expand Down Expand Up @@ -393,7 +393,7 @@ def post(self, query_id):
Responds with created :ref:`query <query-response-label>` object.
"""
query = get_object_or_404(models.Query.get_by_id_and_org, query_id, self.current_org)
require_access(query.data_source.groups, self.current_user, not_view_only)
require_access(query.data_source, self.current_user, not_view_only)
forked_query = query.fork(self.current_user)
models.db.session.commit()

Expand Down Expand Up @@ -422,7 +422,7 @@ def post(self, query_id):
abort(403, message="Please use a user API key.")

query = get_object_or_404(models.Query.get_by_id_and_org, query_id, self.current_org)
require_access(query.groups, self.current_user, not_view_only)
require_access(query, self.current_user, not_view_only)

parameter_values = collect_parameters_from_request(request.args)
parameterized_query = ParameterizedQuery(query.query_text)
Expand Down
6 changes: 3 additions & 3 deletions redash/handlers/query_results.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ def post(self):

data_source = models.DataSource.get_by_id_and_org(params.get('data_source_id'), self.current_org)

if not has_access(data_source.groups, self.current_user, not_view_only):
if not has_access(data_source, self.current_user, not_view_only):
return {'job': {'status': 4, 'error': 'You do not have permission to run queries with this data source.'}}, 403

self.record_event({
Expand Down Expand Up @@ -213,7 +213,7 @@ def post(self, query_id):

allow_executing_with_view_only_permissions = query.parameterized.is_safe

if has_access(query.data_source.groups, self.current_user, allow_executing_with_view_only_permissions):
if has_access(query.data_source, self.current_user, allow_executing_with_view_only_permissions):
return run_query(query.parameterized, parameters, query.data_source, query_id, max_age)
else:
return {'job': {'status': 4, 'error': 'You do not have permission to run queries with this data source.'}}, 403
Expand Down Expand Up @@ -264,7 +264,7 @@ def get(self, query_id=None, query_result_id=None, filetype='json'):
abort(404, message='No cached result found for this query.')

if query_result:
require_access(query_result.data_source.groups, self.current_user, view_only)
require_access(query_result.data_source, self.current_user, view_only)

if isinstance(self.current_user, models.ApiUser):
event = {
Expand Down
2 changes: 1 addition & 1 deletion redash/handlers/widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def post(self):
visualization_id = widget_properties.pop('visualization_id')
if visualization_id:
visualization = models.Visualization.get_by_id_and_org(visualization_id, self.current_org)
require_access(visualization.query_rel.groups, self.current_user, view_only)
require_access(visualization.query_rel, self.current_user, view_only)
else:
visualization = None

Expand Down
2 changes: 1 addition & 1 deletion redash/models/parameterized_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def _load_result(query_id, should_require_access):
query = models.Query.get_by_id_and_org(query_id, current_org)

if should_require_access:
require_access(query.data_source.groups, current_user, view_only)
require_access(query.data_source, current_user, view_only)

query_result = models.QueryResult.get_by_id_and_org(query.latest_query_data_id, current_org)

Expand Down
12 changes: 7 additions & 5 deletions redash/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,24 +14,26 @@
ACCESS_TYPES = (ACCESS_TYPE_VIEW, ACCESS_TYPE_MODIFY, ACCESS_TYPE_DELETE)


def has_access(object_groups, user, need_view_only):
def has_access(obj, user, need_view_only):
groups = obj.groups if hasattr(obj, 'groups') else obj

if 'admin' in user.permissions:
return True

matching_groups = set(object_groups.keys()).intersection(user.group_ids)
matching_groups = set(groups.keys()).intersection(user.group_ids)

if not matching_groups:
return False

required_level = 1 if need_view_only else 2

group_level = 1 if all(flatten([object_groups[group] for group in matching_groups])) else 2
group_level = 1 if all(flatten([groups[group] for group in matching_groups])) else 2

return required_level <= group_level


def require_access(object_groups, user, need_view_only):
if not has_access(object_groups, user, need_view_only):
def require_access(obj, user, need_view_only):
if not has_access(obj, user, need_view_only):
abort(403)


Expand Down
2 changes: 1 addition & 1 deletion redash/query_runner/query_results.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def _load_query(user, query_id):
if user.org_id != query.org_id:
raise PermissionError("Query id {} not found.".format(query.id))

if not has_access(query.data_source.groups, user, not_view_only):
if not has_access(query.data_source, user, not_view_only):
raise PermissionError(u"You are not allowed to execute queries on {} data source (used for query id {}).".format(
query.data_source.name, query.id))

Expand Down
2 changes: 1 addition & 1 deletion redash/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ def serialize_dashboard(obj, with_widgets=False, user=None, with_favorite_state=
for w in obj.widgets:
if w.visualization_id is None:
widgets.append(serialize_widget(w))
elif user and has_access(w.visualization.query_rel.groups, user, view_only):
elif user and has_access(w.visualization.query_rel, user, view_only):
widgets.append(serialize_widget(w))
else:
widget = project(serialize_widget(w),
Expand Down

0 comments on commit ec4f77c

Please sign in to comment.