Implement global app permissions for the "All Users" group (#25679)

src/metabase/api/app.clj

@@ -8,6 +8,7 @@
     [metabase.api.collection :as api.collection]
     [metabase.api.common :as api]
     [metabase.models :refer [App Collection Dashboard Table]]
+    [metabase.models.app.graph :as app.graph]
     [metabase.models.collection :as collection]
     [metabase.models.dashboard :as dashboard]
     [metabase.util.i18n :as i18n]
@@ -27,6 +28,7 @@
                         (select-keys [:dashboard_id :options :nav_items])
                         (assoc :collection_id (:id collection-instance)))
          app (db/insert! App app-params)]
+     (app.graph/set-default-permissions! app)
      (hydrate-details app))))
 
 (api/defendpoint POST "/"
@@ -268,4 +270,37 @@
       (create-scaffold-dashcards! scaffold-target->id pages)
       (hydrate-details (db/select-one App :id app-id)))))
 
+
+;;; ------------------------------------------------ GRAPH ENDPOINTS -------------------------------------------------
+
+(api/defendpoint GET "/global-graph"
+  "Fetch the global graph of all App Permissions."
+  []
+  (api/check-superuser)
+  (app.graph/global-graph))
+
+(defn- ->int [id] (Integer/parseInt (name id)))
+
+(defn- dejsonify-id->permission-map [m]
+  (into {}
+        (map (fn [[k v]]
+               [(->int k) (keyword v)]))
+        m))
+
+(defn- dejsonify-global-graph
+  "Fix the types in the graph when it comes in from the API, e.g. converting things like `\"none\"` to `:none` and
+  parsing object keys as integers."
+  [graph]
+  (update graph :groups dejsonify-id->permission-map))
+
+(api/defendpoint PUT "/global-graph"
+  "Do a batch update of the global App Permissions by passing in a modified graph."
+  [:as {body :body}]
+  {body su/Map}
+  (api/check-superuser)
+  (-> body
+      dejsonify-global-graph
+      app.graph/update-global-graph!)
+  (app.graph/global-graph))
+
 (api/define-routes)

src/metabase/models/app/graph.clj

+(ns metabase.models.app.graph
+  "Code for generating and updating the App permissions graph. App permissions
+  are based on the permissions of the app's collection."
+  (:require [clojure.data :as data]
+            [metabase.models.collection-permission-graph-revision :as c-perm-revision
+             :refer [CollectionPermissionGraphRevision]]
+            [metabase.models.collection.graph :as graph]
+            [metabase.models.permissions :as perms]
+            [metabase.models.permissions-group :as perms-group]
+            [metabase.models.setting :as setting :refer [defsetting]]
+            [metabase.public-settings.premium-features :as premium-features]
+            [metabase.util.i18n :as i18n :refer [tru]]
+            [metabase.util.schema :as su]
+            [schema.core :as s]
+            [toucan.db :as db]))
+
+(def ^:private AppPermissions
+  graph/CollectionPermissions)
+
+(def ^:private GlobalPermissionsGraph
+  {:revision s/Int
+   :groups   {su/IntGreaterThanZero AppPermissions}})
+
+(s/defn ^:private set-all-users-app-permission!
+  [permission :- AppPermissions]
+  (setting/set-value-of-type! :keyword :all-users-app-permission permission))
+
+(defsetting all-users-app-permission
+  "App permission of the All Users group"
+  :type :keyword
+  :visibility :internal
+  :default :none
+  :setter (fn [v]
+            (set-all-users-app-permission! (cond-> v (string? v) keyword))))
+
+(defn set-default-permissions!
+  "Sets the default permissions for the ''All Users'' group on`app` as specified
+  by `all-users-app-permission` if advanced permissions are not available."
+  [app]
+  (when-not (premium-features/has-feature? :advanced-permissions)
+    (graph/update-collection-permissions! (:id (perms-group/all-users))
+                                          (:collection_id app)
+                                          (all-users-app-permission))))
+
+(s/defn global-graph :- GlobalPermissionsGraph
+  "Fetch the global app permission graph."
+  []
+  {:revision (c-perm-revision/latest-id)
+   :groups   {(:id (perms-group/admin)) :write
+              (:id (perms-group/all-users)) (all-users-app-permission)}})
+
+(s/defn update-global-graph!
+  "Update the global app permission graph to the state specified by
+  `new-graph`."
+  [new-graph :- GlobalPermissionsGraph]
+  (let [old-graph (global-graph)
+        [diff-old changes] (data/diff (:groups old-graph) (:groups new-graph))]
+    (perms/log-permissions-changes diff-old changes)
+    (perms/check-revision-numbers old-graph new-graph)
+    (when-let [[[group-id permission] & other-groups] (seq changes)]
+      (when (or (not= group-id (:id (perms-group/all-users)))
+                (seq other-groups))
+        (throw (ex-info (tru "You can configure for the ''All Users'' group only")
+                        {:group-ids (keys changes)
+                         :status-code 400})))
+      (db/transaction
+        (when (not= permission (all-users-app-permission))
+          (all-users-app-permission! permission)
+          (doseq [collection-id (db/select-field :collection_id 'App)]
+            (graph/update-collection-permissions! group-id collection-id permission))
+          (perms/save-perms-revision!
+           CollectionPermissionGraphRevision (:revision old-graph) old-graph changes))))))

src/metabase/models/collection/graph.clj

@@ -20,7 +20,8 @@
 
 ;;; ---------------------------------------------------- Schemas -----------------------------------------------------
 
-(def ^:private CollectionPermissions
+(def CollectionPermissions
+  "The valid collection permissions."
   (s/enum :write :read :none))
 
 (def ^:private GroupPermissionsGraph
@@ -106,20 +107,24 @@
                       {:status-code 400
                        :app-ids app-ids})))))
 
