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

* Allow executing query with either view_query or execute_query permissions. * Render AuthHeader according to permissions. * Don't return dashboards where you only have access to textbox widget. Closes #4099.
getredash · Dec 30, 2019 · 30bc1e2 · 30bc1e2
1 parent fd46194
commit 30bc1e2
Show file tree

Hide file tree

Showing 6 changed files with 73 additions and 39 deletions.
diff --git a/client/app/components/app-header/AppHeader.jsx b/client/app/components/app-header/AppHeader.jsx
@@ -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="./">
@@ -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>

diff --git a/client/app/services/auth.js b/client/app/services/auth.js
@@ -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);
   },

diff --git a/redash/handlers/query_results.py b/redash/handlers/query_results.py
@@ -11,6 +11,7 @@
     not_view_only,
     require_access,
     require_permission,
+    require_any_of_permission,
     view_only,
 )
 from redash.tasks import QueryTask
@@ -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)
@@ -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.
@@ -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.

diff --git a/redash/models/__init__.py b/redash/models/__init__.py
@@ -1073,7 +1073,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,
             )

diff --git a/redash/permissions.py b/redash/permissions.py
@@ -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)
@@ -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)
 

diff --git a/tests/test_models.py b/tests/test_models.py
@@ -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))
         )