diff --git a/enterprise/backend/src/metabase_enterprise/advanced_permissions/models/connection_impersonation.clj b/enterprise/backend/src/metabase_enterprise/advanced_permissions/models/connection_impersonation.clj index bf9e02d3f7f7ed9f7f57fa3157d753a512108bd1..79324855da302596763e2ac6f715951877bb135f 100644 --- a/enterprise/backend/src/metabase_enterprise/advanced_permissions/models/connection_impersonation.clj +++ b/enterprise/backend/src/metabase_enterprise/advanced_permissions/models/connection_impersonation.clj @@ -29,14 +29,18 @@ {} impersonations)))) -(defenterprise upsert-impersonations! - "Create new Connection Impersonation records or update existing ones, if they have an `:id`." +(defenterprise insert-impersonations! + "Create new Connection Impersonation records. Deletes any existing Connection Impersonation records for the same + group and database before creating new ones." :feature :advanced-permissions [impersonations] (doall (for [impersonation impersonations] - (if-let [id (:id impersonation)] - (t2/update! :model/ConnectionImpersonation id impersonation) + + (do + (t2/delete! :model/ConnectionImpersonation + :group_id (:group_id impersonation) + :db_id (:db_id impersonation)) (-> (t2/insert-returning-instances! :model/ConnectionImpersonation impersonation) first))))) diff --git a/enterprise/backend/test/metabase_enterprise/advanced_permissions/api/impersonation_test.clj b/enterprise/backend/test/metabase_enterprise/advanced_permissions/api/impersonation_test.clj index 756dc6bab354e8dab3ae1176207749cc31be924d..d13653690224967f52819d25e5dde416ce74b869 100644 --- a/enterprise/backend/test/metabase_enterprise/advanced_permissions/api/impersonation_test.clj +++ b/enterprise/backend/test/metabase_enterprise/advanced_permissions/api/impersonation_test.clj @@ -21,21 +21,24 @@ (let [impersonation {:group_id (u/the-id group) :db_id (mt/id) :attribute "Attribute Name"} - graph (assoc (perms/data-perms-graph) :impersonations [impersonation]) - result (mt/user-http-request :crowberto :put 200 "permissions/graph" graph)] - (is (= [(assoc impersonation :id (-> result :impersonations first :id))] - (t2/select :model/ConnectionImpersonation :group_id (u/the-id group))))) + graph (assoc (perms/data-perms-graph) :impersonations [impersonation])] + (mt/user-http-request :crowberto :put 200 "permissions/graph" graph) + (is (=? [impersonation] + (t2/select :model/ConnectionImpersonation :group_id (u/the-id group))))) (testing "A connection impersonation policy can be updated via the permissions graph endpoint" - (let [impersonation (-> (t2/select :model/ConnectionImpersonation - :group_id (u/the-id group)) - first - (assoc :attribute "New Attribute Name")) + (let [impersonation {:group_id (u/the-id group) + :db_id (mt/id) + :attribute "New Attribute Name"} graph (assoc (perms/data-perms-graph) :impersonations [impersonation])] (mt/user-http-request :crowberto :put 200 "permissions/graph" graph) - (is (= [impersonation] - (t2/select :model/ConnectionImpersonation - :group_id (u/the-id group))))))))))) + (is (=? + [{:group_id (u/the-id group) + :db_id (mt/id) + :attribute "New Attribute Name"}] + (t2/select :model/ConnectionImpersonation + :group_id (u/the-id group)))) + (is (= 1 (t2/count :model/ConnectionImpersonation :group_id (u/the-id group))))))))))) (deftest fetch-impersonation-policy-test (testing "GET /api/ee/advanced-permissions/impersonation" diff --git a/resources/migrations/000_migrations.yaml b/resources/migrations/000_migrations.yaml index 6af0e535811227da301fd73bea7d444cc2b066e5..5d48b6c883f2647749ce38918529a4e25965fc94 100644 --- a/resources/migrations/000_migrations.yaml +++ b/resources/migrations/000_migrations.yaml @@ -15922,6 +15922,104 @@ databaseChangeLog: constraints: nullable: false + - changeSet: + id: v47.00-051 + author: noahmoss + comment: Added 0.47.0 - Drop foreign key constraint on connection_impersonations.db_id + changes: + - dropForeignKeyConstraint: + baseTableName: connection_impersonations + constraintName: fk_conn_impersonation_db_id + rollback: + - addForeignKeyConstraint: + baseTableName: connection_impersonations + baseColumnNames: db_id + referencedTableName: metabase_database + referencedColumnNames: id + constraintName: fk_conn_impersonation_db_id + onDelete: CASCADE + + - changeSet: + id: v47.00-052 + author: noahmoss + comment: Added 0.47.0 - Drop foreign key constraint on connection_impersonations.group_id + changes: + - dropForeignKeyConstraint: + baseTableName: connection_impersonations + constraintName: fk_conn_impersonation_group_id + rollback: + - addForeignKeyConstraint: + baseTableName: connection_impersonations + baseColumnNames: group_id + referencedTableName: permissions_group + referencedColumnNames: id + constraintName: fk_conn_impersonation_group_id + onDelete: CASCADE + + - changeSet: + id: v47.00-053 + author: noahmoss + comment: Added 0.47.0 -- connection_impersonations index for db_id column + changes: + - createIndex: + tableName: connection_impersonations + columns: + - column: + name: db_id + indexName: idx_conn_impersonations_db_id + + - changeSet: + id: v47.00-054 + author: noahmoss + comment: Added 0.47.0 -- connection_impersonations index for group_id column + changes: + - createIndex: + tableName: connection_impersonations + columns: + - column: + name: group_id + indexName: idx_conn_impersonations_group_id + + - changeSet: + id: v47.00-055 + author: noahmoss + comment: Added 0.47.0 - unique constraint for connection impersonations + changes: + - addUniqueConstraint: + tableName: connection_impersonations + columnNames: group_id, db_id + constraintName: conn_impersonation_unique_group_id_db_id + rollback: + - dropUniqueConstraint: + tableName: connection_impersonations + constraintName: conn_impersonation_unique_group_id_db_id + + - changeSet: + id: v47.00-056 + author: noahmoss + comment: Added 0.47.0 - re-add foreign key constraint on connection_impersonations.db_id + changes: + - addForeignKeyConstraint: + baseTableName: connection_impersonations + baseColumnNames: db_id + referencedTableName: metabase_database + referencedColumnNames: id + constraintName: fk_conn_impersonation_db_id + onDelete: CASCADE + + - changeSet: + id: v47.00-057 + author: noahmoss + comment: Added 0.47.0 - re-add foreign key constraint on connection_impersonations.group_id + changes: + - addForeignKeyConstraint: + baseTableName: connection_impersonations + baseColumnNames: group_id + referencedTableName: permissions_group + referencedColumnNames: id + constraintName: fk_conn_impersonation_group_id + onDelete: CASCADE + - changeSet: id: v48.00-003 author: qnkhuat diff --git a/src/metabase/api/permissions.clj b/src/metabase/api/permissions.clj index e2c9e136bf9c44d53cb993ce94999488bd3b796b..51c1035f2b612789cc507fbff5d8ef49270907b7 100644 --- a/src/metabase/api/permissions.clj +++ b/src/metabase/api/permissions.clj @@ -53,8 +53,8 @@ (throw (ex-info (tru "Sandboxes are an Enterprise feature. Please upgrade to a paid plan to use this feature.") {:status-code 402}))) -(defenterprise upsert-impersonations! - "OSS implementation of `upsert-impersonations!`. Errors since this is an enterprise feature." +(defenterprise insert-impersonations! + "OSS implementation of `insert-impersonations!`. Errors since this is an enterprise feature." metabase-enterprise.advanced-permissions.models.connection-impersonation [_impersonations] (throw (ex-info (tru "Connection impersonation is an Enterprise feature. Please upgrade to a paid plan to use this feature.") @@ -97,7 +97,7 @@ (upsert-sandboxes! sandbox-updates)) impersonation-updates (:impersonations graph) impersonations (when impersonation-updates - (upsert-impersonations! impersonation-updates))] + (insert-impersonations! impersonation-updates))] (merge (perms/data-perms-graph) (when sandboxes {:sandboxes sandboxes})