adding perm-graph filtering on db or group id (#36543)

* a more refined first crack at adding perm-graph access for db or group id

* new routes: filter data-perm-graph on db or group

add tests

* remove inline defs

* code review responses

* fix typo

* fix the other tests

* update the tests to use the right malli schema

* reuse private vars in tests

* pull graph checker into test util ns and apply it

.clj-kondo/config.edn

@@ -228,6 +228,7 @@
     metabase.api.notify                                             api.notify
     metabase.api.permission-graph                                   api.permission-graph
     metabase.api.permissions                                        api.permissions
+    metabase.api.permissions-test-util                              perm-test-util
     metabase.api.pivots                                             api.pivots
     metabase.api.premium-features                                   api.premium-features
     metabase.api.preview-embed                                      api.preview-embed

dev/src/dev/model_tracking.clj

@@ -52,7 +52,7 @@
 (defn- new-change
   "Add a change to the [[changes]] atom.
 
-    > (new-change :models/Card :insert {:name \"new card\"})
+    > (new-change :model/Card :insert {:name \"new card\"})
     instance
 
     > @changes*

src/metabase/api/permissions.clj

@@ -38,6 +38,20 @@
   (api/check-superuser)
   (perms/data-perms-graph))
 
+(api/defendpoint GET "/graph/db/:db-id"
+  "Fetch a graph of all v1 Permissions for db-id `db-id` (excludes v2 query and data permissions)."
+  [db-id]
+  {db-id ms/PositiveInt}
+  (api/check-superuser)
+  (perms/data-graph-for-db db-id))
+
+(api/defendpoint GET "/graph/group/:group-id"
+  "Fetch a graph of all v1 Permissions for group-id `group-id` (excludes v2 query and data permissions)."
+  [group-id]
+  {group-id ms/PositiveInt}
+  (api/check-superuser)
+  (perms/data-graph-for-group group-id))
+
 (api/defendpoint GET "/graph-v2"
   "Fetch a graph of all v2 Permissions (excludes v1 data permissions)."
   []

src/metabase/models/permissions.clj

@@ -861,6 +861,13 @@
        post-process-graph
        add-impersonations-to-permissions-graph))
 
+(defn ->v1-paths
+  "keep v1 paths, implicitly remove v2"
+  [group-id->permissions]
+  (m/map-vals (fn [paths]
+                (filter (fn [path] (mc/validate [:re path-regex-v1] path)) paths))
+              group-id->permissions))
+
 (defn data-perms-graph
   "Fetch a graph representing the current *data* permissions status for every Group and all permissioned databases.
   See [[metabase.models.collection.graph]] for the Collection permissions graph code. Keeps v1 paths, hence implictly removes v2 paths.
@@ -880,14 +887,25 @@
         group-id->v1-paths (->> (permissions-by-group-ids [:or
                                                            [:= :object (h2x/literal "/")]
                                                            [:like :object (h2x/literal "%/db/%")]])
-                                ;;  keep v1 paths, implicitly remove v2
-                                (m/map-vals (fn [paths]
-                                              (filter (fn [path]
-                                                        (mc/validate [:re path-regex-v1] path))
-                                                      paths))))]
+                                ->v1-paths)]
     {:revision (perms-revision/latest-id)
      :groups   (generate-graph @db-ids group-id->v1-paths)}))
 
