diff --git a/deploy/README.md b/deploy/README.md index 616d714..7378188 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -274,6 +274,69 @@ 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. 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 +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"]}]} +``` + +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.** 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. + +**Who sees the user directory.** Only a metastore owner may create, deactivate, delete or +promote accounts (SCIM `POST`/`PUT`/`PATCH`/`DELETE` on `/scim2/Users`), so the UI shows the +*Users* page to metastore admins only; anyone else gets no menu entry and is sent back to the +catalogs if they open `/users` directly. The SCIM *listing* stays readable by any signed-in user: +the *Grant access* dialog on a catalog, schema or table page picks the grantee from it, and an +owner granting read on their own table needs that list. + +`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 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 1d83b7b..84b65a6 100644 --- a/deploy/README.zh-CN.md +++ b/deploy/README.zh-CN.md @@ -231,6 +231,61 @@ ConfigMap 只是"改这个文件"的一种投递方式。 | 换 tag 升级后跑的仍是旧版本 | 整个 `UC_HOME` 被挂成了卷,遮蔽了镜像里的二进制;只挂 `etc/conf` 和 `etc/db` | +## 授权模型 + +有两类权限,很容易混。 + +| 权限 | 是什么 | 作用在哪 | +|---|---|---| +| `USE CATALOG`、`USE SCHEMA` | 进入容器的"通行权",本身不授予对里面任何对象的操作;服务端要求对被访问对象的每一层祖先都持有 | 只在被授予的那个对象上 | +| `SELECT`、`MODIFY` | 读(写)数据的权限 | 被授予的对象**及其之下的一切** | + +`SELECT` 与 `MODIFY` 沿层级向下继承:授在 schema 上即覆盖其中全部表,**含之后新建的表**;授在 +catalog 上即覆盖所有 schema 的表。对表的判定会顺着父子关系向上找,没有任何复制动作,所以在 schema +或 catalog 上撤销,其下所有表同时失效。覆盖范围就是那棵子树,再无其它:对某个 schema 的 read 不涉及 +兄弟 schema,也不涉及别的 catalog。`OWNER` 永不继承。`USE SCHEMA` 按 schema 逐个检查,但授在 +*catalog* 上时同样会继承到其中每个 schema——catalog 级整体可读正需要这一点(见下)。 + +因此读一张表需要 `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"]}]} +``` + +要读某个 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"]}]} +``` + +**谁能授什么。** 没有 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` 档, +原因相同。 + +**谁能看到用户目录。** 建用户、停用、删除、任命管理员(SCIM `/scim2/Users` 的 `POST`/`PUT`/ +`PATCH`/`DELETE`)只有 metastore owner 能做,所以 UI 的 *Users* 页只对 metastore admin 显示;其他人 +没有菜单入口,直接打开 `/users` 会被送回 catalog 列表。SCIM 的*列表*接口仍对任何登录用户开放: +catalog / schema / table 页的 *Grant access* 对话框要从里面选被授权人,owner 给自己表授 read 需要这份 +名单。 + +`USE CATALOG` 与 `USE SCHEMA` 在 schema 和 catalog 上**有意不收**到 metastore owner 手里。它们只是 +通行权:一个用户只有 `USE_*` 时,能列出自己可以进入的容器名字,读不到任何表、任何列、任何数据。 +收掉它们会打断 owner 必须能自助完成的一件事——给同事授某一张表的 read,这需要祖先上的 +`USE CATALOG` + `USE SCHEMA`——却换不来任何安全收益,因为数据权限在 `SELECT`/`MODIFY` 上,而那两个 +已经收了。授在 catalog 上的 `USE SCHEMA` 同理:它打开每个 schema 的门,但仍然没有一张表。 + ## UI 登录 UI 必须登录。**每个入口都有自己的地址,应用地址不是入口**:没有会话时应用地址会跳到 `/login`。 diff --git a/features.md b/features.md index 8153d97..79f5571 100644 --- a/features.md +++ b/features.md @@ -78,3 +78,10 @@ Feature history of this fork, by PR merge date. - features: — - bugfix: `deploy-uc.sh` accepts a path in `UC_EXTERNAL_URL` (`https://api.example.com/unitycatalog`), which the #21 state-cookie fix depends on and which the script rejected outright, so the fix could not be reached through the repo's own deployment path. Trailing slash, empty segment, query, fragment and a missing scheme are still rejected. The sample file and both deployment guides now describe the behind-a-prefix case and the redirect URI to register. + +## 2026-10-10 + +#25 (https://github.com/relytcloud/unitycatalog/pull/25) + +- features: Read access at the schema and catalog level. `SELECT` (and `MODIFY`) granted on a schema covers every table beneath it, tables created later included, and granted on a catalog -- together with `USE SCHEMA` on the catalog, which inherits the same way -- covers every schema's tables, schemas created later included; the reach is that subtree and nothing else. Because the reach is wide, granting or revoking `SELECT`/`MODIFY` on a schema or catalog is reserved to metastore administrators (the built-in `admin` and anyone appointed through `PUT /api/1.0/unity-control/metastore/admins/`); owners keep `USE_*`, `CREATE_*` and table-level grants, and a request mixing `USE_*` with `SELECT` is refused as a whole. The UI offers `read` on a schema or catalog page to administrators, lists `read` and `create` rows separately, applies the same gate to the Users page's *Grant access* dialog, and shows the Users page to administrators only. Both deployment guides gain an *Authorization model* section: the two kinds of privilege, inheritance and its limits, the schema-wide and catalog-wide recipes, who may grant what and why `USE_*` is not reserved, and who sees the user directory. +- bugfix: — 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 4375265..db0f84d 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 0000000..b782b9d --- /dev/null +++ b/server/src/test/java/io/unitycatalog/server/sdk/access/SdkHierarchyGrantAccessControlTest.java @@ -0,0 +1,573 @@ +package io.unitycatalog.server.sdk.access; + +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; +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. 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_NAME = "sch_pr1"; + private static final String SCHEMA = CATALOG + "." + SCHEMA_NAME; + + @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, 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, 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, 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, remove(REGULAR_1, Privilege.SELECT)); + assertThat(listAllTables(regular1Tables, CATALOG, SCHEMA_NAME)).isEmpty(); + assertPermissionDenied(() -> regular1Tables.getTable(SCHEMA + ".t_after", false, false)); + } + + @Test + @SneakyThrows + public void schemaSelectStopsAtSiblingSchemasAndOtherCatalogs() { + createCommonTestUsers(); + setupCommonCatalogAndSchema(); + 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); + + 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"); + + // 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.USE_SCHEMA, + Privileges.SELECT); + + 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 + @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, 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)); + 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, SCHEMA_NAME)) + .extracting(TableInfo::getName) + .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_* + + // 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( + 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)))); + } +} diff --git a/ui/src/App.test.tsx b/ui/src/App.test.tsx index 2c45522..745fa49 100644 --- a/ui/src/App.test.tsx +++ b/ui/src/App.test.tsx @@ -5,6 +5,7 @@ import { NotificationProvider } from './utils/NotificationContext'; import { appRoutes } from './App'; import { useAuth } from './context/auth-context'; import { useAuthProviders } from './hooks/auth-providers'; +import { useIsMetastoreAdmin } from './hooks/authz'; // The real route table is under test, so only the leaves are replaced: the // identity hooks, the sign-in buttons that need a live SDK, and the two heavy @@ -52,9 +53,25 @@ jest.mock( return
Catalogs page
; }, ); +jest.mock( + './pages/UsersList', + () => + function UsersList() { + return
Users page
; + }, +); +jest.mock('./hooks/authz', () => ({ + ...jest.requireActual('./hooks/authz'), + useIsMetastoreAdmin: jest.fn(), +})); const mockUseAuth = useAuth as jest.Mock; const mockUseAuthProviders = useAuthProviders as jest.Mock; +const mockUseIsMetastoreAdmin = useIsMetastoreAdmin as jest.Mock; + +function metastoreAdmin(isAdmin: boolean) { + mockUseIsMetastoreAdmin.mockReturnValue({ data: isAdmin, isPending: false }); +} function signedIn(user: object | null) { mockUseAuth.mockReturnValue({ @@ -92,6 +109,34 @@ describe('App routing', () => { }, isPending: false, }); + metastoreAdmin(false); + }); + + /** + * The user directory belongs to the metastore administrators: only they may + * create, deactivate or delete accounts. An ordinary user gets neither the + * menu entry nor the page, even by address. + */ + it('keeps an ordinary user out of the user directory', async () => { + signedIn({ id: '1', displayName: 'Someone' }); + + const router = openAt('/users'); + + expect(await screen.findByText('Catalogs page')).toBeVisible(); + await waitFor(() => expect(router.state.location.pathname).toBe('/')); + expect(screen.queryByText('Users page')).toBeNull(); + expect(screen.queryByRole('menuitem', { name: 'Users' })).toBeNull(); + }); + + it('shows the user directory to a metastore administrator', async () => { + metastoreAdmin(true); + signedIn({ id: '1', displayName: 'Admin' }); + + const router = openAt('/users'); + + expect(await screen.findByText('Users page')).toBeVisible(); + expect(router.state.location.pathname).toBe('/users'); + expect(screen.getByRole('menuitem', { name: 'Users' })).toBeVisible(); }); it('sends a visitor without a session away from the application', async () => { diff --git a/ui/src/App.tsx b/ui/src/App.tsx index 942ab81..a44e2c7 100644 --- a/ui/src/App.tsx +++ b/ui/src/App.tsx @@ -40,6 +40,7 @@ import ExternalData from './pages/ExternalData'; import CredentialDetails from './pages/CredentialDetails'; import ExternalLocationDetails from './pages/ExternalLocationDetails'; import UsersList from './pages/UsersList'; +import { useIsMetastoreAdmin } from './hooks/authz'; import ResizableSplit from './components/layouts/ResizableSplit'; // TODO: @@ -144,7 +145,11 @@ export const appRoutes: RouteObject[] = [ }, { path: '/users', - element: , + element: ( + + + + ), }, ], }, @@ -184,12 +189,38 @@ function SignInRoute({ children }: { children: React.ReactNode }) { return <>{children}; } +/** + * A page for metastore administrators only. The user directory is theirs: + * only they may create, deactivate or delete accounts, so there is nothing on + * it for anyone else, and a direct address lands on the catalogs instead. + * With authorization switched off nobody is restricted and the gate is open. + */ +function AdminRoute({ children }: { children: React.ReactNode }) { + const { loginRequired } = useSignInState(); + const { data: isAdmin, isPending } = useIsMetastoreAdmin(); + + if (!loginRequired) { + return <>{children}; + } + if (isPending) { + return

Loading...

; + } + if (!isAdmin) { + return ; + } + return <>{children}; +} + function AppProvider() { const { logout, currentUser } = useAuth(); const navigate = useNavigate(); const location = useLocation(); const { pathname } = location; const { loginRequired, pending } = useSignInState(); + // The user directory is the administrators'; everyone else gets no entry to + // it (see AdminRoute for the address itself). + const { data: isAdmin = false } = useIsMetastoreAdmin(); + const showUsers = !loginRequired || isAdmin; const selectedNavKey = pathname.startsWith('/external-data') ? 'external-data' @@ -286,11 +317,15 @@ function AppProvider() { label: 'External Data', onClick: () => navigate('/external-data'), }, - { - key: 'users', - label: 'Users', - onClick: () => navigate('/users'), - }, + ...(showUsers + ? [ + { + key: 'users', + label: 'Users', + onClick: () => navigate('/users'), + }, + ] + : []), ]} style={{ flex: 1, minWidth: 0 }} /> diff --git a/ui/src/components/access/AccessPanel.test.tsx b/ui/src/components/access/AccessPanel.test.tsx index 2df9e9a..e320e91 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 7becc9d..4bbdbd6 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 4e797eb..629d2d2 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 d1c5d45..b576237 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/components/users/GrantToUserModal.test.tsx b/ui/src/components/users/GrantToUserModal.test.tsx index facc653..7cefe53 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 e6c044b..cb6aaab 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. + + )}