diff --git a/.clj-kondo/config.edn b/.clj-kondo/config.edn index f7d6c23554c4096651f6811b7c4f4ba92cd5a745..b13db5456624f95266f4f344954ef36947cfeda9 100644 --- a/.clj-kondo/config.edn +++ b/.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 diff --git a/dev/src/dev/model_tracking.clj b/dev/src/dev/model_tracking.clj index d15065cfc583a5f5c707a9517a8d2e71030374b9..b5ddbc1d11eff3d41cbfc5ed1097054b59f11820 100644 --- a/dev/src/dev/model_tracking.clj +++ b/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* diff --git a/src/metabase/api/permissions.clj b/src/metabase/api/permissions.clj index 20019ada73d0dad1642b0afce67856baef6af2bc..892a3bb4bae124e1f49e11f2c64197faf9bdd3e6 100644 --- a/src/metabase/api/permissions.clj +++ b/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)." [] diff --git a/src/metabase/models/permissions.clj b/src/metabase/models/permissions.clj index 6715bf7abed0825ca7c5fe3cb1c6802b13adf4e7..2ab20cf4ec527c67c39d0cf5c8465ebef68da15a 100644 --- a/src/metabase/models/permissions.clj +++ b/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 diff --git a/test/metabase/api/permissions_test.clj b/test/metabase/api/permissions_test.clj index 11629a9ef3fe5bdf85d863ff69eebee34d7c3447..2d622234199ce2ce9d6615cacf99e837a6fb185c 100644 --- a/test/metabase/api/permissions_test.clj +++ b/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" diff --git a/test/metabase/api/permissions_test_util.clj b/test/metabase/api/permissions_test_util.clj new file mode 100644 index 0000000000000000000000000000000000000000..1ef192ed6ea0282494e06fec3015527b34aa279e --- /dev/null +++ b/test/metabase/api/permissions_test_util.clj @@ -0,0 +1,15 @@ +(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)) diff --git a/test/metabase/models/permissions_group_test.clj b/test/metabase/models/permissions_group_test.clj index c82445773058ab77e19a3cde40ef2001b1745f36..df4d9acf3b3e96cc6fcb5bfbf4f2c3a49537cc0b 100644 --- a/test/metabase/models/permissions_group_test.clj +++ b/test/metabase/models/permissions_group_test.clj @@ -1,6 +1,7 @@ (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)))))))))