Skip to content
Snippets Groups Projects
Unverified Commit 00ff56e8 authored by bryan's avatar bryan Committed by GitHub
Browse files

Ensure blocked tables downgrade into blocked dbs (#47093) 

* tests ensure blocked tables downgrade > blocked db

* implement the blocked table -> blocked db rollback

- move/add tests for it
parent 8bed390b
Branches
Tags
No related merge requests found
Hide whitespace changes
Inline Side-by-side
resources/migrations/permissions/rollback/view_data.sql
+ 6
− 0
View file @ 00ff56e8
......@@ -20,6 +20,12 @@ WHERE EXISTS
AND dp.perm_type = 'perms/create-queries'
AND dp.perm_value = 'no'
AND s.table_id IS NOT NULL )
OR EXISTS (SELECT 1
FROM data_permissions dp_block
WHERE dp_block.group_id = pg.id
AND dp_block.db_id = mt.db_id
AND dp_block.perm_type = 'perms/view-data'
AND dp_block.perm_value = 'blocked')
AND pg.name != 'Administrators'
AND NOT EXISTS
(SELECT 1
......
test/metabase/db/schema_migrations_test.clj
+ 57
− 3
View file @ 00ff56e8
......@@ -1364,6 +1364,7 @@
:schema_name schema
:perm_type perm-type
:perm_value perm-value))]
(migrate-up!)
(insert-perm! "perms/data-access" "unrestricted")
(migrate! :down 49)
......@@ -1741,6 +1742,7 @@
(t2/select-one-fn :perm_value (t2/table-name :model/DataPermissions)
:db_id db-id :table_id table-id-2 :group_id group-id :perm_type "perms/create-queries"))))))))
#_{:clj-kondo/ignore [:metabase/i-like-making-cams-eyes-bleed-with-horrifically-long-tests]}
(deftest split-data-permissions-legacy-no-self-service-migration-test
(testing "view-data is set to `legacy-no-self-service` for groups that meet specific conditions"
......@@ -2046,13 +2048,65 @@
(t2/select-fn-set :object (t2/table-name :model/Permissions) :group_id group-id)))
(migrate-up!)
(t2/insert-returning-pks! :connection_impersonations {:group_id group-id :db_id db-id :attribute "foo"})
(insert-perm! "perms/view-data" "unrestricted")
(insert-perm! "perms/create-queries" "no")
(insert-perm! "perms/view-data" "blocked" table-id)
(migrate! :down 49)
(is (= #{(format "/block/db/%d/" db-id)}
(t2/select-fn-set :object (t2/table-name :model/Permissions) :group_id group-id)))))))
(deftest dbs-with-a-single-blocked-table-downgrade-to-blocked-dbs
(impl/test-migrations ["v50.2024-01-04T13:52:51" "v50.2024-02-26T22:15:55"] [migrate!]
(let [user-id (t2/insert-returning-pk! (t2/table-name :model/User)
{:first_name "Howard"
:last_name "Hughes"
:email "howard@aircraft.com"
:password "superstrong"
:date_joined :%now})
db-id (t2/insert-returning-pk! (t2/table-name :model/Database)
{:name "DB"
:engine "h2"
:created_at :%now
:updated_at :%now
:details "{}"})
table-id (t2/insert-returning-pk! (t2/table-name :model/Table)
{:name "orders"
:active true
:schema "PUBLIC"
:db_id db-id
:created_at #t "2020"
:updated_at #t "2020"})
other-table-id (t2/insert-returning-pk! (t2/table-name :model/Table)
{:name "other"
:active true
:schema "PUBLIC"
:db_id db-id
:created_at #t "2020"
:updated_at #t "2020"})
group-id (t2/insert-returning-pk! :permissions_group {:name "Test Group"})]
(t2/insert! :model/PermissionsGroupMembership {:user_id user-id :group_id group-id})
(migrate!)
(clear-permissions!)
;; set one table to be unrestricted
(t2/insert! (t2/table-name :model/DataPermissions)
:db_id db-id
:group_id group-id
:table_id table-id
:schema_name "PUBLIC"
:perm_type "perms/view-data"
:perm_value "unrestricted")
;; set the other table to be blocked
(t2/insert! (t2/table-name :model/DataPermissions)
:db_id db-id
:group_id group-id
:table_id other-table-id
:schema_name "PUBLIC"
:perm_type "perms/view-data"
:perm_value "blocked")
(is (= 1 (t2/count (t2/table-name :model/DataPermissions) :group_id group-id :table_id table-id :perm_type "perms/view-data")))
(migrate! :down 49)
(is (contains?
(t2/select-fn-set :object :model/Permissions :group_id group-id)
(str "/block/db/" db-id "/"))))))
(deftest sandboxing-rollback-test
;; Rollback tests flake on MySQL, so only run on Postgres/H2
(mt/test-drivers [:postgres :h2]
......
test/metabase/models/data_permissions_test.clj
+ 61
− 0
View file @ 00ff56e8
......@@ -2,6 +2,7 @@
(:require
[clojure.test :refer :all]
[metabase.api.common :as api]
[metabase.db.schema-migrations-test.impl :as impl]
[metabase.models.data-permissions :as data-perms]
[metabase.models.permissions-group :as perms-group]
[metabase.test :as mt]
......@@ -639,3 +640,63 @@
(is (= :blocked (data-perms/table-permission-for-user user-id :perms/view-data db-id table-id)))
(data-perms/with-additional-table-permission :perms/view-data db-id table-id :unrestricted
(is (= :unrestricted (data-perms/table-permission-for-user user-id :perms/view-data db-id table-id)))))))))
(deftest blocked-tables-downgrade-to-blocked-dbs
(impl/test-migrations ["v51.2024-08-07T11:00:00"] [migrate!]
(let [user-id (t2/insert-returning-pk! :core_user {:first_name "Howard"
:last_name "Hughes"
:email "howard@aircraft.com"
:password "superstrong"
:date_joined :%now})
db-id (t2/insert-returning-pk! :metabase_database {:name "DB"
:engine "h2"
:created_at :%now
:updated_at :%now
:details "{}"})
table-id (t2/insert-returning-pk! :metabase_table {:name "orders"
:active true
:db_id db-id
:created_at #t "2020"
:updated_at #t "2020"})
group-id (t2/insert-returning-pk! :model/PermissionsGroup {:name "Test Group"})]
(t2/insert! :model/PermissionsGroupMembership {:user_id user-id :group_id group-id})
(migrate!)
(data-perms/set-table-permission! group-id table-id :perms/view-data :blocked)
(is (= :blocked (data-perms/table-permission-for-user user-id :perms/view-data db-id table-id)))
(migrate! :down 49)
(is (contains?
(t2/select-fn-set :object :model/Permissions :group_id group-id)
(str "/block/db/" db-id "/"))))))
(deftest dbs-with-a-single-blocked-table-downgrade-to-blocked-dbs
(impl/test-migrations ["v51.2024-08-07T11:00:00"] [migrate!]
(let [user-id (t2/insert-returning-pk! :core_user {:first_name "Howard"
:last_name "Hughes"
:email "howard@aircraft.com"
:password "superstrong"
:date_joined :%now})
db-id (t2/insert-returning-pk! :metabase_database {:name "DB"
:engine "h2"
:created_at :%now
:updated_at :%now
:details "{}"})
table-id (t2/insert-returning-pk! :metabase_table {:name "orders"
:active true
:db_id db-id
:created_at #t "2020"
:updated_at #t "2020"})
_other-table (t2/insert-returning-pk! :metabase_table {:name "other"
:active true
:db_id db-id
:created_at #t "2020"
:updated_at #t "2020"})
group-id (t2/insert-returning-pk! :model/PermissionsGroup {:name "Test Group"})]
(t2/insert! :model/PermissionsGroupMembership {:user_id user-id :group_id group-id})
(migrate!)
(data-perms/set-database-permission! group-id db-id :perms/view-data :unrestricted)
(data-perms/set-table-permission! group-id table-id :perms/view-data :blocked)
(is (= :blocked (data-perms/table-permission-for-user user-id :perms/view-data db-id table-id)))
(migrate! :down 49)
(is (contains?
(t2/select-fn-set :object :model/Permissions :group_id group-id)
(str "/block/db/" db-id "/"))))))
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment