From 28c94fc72b17c0a1a2181030d30141022a9d2d8e Mon Sep 17 00:00:00 2001
From: Jerry Huang <34140255+qwef@users.noreply.github.com>
Date: Thu, 16 Mar 2023 09:38:05 -0700
Subject: [PATCH] Add 404 error when fetching a group that doesn't exist
(#29249)
* add permissions and test
* address comments
* fix whitespace
* remove ehitespace
* revert vscode changes
---
src/metabase/api/permissions.clj | 19 ++++++++++---------
test/metabase/api/permissions_test.clj | 4 ++++
2 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/src/metabase/api/permissions.clj b/src/metabase/api/permissions.clj
index 5fb65ecf3a0..617ec8248c6 100644
--- a/src/metabase/api/permissions.clj
+++ b/src/metabase/api/permissions.clj
@@ -134,9 +134,9 @@
is manager of."
[]
(try
- (validation/check-group-manager)
- (catch clojure.lang.ExceptionInfo _e
- (validation/check-has-application-permission :setting)))
+ (validation/check-group-manager)
+ (catch clojure.lang.ExceptionInfo _e
+ (validation/check-has-application-permission :setting)))
(let [query (when (and (not api/*is-superuser?*)
(premium-features/enable-advanced-permissions?)
api/*is-group-manager?*)
@@ -153,8 +153,9 @@
"Fetch the details for a certain permissions group."
[id]
(validation/check-group-manager id)
- (-> (db/select-one PermissionsGroup :id id)
- (hydrate :members)))
+ (api/check-404
+ (-> (db/select-one PermissionsGroup :id id)
+ (hydrate :members))))
#_{:clj-kondo/ignore [:deprecated-var]}
(api/defendpoint-schema POST "/group"
@@ -163,7 +164,7 @@
{name su/NonBlankString}
(api/check-superuser)
(db/insert! PermissionsGroup
- :name name))
+ :name name))
#_{:clj-kondo/ignore [:deprecated-var]}
(api/defendpoint-schema PUT "/group/:group-id"
@@ -173,7 +174,7 @@
(validation/check-manager-of-group group-id)
(api/check-404 (db/exists? PermissionsGroup :id group-id))
(db/update! PermissionsGroup group-id
- :name name)
+ :name name)
;; return the updated group
(db/select-one PermissionsGroup :id group-id))
@@ -243,8 +244,8 @@
(api/check-404 old)
(validation/check-manager-of-group (:group_id old))
(api/check
- (db/exists? User :id (:user_id old) :is_superuser false)
- [400 (tru "Admin cant be a group manager.")])
+ (db/exists? User :id (:user_id old) :is_superuser false)
+ [400 (tru "Admin cant be a group manager.")])
(db/update! PermissionsGroupMembership (:id old)
:is_group_manager is_group_manager)
(db/select-one PermissionsGroupMembership :id (:id old))))
diff --git a/test/metabase/api/permissions_test.clj b/test/metabase/api/permissions_test.clj
index fdfe0d0f94d..c77dae7da0b 100644
--- a/test/metabase/api/permissions_test.clj
+++ b/test/metabase/api/permissions_test.clj
@@ -100,6 +100,10 @@
(is (= nil
(get id->member :trashbird)))))
+ (testing "returns 404 for nonexistent id"
+ (is (= "Not found."
+ (mt/user-http-request :crowberto :get 404 "permissions/group/10000"))))
+
(testing "requires superuers"
(is (= "You don't have permissions to do that."
(mt/user-http-request :rasta :get 403 (format "permissions/group/%d" (:id (perms-group/all-users)))))))))
--
GitLab