Copilot commented on code in PR #9380:
URL: https://github.com/apache/gravitino/pull/9380#discussion_r2592154548


##########
docs/iceberg-rest-service.md:
##########
@@ -284,6 +284,29 @@ The detailed configuration items are as follows:
 
 Please refer to [Credential vending](./security/credential-vending.md) for 
more details.
 
+### Access control
+
+#### Prerequisites
+
+To use access control with the Iceberg REST service:
+
+1. Enable authorization in the Gravitino server by setting 
`gravitino.authorization.enable = true`
+2. Use the [dynamic configuration 
provider](#dynamic-catalog-configuration-provider) to retrieve catalog 
configurations from Gravitino
+
+Please refer to [Access Control](./security/access-control.md) for details on 
how to configure authorization, create roles, and grant privileges in Gravitino.
+
+#### How access control works
+
+When access control is enabled:
+
+1. Clients authenticate with the Iceberg REST service (Now we supports Basic 
auth and OAuth2)

Review Comment:
   Grammar error: "we supports" should be "we support". The subject "we" 
requires the base form of the verb.
   ```suggestion
   1. Clients authenticate with the Iceberg REST service (Now we support Basic 
auth and OAuth2)
   ```



##########
docs/iceberg-rest-service.md:
##########
@@ -676,3 +699,97 @@ sh ./dev/docker/build-docker.sh --platform linux/arm64 
--type iceberg-rest-serve
 ```
 
 You could try Spark with Gravitino REST catalog service in our 
[playground](./how-to-use-the-playground.md#using-apache-iceberg-rest-service).
+
+## Quick Start: Enable Access Control for Iceberg REST Server
+
+To enable access control for the Iceberg REST server using Gravitino's dynamic 
configuration provider, follow these steps:
+
+### 1. Enable Authorization and Dynamic Config Provider
+
+Add the following to your Gravitino Iceberg REST server configuration (e.g., 
`gravitino-iceberg-rest-server.conf`):
+
+```properties
+gravitino.authorization.enable = true
+gravitino.authorization.serviceAdmins = adminUser
+
+gravitino.iceberg-rest.catalog-config-provider = dynamic-config-provider
+gravitino.iceberg-rest.gravitino-uri = http://127.0.0.1:8090
+gravitino.iceberg-rest.gravitino-metalake = test
+```
+
+Restart the Iceberg REST server after updating the configuration.
+
+---
+
+### 2. Create a Metalake
+
+```shell
+curl -X POST -H "Accept: application/vnd.gravitino.v1+json" \
+-H "Content-Type: application/json" -d '{
+  "name": "test"
+}' http://localhost:8090/api/metalakes
+```
+
+---
+
+### 3. Create a Catalog
+
+```shell
+curl -X POST -H "Accept: application/vnd.gravitino.v1+json" \
+-H "Content-Type: application/json" -d '{
+  "name": "catalog1",
+  "type": "ICEBERG",
+  "comment": "Iceberg catalog",
+  "properties": {}
+}' http://localhost:8090/api/metalakes/test/catalogs
+```
+
+---
+
+### 4. Create a Role and Grant Privileges
+
+```shell
+curl -X POST -H "Accept: application/vnd.gravitino.v1+json" \
+-H "Content-Type: application/json" -d '{
+   "name": "role1",
+   "properties": {},
+   "securableObjects": [
+      {
+         "fullName": "catalog1",
+         "type": "CATALOG",
+         "privileges": [
+            {
+               "name": "USE_SCHEMA",
+               "condition": "ALLOW"
+            },
+            {
+               "name": "SELECT_TABLE",
+               "condition": "ALLOW"
+            }
+         ]
+      }
+   ]
+}' http://localhost:8090/api/metalakes/test/roles
+```
+
+---
+
+### 5. Grant Role to User
+
+```shell
+curl -X PUT -H "Accept: application/vnd.gravitino.v1+json" \
+-H "Content-Type: application/json" -d '{
+    "roleNames": ["role1"]
+}' http://localhost:8090/api/metalakes/test/permissions/users/user1/grant
+```

Review Comment:
   Missing step to create the user before granting the role. The Quick Start 
guide attempts to grant a role to "user1" (step 5), but there's no step that 
creates this user first. According to the access control documentation, users 
must be added to the metalake before they can be used in authorization. 
Consider adding a step between step 4 and step 5 to create the user:
   
   ```shell
   curl -X POST -H "Accept: application/vnd.gravitino.v1+json" \
   -H "Content-Type: application/json" -d '{
     "name": "user1"
   }' http://localhost:8090/api/metalakes/test/users
   ```



##########
docs/iceberg-rest-service.md:
##########
@@ -676,3 +699,97 @@ sh ./dev/docker/build-docker.sh --platform linux/arm64 
--type iceberg-rest-serve
 ```
 
 You could try Spark with Gravitino REST catalog service in our 
