From 63c382d2b74ce7a1cf1052e9655e0b07f7842f4a Mon Sep 17 00:00:00 2001 From: hujincalrin41 Date: Sat, 10 Oct 2026 08:47:34 +0000 Subject: [PATCH 01/12] Reserve schema- and catalog-level SELECT/MODIFY grants to the metastore owner SELECT and MODIFY resolve through the hierarchy: granted on a schema they cover every table in it, tables created later included, and on a catalog every schema's tables. That is what a service account needs instead of a grant per table -- and it is why the grant must not be an owner's to give: a schema's owner is whoever created it, and could open everything beneath it. Require the metastore owner for SELECT/MODIFY changes on a catalog or schema. USE_* and CREATE_* stay with the owners, since a table's owner granting read on that one table must be able to add the USE_* its ancestors require. Tests pin the reach (existing and future tables, list/get, revoke taking all of them at once, catalog level through two levels) and the rule (owner refused on SELECT/MODIFY at both levels, including revoke; owner still grants USE_*, CREATE_* and table-level read). Co-Authored-By: Claude Fable 5.1 --- .../server/service/PermissionService.java | 40 ++++ .../SdkHierarchyGrantAccessControlTest.java | 185 ++++++++++++++++++ 2 files changed, 225 insertions(+) create mode 100644 server/src/test/java/io/unitycatalog/server/sdk/access/SdkHierarchyGrantAccessControlTest.java diff --git a/server/src/main/java/io/unitycatalog/server/service/PermissionService.java b/server/src/main/java/io/unitycatalog/server/service/PermissionService.java index 4375265a..db0f84db 100644 --- a/server/src/main/java/io/unitycatalog/server/service/PermissionService.java +++ b/server/src/main/java/io/unitycatalog/server/service/PermissionService.java @@ -43,6 +43,7 @@ import java.util.Set; import java.util.UUID; import java.util.stream.Collectors; +import java.util.stream.Stream; import com.linecorp.armeria.common.HttpResponse; import com.linecorp.armeria.server.annotation.ExceptionHandler; import com.linecorp.armeria.server.annotation.Get; @@ -283,10 +284,49 @@ public HttpResponse updateCredentialAuthorization( return updateAuthorization(CREDENTIAL, name, request); } + /** + * SELECT and MODIFY inherit down the hierarchy: granted on a catalog or schema they cover every + * table beneath it, including tables that do not exist yet (the authorizer resolves a table's + * privileges through its parents, see jcasbin_auth_model.conf's g2). That reach is the reason to + * grant them there -- and the reason only a metastore owner may: a schema's owner, which is + * whoever created it, would otherwise be able to open everything under it. USE_* and CREATE_* + * stay with the owners, because a table's owner granting read on that one table has to be able + * to add the USE_* its ancestors require. + */ + private static final Set HIERARCHY_WIDE_PRIVILEGES = + Set.of(Privilege.SELECT, Privilege.MODIFY); + + private void requireMetastoreOwnerForHierarchyWideGrants( + SecurableType securableType, List changes) { + if (securableType != SecurableType.CATALOG && securableType != SecurableType.SCHEMA) { + return; + } + boolean touchesHierarchyWide = + changes.stream() + .anyMatch( + change -> + Stream.concat(change.getAdd().stream(), change.getRemove().stream()) + .anyMatch(HIERARCHY_WIDE_PRIVILEGES::contains)); + if (!touchesHierarchyWide) { + return; + } + UUID principalId = userRepository.findPrincipalId(); + if (!authorizer.authorize( + principalId, metastoreRepository.getMetastoreId(), Privileges.OWNER)) { + throw new BaseException( + ErrorCode.PERMISSION_DENIED, + "SELECT and MODIFY on a " + + securableType.getValue() + + " apply to every table beneath it;" + + " only a metastore owner may grant or revoke them"); + } + } + private HttpResponse updateAuthorization( SecurableType securableType, String name, UpdatePermissions request) { UUID resourceId = getResourceId(securableType, name); List changes = request.getChanges(); + requireMetastoreOwnerForHierarchyWideGrants(securableType, changes); Set principalIds = new HashSet<>(); changes.forEach( change -> { diff --git a/server/src/test/java/io/unitycatalog/server/sdk/access/SdkHierarchyGrantAccessControlTest.java b/server/src/test/java/io/unitycatalog/server/sdk/access/SdkHierarchyGrantAccessControlTest.java new file mode 100644 index 00000000..cedb9d81 --- /dev/null +++ b/server/src/test/java/io/unitycatalog/server/sdk/access/SdkHierarchyGrantAccessControlTest.java @@ -0,0 +1,185 @@ +package io.unitycatalog.server.sdk.access; + +import static io.unitycatalog.server.utils.TestUtils.assertPermissionDenied; +import static org.assertj.core.api.Assertions.assertThat; + +import io.unitycatalog.client.api.GrantsApi; +import io.unitycatalog.client.api.TablesApi; +import io.unitycatalog.client.model.PermissionsChange; +import io.unitycatalog.client.model.Privilege; +import io.unitycatalog.client.model.SecurableType; +import io.unitycatalog.client.model.TableInfo; +import io.unitycatalog.client.model.UpdatePermissions; +import io.unitycatalog.server.base.ServerConfig; +import io.unitycatalog.server.persist.model.Privileges; +import io.unitycatalog.server.utils.TestUtils; +import java.util.List; +import lombok.SneakyThrows; +import org.junit.jupiter.api.Test; + +/** + * SELECT granted on a schema or a catalog reaches every table beneath it, including tables created + * afterwards, which is what a service account wants instead of a grant per table. Because that + * reach is wide, only a metastore owner may grant or revoke SELECT (and MODIFY) at those levels; + * owners keep the table-level grants and the USE_* those need. + */ +public class SdkHierarchyGrantAccessControlTest extends SdkAccessControlBaseCRUDTest { + + private static final String CATALOG = "cat_pr1"; + private static final String SCHEMA = CATALOG + ".sch_pr1"; + + @Test + @SneakyThrows + public void schemaSelectReachesEveryTableIncludingFutureOnes() { + createCommonTestUsers(); + setupCommonCatalogAndSchema(); + ServerConfig principal1Config = createTestUserServerConfig(PRINCIPAL_1); + ServerConfig regular1Config = createTestUserServerConfig(REGULAR_1); + TablesApi principal1Tables = new TablesApi(TestUtils.createApiClient(principal1Config)); + TablesApi regular1Tables = new TablesApi(TestUtils.createApiClient(regular1Config)); + + // The schema's owner creates a table; nobody else has anything on it. + grantPermissions(PRINCIPAL_1, SecurableType.SCHEMA, SCHEMA, Privileges.USE_SCHEMA); + createExternalTable( + principal1Tables, CATALOG, "sch_pr1", "t_before", testDirectoryRoot + "/t_before"); + assertThat(listAllTables(regular1Tables, CATALOG, "sch_pr1")).isEmpty(); + + // Read access at the schema: the two USE_* to reach it, and SELECT on the schema itself. + grantPermissions(REGULAR_1, SecurableType.CATALOG, CATALOG, Privileges.USE_CATALOG); + grantPermissions( + REGULAR_1, SecurableType.SCHEMA, SCHEMA, Privileges.USE_SCHEMA, Privileges.SELECT); + + List visible = listAllTables(regular1Tables, CATALOG, "sch_pr1"); + assertThat(visible).extracting(TableInfo::getName).containsExactly("t_before"); + assertThat(getTable(regular1Tables, SCHEMA + ".t_before")).isNotNull(); + + // A table created after the grant, with no grant of its own, is covered as well. + createExternalTable( + principal1Tables, CATALOG, "sch_pr1", "t_after", testDirectoryRoot + "/t_after"); + assertThat(listAllTables(regular1Tables, CATALOG, "sch_pr1")) + .extracting(TableInfo::getName) + .containsExactlyInAnyOrder("t_before", "t_after"); + assertThat(getTable(regular1Tables, SCHEMA + ".t_after")).isNotNull(); + + // Revoking the schema-level SELECT takes every table with it at once. + grantsApi.update( + SecurableType.SCHEMA, + SCHEMA, + new UpdatePermissions() + .changes( + List.of( + new PermissionsChange() + .principal(REGULAR_1) + .add(List.of()) + .remove(List.of(Privilege.SELECT))))); + assertThat(listAllTables(regular1Tables, CATALOG, "sch_pr1")).isEmpty(); + assertPermissionDenied(() -> regular1Tables.getTable(SCHEMA + ".t_after", false, false)); + } + + @Test + @SneakyThrows + public void catalogSelectReachesEverySchemaAndTable() { + createCommonTestUsers(); + setupCommonCatalogAndSchema(); + ServerConfig principal1Config = createTestUserServerConfig(PRINCIPAL_1); + ServerConfig regular2Config = createTestUserServerConfig(REGULAR_2); + TablesApi principal1Tables = new TablesApi(TestUtils.createApiClient(principal1Config)); + TablesApi regular2Tables = new TablesApi(TestUtils.createApiClient(regular2Config)); + + grantPermissions(PRINCIPAL_1, SecurableType.SCHEMA, SCHEMA, Privileges.USE_SCHEMA); + createExternalTable(principal1Tables, CATALOG, "sch_pr1", "t1", testDirectoryRoot + "/t1"); + + // USE_SCHEMA is still needed to enter the schema; SELECT comes from the catalog. + grantPermissions( + REGULAR_2, SecurableType.CATALOG, CATALOG, Privileges.USE_CATALOG, Privileges.SELECT); + grantPermissions(REGULAR_2, SecurableType.SCHEMA, SCHEMA, Privileges.USE_SCHEMA); + + assertThat(listAllTables(regular2Tables, CATALOG, "sch_pr1")) + .extracting(TableInfo::getName) + .containsExactly("t1"); + assertThat(getTable(regular2Tables, SCHEMA + ".t1")).isNotNull(); + } + + @Test + @SneakyThrows + public void onlyAMetastoreOwnerMayGrantSelectOnASchemaOrCatalog() { + createCommonTestUsers(); + setupCommonCatalogAndSchema(); + ServerConfig principal1Config = createTestUserServerConfig(PRINCIPAL_1); + GrantsApi principal1Grants = new GrantsApi(TestUtils.createApiClient(principal1Config)); + + // principal-1 owns both the catalog and the schema, and may still grant what does not + // reach down: USE_SCHEMA and CREATE_TABLE on the schema, USE_CATALOG on the catalog. + principal1Grants.update( + SecurableType.SCHEMA, SCHEMA, add(REGULAR_1, Privilege.USE_SCHEMA, Privilege.CREATE_TABLE)); + principal1Grants.update(SecurableType.CATALOG, CATALOG, add(REGULAR_1, Privilege.USE_CATALOG)); + + // SELECT and MODIFY at those levels are the metastore owner's alone. + assertPermissionDenied( + () -> + principal1Grants.update( + SecurableType.SCHEMA, SCHEMA, add(REGULAR_1, Privilege.SELECT))); + assertPermissionDenied( + () -> + principal1Grants.update( + SecurableType.SCHEMA, SCHEMA, add(REGULAR_1, Privilege.MODIFY))); + assertPermissionDenied( + () -> + principal1Grants.update( + SecurableType.CATALOG, CATALOG, add(REGULAR_1, Privilege.SELECT))); + // ... including taking it away again. + grantPermissions(REGULAR_1, SecurableType.SCHEMA, SCHEMA, Privileges.SELECT); + assertPermissionDenied( + () -> + principal1Grants.update( + SecurableType.SCHEMA, SCHEMA, remove(REGULAR_1, Privilege.SELECT))); + + // The metastore owner (admin) may, and the owner's other grants were not disturbed. + grantsApi.update(SecurableType.SCHEMA, SCHEMA, remove(REGULAR_1, Privilege.SELECT)); + } + + @Test + @SneakyThrows + public void tableOwnerStillGrantsReadOnTheirOwnTable() { + createCommonTestUsers(); + setupCommonCatalogAndSchema(); + ServerConfig principal1Config = createTestUserServerConfig(PRINCIPAL_1); + ServerConfig regular2Config = createTestUserServerConfig(REGULAR_2); + TablesApi principal1Tables = new TablesApi(TestUtils.createApiClient(principal1Config)); + TablesApi regular2Tables = new TablesApi(TestUtils.createApiClient(regular2Config)); + GrantsApi principal1Grants = new GrantsApi(TestUtils.createApiClient(principal1Config)); + + grantPermissions(PRINCIPAL_1, SecurableType.SCHEMA, SCHEMA, Privileges.USE_SCHEMA); + createExternalTable(principal1Tables, CATALOG, "sch_pr1", "mine", testDirectoryRoot + "/mine"); + + // The usual one-table read: USE_* on the ancestors plus SELECT on the table, all by the owner. + principal1Grants.update(SecurableType.CATALOG, CATALOG, add(REGULAR_2, Privilege.USE_CATALOG)); + principal1Grants.update(SecurableType.SCHEMA, SCHEMA, add(REGULAR_2, Privilege.USE_SCHEMA)); + principal1Grants.update( + SecurableType.TABLE, SCHEMA + ".mine", add(REGULAR_2, Privilege.SELECT)); + + assertThat(listAllTables(regular2Tables, CATALOG, "sch_pr1")) + .extracting(TableInfo::getName) + .containsExactly("mine"); + } + + private static UpdatePermissions add(String principal, Privilege... privileges) { + return new UpdatePermissions() + .changes( + List.of( + new PermissionsChange() + .principal(principal) + .add(List.of(privileges)) + .remove(List.of()))); + } + + private static UpdatePermissions remove(String principal, Privilege... privileges) { + return new UpdatePermissions() + .changes( + List.of( + new PermissionsChange() + .principal(principal) + .add(List.of()) + .remove(List.of(privileges)))); + } +} From 75b814449d9b74ec9466bfafcad03c5f179a333c Mon Sep 17 00:00:00 2001 From: hujincalrin41 Date: Sat, 10 Oct 2026 08:47:34 +0000 Subject: [PATCH 02/12] Offer read on a schema or catalog in the UI, for the metastore admin The simplified Grant access dialog offered only 'create' on a schema or a catalog, and its comments said privileges do not inherit. They do: SELECT on a schema reaches every table beneath it. Offer 'read' there, writing the USE_* the ancestors need plus SELECT on the securable; revoke removes only that SELECT. The access panel lists one row per (principal, level) so read and create holders are told apart. Read on a schema or catalog is reserved to the metastore owner server-side, so the dialog disables it for anyone else and says why, and the panel's revoke control for such a row follows the same signal rather than ownership. Co-Authored-By: Claude Fable 5.1 --- ui/src/components/access/AccessPanel.test.tsx | 73 ++++++++++ ui/src/components/access/AccessPanel.tsx | 100 ++++++++----- .../access/GrantAccessModal.test.tsx | 92 +++++++++++- ui/src/components/access/GrantAccessModal.tsx | 41 +++++- ui/src/hooks/access.test.ts | 96 ++++++++++++- ui/src/hooks/access.ts | 136 +++++++++++++----- 6 files changed, 461 insertions(+), 77 deletions(-) diff --git a/ui/src/components/access/AccessPanel.test.tsx b/ui/src/components/access/AccessPanel.test.tsx index 2df9e9aa..e320e91b 100644 --- a/ui/src/components/access/AccessPanel.test.tsx +++ b/ui/src/components/access/AccessPanel.test.tsx @@ -76,3 +76,76 @@ describe('AccessPanel (table)', () => { expect(requestsTo('patch', '/permissions/schema/')).toHaveLength(0); }); }); + +const SCHEMA_PERMISSIONS = { + privilege_assignments: [ + // Reads every table beneath the schema, present and future. + { principal: 'svc@x.com', privileges: ['USE SCHEMA', 'SELECT'] }, + // May create tables but reads none of them. + { principal: 'builder@x.com', privileges: ['USE SCHEMA', 'CREATE TABLE'] }, + // USE alone is plumbing and shows up nowhere. + { principal: 'visitor@x.com', privileges: ['USE SCHEMA'] }, + ], +}; + +describe('AccessPanel (schema)', () => { + it('lists read and create rows, from SELECT and CREATE_TABLE respectively', async () => { + programClient([ + { + method: 'get', + url: '/permissions/schema/c.s', + response: SCHEMA_PERMISSIONS, + }, + { + method: 'get', + url: '/auth/capabilities', + response: { metastore_admin: true }, + }, + ]); + renderWithProviders( + , + ); + + expect(await screen.findByText('svc@x.com')).toBeInTheDocument(); + expect(screen.getByText('builder@x.com')).toBeInTheDocument(); + expect(screen.queryByText('visitor@x.com')).toBeNull(); + expect(screen.getByText('read')).toBeInTheDocument(); + expect(screen.getByText('create')).toBeInTheDocument(); + // The panel says what read here means. + expect( + screen.getByText(/covers every table beneath this schema/), + ).toBeInTheDocument(); + }); + + it('lets only a metastore admin revoke read on a schema', async () => { + programClient([ + { + method: 'get', + url: '/permissions/schema/c.s', + response: { + privilege_assignments: [ + { principal: 'svc@x.com', privileges: ['USE SCHEMA', 'SELECT'] }, + ], + }, + }, + { + method: 'get', + url: '/auth/capabilities', + response: { metastore_admin: false }, + }, + ]); + renderWithProviders( + , + ); + + await screen.findByText('svc@x.com'); + // The read tag renders, but without a close control for a non-admin. + expect(screen.getByText('read')).toBeInTheDocument(); + // eslint-disable-next-line testing-library/no-node-access -- antd Tag close icon has no role + expect(document.querySelector('.ant-tag-close-icon')).toBeNull(); + }); +}); diff --git a/ui/src/components/access/AccessPanel.tsx b/ui/src/components/access/AccessPanel.tsx index 7becc9dc..4bbdbd62 100644 --- a/ui/src/components/access/AccessPanel.tsx +++ b/ui/src/components/access/AccessPanel.tsx @@ -15,10 +15,11 @@ import { AccessLevel, accessLevelsFor, ancestors, + isAdminOnly, leafPrivilegeFor, useUpdateAccess, } from '../../hooks/access'; -import { useCanManageGrants } from '../../hooks/authz'; +import { useCanManageGrants, useIsMetastoreAdmin } from '../../hooks/authz'; import { useNotification } from '../../utils/NotificationContext'; import { GrantAccessModal } from './GrantAccessModal'; import { SecurableType } from '../../types/api/catalog.gen'; @@ -32,13 +33,22 @@ interface AccessPanelProps { owners?: (string | undefined)[]; } +interface AccessRow { + principal: string; + level: AccessLevel; +} + /** - * Simplified access list for one catalog / schema / table. A principal is - * shown here when they hold the securable's defining leaf privilege - * (table:SELECT → read, schema:CREATE_TABLE → create, catalog:CREATE_SCHEMA → - * create); the USE_* plumbing grants are intentionally hidden — the per-user - * detail view on the Users page lists every raw privilege. Revoking removes - * only the leaf privilege (see hooks/access.ts). + * Simplified access list for one catalog / schema / table: one row per + * (principal, level), where a principal holds a level when they hold its leaf + * privilege on this securable (read → SELECT, create → CREATE_TABLE / + * CREATE_SCHEMA). The USE_* plumbing grants are intentionally hidden — the + * per-user detail view on the Users page lists every raw privilege. Revoking + * removes only the leaf privilege (see hooks/access.ts). + * + * A read row on a schema or catalog means every table beneath it, tables + * created later included; the server lets only a metastore owner grant or + * revoke those, so their controls follow that signal rather than ownership. */ export default function AccessPanel({ securableType, @@ -52,32 +62,39 @@ export default function AccessPanel({ const mutation = useUpdateAccess(); const { setNotification } = useNotification(); const [grantOpen, setGrantOpen] = useState(false); - // Granting any simplified level always writes USE_CATALOG on the owning - // catalog (privileges don't inherit), so the grantor must be able to manage - // the CATALOG — its owner or a metastore admin. A schema/table owner who is - // NOT the catalog owner cannot complete the grant (the USE_CATALOG PATCH - // 403s), so gate the Grant button on catalog authority. The catalog owner is - // always the last entry of the resource→…→catalog `owners` chain. + // Granting any simplified level writes USE_CATALOG on the owning catalog, + // so the grantor must be able to manage the CATALOG — its owner or a + // metastore admin. A schema/table owner who is NOT the catalog owner cannot + // complete the grant (the USE_CATALOG PATCH 403s), so gate the Grant button + // on catalog authority. The catalog owner is always the last entry of the + // resource→…→catalog `owners` chain. const canGrant = useCanManageGrants( SecurableType.catalog, ancestors(fullName).catalog, [owners[owners.length - 1]], ); // Revoking removes only the leaf privilege on THIS securable, which its own - // owner (or an ancestor / metastore admin) can do — keep the resource-chain - // gate for the per-row revoke control. + // owner (or an ancestor / metastore admin) can do — except read on a schema + // or catalog, which only a metastore admin may touch. const canRevoke = useCanManageGrants(securableType, fullName, owners); + const { data: isAdmin = false } = useIsMetastoreAdmin(); - const level: AccessLevel | undefined = accessLevelsFor(securableType)[0]; - const leafPrivilege = leafPrivilegeFor(securableType); + const levels = accessLevelsFor(securableType); - const rows = (data?.privilege_assignments ?? []).filter( + const rows: AccessRow[] = (data?.privilege_assignments ?? []).flatMap( (assignment) => - leafPrivilege && (assignment.privileges ?? []).includes(leafPrivilege), + levels + .filter((level) => { + const leaf = leafPrivilegeFor(securableType, level); + return leaf && (assignment.privileges ?? []).includes(leaf); + }) + .map((level) => ({ principal: assignment.principal ?? '', level })), ); - const revoke = (principal: string) => { - if (!level) return; + const mayRevoke = (level: AccessLevel) => + isAdminOnly(securableType, level) ? isAdmin : canRevoke.allowed; + + const revoke = (principal: string, level: AccessLevel) => { mutation.mutate( { principal, @@ -101,18 +118,24 @@ export default function AccessPanel({ // The tag's close (X) handler stopsPropagation, so a wrapping Popconfirm // never sees the click; drive the confirm imperatively from onClose instead. - const confirmRevoke = (principal: string) => { + const confirmRevoke = (principal: string, level: AccessLevel) => { + const leaf = leafPrivilegeFor(securableType, level); Modal.confirm({ title: `Revoke ${level} on ${fullName} from ${principal}?`, - content: `Only ${leafPrivilege} is removed; the USE grants on the catalog/schema stay because other objects may rely on them.`, + content: `Only ${leaf} is removed; the USE grants on the catalog/schema stay because other objects may rely on them.`, okText: 'Revoke', okButtonProps: { danger: true }, cancelText: 'Cancel', - onOk: () => revoke(principal), + onOk: () => revoke(principal, level), }); }; - if (!level || !leafPrivilege) return null; + if (levels.length === 0) return null; + + const levelList = levels.join(' / '); + const readReachesDown = + securableType === SecurableType.schema || + securableType === SecurableType.catalog; return ( @@ -144,9 +167,12 @@ export default function AccessPanel({ - Users with {level} access on this {securableType}. Only grants you are - allowed to see are listed; a non-owner sees just their own. Raw - privileges are listed in the per-user view on the Users page. + Users with {levelList} access on this {securableType}. + {readReachesDown && + ` Read here covers every table beneath this ${securableType}, including tables created later; only a metastore admin can grant or revoke it.`}{' '} + Only grants you are allowed to see are listed; a non-owner sees just + their own. Raw privileges are listed in the per-user view on the Users + page. {isError && ( `access-${record.principal}`} + rowKey={(record) => `access-${record.principal}-${record.level}`} dataSource={isError ? [] : rows} pagination={{ hideOnSinglePage: true, pageSize: 10 }} locale={{ - emptyText: `No ${level} access granted on this ${securableType}`, + emptyText: `No ${levelList} access granted on this ${securableType}`, }} columns={[ { @@ -175,17 +201,19 @@ export default function AccessPanel({ { title: 'Access', key: 'access', - render: (_, record) => ( + render: (_, record: AccessRow) => ( { e.preventDefault(); - confirmRevoke(record.principal ?? ''); + confirmRevoke(record.principal, record.level); + }} + style={{ + cursor: mayRevoke(record.level) ? 'pointer' : 'default', }} - style={{ cursor: canRevoke.allowed ? 'pointer' : 'default' }} > - {level} + {record.level} ), }, diff --git a/ui/src/components/access/GrantAccessModal.test.tsx b/ui/src/components/access/GrantAccessModal.test.tsx index 4e797ebf..629d2d21 100644 --- a/ui/src/components/access/GrantAccessModal.test.tsx +++ b/ui/src/components/access/GrantAccessModal.test.tsx @@ -1,4 +1,4 @@ -import { waitFor } from '@testing-library/react'; +import { screen, waitFor } from '@testing-library/react'; import { renderWithProviders } from '../../test-utils/render'; import { programClient, requestsTo } from '../../test-utils/mockClient'; import { clickModalOk, selectAntdOption } from '../../test-utils/antd'; @@ -97,4 +97,94 @@ describe('GrantAccessModal', () => { ], }); }); + + it('schema read, as a metastore admin, writes USE_* and SELECT on the schema', async () => { + programClient([ + { method: 'get', url: '/scim2/Users', response: USERS }, + { + method: 'get', + url: '/auth/capabilities', + response: { metastore_admin: true }, + }, + { + method: 'patch', + url: '/permissions/', + response: { privilege_assignments: [] }, + }, + ]); + renderWithProviders( + , + ); + + // read is the first level on a schema and is enabled for an admin. + expect( + await screen.findByText(/covers every table beneath it/), + ).toBeInTheDocument(); + await selectAntdOption(0, /Demo Reader/); + clickModalOk(); + + await waitFor(() => + expect(requestsTo('patch', '/permissions/schema/c.s')).toHaveLength(1), + ); + expect(requestsTo('patch', '/permissions/schema/c.s')[0].data).toEqual({ + changes: [ + { + principal: 'demo.reader@x.com', + add: ['USE SCHEMA', 'SELECT'], + remove: [], + }, + ], + }); + expect(requestsTo('patch', '/permissions/catalog/c')[0].data).toEqual({ + changes: [ + { principal: 'demo.reader@x.com', add: ['USE CATALOG'], remove: [] }, + ], + }); + }); + + it('schema read is disabled for a non-admin, who gets create by default', async () => { + programClient([ + { method: 'get', url: '/scim2/Users', response: USERS }, + { + method: 'get', + url: '/auth/capabilities', + response: { metastore_admin: false }, + }, + { + method: 'patch', + url: '/permissions/', + response: { privilege_assignments: [] }, + }, + ]); + renderWithProviders( + , + ); + + await screen.findByText('create'); + await waitFor(() => expect(screen.getByLabelText('read')).toBeDisabled()); + expect(screen.getByLabelText('create')).toBeChecked(); + await selectAntdOption(0, /Demo Reader/); + clickModalOk(); + + await waitFor(() => + expect(requestsTo('patch', '/permissions/schema/c.s')).toHaveLength(1), + ); + expect(requestsTo('patch', '/permissions/schema/c.s')[0].data).toEqual({ + changes: [ + { + principal: 'demo.reader@x.com', + add: ['USE SCHEMA', 'CREATE TABLE'], + remove: [], + }, + ], + }); + }); }); diff --git a/ui/src/components/access/GrantAccessModal.tsx b/ui/src/components/access/GrantAccessModal.tsx index d1c5d456..b5762371 100644 --- a/ui/src/components/access/GrantAccessModal.tsx +++ b/ui/src/components/access/GrantAccessModal.tsx @@ -5,9 +5,11 @@ import { AccessLevel, AccessTarget, accessLevelsFor, + isAdminOnly, grantsFor, useUpdateAccess, } from '../../hooks/access'; +import { useIsMetastoreAdmin } from '../../hooks/authz'; import { useScimUserOptions } from '../../hooks/users'; interface GrantAccessFormValues { @@ -41,7 +43,11 @@ export function GrantAccessModal({ const userOptions = useScimUserOptions(); const levels = accessLevelsFor(target.securableType); - const watchedLevel = Form.useWatch('level', form) ?? levels[0]; + const { data: isAdmin = false } = useIsMetastoreAdmin(); + const isDisabled = (level: AccessLevel) => + isAdminOnly(target.securableType, level) && !isAdmin; + const defaultLevel = levels.find((level) => !isDisabled(level)) ?? levels[0]; + const watchedLevel = Form.useWatch('level', form) ?? defaultLevel; // Reset the form each time the modal opens so a prior grant's values don't // linger (the useForm store outlives destroyOnClose, which only unmounts @@ -49,6 +55,20 @@ export function GrantAccessModal({ useEffect(() => { if (open) form.resetFields(); }, [open, form]); + // The admin signal arrives after the first render; when it does, move off a + // level that turned out to be unavailable (or onto read once it is allowed). + useEffect(() => { + if (!open) return; + const current = form.getFieldValue('level') as AccessLevel | undefined; + if ( + current === undefined || + isDisabled(current) || + current !== defaultLevel + ) { + form.setFieldValue('level', defaultLevel); + } + // eslint-disable-next-line react-hooks/exhaustive-deps -- defaultLevel already derives from isAdmin + }, [open, form, defaultLevel]); const handleSubmit = useCallback(() => { submitRef.current?.click(); @@ -75,7 +95,7 @@ export function GrantAccessModal({ form={form} layout="vertical" - initialValues={{ level: levels[0] }} + initialValues={{ level: defaultLevel }} onFinish={(values) => { mutation.mutate( { @@ -122,11 +142,24 @@ export function GrantAccessModal({ rules={[{ required: true, message: 'Access level is required' }]} > ({ value: level, label: level }))} + options={levels.map((level) => ({ + value: level, + label: level, + disabled: isDisabled(level), + }))} /> + {watchedLevel && isAdminOnly(target.securableType, watchedLevel) && ( + + Read on a {target.securableType} covers every table beneath it, + including tables created later. Only a metastore admin can grant or + revoke it + {isAdmin ? '.' : ', which is why it is unavailable to you.'} + + )} - Privileges do not inherit in Unity Catalog, so this grant writes: + The USE grants on the ancestors are required to reach the object, so + this grant writes:
    {plannedGrants.map((grant) => ( diff --git a/ui/src/hooks/access.test.ts b/ui/src/hooks/access.test.ts index 12de7533..ce223f74 100644 --- a/ui/src/hooks/access.test.ts +++ b/ui/src/hooks/access.test.ts @@ -1,6 +1,7 @@ import { accessLevelsFor, grantsFor, + isAdminOnly, leafPrivilegeFor, revokesFor, } from './access'; @@ -118,16 +119,103 @@ describe('simplified access mapping', () => { ]); }); + it('schema read grants USE_CATALOG + USE_SCHEMA + SELECT on the schema', () => { + expect( + grantsFor( + { securableType: SecurableType.schema, fullName: 'c.s' }, + 'read', + ), + ).toEqual([ + { + securable_type: SecurableType.catalog, + full_name: 'c', + privilege: Privilege.USE_CATALOG, + }, + { + securable_type: SecurableType.schema, + full_name: 'c.s', + privilege: Privilege.USE_SCHEMA, + }, + { + securable_type: SecurableType.schema, + full_name: 'c.s', + privilege: Privilege.SELECT, + }, + ]); + }); + + it('catalog read grants USE_CATALOG + SELECT on the catalog', () => { + expect( + grantsFor( + { securableType: SecurableType.catalog, fullName: 'c' }, + 'read', + ), + ).toEqual([ + { + securable_type: SecurableType.catalog, + full_name: 'c', + privilege: Privilege.USE_CATALOG, + }, + { + securable_type: SecurableType.catalog, + full_name: 'c', + privilege: Privilege.SELECT, + }, + ]); + }); + + it('revoking read on a schema or catalog removes only that SELECT', () => { + expect( + revokesFor( + { securableType: SecurableType.schema, fullName: 'c.s' }, + 'read', + ), + ).toEqual([ + { + securable_type: SecurableType.schema, + full_name: 'c.s', + privilege: Privilege.SELECT, + }, + ]); + expect( + revokesFor( + { securableType: SecurableType.catalog, fullName: 'c' }, + 'read', + ), + ).toEqual([ + { + securable_type: SecurableType.catalog, + full_name: 'c', + privilege: Privilege.SELECT, + }, + ]); + }); + it('levels and leaf privileges per securable type', () => { expect(accessLevelsFor(SecurableType.table)).toEqual(['read']); - expect(accessLevelsFor(SecurableType.schema)).toEqual(['create']); - expect(accessLevelsFor(SecurableType.catalog)).toEqual(['create']); + expect(accessLevelsFor(SecurableType.schema)).toEqual(['read', 'create']); + expect(accessLevelsFor(SecurableType.catalog)).toEqual(['read', 'create']); expect(accessLevelsFor(SecurableType.credential)).toEqual([]); + // Without a level, the first level of the securable is meant. expect(leafPrivilegeFor(SecurableType.table)).toBe(Privilege.SELECT); - expect(leafPrivilegeFor(SecurableType.schema)).toBe(Privilege.CREATE_TABLE); - expect(leafPrivilegeFor(SecurableType.catalog)).toBe( + expect(leafPrivilegeFor(SecurableType.schema)).toBe(Privilege.SELECT); + expect(leafPrivilegeFor(SecurableType.schema, 'create')).toBe( + Privilege.CREATE_TABLE, + ); + expect(leafPrivilegeFor(SecurableType.catalog, 'read')).toBe( + Privilege.SELECT, + ); + expect(leafPrivilegeFor(SecurableType.catalog, 'create')).toBe( Privilege.CREATE_SCHEMA, ); expect(leafPrivilegeFor(SecurableType.external_location)).toBeUndefined(); }); + + it('read on a schema or catalog is reserved to the metastore admin', () => { + expect(isAdminOnly(SecurableType.schema, 'read')).toBe(true); + expect(isAdminOnly(SecurableType.catalog, 'read')).toBe(true); + expect(isAdminOnly(SecurableType.table, 'read')).toBe(false); + expect(isAdminOnly(SecurableType.schema, 'create')).toBe(false); + expect(isAdminOnly(SecurableType.catalog, 'create')).toBe(false); + }); }); diff --git a/ui/src/hooks/access.ts b/ui/src/hooks/access.ts index 00f2f258..c9d58221 100644 --- a/ui/src/hooks/access.ts +++ b/ui/src/hooks/access.ts @@ -7,27 +7,30 @@ import type { Route, SuccessResponseBody } from '../utils/openapi'; import type { PrivilegeType } from './permissions'; /** - * Simplified access model for the permissions UI (issue #6). + * Simplified access model for the permissions UI (issue #6, extended by #19). * - * UnityCatalog's server-side authorizer does NOT inherit privileges down the - * hierarchy: to read one table a non-owner needs USE_CATALOG (on the catalog) + - * USE_SCHEMA (on the schema) + SELECT (on the table); a SELECT/USE granted on - * the catalog or schema does NOT cascade to the table. Exposing every raw - * privilege in the UI is error-prone, so we collapse them into two levels — - * `read` and `create` — and "auto-complete" the multiple securables each level - * needs. This is a pure UI convenience; the backend authorizer is unchanged - * (no inheritance is introduced). + * Two kinds of privilege are in play. USE_CATALOG / USE_SCHEMA are the right to + * enter a container and grant nothing on what is inside; the server requires + * them on every ancestor of whatever is being reached. SELECT (and MODIFY) is + * the right to read data, and the server resolves it through the hierarchy: + * SELECT granted on a schema covers every table beneath it, tables created + * later included, and SELECT on a catalog covers every schema's tables. * - * Level → granted (securable, privilege) set: - * - table · read = catalog:USE_CATALOG + schema:USE_SCHEMA + table:SELECT - * - schema · create = catalog:USE_CATALOG + schema:USE_SCHEMA + schema:CREATE_TABLE - * - catalog· create = catalog:USE_CATALOG + catalog:CREATE_SCHEMA + * The UI collapses these into two levels and writes the USE_* each needs: + * - table · read = catalog:USE_CATALOG + schema:USE_SCHEMA + table:SELECT + * - schema · read = catalog:USE_CATALOG + schema:USE_SCHEMA + schema:SELECT + * - catalog · read = catalog:USE_CATALOG + catalog:SELECT + * - schema · create = catalog:USE_CATALOG + schema:USE_SCHEMA + schema:CREATE_TABLE + * - catalog · create = catalog:USE_CATALOG + catalog:CREATE_SCHEMA + * + * Because read on a schema or catalog reaches everything beneath it, the server + * lets only a metastore owner grant or revoke it (see isAdminOnly). Owners keep + * the table-level read and the create levels. * * Revoke intentionally removes ONLY the leaf privilege and keeps the USE_* - * grants, because USE_CATALOG / USE_SCHEMA are shared by every read on sibling - * tables in the same catalog/schema — cascading their removal would silently - * break other tables' access. (Revoking read on a table only removes that - * table's SELECT.) + * grants, because USE_CATALOG / USE_SCHEMA are shared by every grant on sibling + * objects in the same catalog/schema -- cascading their removal would silently + * break other objects' access. */ export type AccessLevel = 'read' | 'create'; @@ -88,6 +91,33 @@ export function grantsFor( return list; } + if (target.securableType === SecurableType.schema && level === 'read') { + return [ + useCatalog, + { + securable_type: SecurableType.schema, + full_name: target.fullName, + privilege: Privilege.USE_SCHEMA, + }, + { + securable_type: SecurableType.schema, + full_name: target.fullName, + privilege: Privilege.SELECT, + }, + ]; + } + + if (target.securableType === SecurableType.catalog && level === 'read') { + return [ + useCatalog, + { + securable_type: SecurableType.catalog, + full_name: target.fullName, + privilege: Privilege.SELECT, + }, + ]; + } + if (target.securableType === SecurableType.schema && level === 'create') { return [ useCatalog, @@ -135,6 +165,19 @@ export function revokesFor( }, ]; } + if ( + (target.securableType === SecurableType.schema || + target.securableType === SecurableType.catalog) && + level === 'read' + ) { + return [ + { + securable_type: target.securableType, + full_name: target.fullName, + privilege: Privilege.SELECT, + }, + ]; + } if (target.securableType === SecurableType.schema && level === 'create') { return [ { @@ -157,24 +200,36 @@ export function revokesFor( } /** - * The leaf privilege that *defines* the simplified level on a securable — - * the one shown as a read/create tag and the only one a revoke removes: - * table has SELECT → read, schema has CREATE_TABLE → create, catalog has - * CREATE_SCHEMA → create. + * The leaf privilege that *defines* a simplified level on a securable -- the + * one shown as a tag and the only one a revoke removes. `read` is SELECT on + * any securable; `create` is CREATE_TABLE on a schema and CREATE_SCHEMA on a + * catalog. Without a level, the securable's first level is meant. */ export function leafPrivilegeFor( securableType: SecurableType, + level?: AccessLevel, ): PrivilegeType | undefined { - switch (securableType) { - case SecurableType.table: - return Privilege.SELECT; - case SecurableType.schema: - return Privilege.CREATE_TABLE; - case SecurableType.catalog: - return Privilege.CREATE_SCHEMA; - default: - return undefined; + const resolved = level ?? accessLevelsFor(securableType)[0]; + if (resolved === 'read') { + return [ + SecurableType.table, + SecurableType.schema, + SecurableType.catalog, + ].includes(securableType) + ? Privilege.SELECT + : undefined; } + if (resolved === 'create') { + switch (securableType) { + case SecurableType.schema: + return Privilege.CREATE_TABLE; + case SecurableType.catalog: + return Privilege.CREATE_SCHEMA; + default: + return undefined; + } + } + return undefined; } /** The access levels offered for a given securable type in the simplified UI. */ @@ -183,14 +238,31 @@ export function accessLevelsFor(securableType: SecurableType): AccessLevel[] { case SecurableType.table: return ['read']; case SecurableType.schema: - return ['create']; + return ['read', 'create']; case SecurableType.catalog: - return ['create']; + return ['read', 'create']; default: return []; } } +/** + * Whether only a metastore owner may grant or revoke this level here: read on + * a schema or catalog reaches every table beneath it, so the server reserves + * it (PermissionService). The UI only uses this to explain a disabled control; + * the server decides. + */ +export function isAdminOnly( + securableType: SecurableType, + level: AccessLevel, +): boolean { + return ( + level === 'read' && + (securableType === SecurableType.schema || + securableType === SecurableType.catalog) + ); +} + /** Group (securable, privilege) items so each securable gets ONE PATCH. */ function groupBySecurable(items: SecurablePrivilege[]): { securable_type: SecurableType; From 00f5c95a770dd333760732d8712e8a2579f88de7 Mon Sep 17 00:00:00 2001 From: hujincalrin41 Date: Sat, 10 Oct 2026 08:47:34 +0000 Subject: [PATCH 03/12] Document the authorization model Which privileges inherit and which do not, what USE_* is for, how to give a service account every table in a schema with one grant, and who may grant what. Both guides. Co-Authored-By: Claude Fable 5.1 --- deploy/README.md | 32 ++++++++++++++++++++++++++++++++ deploy/README.zh-CN.md | 29 +++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/deploy/README.md b/deploy/README.md index 616d7145..ec96bc26 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -274,6 +274,38 @@ Notes: (the third argument binds the issuer to that key, so UC only uses it to verify tokens from that issuer). +## Authorization model + +Two kinds of privilege are in play, and they are easy to confuse. + +| Privilege | What it is | Where it acts | +|---|---|---| +| `USE CATALOG`, `USE SCHEMA` | The right to enter a container. On its own it grants nothing on what is inside; the server requires it on every ancestor of whatever is being reached | Exactly the securable it is granted on | +| `SELECT`, `MODIFY` | The right to read (write) data | The securable it is granted on **and everything beneath it** | + +`SELECT` and `MODIFY` inherit down the hierarchy: granted on a schema they cover every table in it, +**including tables created later**; granted on a catalog they cover every schema's tables. The +check on a table resolves through its parents, so nothing is copied and a revoke at the schema or +catalog takes every table with it at once. `USE_*`, `CREATE_*` and `OWNER` do not inherit. + +To read one table a principal therefore needs `USE CATALOG` + `USE SCHEMA` + `SELECT` on the +table. To read every table in a schema, present and future -- what a service account usually +wants -- grant the same two `USE_*` and `SELECT` on the schema instead: + +```bash +PATCH /api/2.1/unity-catalog/permissions/catalog/ + {"changes":[{"principal":"","add":["USE CATALOG"]}]} +PATCH /api/2.1/unity-catalog/permissions/schema/. + {"changes":[{"principal":"","add":["USE SCHEMA","SELECT"]}]} +``` + +**Who may grant what.** An owner (whoever created the securable, or was made its owner) grants +on it and on what it contains: `USE_*`, `CREATE_*`, and `SELECT`/`MODIFY` on a table. Because +`SELECT`/`MODIFY` on a **schema or catalog** reach everything beneath them, only a **metastore +owner** may grant or revoke them there -- the built-in `admin`, and anyone granted `OWNER` on the +metastore. A schema's owner who tries gets `PERMISSION_DENIED`. The UI's *Grant access* dialog +offers `read` on a schema or catalog only to a metastore admin for the same reason. + ## UI sign-in The UI requires sign-in. Each way in has an address of its own; the application's address is not one diff --git a/deploy/README.zh-CN.md b/deploy/README.zh-CN.md index 1d83b7b6..c595ca8e 100644 --- a/deploy/README.zh-CN.md +++ b/deploy/README.zh-CN.md @@ -231,6 +231,35 @@ ConfigMap 只是"改这个文件"的一种投递方式。 | 换 tag 升级后跑的仍是旧版本 | 整个 `UC_HOME` 被挂成了卷，遮蔽了镜像里的二进制；只挂 `etc/conf` 和 `etc/db` | +## 授权模型 + +有两类权限，很容易混。 + +| 权限 | 是什么 | 作用在哪 | +|---|---|---| +| `USE CATALOG`、`USE SCHEMA` | 进入容器的"通行权"，本身不授予对里面任何对象的操作；服务端要求对被访问对象的每一层祖先都持有 | 只在被授予的那个对象上 | +| `SELECT`、`MODIFY` | 读（写）数据的权限 | 被授予的对象**及其之下的一切** | + +`SELECT` 与 `MODIFY` 沿层级向下继承：授在 schema 上即覆盖其中全部表，**含之后新建的表**；授在 +catalog 上即覆盖所有 schema 的表。对表的判定会顺着父子关系向上找，没有任何复制动作，所以在 schema +或 catalog 上撤销，其下所有表同时失效。`USE_*`、`CREATE_*`、`OWNER` 不继承。 + +因此读一张表需要 `USE CATALOG` + `USE SCHEMA` + 表上的 `SELECT`；要读某个 schema 下的全部表（含 +未来的，服务账号通常要的就是这个），把 `SELECT` 授在 schema 上即可： + +```bash +PATCH /api/2.1/unity-catalog/permissions/catalog/ + {"changes":[{"principal":"","add":["USE CATALOG"]}]} +PATCH /api/2.1/unity-catalog/permissions/schema/. + {"changes":[{"principal":"","add":["USE SCHEMA","SELECT"]}]} +``` + +**谁能授什么。** owner（创建者，或被设为 owner 的人）可以在自己的对象及其内容上授 `USE_*`、 +`CREATE_*`，以及表上的 `SELECT`/`MODIFY`。因为 **schema 或 catalog** 上的 `SELECT`/`MODIFY` 会 +覆盖其下一切，这两层上的授予与撤销只有 **metastore owner** 能做——内置的 `admin`，以及被授予 +metastore `OWNER` 的账号；schema 的 owner 去做会得到 `PERMISSION_DENIED`。UI 的 *Grant access* +对话框在 schema / catalog 上只对 metastore admin 开放 `read` 档，原因相同。 + ## UI 登录 UI 必须登录。**每个入口都有自己的地址，应用地址不是入口**：没有会话时应用地址会跳到 `/login`。 From d95409a8640e9788b995f1c87950a5b65181462b Mon Sep 17 00:00:00 2001 From: hujincalrin41 Date: Sat, 10 Oct 2026 08:47:34 +0000 Subject: [PATCH 04/12] test(server): schema/catalog SELECT stops at siblings and other catalogs; catalog read needs USE_SCHEMA on the catalog Three new scenarios beside the two inheritance ones: a schema-level read sees nothing in a sibling schema created later (even once let in with USE_SCHEMA) nor in a new catalog; a catalog-level read reaches schemas created after the grant; a catalog-level read sees nothing in another catalog. The catalog read recipe is USE_CATALOG + USE_SCHEMA + SELECT, all on the catalog: USE_SCHEMA is checked per schema and only inherits to new schemas when granted on the catalog. Catalogs, schemas and tables in these tests are created by principal-1, since the create expressions have no metastore-OWNER clause and the admin token cannot create inside someone else's catalog. Co-Authored-By: Claude Fable 5.1 --- .../SdkHierarchyGrantAccessControlTest.java | 200 +++++++++++++++--- 1 file changed, 167 insertions(+), 33 deletions(-) diff --git a/server/src/test/java/io/unitycatalog/server/sdk/access/SdkHierarchyGrantAccessControlTest.java b/server/src/test/java/io/unitycatalog/server/sdk/access/SdkHierarchyGrantAccessControlTest.java index cedb9d81..7f79fe3a 100644 --- a/server/src/test/java/io/unitycatalog/server/sdk/access/SdkHierarchyGrantAccessControlTest.java +++ b/server/src/test/java/io/unitycatalog/server/sdk/access/SdkHierarchyGrantAccessControlTest.java @@ -3,10 +3,16 @@ import static io.unitycatalog.server.utils.TestUtils.assertPermissionDenied; import static org.assertj.core.api.Assertions.assertThat; +import io.unitycatalog.client.ApiClient; +import io.unitycatalog.client.api.CatalogsApi; import io.unitycatalog.client.api.GrantsApi; +import io.unitycatalog.client.api.SchemasApi; import io.unitycatalog.client.api.TablesApi; +import io.unitycatalog.client.model.CreateCatalog; +import io.unitycatalog.client.model.CreateSchema; import io.unitycatalog.client.model.PermissionsChange; import io.unitycatalog.client.model.Privilege; +import io.unitycatalog.client.model.SchemaInfo; import io.unitycatalog.client.model.SecurableType; import io.unitycatalog.client.model.TableInfo; import io.unitycatalog.client.model.UpdatePermissions; @@ -19,14 +25,16 @@ /** * SELECT granted on a schema or a catalog reaches every table beneath it, including tables created - * afterwards, which is what a service account wants instead of a grant per table. Because that - * reach is wide, only a metastore owner may grant or revoke SELECT (and MODIFY) at those levels; - * owners keep the table-level grants and the USE_* those need. + * afterwards, which is what a service account wants instead of a grant per table. It reaches + * nothing outside that subtree: a sibling schema or another catalog stays closed. Because the reach + * is wide, only a metastore owner may grant or revoke SELECT (and MODIFY) at those levels; owners + * keep the table-level grants and the USE_* those need. */ public class SdkHierarchyGrantAccessControlTest extends SdkAccessControlBaseCRUDTest { private static final String CATALOG = "cat_pr1"; - private static final String SCHEMA = CATALOG + ".sch_pr1"; + private static final String SCHEMA_NAME = "sch_pr1"; + private static final String SCHEMA = CATALOG + "." + SCHEMA_NAME; @Test @SneakyThrows @@ -41,63 +49,167 @@ public void schemaSelectReachesEveryTableIncludingFutureOnes() { // The schema's owner creates a table; nobody else has anything on it. grantPermissions(PRINCIPAL_1, SecurableType.SCHEMA, SCHEMA, Privileges.USE_SCHEMA); createExternalTable( - principal1Tables, CATALOG, "sch_pr1", "t_before", testDirectoryRoot + "/t_before"); - assertThat(listAllTables(regular1Tables, CATALOG, "sch_pr1")).isEmpty(); + principal1Tables, CATALOG, SCHEMA_NAME, "t_before", testDirectoryRoot + "/t_before"); + assertThat(listAllTables(regular1Tables, CATALOG, SCHEMA_NAME)).isEmpty(); // Read access at the schema: the two USE_* to reach it, and SELECT on the schema itself. grantPermissions(REGULAR_1, SecurableType.CATALOG, CATALOG, Privileges.USE_CATALOG); grantPermissions( REGULAR_1, SecurableType.SCHEMA, SCHEMA, Privileges.USE_SCHEMA, Privileges.SELECT); - List visible = listAllTables(regular1Tables, CATALOG, "sch_pr1"); + List visible = listAllTables(regular1Tables, CATALOG, SCHEMA_NAME); assertThat(visible).extracting(TableInfo::getName).containsExactly("t_before"); assertThat(getTable(regular1Tables, SCHEMA + ".t_before")).isNotNull(); // A table created after the grant, with no grant of its own, is covered as well. createExternalTable( - principal1Tables, CATALOG, "sch_pr1", "t_after", testDirectoryRoot + "/t_after"); - assertThat(listAllTables(regular1Tables, CATALOG, "sch_pr1")) + principal1Tables, CATALOG, SCHEMA_NAME, "t_after", testDirectoryRoot + "/t_after"); + assertThat(listAllTables(regular1Tables, CATALOG, SCHEMA_NAME)) .extracting(TableInfo::getName) .containsExactlyInAnyOrder("t_before", "t_after"); assertThat(getTable(regular1Tables, SCHEMA + ".t_after")).isNotNull(); // Revoking the schema-level SELECT takes every table with it at once. - grantsApi.update( - SecurableType.SCHEMA, - SCHEMA, - new UpdatePermissions() - .changes( - List.of( - new PermissionsChange() - .principal(REGULAR_1) - .add(List.of()) - .remove(List.of(Privilege.SELECT))))); - assertThat(listAllTables(regular1Tables, CATALOG, "sch_pr1")).isEmpty(); + grantsApi.update(SecurableType.SCHEMA, SCHEMA, remove(REGULAR_1, Privilege.SELECT)); + assertThat(listAllTables(regular1Tables, CATALOG, SCHEMA_NAME)).isEmpty(); assertPermissionDenied(() -> regular1Tables.getTable(SCHEMA + ".t_after", false, false)); } @Test @SneakyThrows - public void catalogSelectReachesEverySchemaAndTable() { + public void schemaSelectStopsAtSiblingSchemasAndOtherCatalogs() { createCommonTestUsers(); setupCommonCatalogAndSchema(); - ServerConfig principal1Config = createTestUserServerConfig(PRINCIPAL_1); - ServerConfig regular2Config = createTestUserServerConfig(REGULAR_2); - TablesApi principal1Tables = new TablesApi(TestUtils.createApiClient(principal1Config)); - TablesApi regular2Tables = new TablesApi(TestUtils.createApiClient(regular2Config)); + ApiClient principal1Client = TestUtils.createApiClient(createTestUserServerConfig(PRINCIPAL_1)); + ApiClient regular1Client = TestUtils.createApiClient(createTestUserServerConfig(REGULAR_1)); + TablesApi principal1Tables = new TablesApi(principal1Client); + TablesApi regular1Tables = new TablesApi(regular1Client); + SchemasApi regular1Schemas = new SchemasApi(regular1Client); - grantPermissions(PRINCIPAL_1, SecurableType.SCHEMA, SCHEMA, Privileges.USE_SCHEMA); - createExternalTable(principal1Tables, CATALOG, "sch_pr1", "t1", testDirectoryRoot + "/t1"); + createExternalTable( + principal1Tables, CATALOG, SCHEMA_NAME, "t_in", testDirectoryRoot + "/t_in"); + grantPermissions(REGULAR_1, SecurableType.CATALOG, CATALOG, Privileges.USE_CATALOG); + grantPermissions( + REGULAR_1, SecurableType.SCHEMA, SCHEMA, Privileges.USE_SCHEMA, Privileges.SELECT); + assertThat(listAllTables(regular1Tables, CATALOG, SCHEMA_NAME)) + .extracting(TableInfo::getName) + .containsExactly("t_in"); + + // A sibling schema created later in the same catalog, with a table in it. + String sibling = "sch_sibling"; + createSchema(principal1Client, CATALOG, sibling); + createExternalTable(principal1Tables, CATALOG, sibling, "t_sib", testDirectoryRoot + "/t_sib"); + + // The schema-level read neither lists nor reads anything in the sibling ... + assertThat(listSchemas(regular1Schemas, CATALOG)).containsExactly(SCHEMA_NAME); + assertThat(listAllTables(regular1Tables, CATALOG, sibling)).isEmpty(); + assertPermissionDenied(() -> regular1Schemas.getSchema(CATALOG + "." + sibling)); + assertPermissionDenied( + () -> regular1Tables.getTable(CATALOG + "." + sibling + ".t_sib", false, false)); + + // ... and being let into the sibling (USE_SCHEMA) still reads none of its tables, because + // the SELECT sits on sch_pr1 and does not travel sideways. + grantPermissions( + REGULAR_1, SecurableType.SCHEMA, CATALOG + "." + sibling, Privileges.USE_SCHEMA); + assertThat(listSchemas(regular1Schemas, CATALOG)) + .containsExactlyInAnyOrder(SCHEMA_NAME, sibling); + assertThat(listAllTables(regular1Tables, CATALOG, sibling)).isEmpty(); + assertPermissionDenied( + () -> regular1Tables.getTable(CATALOG + "." + sibling + ".t_sib", false, false)); + + // A brand-new catalog with its own schema and table is out of reach entirely. + String other = "cat_other"; + createCatalog(principal1Client, other); + createSchema(principal1Client, other, "sch_x"); + createExternalTable(principal1Tables, other, "sch_x", "t_x", testDirectoryRoot + "/t_x"); + assertThat(listSchemas(regular1Schemas, other)).isEmpty(); + assertThat(listAllTables(regular1Tables, other, "sch_x")).isEmpty(); + assertPermissionDenied(() -> regular1Schemas.getSchema(other + ".sch_x")); + assertPermissionDenied(() -> regular1Tables.getTable(other + ".sch_x.t_x", false, false)); + } + + @Test + @SneakyThrows + public void catalogSelectReachesEverySchemaAndTableIncludingFutureSchemas() { + createCommonTestUsers(); + setupCommonCatalogAndSchema(); + ApiClient principal1Client = TestUtils.createApiClient(createTestUserServerConfig(PRINCIPAL_1)); + ApiClient regular2Client = TestUtils.createApiClient(createTestUserServerConfig(REGULAR_2)); + TablesApi principal1Tables = new TablesApi(principal1Client); + TablesApi regular2Tables = new TablesApi(regular2Client); + SchemasApi regular2Schemas = new SchemasApi(regular2Client); + + createExternalTable(principal1Tables, CATALOG, SCHEMA_NAME, "t1", testDirectoryRoot + "/t1"); - // USE_SCHEMA is still needed to enter the schema; SELECT comes from the catalog. + // Read access at the catalog. USE_SCHEMA is granted on the catalog too: the entry right + // is checked per schema, and granted this high it inherits to every schema, present and + // future, exactly like SELECT does. grantPermissions( - REGULAR_2, SecurableType.CATALOG, CATALOG, Privileges.USE_CATALOG, Privileges.SELECT); - grantPermissions(REGULAR_2, SecurableType.SCHEMA, SCHEMA, Privileges.USE_SCHEMA); + REGULAR_2, + SecurableType.CATALOG, + CATALOG, + Privileges.USE_CATALOG, + Privileges.USE_SCHEMA, + Privileges.SELECT); - assertThat(listAllTables(regular2Tables, CATALOG, "sch_pr1")) + assertThat(listSchemas(regular2Schemas, CATALOG)).containsExactly(SCHEMA_NAME); + assertThat(listAllTables(regular2Tables, CATALOG, SCHEMA_NAME)) .extracting(TableInfo::getName) .containsExactly("t1"); assertThat(getTable(regular2Tables, SCHEMA + ".t1")).isNotNull(); + + // A schema and table created after the grant, with no grant of their own. + String later = "sch_later"; + createSchema(principal1Client, CATALOG, later); + createExternalTable( + principal1Tables, CATALOG, later, "t_later", testDirectoryRoot + "/t_later"); + assertThat(listSchemas(regular2Schemas, CATALOG)).containsExactlyInAnyOrder(SCHEMA_NAME, later); + assertThat(listAllTables(regular2Tables, CATALOG, later)) + .extracting(TableInfo::getName) + .containsExactly("t_later"); + assertThat(getTable(regular2Tables, CATALOG + "." + later + ".t_later")).isNotNull(); + + // Revoking the catalog-level SELECT closes every table in every schema at once; the USE_* + // stay, so the schemas themselves remain listed (an empty room, not a locked door). + grantsApi.update(SecurableType.CATALOG, CATALOG, remove(REGULAR_2, Privilege.SELECT)); + assertThat(listSchemas(regular2Schemas, CATALOG)).containsExactlyInAnyOrder(SCHEMA_NAME, later); + assertThat(listAllTables(regular2Tables, CATALOG, SCHEMA_NAME)).isEmpty(); + assertThat(listAllTables(regular2Tables, CATALOG, later)).isEmpty(); + assertPermissionDenied(() -> regular2Tables.getTable(SCHEMA + ".t1", false, false)); + assertPermissionDenied( + () -> regular2Tables.getTable(CATALOG + "." + later + ".t_later", false, false)); + } + + @Test + @SneakyThrows + public void catalogSelectStopsAtOtherCatalogs() { + createCommonTestUsers(); + setupCommonCatalogAndSchema(); + ApiClient principal1Client = TestUtils.createApiClient(createTestUserServerConfig(PRINCIPAL_1)); + ApiClient regular2Client = TestUtils.createApiClient(createTestUserServerConfig(REGULAR_2)); + TablesApi principal1Tables = new TablesApi(principal1Client); + TablesApi regular2Tables = new TablesApi(regular2Client); + SchemasApi regular2Schemas = new SchemasApi(regular2Client); + + grantPermissions( + REGULAR_2, + SecurableType.CATALOG, + CATALOG, + Privileges.USE_CATALOG, + Privileges.USE_SCHEMA, + Privileges.SELECT); + + String other = "cat_other"; + createCatalog(principal1Client, other); + createSchema(principal1Client, other, "sch_x"); + createExternalTable(principal1Tables, other, "sch_x", "t_x", testDirectoryRoot + "/t_x"); + + assertThat(listSchemas(regular2Schemas, other)).isEmpty(); + assertThat(listAllTables(regular2Tables, other, "sch_x")).isEmpty(); + assertPermissionDenied(() -> regular2Schemas.getSchema(other + ".sch_x")); + assertPermissionDenied(() -> regular2Tables.getTable(other + ".sch_x.t_x", false, false)); + // The catalog the grant was made on is unaffected by the new neighbour. + assertThat(listSchemas(regular2Schemas, CATALOG)).containsExactly(SCHEMA_NAME); } @Test @@ -150,7 +262,8 @@ public void tableOwnerStillGrantsReadOnTheirOwnTable() { GrantsApi principal1Grants = new GrantsApi(TestUtils.createApiClient(principal1Config)); grantPermissions(PRINCIPAL_1, SecurableType.SCHEMA, SCHEMA, Privileges.USE_SCHEMA); - createExternalTable(principal1Tables, CATALOG, "sch_pr1", "mine", testDirectoryRoot + "/mine"); + createExternalTable( + principal1Tables, CATALOG, SCHEMA_NAME, "mine", testDirectoryRoot + "/mine"); // The usual one-table read: USE_* on the ancestors plus SELECT on the table, all by the owner. principal1Grants.update(SecurableType.CATALOG, CATALOG, add(REGULAR_2, Privilege.USE_CATALOG)); @@ -158,11 +271,32 @@ public void tableOwnerStillGrantsReadOnTheirOwnTable() { principal1Grants.update( SecurableType.TABLE, SCHEMA + ".mine", add(REGULAR_2, Privilege.SELECT)); - assertThat(listAllTables(regular2Tables, CATALOG, "sch_pr1")) + assertThat(listAllTables(regular2Tables, CATALOG, SCHEMA_NAME)) .extracting(TableInfo::getName) .containsExactly("mine"); } + // principal-1 holds CREATE CATALOG on the metastore (common setup) and owns what it creates, + // so it can go on to create schemas and tables inside; the admin's metastore OWNER does not + // satisfy the create expressions, which look for the catalog / schema owner or USE_* + + // CREATE_*. + @SneakyThrows + private static void createCatalog(ApiClient client, String name) { + new CatalogsApi(client).createCatalog(new CreateCatalog().name(name)); + } + + @SneakyThrows + private static void createSchema(ApiClient client, String catalog, String name) { + new SchemasApi(client).createSchema(new CreateSchema().name(name).catalogName(catalog)); + } + + @SneakyThrows + private static List listSchemas(SchemasApi schemasApi, String catalog) { + return schemasApi.listSchemas(catalog, null, null).getSchemas().stream() + .map(SchemaInfo::getName) + .toList(); + } + private static UpdatePermissions add(String principal, Privilege... privileges) { return new UpdatePermissions() .changes( From 94205b4fbe4112cdef7970041866a15a769b26df Mon Sep 17 00:00:00 2001 From: hujincalrin41 Date: Sat, 10 Oct 2026 08:47:34 +0000 Subject: [PATCH 05/12] ui: catalog read also writes USE_SCHEMA on the catalog Without it the principal cannot enter any schema, so the catalog-wide SELECT reached nothing; granted on the catalog, USE_SCHEMA inherits to every schema, present and future. One PATCH on the catalog carries all three. Co-Authored-By: Claude Fable 5.1 --- ui/src/hooks/access.test.ts | 8 +++++++- ui/src/hooks/access.ts | 12 +++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/ui/src/hooks/access.test.ts b/ui/src/hooks/access.test.ts index ce223f74..ed8766bc 100644 --- a/ui/src/hooks/access.test.ts +++ b/ui/src/hooks/access.test.ts @@ -144,7 +144,8 @@ describe('simplified access mapping', () => { ]); }); - it('catalog read grants USE_CATALOG + SELECT on the catalog', () => { + it('catalog read grants USE_CATALOG + USE_SCHEMA + SELECT, all on the catalog', () => { + // USE_SCHEMA on the catalog is what lets the read reach schemas created later. expect( grantsFor( { securableType: SecurableType.catalog, fullName: 'c' }, @@ -156,6 +157,11 @@ describe('simplified access mapping', () => { full_name: 'c', privilege: Privilege.USE_CATALOG, }, + { + securable_type: SecurableType.catalog, + full_name: 'c', + privilege: Privilege.USE_SCHEMA, + }, { securable_type: SecurableType.catalog, full_name: 'c', diff --git a/ui/src/hooks/access.ts b/ui/src/hooks/access.ts index c9d58221..d14b4e19 100644 --- a/ui/src/hooks/access.ts +++ b/ui/src/hooks/access.ts @@ -19,7 +19,9 @@ import type { PrivilegeType } from './permissions'; * The UI collapses these into two levels and writes the USE_* each needs: * - table · read = catalog:USE_CATALOG + schema:USE_SCHEMA + table:SELECT * - schema · read = catalog:USE_CATALOG + schema:USE_SCHEMA + schema:SELECT - * - catalog · read = catalog:USE_CATALOG + catalog:SELECT + * - catalog · read = catalog:USE_CATALOG + catalog:USE_SCHEMA + catalog:SELECT + * (USE_SCHEMA is checked per schema; granted on the catalog it inherits to + * every schema, present and future, so the catalog-wide read reaches them) * - schema · create = catalog:USE_CATALOG + schema:USE_SCHEMA + schema:CREATE_TABLE * - catalog · create = catalog:USE_CATALOG + catalog:CREATE_SCHEMA * @@ -108,8 +110,16 @@ export function grantsFor( } if (target.securableType === SecurableType.catalog && level === 'read') { + // USE_SCHEMA is checked per schema; granted on the catalog it inherits to + // every schema, present and future, which is what makes the catalog-wide + // SELECT reachable at all. return [ useCatalog, + { + securable_type: SecurableType.catalog, + full_name: target.fullName, + privilege: Privilege.USE_SCHEMA, + }, { securable_type: SecurableType.catalog, full_name: target.fullName, From 5d65e33f9dc85cace8aa97d9d7039821413fd28e Mon Sep 17 00:00:00 2001 From: hujincalrin41 Date: Sat, 10 Oct 2026 08:47:34 +0000 Subject: [PATCH 06/12] docs(deploy): catalog-wide read recipe and the scope of inheritance Adds the catalog recipe (USE CATALOG + USE SCHEMA + SELECT on the catalog), states that a read covers its subtree only (no sibling schema, no other catalog), and corrects the claim that USE_* never inherits: USE SCHEMA granted on a catalog does reach every schema in it. Only OWNER never inherits. Co-Authored-By: Claude Fable 5.1 --- deploy/README.md | 14 +++++++++++++- deploy/README.zh-CN.md | 12 +++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/deploy/README.md b/deploy/README.md index ec96bc26..3f2e8140 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -286,7 +286,10 @@ Two kinds of privilege are in play, and they are easy to confuse. `SELECT` and `MODIFY` inherit down the hierarchy: granted on a schema they cover every table in it, **including tables created later**; granted on a catalog they cover every schema's tables. The check on a table resolves through its parents, so nothing is copied and a revoke at the schema or -catalog takes every table with it at once. `USE_*`, `CREATE_*` and `OWNER` do not inherit. +catalog takes every table with it at once. The reach is the subtree and nothing else: a read on +one schema says nothing about a sibling schema or another catalog. `OWNER` never inherits. +`USE SCHEMA` is checked on each schema, but granted on a *catalog* it inherits to every schema in +it the same way -- which is what a catalog-wide read needs (below). To read one table a principal therefore needs `USE CATALOG` + `USE SCHEMA` + `SELECT` on the table. To read every table in a schema, present and future -- what a service account usually @@ -299,6 +302,15 @@ PATCH /api/2.1/unity-catalog/permissions/schema/. {"changes":[{"principal":"","add":["USE SCHEMA","SELECT"]}]} ``` +To read every table in every schema of a catalog, schemas created later included, put all three on +the catalog. Without `USE SCHEMA` there the principal cannot enter any schema and the `SELECT` +reaches nothing: + +```bash +PATCH /api/2.1/unity-catalog/permissions/catalog/ + {"changes":[{"principal":"","add":["USE CATALOG","USE SCHEMA","SELECT"]}]} +``` + **Who may grant what.** An owner (whoever created the securable, or was made its owner) grants on it and on what it contains: `USE_*`, `CREATE_*`, and `SELECT`/`MODIFY` on a table. Because `SELECT`/`MODIFY` on a **schema or catalog** reach everything beneath them, only a **metastore diff --git a/deploy/README.zh-CN.md b/deploy/README.zh-CN.md index c595ca8e..8d37cfe0 100644 --- a/deploy/README.zh-CN.md +++ b/deploy/README.zh-CN.md @@ -242,7 +242,9 @@ ConfigMap 只是"改这个文件"的一种投递方式。 `SELECT` 与 `MODIFY` 沿层级向下继承：授在 schema 上即覆盖其中全部表，**含之后新建的表**；授在 catalog 上即覆盖所有 schema 的表。对表的判定会顺着父子关系向上找，没有任何复制动作，所以在 schema -或 catalog 上撤销，其下所有表同时失效。`USE_*`、`CREATE_*`、`OWNER` 不继承。 +或 catalog 上撤销，其下所有表同时失效。覆盖范围就是那棵子树，再无其它：对某个 schema 的 read 不涉及 +兄弟 schema，也不涉及别的 catalog。`OWNER` 永不继承。`USE SCHEMA` 按 schema 逐个检查，但授在 +*catalog* 上时同样会继承到其中每个 schema——catalog 级整体可读正需要这一点（见下）。 因此读一张表需要 `USE CATALOG` + `USE SCHEMA` + 表上的 `SELECT`；要读某个 schema 下的全部表（含 未来的，服务账号通常要的就是这个），把 `SELECT` 授在 schema 上即可： @@ -254,6 +256,14 @@ PATCH /api/2.1/unity-catalog/permissions/schema/. {"changes":[{"principal":"","add":["USE SCHEMA","SELECT"]}]} ``` +要读某个 catalog 下所有 schema 的全部表（含之后新建的 schema），把三项都授在 catalog 上。少了 +`USE SCHEMA`，该账号进不了任何 schema，`SELECT` 就落不到任何表上： + +```bash +PATCH /api/2.1/unity-catalog/permissions/catalog/ + {"changes":[{"principal":"","add":["USE CATALOG","USE SCHEMA","SELECT"]}]} +``` + **谁能授什么。** owner（创建者，或被设为 owner 的人）可以在自己的对象及其内容上授 `USE_*`、 `CREATE_*`，以及表上的 `SELECT`/`MODIFY`。因为 **schema 或 catalog** 上的 `SELECT`/`MODIFY` 会 覆盖其下一切，这两层上的授予与撤销只有 **metastore owner** 能做——内置的 `admin`，以及被授予 From 8b3b784fcd272f8adc0c04a927635c06aac92b8c Mon Sep 17 00:00:00 2001 From: hujincalrin41 Date: Sat, 10 Oct 2026 08:47:34 +0000 Subject: [PATCH 07/12] test(server): who may grant, who may not, and what USE alone gives Six more scenarios around the schema/catalog-level SELECT: - MODIFY on a catalog, and revoking a catalog SELECT, are reserved to the metastore owner like the rest. - An owner's request that mixes USE_* with SELECT/MODIFY is refused as a whole; the USE_* part does not land either. - A user appointed metastore administrator through the control API can grant and revoke at those levels; standing them down takes it away again. - A user who holds an inherited SELECT can neither pass it on (at the schema, the catalog or a table beneath) nor revoke it from themselves. - USE_* alone lists container names and reads no table; a schema SELECT without USE_CATALOG, or a catalog SELECT without USE_SCHEMA, reaches nothing. - Temporary table credentials follow the inherited grants: READ with schema SELECT, READ_WRITE only once schema MODIFY is added, both closed when SELECT is revoked. Co-Authored-By: Claude Fable 5.1 --- .../SdkHierarchyGrantAccessControlTest.java | 254 ++++++++++++++++++ 1 file changed, 254 insertions(+) diff --git a/server/src/test/java/io/unitycatalog/server/sdk/access/SdkHierarchyGrantAccessControlTest.java b/server/src/test/java/io/unitycatalog/server/sdk/access/SdkHierarchyGrantAccessControlTest.java index 7f79fe3a..b782b9d1 100644 --- a/server/src/test/java/io/unitycatalog/server/sdk/access/SdkHierarchyGrantAccessControlTest.java +++ b/server/src/test/java/io/unitycatalog/server/sdk/access/SdkHierarchyGrantAccessControlTest.java @@ -3,18 +3,29 @@ import static io.unitycatalog.server.utils.TestUtils.assertPermissionDenied; import static org.assertj.core.api.Assertions.assertThat; +import com.linecorp.armeria.client.WebClient; +import com.linecorp.armeria.common.AggregatedHttpResponse; +import com.linecorp.armeria.common.HttpData; +import com.linecorp.armeria.common.HttpHeaderNames; +import com.linecorp.armeria.common.HttpMethod; +import com.linecorp.armeria.common.HttpStatus; +import com.linecorp.armeria.common.RequestHeaders; import io.unitycatalog.client.ApiClient; import io.unitycatalog.client.api.CatalogsApi; import io.unitycatalog.client.api.GrantsApi; import io.unitycatalog.client.api.SchemasApi; import io.unitycatalog.client.api.TablesApi; +import io.unitycatalog.client.api.TemporaryCredentialsApi; import io.unitycatalog.client.model.CreateCatalog; import io.unitycatalog.client.model.CreateSchema; +import io.unitycatalog.client.model.GenerateTemporaryTableCredential; import io.unitycatalog.client.model.PermissionsChange; import io.unitycatalog.client.model.Privilege; +import io.unitycatalog.client.model.PrivilegeAssignment; import io.unitycatalog.client.model.SchemaInfo; import io.unitycatalog.client.model.SecurableType; import io.unitycatalog.client.model.TableInfo; +import io.unitycatalog.client.model.TableOperation; import io.unitycatalog.client.model.UpdatePermissions; import io.unitycatalog.server.base.ServerConfig; import io.unitycatalog.server.persist.model.Privileges; @@ -276,6 +287,249 @@ public void tableOwnerStillGrantsReadOnTheirOwnTable() { .containsExactly("mine"); } + @Test + @SneakyThrows + public void catalogLevelModifyAndRevocationAreReservedToo() { + createCommonTestUsers(); + setupCommonCatalogAndSchema(); + ServerConfig principal1Config = createTestUserServerConfig(PRINCIPAL_1); + GrantsApi principal1Grants = new GrantsApi(TestUtils.createApiClient(principal1Config)); + + assertPermissionDenied( + () -> + principal1Grants.update( + SecurableType.CATALOG, CATALOG, add(REGULAR_1, Privilege.MODIFY))); + + grantPermissions(REGULAR_1, SecurableType.CATALOG, CATALOG, Privileges.SELECT); + assertPermissionDenied( + () -> + principal1Grants.update( + SecurableType.CATALOG, CATALOG, remove(REGULAR_1, Privilege.SELECT))); + grantsApi.update(SecurableType.CATALOG, CATALOG, remove(REGULAR_1, Privilege.SELECT)); + } + + @Test + @SneakyThrows + public void ownerRequestMixingUseAndSelectIsRefusedAsAWhole() { + createCommonTestUsers(); + setupCommonCatalogAndSchema(); + ServerConfig principal1Config = createTestUserServerConfig(PRINCIPAL_1); + GrantsApi principal1Grants = new GrantsApi(TestUtils.createApiClient(principal1Config)); + + // The check runs before anything is written, so the USE_* that the owner could have + // granted on its own does not land either: nothing half-applied to clean up. + assertPermissionDenied( + () -> + principal1Grants.update( + SecurableType.SCHEMA, + SCHEMA, + add(REGULAR_1, Privilege.USE_SCHEMA, Privilege.SELECT))); + assertPermissionDenied( + () -> + principal1Grants.update( + SecurableType.CATALOG, + CATALOG, + add(REGULAR_1, Privilege.USE_CATALOG, Privilege.MODIFY))); + assertThat(privilegesOf(REGULAR_1, SecurableType.SCHEMA, SCHEMA)).isEmpty(); + assertThat(privilegesOf(REGULAR_1, SecurableType.CATALOG, CATALOG)).isEmpty(); + } + + @Test + @SneakyThrows + public void anAppointedMetastoreAdminGrantsAndRevokesWhatAnOwnerCannot() { + createCommonTestUsers(); + setupCommonCatalogAndSchema(); + ServerConfig regular2Config = createTestUserServerConfig(REGULAR_2); + GrantsApi regular2Grants = new GrantsApi(TestUtils.createApiClient(regular2Config)); + TablesApi principal1Tables = + new TablesApi(TestUtils.createApiClient(createTestUserServerConfig(PRINCIPAL_1))); + TablesApi regular1Tables = + new TablesApi(TestUtils.createApiClient(createTestUserServerConfig(REGULAR_1))); + createExternalTable(principal1Tables, CATALOG, SCHEMA_NAME, "t1", testDirectoryRoot + "/t1"); + + // regular-2 owns nothing; as a plain user it may grant nothing at all ... + assertPermissionDenied( + () -> + regular2Grants.update( + SecurableType.SCHEMA, SCHEMA, add(REGULAR_1, Privilege.USE_SCHEMA))); + + // ... until admin appoints it a metastore administrator (OWNER on the metastore, the + // "super user"). From then on it may do what even the catalog's owner may not. + assertThat(setMetastoreAdmin(REGULAR_2, true).status()).isEqualTo(HttpStatus.OK); + regular2Grants.update(SecurableType.CATALOG, CATALOG, add(REGULAR_1, Privilege.USE_CATALOG)); + regular2Grants.update( + SecurableType.SCHEMA, SCHEMA, add(REGULAR_1, Privilege.USE_SCHEMA, Privilege.SELECT)); + assertThat(listAllTables(regular1Tables, CATALOG, SCHEMA_NAME)) + .extracting(TableInfo::getName) + .containsExactly("t1"); + regular2Grants.update( + SecurableType.CATALOG, CATALOG, add(REGULAR_1, Privilege.SELECT, Privilege.MODIFY)); + regular2Grants.update( + SecurableType.CATALOG, CATALOG, remove(REGULAR_1, Privilege.SELECT, Privilege.MODIFY)); + regular2Grants.update(SecurableType.SCHEMA, SCHEMA, remove(REGULAR_1, Privilege.SELECT)); + assertThat(listAllTables(regular1Tables, CATALOG, SCHEMA_NAME)).isEmpty(); + + // Standing it down takes the ability away again. + assertThat(setMetastoreAdmin(REGULAR_2, false).status()).isEqualTo(HttpStatus.OK); + assertPermissionDenied( + () -> + regular2Grants.update(SecurableType.SCHEMA, SCHEMA, add(REGULAR_1, Privilege.SELECT))); + } + + @Test + @SneakyThrows + public void holderOfAnInheritedSelectCanNeitherPassItOnNorGiveItUp() { + createCommonTestUsers(); + setupCommonCatalogAndSchema(); + ServerConfig regular1Config = createTestUserServerConfig(REGULAR_1); + GrantsApi regular1Grants = new GrantsApi(TestUtils.createApiClient(regular1Config)); + TablesApi regular1Tables = new TablesApi(TestUtils.createApiClient(regular1Config)); + TablesApi regular2Tables = + new TablesApi(TestUtils.createApiClient(createTestUserServerConfig(REGULAR_2))); + TablesApi principal1Tables = + new TablesApi(TestUtils.createApiClient(createTestUserServerConfig(PRINCIPAL_1))); + createExternalTable(principal1Tables, CATALOG, SCHEMA_NAME, "t1", testDirectoryRoot + "/t1"); + + grantPermissions(REGULAR_1, SecurableType.CATALOG, CATALOG, Privileges.USE_CATALOG); + grantPermissions( + REGULAR_1, SecurableType.SCHEMA, SCHEMA, Privileges.USE_SCHEMA, Privileges.SELECT); + assertThat(listAllTables(regular1Tables, CATALOG, SCHEMA_NAME)).hasSize(1); + + // There is no grant option: holding a privilege never includes handing it to someone else, + // at this level or on a table beneath it, nor dropping it oneself. + assertPermissionDenied( + () -> + regular1Grants.update(SecurableType.SCHEMA, SCHEMA, add(REGULAR_2, Privilege.SELECT))); + assertPermissionDenied( + () -> + regular1Grants.update( + SecurableType.SCHEMA, SCHEMA, add(REGULAR_2, Privilege.USE_SCHEMA))); + assertPermissionDenied( + () -> + regular1Grants.update( + SecurableType.CATALOG, CATALOG, add(REGULAR_2, Privilege.USE_CATALOG))); + assertPermissionDenied( + () -> + regular1Grants.update( + SecurableType.TABLE, SCHEMA + ".t1", add(REGULAR_2, Privilege.SELECT))); + assertPermissionDenied( + () -> + regular1Grants.update( + SecurableType.SCHEMA, SCHEMA, remove(REGULAR_1, Privilege.SELECT))); + assertThat(listAllTables(regular2Tables, CATALOG, SCHEMA_NAME)).isEmpty(); + assertThat(listAllTables(regular1Tables, CATALOG, SCHEMA_NAME)).hasSize(1); + } + + @Test + @SneakyThrows + public void useAloneReadsNothingAndEveryAncestorUseIsRequired() { + createCommonTestUsers(); + setupCommonCatalogAndSchema(); + ApiClient regular1Client = TestUtils.createApiClient(createTestUserServerConfig(REGULAR_1)); + ApiClient regular2Client = TestUtils.createApiClient(createTestUserServerConfig(REGULAR_2)); + ApiClient principal2Client = TestUtils.createApiClient(createTestUserServerConfig(PRINCIPAL_2)); + TablesApi regular1Tables = new TablesApi(regular1Client); + TablesApi regular2Tables = new TablesApi(regular2Client); + SchemasApi regular2Schemas = new SchemasApi(regular2Client); + TablesApi principal2Tables = new TablesApi(principal2Client); + SchemasApi principal2Schemas = new SchemasApi(principal2Client); + TablesApi principal1Tables = + new TablesApi(TestUtils.createApiClient(createTestUserServerConfig(PRINCIPAL_1))); + createExternalTable(principal1Tables, CATALOG, SCHEMA_NAME, "t1", testDirectoryRoot + "/t1"); + + // USE_* on their own open doors and read nothing: principal-2 can list the schema it may + // enter, but no table in it. + grantPermissions(PRINCIPAL_2, SecurableType.CATALOG, CATALOG, Privileges.USE_CATALOG); + grantPermissions(PRINCIPAL_2, SecurableType.SCHEMA, SCHEMA, Privileges.USE_SCHEMA); + assertThat(listSchemas(principal2Schemas, CATALOG)).containsExactly(SCHEMA_NAME); + assertThat(listAllTables(principal2Tables, CATALOG, SCHEMA_NAME)).isEmpty(); + assertPermissionDenied(() -> principal2Tables.getTable(SCHEMA + ".t1", false, false)); + + // A schema-level SELECT without USE_CATALOG reaches nothing either; the entry right is + // required on every ancestor. + grantPermissions( + REGULAR_1, SecurableType.SCHEMA, SCHEMA, Privileges.USE_SCHEMA, Privileges.SELECT); + assertThat(listAllTables(regular1Tables, CATALOG, SCHEMA_NAME)).isEmpty(); + assertPermissionDenied(() -> regular1Tables.getTable(SCHEMA + ".t1", false, false)); + grantPermissions(REGULAR_1, SecurableType.CATALOG, CATALOG, Privileges.USE_CATALOG); + assertThat(listAllTables(regular1Tables, CATALOG, SCHEMA_NAME)).hasSize(1); + + // Same at the catalog: USE_CATALOG + SELECT without USE_SCHEMA sees no schema and no table; + // USE_SCHEMA on the catalog completes it. + grantPermissions( + REGULAR_2, SecurableType.CATALOG, CATALOG, Privileges.USE_CATALOG, Privileges.SELECT); + assertThat(listSchemas(regular2Schemas, CATALOG)).isEmpty(); + assertThat(listAllTables(regular2Tables, CATALOG, SCHEMA_NAME)).isEmpty(); + assertPermissionDenied(() -> regular2Tables.getTable(SCHEMA + ".t1", false, false)); + grantPermissions(REGULAR_2, SecurableType.CATALOG, CATALOG, Privileges.USE_SCHEMA); + assertThat(listSchemas(regular2Schemas, CATALOG)).containsExactly(SCHEMA_NAME); + assertThat(listAllTables(regular2Tables, CATALOG, SCHEMA_NAME)).hasSize(1); + } + + @Test + @SneakyThrows + public void temporaryCredentialsFollowTheInheritedSelectAndModify() { + createCommonTestUsers(); + setupCommonCatalogAndSchema(); + ApiClient regular1Client = TestUtils.createApiClient(createTestUserServerConfig(REGULAR_1)); + TemporaryCredentialsApi regular1Credentials = new TemporaryCredentialsApi(regular1Client); + TablesApi principal1Tables = + new TablesApi(TestUtils.createApiClient(createTestUserServerConfig(PRINCIPAL_1))); + + grantPermissions(REGULAR_1, SecurableType.CATALOG, CATALOG, Privileges.USE_CATALOG); + grantPermissions( + REGULAR_1, SecurableType.SCHEMA, SCHEMA, Privileges.USE_SCHEMA, Privileges.SELECT); + // Created after the grant: this is the path a query engine takes to read it. + TableInfo later = + createExternalTable( + principal1Tables, CATALOG, SCHEMA_NAME, "t_later", testDirectoryRoot + "/t_later"); + GenerateTemporaryTableCredential read = + new GenerateTemporaryTableCredential() + .tableId(later.getTableId()) + .operation(TableOperation.READ); + GenerateTemporaryTableCredential readWrite = + new GenerateTemporaryTableCredential() + .tableId(later.getTableId()) + .operation(TableOperation.READ_WRITE); + + assertThat(regular1Credentials.generateTemporaryTableCredentials(read)).isNotNull(); + // SELECT alone does not write ... + assertPermissionDenied(() -> regular1Credentials.generateTemporaryTableCredentials(readWrite)); + // ... MODIFY on the schema inherits the same way and completes it. + grantPermissions(REGULAR_1, SecurableType.SCHEMA, SCHEMA, Privileges.MODIFY); + assertThat(regular1Credentials.generateTemporaryTableCredentials(readWrite)).isNotNull(); + + // Taking SELECT away closes both, MODIFY on its own being no read right. + grantsApi.update(SecurableType.SCHEMA, SCHEMA, remove(REGULAR_1, Privilege.SELECT)); + assertPermissionDenied(() -> regular1Credentials.generateTemporaryTableCredentials(read)); + assertPermissionDenied(() -> regular1Credentials.generateTemporaryTableCredentials(readWrite)); + } + + /** What `principal` holds on the securable, as the admin sees it. */ + @SneakyThrows + private List privilegesOf(String principal, SecurableType type, String fullName) { + return grantsApi.get(type, fullName, principal).getPrivilegeAssignments().stream() + .filter(assignment -> principal.equals(assignment.getPrincipal())) + .map(PrivilegeAssignment::getPrivileges) + .flatMap(List::stream) + .toList(); + } + + /** Appoints (or stands down) a metastore administrator through the control API, as admin. */ + private AggregatedHttpResponse setMetastoreAdmin(String email, boolean grant) { + RequestHeaders headers = + RequestHeaders.builder() + .method(grant ? HttpMethod.PUT : HttpMethod.DELETE) + .path("/api/1.0/unity-control/metastore/admins/" + email) + .add(HttpHeaderNames.AUTHORIZATION, "Bearer " + adminConfig.getAuthToken()) + .build(); + return WebClient.builder(serverConfig.getServerUrl()) + .build() + .execute(headers, HttpData.empty()) + .aggregate() + .join(); + } + // principal-1 holds CREATE CATALOG on the metastore (common setup) and owns what it creates, // so it can go on to create schemas and tables inside; the admin's metastore OWNER does not // satisfy the create expressions, which look for the catalog / schema owner or USE_* + From a3ba7d4eb6adc6491de9fd272a31e0d94764cc6b Mon Sep 17 00:00:00 2001 From: hujincalrin41 Date: Sat, 10 Oct 2026 08:47:34 +0000 Subject: [PATCH 08/12] docs(deploy): why USE_* stays grantable by owners while SELECT/MODIFY above a table does not States the decision and its reasoning: there is no grant option, holding a privilege never includes passing it on; USE CATALOG / USE SCHEMA are entry rights that read nothing and are needed for an owner's single-table grants, so reserving them would cost self-service and buy no safety; the data right is SELECT/MODIFY, and that is what is reserved to the metastore owner. Also names the control-API call that appoints a metastore administrator, and that a mixed USE+SELECT request is refused whole. Co-Authored-By: Claude Fable 5.1 --- deploy/README.md | 24 ++++++++++++++++++------ deploy/README.zh-CN.md | 20 +++++++++++++++----- 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/deploy/README.md b/deploy/README.md index 3f2e8140..8d157357 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -311,12 +311,24 @@ PATCH /api/2.1/unity-catalog/permissions/catalog/ {"changes":[{"principal":"","add":["USE CATALOG","USE SCHEMA","SELECT"]}]} ``` -**Who may grant what.** An owner (whoever created the securable, or was made its owner) grants -on it and on what it contains: `USE_*`, `CREATE_*`, and `SELECT`/`MODIFY` on a table. Because -`SELECT`/`MODIFY` on a **schema or catalog** reach everything beneath them, only a **metastore -owner** may grant or revoke them there -- the built-in `admin`, and anyone granted `OWNER` on the -metastore. A schema's owner who tries gets `PERMISSION_DENIED`. The UI's *Grant access* dialog -offers `read` on a schema or catalog only to a metastore admin for the same reason. +**Who may grant what.** There is no grant option: holding a privilege never includes passing it +on, and a user who only *holds* `SELECT` on a schema can neither grant it to anyone else nor give +it up. The right to grant comes from ownership alone. An owner (whoever created the securable, or +was made its owner) grants on it and on what it contains: `USE_*`, `CREATE_*`, and +`SELECT`/`MODIFY` on a table. Because `SELECT`/`MODIFY` on a **schema or catalog** reach +everything beneath them, only a **metastore owner** may grant or revoke them there -- the +built-in `admin`, and anyone appointed through `PUT /api/1.0/unity-control/metastore/admins/` +(the "super user"). A schema's owner who tries gets `PERMISSION_DENIED`, and a request that mixes +`USE SCHEMA` with `SELECT` is refused as a whole, nothing half-applied. The UI's *Grant access* +dialog offers `read` on a schema or catalog only to a metastore admin for the same reason. + +`USE CATALOG` and `USE SCHEMA` are deliberately **not** reserved to the metastore owner, on +schemas and catalogs alike. They are entry rights only: with nothing but `USE_*` a user can list +the names of the containers they may enter and reads no table, no column and no data. Reserving +them would break the one thing owners must be able to do on their own -- give a colleague read on a +single table, which needs `USE CATALOG` + `USE SCHEMA` on the ancestors -- and would buy nothing, +because the data right is in `SELECT`/`MODIFY`, and those are reserved. The same reasoning covers +`USE SCHEMA` granted on a catalog: it opens every schema's door, and still no table. ## UI sign-in diff --git a/deploy/README.zh-CN.md b/deploy/README.zh-CN.md index 8d37cfe0..82699beb 100644 --- a/deploy/README.zh-CN.md +++ b/deploy/README.zh-CN.md @@ -264,11 +264,21 @@ PATCH /api/2.1/unity-catalog/permissions/catalog/ {"changes":[{"principal":"","add":["USE CATALOG","USE SCHEMA","SELECT"]}]} ``` -**谁能授什么。** owner（创建者，或被设为 owner 的人）可以在自己的对象及其内容上授 `USE_*`、 -`CREATE_*`，以及表上的 `SELECT`/`MODIFY`。因为 **schema 或 catalog** 上的 `SELECT`/`MODIFY` 会 -覆盖其下一切，这两层上的授予与撤销只有 **metastore owner** 能做——内置的 `admin`，以及被授予 -metastore `OWNER` 的账号；schema 的 owner 去做会得到 `PERMISSION_DENIED`。UI 的 *Grant access* -对话框在 schema / catalog 上只对 metastore admin 开放 `read` 档，原因相同。 +**谁能授什么。** 没有 grant option：持有某个权限不等于可以转授，只是*持有* schema 上 `SELECT` 的 +用户既不能把它授给别人，也不能自己撤掉。能授，只来自 owner 身份。owner（创建者，或被设为 owner 的 +人）可以在自己的对象及其内容上授 `USE_*`、`CREATE_*`，以及表上的 `SELECT`/`MODIFY`。因为 +**schema 或 catalog** 上的 `SELECT`/`MODIFY` 会覆盖其下一切，这两层上的授予与撤销只有 +**metastore owner** 能做——内置的 `admin`，以及经 +`PUT /api/1.0/unity-control/metastore/admins/` 任命的账号（"超级账号"）。schema 的 owner +去做会得到 `PERMISSION_DENIED`；把 `USE SCHEMA` 和 `SELECT` 放在同一个请求里也会整单拒绝，不会 +应用一半。UI 的 *Grant access* 对话框在 schema / catalog 上只对 metastore admin 开放 `read` 档， +原因相同。 + +`USE CATALOG` 与 `USE SCHEMA` 在 schema 和 catalog 上**有意不收**到 metastore owner 手里。它们只是 +通行权：一个用户只有 `USE_*` 时，能列出自己可以进入的容器名字，读不到任何表、任何列、任何数据。 +收掉它们会打断 owner 必须能自助完成的一件事——给同事授某一张表的 read，这需要祖先上的 +`USE CATALOG` + `USE SCHEMA`——却换不来任何安全收益，因为数据权限在 `SELECT`/`MODIFY` 上，而那两个 +已经收了。授在 catalog 上的 `USE SCHEMA` 同理：它打开每个 schema 的门，但仍然没有一张表。 ## UI 登录 From a17c2fda87a32116fc1d91ea425adaf5cc4a5cdc Mon Sep 17 00:00:00 2001 From: hujincalrin41 Date: Sat, 10 Oct 2026 08:47:34 +0000 Subject: [PATCH 09/12] ui: gate the per-user grant dialog like the per-securable one The Users page's "Grant access" dialog (GrantToUserModal) offered read on a catalog or schema to any signed-in user, and its Grant button lit up on any catalog. The server refused both (403), but the dialog should not promise what the server will not do. Now, as in GrantAccessModal: read on a catalog or schema is disabled for a non-admin with the reason shown, the level falls back to create, and the button stays off on a catalog the user neither owns nor administers, since every level writes USE_CATALOG there. Co-Authored-By: Claude Fable 5.1 --- .../users/GrantToUserModal.test.tsx | 78 ++++++++++++++++++- ui/src/components/users/GrantToUserModal.tsx | 65 +++++++++++++++- 2 files changed, 139 insertions(+), 4 deletions(-) diff --git a/ui/src/components/users/GrantToUserModal.test.tsx b/ui/src/components/users/GrantToUserModal.test.tsx index facc653e..7cefe53d 100644 --- a/ui/src/components/users/GrantToUserModal.test.tsx +++ b/ui/src/components/users/GrantToUserModal.test.tsx @@ -5,6 +5,7 @@ import { useListCatalogs } from '../../hooks/catalog'; import { useListSchemas } from '../../hooks/schemas'; import { useListTables } from '../../hooks/tables'; import { useUpdateAccess } from '../../hooks/access'; +import { useCanManageGrants, useIsMetastoreAdmin } from '../../hooks/authz'; jest.mock('../../hooks/catalog'); jest.mock('../../hooks/schemas'); @@ -16,11 +17,17 @@ jest.mock('../../hooks/access', () => ({ jest.mock('../../utils/NotificationContext', () => ({ useNotification: () => ({ setNotification: jest.fn() }), })); +jest.mock('../../hooks/authz', () => ({ + useIsMetastoreAdmin: jest.fn(), + useCanManageGrants: jest.fn(), +})); const mockUseListCatalogs = useListCatalogs as jest.Mock; const mockUseListSchemas = useListSchemas as jest.Mock; const mockUseListTables = useListTables as jest.Mock; const mockUseUpdateAccess = useUpdateAccess as jest.Mock; +const mockUseIsMetastoreAdmin = useIsMetastoreAdmin as jest.Mock; +const mockUseCanManageGrants = useCanManageGrants as jest.Mock; const mutate = jest.fn(); @@ -51,8 +58,11 @@ async function pick(selectIndex: number, optionText: string) { beforeEach(() => { jest.clearAllMocks(); mockUseUpdateAccess.mockReturnValue({ mutate, isPending: false }); + // Default: an ordinary user who owns the catalog they pick. + mockUseIsMetastoreAdmin.mockReturnValue({ data: false }); + mockUseCanManageGrants.mockReturnValue({ allowed: true, ready: true }); mockUseListCatalogs.mockReturnValue({ - data: { catalogs: [{ name: 'main' }] }, + data: { catalogs: [{ name: 'main', owner: 'me@example.com' }] }, isLoading: false, }); mockUseListSchemas.mockReturnValue({ @@ -118,4 +128,70 @@ describe('GrantToUserModal', () => { expect.anything(), ); }); + + /** + * Read on a catalog or schema reaches every table beneath it and is the + * metastore admin's alone (same gate as GrantAccessModal): an ordinary user + * sees it disabled, lands on create, and the plan shows the create grants. + */ + it('disables read on a catalog for a non-admin and defaults to create', async () => { + renderModal(); + + await pick(0, 'main'); + + const read = await screen.findByRole('radio', { name: 'read' }); + expect(read).toBeDisabled(); + await waitFor(() => + expect(screen.getByRole('radio', { name: 'create' })).toBeChecked(), + ); + expect(screen.getByText('CREATE SCHEMA')).toBeInTheDocument(); + expect(screen.queryByText('SELECT')).toBeNull(); + expect( + screen.getByText(/which is why it is unavailable to you/), + ).toBeInTheDocument(); + }); + + it('lets a metastore admin grant catalog read, writing USE_* and SELECT on the catalog', async () => { + mockUseIsMetastoreAdmin.mockReturnValue({ data: true }); + renderModal(); + + await pick(0, 'main'); + + const read = await screen.findByRole('radio', { name: 'read' }); + expect(read).toBeEnabled(); + await waitFor(() => expect(read).toBeChecked()); + expect(screen.getByText('USE CATALOG')).toBeInTheDocument(); + expect(screen.getByText('USE SCHEMA')).toBeInTheDocument(); + expect(screen.getByText('SELECT')).toBeInTheDocument(); + + fireEvent.click(screen.getByRole('button', { name: 'Grant' })); + + await waitFor(() => expect(mutate).toHaveBeenCalled()); + expect(mutate).toHaveBeenCalledWith( + expect.objectContaining({ + level: 'read', + target: expect.objectContaining({ fullName: 'main' }), + }), + expect.anything(), + ); + }); + + /** + * Every level writes USE_CATALOG on the chosen catalog, so a user who is + * neither its owner nor an admin cannot complete any grant there: the + * button stays off and the reason is shown, instead of a 403 after the fact. + */ + it('blocks granting on a catalog the user does not own', async () => { + mockUseCanManageGrants.mockReturnValue({ allowed: false, ready: true }); + renderModal(); + + await pick(0, 'main'); + + expect( + await screen.findByText( + /requires owning it \(or being a metastore admin\)/, + ), + ).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Grant' })).toBeDisabled(); + }); }); diff --git a/ui/src/components/users/GrantToUserModal.tsx b/ui/src/components/users/GrantToUserModal.tsx index e6c044b7..cb6aaab6 100644 --- a/ui/src/components/users/GrantToUserModal.tsx +++ b/ui/src/components/users/GrantToUserModal.tsx @@ -6,8 +6,10 @@ import { AccessTarget, accessLevelsFor, grantsFor, + isAdminOnly, useUpdateAccess, } from '../../hooks/access'; +import { useCanManageGrants, useIsMetastoreAdmin } from '../../hooks/authz'; import { useListCatalogs } from '../../hooks/catalog'; import { useListSchemas } from '../../hooks/schemas'; import { useListTables } from '../../hooks/tables'; @@ -36,6 +38,11 @@ interface GrantToUserFormValues { * traverse every catalog, schema and table (see UserAccessDetails). Granting * must not pay that cost: each level here is fetched only once its parent is * chosen, so opening this modal costs a single catalog listing. + * + * The same gates as GrantAccessModal apply: read on a schema or catalog is a + * metastore admin's alone, and every grant writes USE_CATALOG, so the caller + * must own the chosen catalog (or be an admin) for the Grant button to light + * up. The server re-authorizes each PATCH regardless. */ export default function GrantToUserModal({ open, @@ -64,6 +71,16 @@ export default function GrantToUserModal({ options: { enabled: open && !!catalog && !!schema }, }); + const { data: isAdmin = false } = useIsMetastoreAdmin(); + // Every level writes USE_CATALOG on the chosen catalog, so that is the + // authority to check — its owner, or a metastore admin. + const catalogOwner = (catalogs.data?.catalogs ?? []).find( + (c) => c.name === catalog, + )?.owner; + const canGrant = useCanManageGrants(SecurableType.catalog, catalog, [ + catalogOwner, + ]); + useEffect(() => { if (open) { form.resetFields(); @@ -87,11 +104,32 @@ export default function GrantToUserModal({ : null; const levels = target ? accessLevelsFor(target.securableType) : []; + const isDisabled = (level: AccessLevel) => + !!target && isAdminOnly(target.securableType, level) && !isAdmin; + const defaultLevel = levels.find((level) => !isDisabled(level)) ?? levels[0]; const watchedLevel = Form.useWatch('level', form); const effectiveLevel = - watchedLevel && levels.includes(watchedLevel) ? watchedLevel : levels[0]; + watchedLevel && levels.includes(watchedLevel) && !isDisabled(watchedLevel) + ? watchedLevel + : defaultLevel; + // The admin signal arrives after the first render, and the target changes + // with every pick; keep the form off a level that is not available. + useEffect(() => { + if (!open || !target) return; + const current = form.getFieldValue('level') as AccessLevel | undefined; + if ( + current === undefined || + !levels.includes(current) || + isDisabled(current) + ) { + form.setFieldValue('level', defaultLevel); + } + // eslint-disable-next-line react-hooks/exhaustive-deps -- defaultLevel already derives from target and isAdmin + }, [open, form, defaultLevel, target?.fullName]); + const plannedGrants = target && effectiveLevel ? grantsFor(target, effectiveLevel) : []; + const grantBlocked = !!catalog && canGrant.ready && !canGrant.allowed; const handleSubmit = useCallback(() => { submitRef.current?.click(); @@ -105,7 +143,7 @@ export default function GrantToUserModal({ okText="Grant" okButtonProps={{ loading: mutation.isPending, - disabled: !target || !effectiveLevel, + disabled: !target || !effectiveLevel || grantBlocked, }} onOk={handleSubmit} destroyOnClose @@ -155,6 +193,13 @@ export default function GrantToUserModal({ }))} /> + {grantBlocked && ( + + Granting on this catalog requires owning it (or being a metastore + admin), because every level also writes USE CATALOG on it; the + server enforces this. + + )}