metabase
metabase copied to clipboard
Migration search API to DataPermissions
First, I added a new function to models.data-permissions, full-database-permission-for-user, that is equivalent to full-schema-permission-for-user but for an entire database rather than a table. For each group, it gets the least permissive permission in the database, then coalesces the result like normal.
If permissions look like this:
id | object | group_id
-----+---------------+----------
156 | /db/1/native/ | 1
155 | /db/1/schema/ | 2
154 | /db/1/ | 3
158 | /db/1/schema/ | 4
159 | /db/1/native/ | 4
DataPermissions end up looking like this:
id | group_id | perm_type | db_id | schema_name | table_id | perm_value
-----+----------+-----------------------------+-------+-------------+----------+-----------------
809 | 1 | perms/data-access | 1 | | | no-self-service
816 | 1 | perms/download-results | 1 | | | no
825 | 1 | perms/manage-database | 1 | | | no
832 | 1 | perms/manage-table-metadata | 1 | | | no
841 | 1 | perms/native-query-editing | 1 | | | yes
813 | 2 | perms/data-access | 1 | | | unrestricted
818 | 2 | perms/download-results | 1 | | | no
829 | 2 | perms/manage-database | 1 | | | no
834 | 2 | perms/manage-table-metadata | 1 | | | no
845 | 2 | perms/native-query-editing | 1 | | | no
811 | 3 | perms/data-access | 1 | | | unrestricted
817 | 3 | perms/download-results | 1 | | | no
827 | 3 | perms/manage-database | 1 | | | no
833 | 3 | perms/manage-table-metadata | 1 | | | no
843 | 3 | perms/native-query-editing | 1 | | | yes
815 | 4 | perms/data-access | 1 | | | unrestricted
819 | 4 | perms/download-results | 1 | | | no
831 | 4 | perms/manage-database | 1 | | | no
835 | 4 | perms/manage-table-metadata | 1 | | | no
847 | 4 | perms/native-query-editing | 1 | | | yes
So to match the existing logic (matching permissions to /db/:id/ OR BOTH OF /db/:id/native/ and /db/:id/schema) we can just check that the user has yes for native-query-editing, plus unrestricted data-access, for every table in the database.
Note that we are now filtering tables after selecting them from the database. There is an existing comment in metabase.api.search saying:
We get to do this slicing and dicing with the result data because the pagination of search is for UI improvement, not for performance. We intend for the cardinality of the search results to be below the default max before this slicing occurs
So I think this ok.
| Status | Complete ↗︎ |
| Commit | cde831ddb277881cd27498373171fc98b5c33f26 |
| Results | ⚠️ 2 Flaky
✅ 2313 Passed
|