[playground](./how-to-use-the-playground.md#using-apache-iceberg-rest-service).
+
+## Quick Start: Enable Access Control for Iceberg REST Server
+
+To enable access control for the Iceberg REST server using Gravitino's dynamic 
configuration provider, follow these steps:
+
+### 1. Enable Authorization and Dynamic Config Provider
+
+Add the following to your Gravitino Iceberg REST server configuration (e.g., 
`gravitino-iceberg-rest-server.conf`):
+
+```properties
+gravitino.authorization.enable = true
+gravitino.authorization.serviceAdmins = adminUser
+
+gravitino.iceberg-rest.catalog-config-provider = dynamic-config-provider
+gravitino.iceberg-rest.gravitino-uri = http://127.0.0.1:8090
+gravitino.iceberg-rest.gravitino-metalake = test
+```
+
+Restart the Iceberg REST server after updating the configuration.
+
+---
+
+### 2. Create a Metalake
+
+```shell
+curl -X POST -H "Accept: application/vnd.gravitino.v1+json" \
+-H "Content-Type: application/json" -d '{
+  "name": "test"
+}' http://localhost:8090/api/metalakes
+```
+
+---
+
+### 3. Create a Catalog
+
+```shell
+curl -X POST -H "Accept: application/vnd.gravitino.v1+json" \
+-H "Content-Type: application/json" -d '{
+  "name": "catalog1",
+  "type": "ICEBERG",
+  "comment": "Iceberg catalog",
+  "properties": {}
+}' http://localhost:8090/api/metalakes/test/catalogs
+```
+
+---
+
+### 4. Create a Role and Grant Privileges
+
+```shell
+curl -X POST -H "Accept: application/vnd.gravitino.v1+json" \
+-H "Content-Type: application/json" -d '{
+   "name": "role1",
+   "properties": {},
+   "securableObjects": [
+      {
+         "fullName": "catalog1",
+         "type": "CATALOG",
+         "privileges": [
+            {
+               "name": "USE_SCHEMA",
+               "condition": "ALLOW"
+            },
+            {
+               "name": "SELECT_TABLE",
+               "condition": "ALLOW"
+            }
+         ]
+      }
+   ]
+}' http://localhost:8090/api/metalakes/test/roles
+```
+
+---
+
+### 5. Grant Role to User
+
+```shell
+curl -X PUT -H "Accept: application/vnd.gravitino.v1+json" \
+-H "Content-Type: application/json" -d '{
+    "roleNames": ["role1"]
+}' http://localhost:8090/api/metalakes/test/permissions/users/user1/grant
+```
+
+---
+
+**Summary:**
+- Enable authorization and set configuration provider to 
`dynamic-config-provider`
+- Create metalake
+- Create catalog
+- Create role and grant privileges

Review Comment:
   The summary should include the user creation step. Since the guide should 
include a step to create the user before granting roles, the summary should be 
updated to include "Create user" as a separate bullet point between "Create 
role and grant privileges" and "Assign role to user".
   ```suggestion
   - Create role and grant privileges
   - Create user
   ```



##########
docs/iceberg-rest-service.md:
##########
@@ -676,3 +699,97 @@ sh ./dev/docker/build-docker.sh --platform linux/arm64 
--type iceberg-rest-serve
 ```
 
 You could try Spark with Gravitino REST catalog service in our 
[playground](./how-to-use-the-playground.md#using-apache-iceberg-rest-service).
+
+## Quick Start: Enable Access Control for Iceberg REST Server
+
+To enable access control for the Iceberg REST server using Gravitino's dynamic 
configuration provider, follow these steps:
+
+### 1. Enable Authorization and Dynamic Config Provider
+
+Add the following to your Gravitino Iceberg REST server configuration (e.g., 
`gravitino-iceberg-rest-server.conf`):
+
+```properties
+gravitino.authorization.enable = true
+gravitino.authorization.serviceAdmins = adminUser
+
+gravitino.iceberg-rest.catalog-config-provider = dynamic-config-provider
+gravitino.iceberg-rest.gravitino-uri = http://127.0.0.1:8090
+gravitino.iceberg-rest.gravitino-metalake = test
+```
+
+Restart the Iceberg REST server after updating the configuration.
+
+---
+
+### 2. Create a Metalake
+
+```shell
+curl -X POST -H "Accept: application/vnd.gravitino.v1+json" \
+-H "Content-Type: application/json" -d '{
+  "name": "test"
+}' http://localhost:8090/api/metalakes
+```
+
+---
+
+### 3. Create a Catalog
+
+```shell
+curl -X POST -H "Accept: application/vnd.gravitino.v1+json" \
+-H "Content-Type: application/json" -d '{
+  "name": "catalog1",
+  "type": "ICEBERG",
+  "comment": "Iceberg catalog",
+  "properties": {}
+}' http://localhost:8090/api/metalakes/test/catalogs
+```
+
+---
+
+### 4. Create a Role and Grant Privileges
+
+```shell
+curl -X POST -H "Accept: application/vnd.gravitino.v1+json" \
+-H "Content-Type: application/json" -d '{
+   "name": "role1",
+   "properties": {},
+   "securableObjects": [
+      {
+         "fullName": "catalog1",
+         "type": "CATALOG",
+         "privileges": [
+            {
+               "name": "USE_SCHEMA",
+               "condition": "ALLOW"
+            },
+            {
+               "name": "SELECT_TABLE",
+               "condition": "ALLOW"
+            }
+         ]

Review Comment:
   Missing required `USE_CATALOG` privilege. According to the access control 
documentation, users need `USE_CATALOG` privilege to interact with any object 
within the catalog. The example should include this privilege along with 
`USE_SCHEMA` and `SELECT_TABLE`. Add the following privilege to the list:
   
   ```json
   {
      "name": "USE_CATALOG",
      "condition": "ALLOW"
   }
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to