Skip to content

Refine permissions usage in Redash to allow for guest users #4492

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

Merged
merged 3 commits into from
Dec 30, 2019
Merged

Refine permissions usage in Redash to allow for guest users #4492

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
955c047
Allow executing query with either view_query or execute_query permiss…
arikfr Dec 25, 2019
968a392
Render AuthHeader according to permissions.
arikfr Dec 25, 2019
46dc775
Don't return dashboards where you only have access to textbox widget.
arikfr Dec 25, 2019
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
58 changes: 32 additions & 26 deletions client/app/components/app-header/AppHeader.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,29 +51,33 @@ function DesktopNavbar() {
</Menu.Item>
)}
</Menu>
<Dropdown
trigger={["click"]}
overlay={
<Menu>
{currentUser.hasPermission("create_query") && (
<Menu.Item key="new-query">
<a href="queries/new">New Query</a>
</Menu.Item>
)}
{currentUser.hasPermission("create_dashboard") && (
<Menu.Item key="new-dashboard">
<a onMouseUp={() => CreateDashboardDialog.showModal()}>New Dashboard</a>
</Menu.Item>
)}
<Menu.Item key="new-alert">
<a href="alerts/new">New Alert</a>
</Menu.Item>
</Menu>
}>
<Button type="primary" data-test="CreateButton">
Create <Icon type="down" />
</Button>
</Dropdown>
{currentUser.canCreate() && (
<Dropdown
trigger={["click"]}
overlay={
<Menu>
{currentUser.hasPermission("create_query") && (
<Menu.Item key="new-query">
<a href="queries/new">New Query</a>
</Menu.Item>
)}
{currentUser.hasPermission("create_dashboard") && (
<Menu.Item key="new-dashboard">
<a onMouseUp={() => CreateDashboardDialog.showModal()}>New Dashboard</a>
</Menu.Item>
)}
{currentUser.hasPermission("list_alerts") && (
<Menu.Item key="new-alert">
<a href="alerts/new">New Alert</a>
</Menu.Item>
)}
</Menu>
}>
<Button type="primary" data-test="CreateButton">
Create <Icon type="down" />
</Button>
</Dropdown>
)}
</div>
<div className="header-logo">
<a href="./">
Expand Down Expand Up @@ -126,9 +130,11 @@ function DesktopNavbar() {
<a href="users">Users</a>
</Menu.Item>
)}
<Menu.Item key="snippets">
<a href="query_snippets">Query Snippets</a>
</Menu.Item>
{currentUser.hasPermission("create_query") && (
<Menu.Item key="snippets">
<a href="query_snippets">Query Snippets</a>
</Menu.Item>
)}
{currentUser.hasPermission("list_users") && (
<Menu.Item key="destinations">
<a href="destinations">Alert Destinations</a>
Expand Down
6 changes: 6 additions & 0 deletions client/app/services/auth.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ export const currentUser = {
return this.hasPermission("admin") || (userId && userId === this.id);
},

canCreate() {
return (
this.hasPermission("create_query") || this.hasPermission("create_dashboard") || this.hasPermission("list_alerts")
);
},

hasPermission(permission) {
return includes(this.permissions, permission);
},
Expand Down
7 changes: 4 additions & 3 deletions redash/handlers/query_results.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
not_view_only,
require_access,
require_permission,
require_any_of_permission,
view_only,
)
from redash.tasks import QueryTask
Expand Down Expand Up @@ -220,7 +221,7 @@ def add_cors_headers(headers):
settings.ACCESS_CONTROL_ALLOW_CREDENTIALS
).lower()

@require_permission("view_query")
@require_any_of_permission(("view_query", "execute_query"))
def options(self, query_id=None, query_result_id=None, filetype="json"):
headers = {}
self.add_cors_headers(headers)
Expand All @@ -237,7 +238,7 @@ def options(self, query_id=None, query_result_id=None, filetype="json"):

return make_response("", 200, headers)

@require_permission("view_query")
@require_any_of_permission(("view_query", "execute_query"))
def post(self, query_id):
"""
Execute a saved query.
Expand Down Expand Up @@ -283,7 +284,7 @@ def post(self, query_id):
else:
return error_messages["no_permission"]

@require_permission("view_query")
@require_any_of_permission(("view_query", "execute_query"))
def get(self, query_id=None, query_result_id=None, filetype="json"):
"""
Retrieve query results.
Expand Down
1 change: 0 additions & 1 deletion redash/models/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1072,7 +1072,6 @@ def all(cls, org, group_ids, user_id):
(
DataSourceGroup.group_id.in_(group_ids)
| (Dashboard.user_id == user_id)
| ((Widget.dashboard != None) & (Widget.visualization == None))
),
Dashboard.org == org,
)
Expand Down
12 changes: 10 additions & 2 deletions redash/permissions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,17 @@ def require_access(obj, user, need_view_only):


class require_permissions(object):
def __init__(self, permissions):
def __init__(self, permissions, allow_one=False):
self.permissions = permissions
self.allow_one = allow_one

def __call__(self, fn):
@functools.wraps(fn)
def decorated(*args, **kwargs):
has_permissions = current_user.has_permissions(self.permissions)
if self.allow_one:
has_permissions = any([current_user.has_permission(permission) for permission in self.permissions])
else:
has_permissions = current_user.has_permissions(self.permissions)

if has_permissions:
return fn(*args, **kwargs)
Expand All @@ -75,6 +79,10 @@ def require_permission(permission):
return require_permissions((permission,))


def require_any_of_permission(permissions):
return require_permissions(permissions, True)


def require_admin(fn):
return require_permission("admin")(fn)

Expand Down
28 changes: 21 additions & 7 deletions tests/test_models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -736,24 +736,38 @@ def test_returns_dashboards_created_by_user(self):
d1, list(models.Dashboard.all(self.u2.org, self.u2.group_ids, self.u2.id))
)

def test_returns_dashboards_with_text_widgets(self):
def test_returns_dashboards_with_text_widgets_to_creator(self):
w1 = self.factory.create_widget(visualization=None)

self.assertEqual(w1.dashboard.user, self.factory.user)
self.assertIn(
w1.dashboard, models.Dashboard.all(self.u1.org, self.u1.group_ids, None)
w1.dashboard,
list(
models.Dashboard.all(
self.factory.user.org,
self.factory.user.group_ids,
self.factory.user.id,
)
),
)
self.assertIn(
w1.dashboard, models.Dashboard.all(self.u2.org, self.u2.group_ids, None)
self.assertNotIn(
w1.dashboard,
list(models.Dashboard.all(self.u1.org, self.u1.group_ids, self.u1.id)),
)

def test_returns_dashboards_from_current_org_only(self):
w1 = self.factory.create_widget(visualization=None)
w1 = self.factory.create_widget()

user = self.factory.create_user(org=self.factory.create_org())

self.assertIn(
w1.dashboard, models.Dashboard.all(self.u1.org, self.u1.group_ids, None)
w1.dashboard,
list(
models.Dashboard.all(
self.factory.user.org, self.factory.user.group_ids, None
)
),
)
self.assertNotIn(
w1.dashboard, models.Dashboard.all(user.org, user.group_ids, None)
w1.dashboard, list(models.Dashboard.all(user.org, user.group_ids, user.id))
)