diff --git a/src/metabase/api/search.clj b/src/metabase/api/search.clj index 267233833a9bde219ade022d29531cb6115b45c1..f35694f6ecc672c06942e2596f1616c0b5ea1a53 100644 --- a/src/metabase/api/search.clj +++ b/src/metabase/api/search.clj @@ -28,6 +28,7 @@ :created-at created_at :created-by (set (u/one-or-many created_by)) :current-user-id api/*current-user-id* + :is-superuser? api/*is-superuser?* :current-user-perms @api/*current-user-permissions-set* :filter-items-in-personal-collection filter_items_in_personal_collection :last-edited-at last_edited_at @@ -85,6 +86,7 @@ :created-at created_at :created-by (set created_by) :current-user-id api/*current-user-id* + :is-superuser? api/*is-superuser?* :current-user-perms @api/*current-user-permissions-set* :filter-items-in-personal-collection filter_items_in_personal_collection :last-edited-at last_edited_at diff --git a/src/metabase/models/collection.clj b/src/metabase/models/collection.clj index 94147368773777a5f07196cb6c6fbfa75002b68e..3e703d1ea3d80c4725f71d1a89fd9987c827d5d1 100644 --- a/src/metabase/models/collection.clj +++ b/src/metabase/models/collection.clj @@ -488,6 +488,11 @@ [:permission-level {:optional true} [:enum :read :write]] [:effective-child-of {:optional true} [:maybe CollectionWithLocationAndIDOrRoot]]]) +(def ^:private UserScope + [:map + [:current-user-id pos-int?] + [:is-superuser? :boolean]]) + (def ^:private default-visibility-config {:include-archived-items :exclude :include-trash-collection? false @@ -544,6 +549,12 @@ (visible-collection-filter-clause collection-id-field {})) ([collection-id-field :- [:or [:tuple [:= :coalesce] :keyword :keyword] :keyword] visibility-config :- CollectionVisibilityConfig] + (visible-collection-filter-clause collection-id-field visibility-config + {:current-user-id api/*current-user-id* + :is-superuser? api/*is-superuser?*})) + ([collection-id-field :- [:or [:tuple [:= :coalesce] :keyword :keyword] :keyword] + visibility-config :- CollectionVisibilityConfig + {:keys [current-user-id is-superuser?]} :- UserScope] (let [visibility-config (merge default-visibility-config visibility-config)] ;; This giant query looks scary, but it's actually only moderately terrifying! Let's walk through it step by ;; step. What we're doing here is adding a filter clause to a surrounding query, to make sure that @@ -569,27 +580,26 @@ ;; the `FROM` clause is where we limit the collections to the ones we have permissions on. For a superuser, ;; that's all of them. For regular users, it's a) the collections they have permission in the DB for, and b) ;; their personal collection and its descendants. - :from [[{:union-all (if api/*is-superuser?* - [{:select [:c.*] - :from [[:collection :c]]}] - [{:select [:c.*] - :from [[:collection :c]] - :join [[:permissions :p] - [:= :c.id :p.collection_id] - [:permissions_group :pg] [:= :pg.id :p.group_id] - [:permissions_group_membership :pgm] [:= :pgm.group_id :pg.id]] - :where [:and - [:= :pgm.user_id api/*current-user-id*] - [:= :p.perm_type "perms/collection-access"] - [:or - [:= :p.perm_value "read-and-write"] - (when (= :read (:permission-level visibility-config)) - [:= :p.perm_value "read"])]]} - (when-let [personal-collection-and-descendant-ids (user->personal-collection-and-descendant-ids api/*current-user-id*)] + :from [(if is-superuser? + [:collection :c] + [{:union-all [{:select [:c.*] + :from [[:collection :c]] + :join [[:permissions :p] + [:= :c.id :p.collection_id] + [:permissions_group :pg] [:= :pg.id :p.group_id] + [:permissions_group_membership :pgm] [:= :pgm.group_id :pg.id]] + :where [:and + [:= :pgm.user_id api/*current-user-id*] + [:= :p.perm_type "perms/collection-access"] + [:or + [:= :p.perm_value "read-and-write"] + (when (= :read (:permission-level visibility-config)) + [:= :p.perm_value "read"])]]} + (when-let [personal-collection-and-descendant-ids (user->personal-collection-and-descendant-ids current-user-id)] {:select [:c.*] - :from [[:collection :c]] - :where [:in :id personal-collection-and-descendant-ids]})])} - :c]] + :from [[:collection :c]] + :where [:in :id personal-collection-and-descendant-ids]})]} + :c])] ;; The `WHERE` clause is where we apply the other criteria we were given: :where [:and ;; hiding the trash collection when desired... diff --git a/src/metabase/search/config.clj b/src/metabase/search/config.clj index fa83bee5b956d429944b58ec497a3ef8415c8f9b..609002e59e908b49882828824719cfb3655c5dcd 100644 --- a/src/metabase/search/config.clj +++ b/src/metabase/search/config.clj @@ -106,6 +106,7 @@ ;; [:archived? [:maybe :boolean]] [:current-user-id pos-int?] + [:is-superuser? :boolean] [:current-user-perms [:set perms.u/PathSchema]] [:model-ancestors? :boolean] [:models [:set SearchableModel]] diff --git a/src/metabase/search/impl.clj b/src/metabase/search/impl.clj index 0ee5b1ffcb161279431569c0a6f6150b16c021cd..a67b620b41eb7578f216a3e0ee15cdd136264c3f 100644 --- a/src/metabase/search/impl.clj +++ b/src/metabase/search/impl.clj @@ -95,7 +95,7 @@ (mu/defn- base-query-for-model :- [:map {:closed true} [:select :any] [:from :any] - [:where :any] + [:where {:optional true} :any] [:join {:optional true} :any] [:left-join {:optional true} :any]] "Create a HoneySQL query map with `:select`, `:from`, and `:where` clauses for `model`, suitable for the `UNION ALL` @@ -111,7 +111,9 @@ [honeysql-query :- ms/Map model :- :string {:keys [filter-items-in-personal-collection - archived]} :- SearchContext] + archived + current-user-id + is-superuser?]} :- SearchContext] (let [collection-id-col (if (= model "collection") :collection.id :collection_id) @@ -121,7 +123,9 @@ :include-trash-collection? true :permission-level (if archived :write - :read)})] + :read)} + {:current-user-id current-user-id + :is-superuser? is-superuser?})] (cond-> honeysql-query true (sql.helpers/where collection-filter-clause (perms/audit-namespace-clause :collection.namespace nil)) @@ -238,8 +242,7 @@ [:= :model.id :action.model_id]) (sql.helpers/left-join :query_action [:= :query_action.action_id :action.id]) - (add-collection-join-and-where-clauses model - search-ctx))) + (add-collection-join-and-where-clauses model search-ctx))) (defmethod search-query-for-model "card" [_model search-ctx] @@ -604,6 +607,7 @@ [:search-string [:maybe ms/NonBlankString]] [:models [:maybe [:set SearchableModel]]] [:current-user-id pos-int?] + [:is-superuser? :boolean] [:current-user-perms [:set perms.u/PathSchema]] [:archived {:optional true} [:maybe :boolean]] [:created-at {:optional true} [:maybe ms/NonBlankString]] @@ -625,6 +629,7 @@ created-at created-by current-user-id + is-superuser? current-user-perms last-edited-at last-edited-by @@ -637,7 +642,7 @@ table-db-id search-native-query verified - ids]} :- ::search-context.input] + ids]} :- ::search-context.input] ;; for prod where Malli is disabled {:pre [(pos-int? current-user-id) (set? current-user-perms)]} (when (some? verified) @@ -647,6 +652,7 @@ (let [models (if (string? models) [models] models) ctx (cond-> {:archived? (boolean archived) :current-user-id current-user-id + :is-superuser? is-superuser? :current-user-perms current-user-perms :model-ancestors? (boolean model-ancestors?) :models models diff --git a/test/metabase/search/filter_test.clj b/test/metabase/search/filter_test.clj index 86b8fcd799b61258a176205b848e27a4b85d257d..a1f6ae2de69304556d47a61332d629e244aa7608 100644 --- a/test/metabase/search/filter_test.clj +++ b/test/metabase/search/filter_test.clj @@ -12,6 +12,7 @@ :models search.config/all-models :model-ancestors? false :current-user-id 1 + :is-superuser? true :current-user-perms #{"/"}}) (deftest ^:parallel ->applicable-models-test diff --git a/test/metabase/search/impl_test.clj b/test/metabase/search/impl_test.clj index ec840aca6d8a11298cbf55593f3f57313e00ad25..57f260bd46287ea079ef1089e2106c4d9752fd51 100644 --- a/test/metabase/search/impl_test.clj +++ b/test/metabase/search/impl_test.clj @@ -63,6 +63,7 @@ :archived? false :models search.config/all-models :current-user-id (mt/user->id :crowberto) + :is-superuser? true :current-user-perms #{"/"} :model-ancestors? false :limit-int 100}))] @@ -137,6 +138,7 @@ :models search.config/all-models :created-at created-at :current-user-id (mt/user->id :crowberto) + :is-superuser? true :current-user-perms @api/*current-user-permissions-set*})) :data (map (juxt :model :id)) @@ -222,6 +224,7 @@ :models search.config/all-models :last-edited-at last-edited-at :current-user-id (mt/user->id :crowberto) + :is-superuser? true :current-user-perms @api/*current-user-permissions-set*})) :data (map (juxt :model :id))