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

Implement view access permission for Dashboard and Query resources. #2011

Closed
wants to merge 8 commits into from
Closed
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
25 changes: 15 additions & 10 deletions client/app/components/permissions-editor/index.js
Expand Up @@ -8,12 +8,14 @@ const PermissionsEditorComponent = {
close: '&',
dismiss: '&',
},
controller($http, User) {
controller($http, User, clientConfig) {
'ngInject';

this.grantees = [];
this.newGrantees = {};
this.aclUrl = this.resolve.aclUrl.url;
this.showViewPermission = clientConfig.showViewPermission;
this.foundUsers = {};

// List users that are granted permissions
const loadGrantees = () => {
Expand All @@ -32,24 +34,26 @@ const PermissionsEditorComponent = {
loadGrantees();

// Search for user
this.findUser = (search) => {
this.findUser = (search, accessType) => {
if (search === '') {
return;
}

if (this.foundUsers === undefined) {
if (this.foundUsers[accessType] === undefined) {
User.query((users) => {
const existingIds = this.grantees.map(m => m.id);
const filtered = this.grantees.filter(m => m.access_type === accessType);
const existingIds = filtered.map(m => m.id);
users.forEach((user) => { user.alreadyGrantee = includes(existingIds, user.id); });
this.foundUsers = users;
this.foundUsers[accessType] = users;
});
}
};


// Add new user to grantees list
this.addGrantee = (user) => {
this.addGrantee = (user, accessType) => {
this.newGrantees.selected = undefined;
const body = { access_type: 'modify', user_id: user.id };
const body = { access_type: accessType, user_id: user.id };
$http.post(this.aclUrl, body).success(() => {
user.alreadyGrantee = true;
loadGrantees();
Expand All @@ -58,7 +62,7 @@ const PermissionsEditorComponent = {

// Remove user from grantees list
this.removeGrantee = (user) => {
const body = { access_type: 'modify', user_id: user.id };
const body = { access_type: user.access_type, user_id: user.id };
$http({
url: this.aclUrl,
method: 'DELETE',
Expand All @@ -67,8 +71,9 @@ const PermissionsEditorComponent = {
}).success(() => {
this.grantees = this.grantees.filter(m => m !== user);

if (this.foundUsers) {
this.foundUsers.forEach((u) => { if (u.id === user.id) { u.alreadyGrantee = false; } });
if (this.foundUsers[user.access_type]) {
const users = this.foundUsers[user.access_type];
users.forEach((u) => { if (u.id === user.id) { u.alreadyGrantee = false; } });
}
});
};
Expand Down
21 changes: 17 additions & 4 deletions client/app/components/permissions-editor/permissions-editor.html
Expand Up @@ -4,10 +4,23 @@ <h4 class="modal-title">Manage Permissions</h4>
</div>
<div class="modal-body">
<div style="overflow: auto; height: 300px">
<ui-select ng-model="$ctrl.newGrantee.selected" on-select="$ctrl.addGrantee($item)">
<ui-select-match placeholder="Add New User"></ui-select-match>
<ui-select-choices repeat="user in $ctrl.foundUsers | filter:$select.search"
refresh="$ctrl.findUser($select.search)"
<ui-select ng-if="$ctrl.showViewPermission" ng-model="$ctrl.newGrantee.selected" on-select="$ctrl.addGrantee($item, 'view')">
<ui-select-match placeholder="Add View New User"></ui-select-match>
<ui-select-choices repeat="user in $ctrl.foundUsers['view'] | filter:$select.search"
refresh="$ctrl.findUser($select.search, 'view')"
refresh-delay="0"
ui-disable-choice="user.alreadyGrantee">
<div>
<img ng-src="{{user.gravatar_url}}" height="24px">&nbsp;{{user.name}}
<small ng-if="user.alreadyGrantee">(already has permission)</small>
</div>
</ui-select-choices>
</ui-select>
<br/>
<ui-select ng-model="$ctrl.newGrantee.selected" on-select="$ctrl.addGrantee($item, 'modify')">
<ui-select-match placeholder="Add Modify New User"></ui-select-match>
<ui-select-choices repeat="user in $ctrl.foundUsers['modify'] | filter:$select.search"
refresh="$ctrl.findUser($select.search, 'modify')"
refresh-delay="0"
ui-disable-choice="user.alreadyGrantee">
<div>
Expand Down
1 change: 1 addition & 0 deletions redash/handlers/authentication.py
Expand Up @@ -176,6 +176,7 @@ def client_config():
defaults = {
'allowScriptsInUserInput': settings.ALLOW_SCRIPTS_IN_USER_INPUT,
'showPermissionsControl': settings.FEATURE_SHOW_PERMISSIONS_CONTROL,
'showViewPermission': settings.FEATURE_ENABLE_VIEW_PERMISSION,
'allowCustomJSVisualizations': settings.FEATURE_ALLOW_CUSTOM_JS_VISUALIZATIONS,
'autoPublishNamedQueries': settings.FEATURE_AUTO_PUBLISH_NAMED_QUERIES,
'mailSettingsMissing': settings.MAIL_DEFAULT_SENDER is None,
Expand Down
8 changes: 6 additions & 2 deletions redash/handlers/dashboards.py
Expand Up @@ -98,7 +98,11 @@ def get(self, dashboard_slug=None):
:>json string widget.created_at: ISO format timestamp for widget creation
:>json string widget.updated_at: ISO format timestamp for last widget modification
"""
dashboard = get_object_or_404(models.Dashboard.get_by_slug_and_org, dashboard_slug, self.current_org)
dashboard = get_object_or_404(models.Dashboard.get_by_slug_and_org,
dashboard_slug,
self.current_org,
self.current_user.id)

response = dashboard.to_dict(with_widgets=True, user=self.current_user)

api_key = models.ApiKey.get_by_object(dashboard)
Expand Down Expand Up @@ -158,7 +162,7 @@ def delete(self, dashboard_slug):

Responds with the archived :ref:`dashboard <dashboard-response-label>`.
"""
dashboard = models.Dashboard.get_by_slug_and_org(dashboard_slug, self.current_org)
dashboard = models.Dashboard.get_by_slug_and_org(dashboard_slug, self.current_org, self.current_user.id)
dashboard.is_archived = True
dashboard.record_changes(changed_by=self.current_user)
models.db.session.add(dashboard)
Expand Down
7 changes: 3 additions & 4 deletions redash/handlers/permissions.py
Expand Up @@ -24,7 +24,7 @@ def get_model_from_type(type):
class ObjectPermissionsListResource(BaseResource):
def get(self, object_type, object_id):
model = get_model_from_type(object_type)
obj = get_object_or_404(model.get_by_id_and_org, object_id, self.current_org)
obj = get_object_or_404(model.get_by_id_and_org, object_id, self.current_org, user_id=self.current_user.id)

# TODO: include grantees in search to avoid N+1 queries
permissions = AccessPermission.find(obj)
Expand All @@ -38,7 +38,7 @@ def get(self, object_type, object_id):

def post(self, object_type, object_id):
model = get_model_from_type(object_type)
obj = get_object_or_404(model.get_by_id_and_org, object_id, self.current_org)
obj = get_object_or_404(model.get_by_id_and_org, object_id, self.current_org, user_id=self.current_user.id)

require_admin_or_owner(obj.user_id)

Expand Down Expand Up @@ -69,8 +69,7 @@ def post(self, object_type, object_id):

def delete(self, object_type, object_id):
model = get_model_from_type(object_type)
obj = get_object_or_404(model.get_by_id_and_org, object_id,
self.current_org)
obj = get_object_or_404(model.get_by_id_and_org, object_id, self.current_org, user_id=self.current_user.id)

require_admin_or_owner(obj.user_id)

Expand Down
13 changes: 7 additions & 6 deletions redash/handlers/queries.py
Expand Up @@ -53,6 +53,7 @@ def get(self):
for q in models.Query.search(term,
self.current_user.group_ids,
include_drafts=include_drafts,
user_id=self.current_user.id,
limit=None)]


Expand All @@ -74,7 +75,7 @@ def get(self):

global_recent = []
if len(recent) < 10:
global_recent = [d.to_dict(with_last_modified_by=False, with_user=False) for d in models.Query.recent(self.current_user.group_ids)]
global_recent = [d.to_dict(with_last_modified_by=False, with_user=False) for d in models.Query.recent(self.current_user.group_ids, self.current_user.id)]

queries = take(20, distinct(chain(recent, global_recent), key=lambda d: d['id']))

Expand Down Expand Up @@ -191,7 +192,7 @@ def post(self, query_id):

Responds with the updated :ref:`query <query-response-label>` object.
"""
query = get_object_or_404(models.Query.get_by_id_and_org, query_id, self.current_org)
query = get_object_or_404(models.Query.get_by_id_and_org, query_id, self.current_org, user_id=self.current_user.id)
query_def = request.get_json(force=True)

require_object_modify_permission(query, self.current_user)
Expand Down Expand Up @@ -227,7 +228,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)
q = get_object_or_404(models.Query.get_by_id_and_org, query_id, self.current_org, self.current_user.id)
require_access(q.groups, self.current_user, view_only)

result = q.to_dict(with_visualizations=True)
Expand All @@ -241,7 +242,7 @@ def delete(self, query_id):

:param query_id: ID of query to archive
"""
query = get_object_or_404(models.Query.get_by_id_and_org, query_id, self.current_org)
query = get_object_or_404(models.Query.get_by_id_and_org, query_id, self.current_org, self.current_user.id)
require_admin_or_owner(query.user_id)
query.archive(self.current_user)
models.db.session.commit()
Expand All @@ -257,7 +258,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)
query = get_object_or_404(models.Query.get_by_id_and_org, query_id, self.current_org, self.current_user.id)
require_access(query.data_source.groups, self.current_user, not_view_only)
forked_query = query.fork(self.current_user)
models.db.session.commit()
Expand All @@ -279,7 +280,7 @@ def post(self, query_id):
if self.current_user.is_api_user():
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)
query = get_object_or_404(models.Query.get_by_id_and_org, query_id, self.current_org, self.current_user.id)
require_access(query.groups, self.current_user, not_view_only)

parameter_values = collect_parameters_from_request(request.args)
Expand Down
8 changes: 4 additions & 4 deletions redash/handlers/query_results.py
Expand Up @@ -184,17 +184,17 @@ def get(self, query_id=None, query_result_id=None, filetype='json'):
query_result = None

if query_result_id:
query_result = get_object_or_404(models.QueryResult.get_by_id_and_org, query_result_id, self.current_org)
query_result = get_object_or_404(models.QueryResult.get_by_id_and_org, query_result_id, self.current_org, self.current_user.id)

if query_id is not None:
query = get_object_or_404(models.Query.get_by_id_and_org, query_id, self.current_org)
query = get_object_or_404(models.Query.get_by_id_and_org, query_id, self.current_org, self.current_user.id)

if query_result is None and query is not None:
if settings.ALLOW_PARAMETERS_IN_EMBEDS and parameter_values:
query_result = run_query_sync(query.data_source, parameter_values, query.to_dict()['query'], max_age=max_age)
elif query.latest_query_data_id is not None:
query_result = get_object_or_404(models.QueryResult.get_by_id_and_org, query.latest_query_data_id, self.current_org)
query_result = get_object_or_404(models.QueryResult.get_by_id_and_org, query.latest_query_data_id, self.current_org, self.current_user.id)

if query is not None and query_result is not None and self.current_user.is_api_user():
if query.query_hash != query_result.query_hash:
abort(404, message='No cached result found for this query.')
Expand Down
2 changes: 1 addition & 1 deletion redash/handlers/visualizations.py
Expand Up @@ -14,7 +14,7 @@ class VisualizationListResource(BaseResource):
def post(self):
kwargs = request.get_json(force=True)

query = get_object_or_404(models.Query.get_by_id_and_org, kwargs.pop('query_id'), self.current_org)
query = get_object_or_404(models.Query.get_by_id_and_org, kwargs.pop('query_id'), self.current_org, user_id=self.current_user.id)
require_object_modify_permission(query, self.current_user)

kwargs['options'] = json.dumps(kwargs['options'])
Expand Down