+(defn data-graph-for-db
+  "Efficiently returns a data permissions graph, which has all the permissions info for `db-id`."
+  [db-id]
+  (let [group-id->permissions (permissions-by-group-ids [:like :object (h2x/literal (str "%/db/" db-id "/%"))])
+        group-id->v1-paths (->v1-paths group-id->permissions)]
+    (generate-graph [db-id] group-id->v1-paths)))
+
+(defn data-graph-for-group
+  "Efficiently returns a data permissions graph, which has all the permissions info for the permission group at `group-id`."
+  [group-id]
+  (let [db-ids (t2/select-pks-set :model/Database)
+        group-id->permissions (permissions-by-group-ids [:= :group_id group-id])
+        group-id->paths (select-keys (->v1-paths group-id->permissions) [group-id])]
+    (generate-graph db-ids group-id->paths)))
+
 (defn data-perms-graph-v2
   "Fetch a graph representing the current *data* permissions status for every Group and all permissioned databases.
   See [[metabase.models.collection.graph]] for the Collection permissions graph code. This version of data-perms-graph

test/metabase/api/permissions_test.clj

@@ -4,6 +4,7 @@
    [clojure.test :refer :all]
    [medley.core :as m]
    [metabase.api.permissions :as api.permissions]
+   [metabase.api.permissions-test-util :as perm-test-util]
    [metabase.config :as config]
    [metabase.models
     :refer [Database
@@ -156,6 +157,26 @@
     (testing "make sure a non-admin cannot fetch the perms graph from the API"
       (mt/user-http-request :rasta :get 403 "permissions/graph"))))
 
+(deftest fetch-perms-graph-by-group-id-test
+  (testing "GET /api/permissions/graph"
+    (testing "make sure we can fetch the perms graph from the API"
+      (t2.with-temp/with-temp [PermissionsGroup {group-id :id :as group}    {}
+                               Database         db                          {}]
+        (perms/grant-permissions! group (perms/data-perms-path db))
+        (let [graph (mt/user-http-request :crowberto :get 200 (format "permissions/graph/group/%s" group-id))]
+          (is (perm-test-util/validate-graph-api-output graph))
+          (is (= #{group-id} (set (keys graph)))))))))
+
+(deftest fetch-perms-graph-by-db-id-test
+  (testing "GET /api/permissions/graph"
+    (testing "make sure we can fetch the perms graph from the API"
+      (t2.with-temp/with-temp [PermissionsGroup group       {}
+                               Database         {db-id :id} {}]
+        (perms/grant-permissions! group (perms/data-perms-path db-id))
+        (let [graph (mt/user-http-request :crowberto :get 200 (format "permissions/graph/db/%s" db-id))]
+          (is (perm-test-util/validate-graph-api-output graph))
+          (is (= #{db-id} (->> graph vals (mapcat keys) set))))))))
+
 (deftest fetch-perms-graph-v2-test
   (testing "GET /api/permissions/graph-v2"
     (testing "make sure we can fetch the perms graph from the API"

test/metabase/api/permissions_test_util.clj

+(ns metabase.api.permissions-test-util
+  (:require [malli.core :as mc]
+            [malli.transform :as mtx]
+            [metabase.api.permission-graph :as api.permission-graph]))
+
+(def ^:private graph-output-schema
+  [:map-of @#'api.permission-graph/GroupId @#'api.permission-graph/DbGraph])
+
+(defn- decode-and-validate [schema value]
+  (mc/validate schema (mc/decode schema value (mtx/string-transformer))))
+
+(defn validate-graph-api-output
+  "Handles string->keyword transofmrations in DataPerms"
+  [graph]
+  (decode-and-validate graph-output-schema graph))

test/metabase/models/permissions_group_test.clj

 (ns metabase.models.permissions-group-test
   (:require
    [clojure.test :refer :all]
+   [metabase.api.permissions-test-util :as perm-test-util]
    [metabase.models.database :refer [Database]]
    [metabase.models.interface :as mi]
    [metabase.models.permissions :as perms :refer [Permissions]]
@@ -105,3 +106,24 @@
       (t2.with-temp/with-temp [User {user-id :id} {:is_superuser true}]
         (t2/update! User user-id {:is_superuser false})
         (is (false? (t2/exists? PermissionsGroupMembership, :user_id user-id, :group_id (u/the-id (perms-group/admin)))))))))
+
+(deftest data-graph-for-group-check-all-groups-test
+  (doseq [group-id (t2/select-fn-set :id :model/PermissionsGroup)]
+    (testing (str "testing data-graph-for-group with group-id: [" group-id "].")
+      (let [graph (perms/data-graph-for-group group-id)]
+        (is (perm-test-util/validate-graph-api-output graph))
+        (is (= #{group-id} (set (keys graph))))))))
+
+(defn- perm-object->db [perm-obj]
+  (some-> (re-find #"/db/(\d+)/" perm-obj) second parse-long))
+
+(deftest data-graph-for-db-check-all-dbs-test
+  (let [perm-objects (t2/select-fn-set :object :model/Permissions)
+        dbs-in-perms (set (keep perm-object->db perm-objects))]
+    (doseq [db-id (t2/select-fn-set :id :model/Database)]
+      (testing (str "testing data-graph-for-db with db-id: [" db-id "].")
+        (let [graph (perms/data-graph-for-db db-id)]
+          (is (perm-test-util/validate-graph-api-output graph))
+          ;; Only check this for dbs with permissions
+          (when (contains? dbs-in-perms db-id)
+            (is (= #{db-id} (->> graph vals (mapcat keys) set)))))))))