-(s/defn ^:private update-collection-permissions!
-  [collection-namespace :- (s/maybe su/KeywordOrString)
-   group-id             :- su/IntGreaterThanZero
-   collection-id        :- (s/cond-pre (s/eq :root) su/IntGreaterThanZero)
-   new-collection-perms :- CollectionPermissions]
-  (let [collection-id (if (= collection-id :root)
-                        (assoc collection/root-collection :namespace collection-namespace)
-                        collection-id)]
-    ;; remove whatever entry is already there (if any) and add a new entry if applicable
-    (perms/revoke-collection-permissions! group-id collection-id)
-    (case new-collection-perms
-      :write (perms/grant-collection-readwrite-permissions! group-id collection-id)
-      :read  (perms/grant-collection-read-permissions! group-id collection-id)
-      :none  nil)))
+(s/defn update-collection-permissions!
+  "Update the permissions for group ID with `group-id` on collection with ID
+  `collection-id` in the optional `collection-namespace` to `new-collection-perms`."
+  ([group-id collection-id new-collection-perms]
+   (update-collection-permissions! nil group-id collection-id new-collection-perms))
+  ([collection-namespace :- (s/maybe su/KeywordOrString)
+    group-id             :- su/IntGreaterThanZero
+    collection-id        :- (s/cond-pre (s/eq :root) su/IntGreaterThanZero)
+    new-collection-perms :- CollectionPermissions]
+   (let [collection-id (if (= collection-id :root)
+                         (assoc collection/root-collection :namespace collection-namespace)
+                         collection-id)]
+     ;; remove whatever entry is already there (if any) and add a new entry if applicable
+     (perms/revoke-collection-permissions! group-id collection-id)
+     (case new-collection-perms
+       :write (perms/grant-collection-readwrite-permissions! group-id collection-id)
+       :read  (perms/grant-collection-read-permissions! group-id collection-id)
+       :none  nil))))
 
 (s/defn ^:private update-group-permissions!
   [collection-namespace :- (s/maybe su/KeywordOrString)

test/metabase/api/app_test.clj

@@ -2,7 +2,8 @@
   (:require
     [clojure.test :refer [deftest is testing]]
     [medley.core :as m]
-    [metabase.models :refer [App Card Collection Dashboard]]
+    [metabase.models :refer [App Card Collection Dashboard Permissions]]
+    [metabase.models.collection.graph :as graph]
     [metabase.models.permissions :as perms]
     [metabase.models.permissions-group :as perms-group]
     [metabase.test :as mt]
@@ -11,24 +12,32 @@
     [toucan.hydrate :refer [hydrate]]))
 
 (deftest create-test
-  (mt/with-model-cleanup [Collection]
+  (mt/with-model-cleanup [Collection Permissions]
     (let [base-params {:name "App collection"
                        :color "#123456"}]
       (mt/test-drivers (mt/normal-drivers-with-feature :actions/custom)
         (testing "parent_id is ignored when creating apps"
-          (mt/with-temp* [Collection [{collection-id :id}]]
-            (let [coll-params (assoc base-params :parent_id collection-id)
-                  response (mt/user-http-request :crowberto :post 200 "app" {:collection coll-params})]
-              (is (pos-int? (:id response)))
-              (is (pos-int? (:collection_id response)))
-              (is (partial= (assoc base-params :location "/")
-                            (:collection response))))))
+          (mt/with-temporary-setting-values [all-users-app-permission :none]
+            (mt/with-temp* [Collection [{collection-id :id}]]
+             (let [coll-params (assoc base-params :parent_id collection-id)
+                   response (mt/user-http-request :crowberto :post 200 "app" {:collection coll-params})]
+               (is (pos-int? (:id response)))
+               (is (pos-int? (:collection_id response)))
+               (is (partial= (assoc base-params :location "/")
+                             (:collection response)))
+               (is (partial= {:groups {(:id (perms-group/all-users)) {(:collection_id response) :none}}}
+                             (graph/graph))
+                   "''All Users'' should have the default permission on the app collection")))))
         (testing "Create app in the root"
-          (let [response (mt/user-http-request :crowberto :post 200 "app" {:collection base-params})]
-            (is (pos-int? (:id response)))
-            (is (pos-int? (:collection_id response)))
-            (is (partial= (assoc base-params :location "/")
-                          (:collection response)))))
+          (mt/with-temporary-setting-values [all-users-app-permission :read]
+            (let [response (mt/user-http-request :crowberto :post 200 "app" {:collection base-params})]
+             (is (pos-int? (:id response)))
+             (is (pos-int? (:collection_id response)))
+             (is (partial= (assoc base-params :location "/")
+                           (:collection response)))
+             (is (partial= {:groups {(:id (perms-group/all-users)) {(:collection_id response) :read}}}
+                           (graph/graph))
+                 "''All Users'' should have the default permission on the app collection"))))
         (testing "With initial dashboard and nav_items"
           (mt/with-temp Dashboard [{dashboard-id :id}]
             (let [nav_items [{:options {:click_behavior {}}}]]
@@ -137,25 +146,29 @@
                  (mt/user-http-request :rasta :get 403 (str "app/" app-id)))))))))
 
 (deftest scaffold-test
-  (mt/with-model-cleanup [Card Dashboard Collection]
+  (mt/with-model-cleanup [Card Dashboard Collection Permissions]
     (testing "Golden path"
-      (let [app (mt/user-http-request
-                  :crowberto :post 200 "app/scaffold"
-                  {:table-ids [(data/id :venues)]
-                   :app-name "My test app"})
-            pages (m/index-by :name (hydrate (db/select Dashboard :collection_id (:collection_id app)) :ordered_cards))
-            list-page (get pages "Venues List")
-            detail-page (get pages "Venues Detail")]
-        (is (partial= {:nav_items [{:page_id (:id list-page)}
-                                   {:page_id (:id detail-page) :hidden true :indent 1}]
-                       :dashboard_id (:id list-page)}
-                      app))
-        (is (partial= {:ordered_cards [{:visualization_settings {:click_behavior
-                                                                 {:type "link",
-                                                                  :linkType "page",
-                                                                  :targetId (:id detail-page)}}}
-                                       {}]}
-                      list-page))))
+      (mt/with-temporary-setting-values [all-users-app-permission :read]
+        (let [app (mt/user-http-request
+                   :crowberto :post 200 "app/scaffold"
+                   {:table-ids [(data/id :venues)]
+                    :app-name "My test app"})
+              pages (m/index-by :name (hydrate (db/select Dashboard :collection_id (:collection_id app)) :ordered_cards))
+              list-page (get pages "Venues List")
+              detail-page (get pages "Venues Detail")]
+          (is (partial= {:nav_items [{:page_id (:id list-page)}
+                                     {:page_id (:id detail-page) :hidden true :indent 1}]
+                         :dashboard_id (:id list-page)}
+                        app))
+          (is (partial= {:ordered_cards [{:visualization_settings {:click_behavior
+                                                                   {:type "link",
+                                                                    :linkType "page",
+                                                                    :targetId (:id detail-page)}}}
+                                         {}]}
+                        list-page))
+          (is (partial= {:groups {(:id (perms-group/all-users)) {(:collection_id app) :read}}}
+                        (graph/graph))
+              "''All Users'' should have the default permission on the app collection"))))
     (testing "Bad or duplicate tables"
       (is (= (format "Some tables could not be found. Given: (%s %s) Found: (%s)"
                      (data/id :venues)
@@ -167,7 +180,7 @@
                 :app-name (str "My test app " (gensym))}))))))
 
 (deftest scaffold-app-test
-  (mt/with-model-cleanup [Card Dashboard Collection]
+  (mt/with-model-cleanup [Card Dashboard]
     (mt/with-temp* [Collection [{collection-id :id}]
                     App [{app-id :id} {:collection_id collection-id}]]
       (testing "Without existing pages"
@@ -204,3 +217,24 @@
                                                                     :targetId (:id detail-page)}}}
                                          {}]}
                         list-page)))))))
+
+(deftest global-graph-test
+  (mt/with-model-cleanup [Collection Permissions]
+    (let [base-params {:name "App collection"
+                       :color "#123456"}]
+      (mt/test-drivers (mt/normal-drivers-with-feature :actions/custom)
+        (testing "changing default permission"
+          (mt/with-temp* [Collection [{collection-id :id}]]
+            (let [coll-params (assoc base-params :parent_id collection-id)
+                  response1 (mt/user-http-request :crowberto :post 200 "app" {:collection coll-params})
+                  response2 (mt/user-http-request :crowberto :post 200 "app" {:collection base-params})]
+              (is (partial= {:groups {(:id (perms-group/all-users)) {(:collection_id response1) :none
+                                                                     (:collection_id response2) :none}}}
+                            (graph/graph)))
+              (mt/user-http-request :crowberto :put 200 "app/global-graph"
+                                    (assoc-in (mt/user-http-request :crowberto :get 200 "app/global-graph")
+                                              [:groups (:id (perms-group/all-users))]
+                                              :write))
+              (is (partial= {:groups {(:id (perms-group/all-users)) {(:collection_id response1) :write
+                                                                     (:collection_id response2) :write}}}
+                            (graph/graph))))))))))