From e98df1b52828eea65d4ba6e85f367a9f1e3ead70 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 27 Aug 2024 16:00:49 +0800 Subject: [PATCH 1/3] [#4701] fix(docs): Fix the inconsistent privilege descriptions (#4703) ### What changes were proposed in this pull request? Fix the inconsistent privilege descriptions ### Why are the changes needed? Fix: #4701 ### Does this PR introduce _any_ user-facing change? Just docs. ### How was this patch tested? NO. Co-authored-by: roryqi --- docs/security/access-control.md | 84 ++++++++++++++++----------------- 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/docs/security/access-control.md b/docs/security/access-control.md index 51e607d2564..b0ffaf75e77 100644 --- a/docs/security/access-control.md +++ b/docs/security/access-control.md @@ -134,83 +134,83 @@ You can also create a dedicated role for your business by API or the client. ### User privileges -| Name | Supports Securable Object | Operation | -|-------------|---------------------------|---------------------| -| ManageUsers | Metalake | Add or remove users | +| Name | Supports Securable Object | Operation | +|--------------|---------------------------|---------------------| +| MANAGE_USERS | Metalake | Add or remove users | ### Group privileges -| Name | Supports Securable Object | Operation | -|--------------|---------------------------|----------------------| -| ManageGroups | Metalake | Add or remove groups | +| Name | Supports Securable Object | Operation | +|---------------|---------------------------|----------------------| +| MANAGE_GROUPS | Metalake | Add or remove groups | ### Role privileges -| Name | Supports Securable Object | Operation | -|------------|---------------------------|---------------| -| CreateRole | Metalake | Create a role | +| Name | Supports Securable Object | Operation | +|-------------|---------------------------|---------------| +| CREATE_ROLE | Metalake | Create a role | ### Permission privileges -| Name | Supports Securable Object | Operation | -|--------------|---------------------------|------------------------| -| ManageGrants | Metalake | grant or revoke a role | +| Name | Supports Securable Object | Operation | +|---------------|---------------------------|------------------------| +| MANAGE_GRANTS | Metalake | grant or revoke a role | ### Catalog privileges -| Name | Supports Securable Object | Operation | -|---------------|---------------------------|------------------| -| CreateCatalog | Metalake | Create a catalog | -| UseCatalog | Metalake, Catalog | | +| Name | Supports Securable Object | Operation | +|----------------|---------------------------|------------------| +| CREATE_CATALOG | Metalake | Create a catalog | +| USE_CATALOG | Metalake, Catalog | | :::info `USE_CATALOG` is needed for a user to interact with any object within the catalog. -For example, to select data from a table, users need to have the SELECT_TABLE privilege on that table and -`USE CATALOG` privileges on its parent catalog as well as `USE SCHEMA` privileges on its parent schema. +For example, to select data from a table, users need to have the `SELECT_TABLE` privilege on that table and +`USE_CATALOG` privileges on its parent catalog as well as `USE_SCHEMA` privileges on its parent schema. ::: ### Schema privileges -| Name | Supports Securable Object | Operation | -|--------------|---------------------------|-----------------| -| CreateSchema | Metalake, Catalog | Create a schema | -| UseSchema | Metalake, Catalog, Schema | Use a schema | +| Name | Supports Securable Object | Operation | +|---------------|---------------------------|-----------------| +| CREATE_SCHEMA | Metalake, Catalog | Create a schema | +| USE_SCHEMA | Metalake, Catalog, Schema | Use a schema | :::info -`UseSchema`is needed for a user to interact with any object within the schema. +`USE_SCHEMA`is needed for a user to interact with any object within the schema. For example, to select data from a table, users need to have the `SELECT_TABLE` privilege on that table -and `USE SCHEMA` privileges on its parent schema. +and `USE_SCHEMA` privileges on its parent schema. ::: ### Table privileges -| Name | Supports Securable Object | Operation | -|-------------|-----------------------------------|------------------------------------------------| -| CreateTable | Metalake, Catalog, Schema | Create a table | -| ModifyTable | Metalake, Catalog, Schema, Table | Use the SQL `UPDATE`,`DELETE`,`INSERT` a table | -| SelectTable | Metalake, Catalog, Schema, Table | Use the SQL `SELECT` data from a table | +| Name | Supports Securable Object | Operation | +|--------------|-----------------------------------|------------------------------------------------| +| CREATE_TABLE | Metalake, Catalog, Schema | Create a table | +| MODIFY_TABLE | Metalake, Catalog, Schema, Table | Use the SQL `UPDATE`,`DELETE`,`INSERT` a table | +| SELECT_TABLE | Metalake, Catalog, Schema, Table | Use the SQL `SELECT` data from a table | ### Topic privileges -| Name | Supports Securable Object | Operation | -|--------------|----------------------------------|-------------------------------------------| -| CreateTopic | Metalake, Catalog, Schema | Create a topic | -| ProduceTopic | Metalake, Catalog, Schema, Topic | Produce a topic (including alter a topic) | -| ConsumeTopic | Metalake, Catalog, Schema, Topic | Consume a topic | +| Name | Supports Securable Object | Operation | +|---------------|----------------------------------|-------------------------------------------| +| CREATE_TOPIC | Metalake, Catalog, Schema | Create a topic | +| PRODUCE_TOPIC | Metalake, Catalog, Schema, Topic | Produce a topic (including alter a topic) | +| CONSUME_TOPIC | Metalake, Catalog, Schema, Topic | Consume a topic | ### Fileset privileges -| Name | Supports Securable Object | Operation | -|---------------|------------------------------------|---------------------------------------------| -| CreateFileset | Metalake, Catalog, Schema | Create a fileset | -| WriteFileset | Metalake, Catalog, Schema, Fileset | Write a fileset (including alter a fileset) | -| ReadFileset | Metalake, Catalog, Schema, Fileset | read a fileset | +| Name | Supports Securable Object | Operation | +|----------------|------------------------------------|---------------------------------------------| +| CREATE_FILESET | Metalake, Catalog, Schema | Create a fileset | +| WRITE_FILESET | Metalake, Catalog, Schema, Fileset | Write a fileset (including alter a fileset) | +| READ_FILESET | Metalake, Catalog, Schema, Fileset | read a fileset | ## Inheritance Model @@ -224,9 +224,9 @@ will be able to select(read) all tables in that catalog. ## Privilege Condition -The privilege supports two condition: `allow` and `deny`. `allow` means that you are able to use the privilege, -`deny` means that you aren't able to use the privilege. -`deny` condition is prior to `allow` condition. If a role has the `allow` condition and `deny` condition at the same time. +The privilege supports two condition: `ALLOW` and `DENY`. `ALLOW` means that you are able to use the privilege, +`DENY` means that you aren't able to use the privilege. +`DENY` condition is prior to `ALLOW` condition. If a role has the `ALLOW` condition and `DENY` condition at the same time. The user won't be able to use the privilege. If parent securable object has the same privilege name with different condition, the securable object won't override the parent object privilege. From 6a175120c0065edf2b8104593f8a3032c488b981 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 27 Aug 2024 17:51:33 +0800 Subject: [PATCH 2/3] [#4695] Fix (python-client): Fix gradlew :clients:client-python:build failed if the package does not contain .git directory (#4705) ### What changes were proposed in this pull request? Fix gradlew :clients:client-python:build failed if the package does not contain .git directory ### Why are the changes needed? Fix: #4695 ### Does this PR introduce _any_ user-facing change? NO ### How was this patch tested? Manually test Co-authored-by: Yuhui --- .../client-python/gravitino/constants/root.py | 1 + .../client-python/scripts/generate_version.py | 24 +++++++++++++++---- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/clients/client-python/gravitino/constants/root.py b/clients/client-python/gravitino/constants/root.py index 613983f1f3b..101a62bc556 100644 --- a/clients/client-python/gravitino/constants/root.py +++ b/clients/client-python/gravitino/constants/root.py @@ -21,4 +21,5 @@ MODULE_NAME = "gravitino" PROJECT_HOME = Path(__file__).parent.parent.parent +PROJECT_ROOT = PROJECT_HOME.parent.parent GRAVITINO_DIR = PROJECT_HOME / MODULE_NAME diff --git a/clients/client-python/scripts/generate_version.py b/clients/client-python/scripts/generate_version.py index bf7e8c44986..226f71530bd 100644 --- a/clients/client-python/scripts/generate_version.py +++ b/clients/client-python/scripts/generate_version.py @@ -17,10 +17,12 @@ under the License. """ +# coding=utf-8 + import re import configparser -import subprocess from datetime import datetime +from gravitino.constants.root import PROJECT_ROOT from gravitino.constants.version import Version, VERSION_INI, SETUP_FILE from gravitino.exceptions.base import GravitinoRuntimeException @@ -28,6 +30,22 @@ VERSION_PATTERN = r"version\s*=\s*['\"]([^'\"]+)['\"]" +def get_git_commit_id(): + try: + commit_id = "" + git_path = f"{PROJECT_ROOT}/.git/" + with open(git_path + "HEAD", "r", encoding="utf-8") as file: + ref = file.readline().strip() + + if ref.startswith("ref:"): + ref_path = ref.split(" ")[1] + with open(git_path + ref_path, "r", encoding="utf-8") as file: + commit_id = file.readline().strip() + return commit_id + except (FileNotFoundError, IOError): + return "" + + def main(): with open(SETUP_FILE, "r", encoding="utf-8") as f: setup_content = f.read() @@ -37,9 +55,7 @@ def main(): else: raise GravitinoRuntimeException("Can't find valid version info in setup.py") - git_commit = ( - subprocess.check_output(["git", "rev-parse", "HEAD"]).decode("ascii").strip() - ) + git_commit = get_git_commit_id() compile_date = datetime.now().strftime("%d/%m/%Y %H:%M:%S") From 60e58f3dc050acd504518cd7d9aff963ac689487 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 28 Aug 2024 08:31:58 +0800 Subject: [PATCH 3/3] [#4699] fix(server): The error response of addUser should meet the REST specification (#4710) ### What changes were proposed in this pull request? The error response of addUser should meet the REST specification ### Why are the changes needed? Fix: #4699 ### Does this PR introduce _any_ user-facing change? No, access control isn't released. ### How was this patch tested? By hand. If we don't enable authorization, it will return ``` {"code":1006,"type":"UnsupportedOperationException","message":"You should set 'gravitino.authorization.enable' to true in the server side `gravitino.conf` to enable the authorization of the system, otherwise these interfaces can't work."} ``` Co-authored-by: roryqi --- .../gravitino/client/ErrorHandlers.java | 15 ++ .../AccessControlNotAllowIT.java | 153 ++++++++++++++++++ .../filter/AccessControlNotAllowedFilter.java | 18 +-- 3 files changed, 174 insertions(+), 12 deletions(-) create mode 100644 integration-test/src/test/java/org/apache/gravitino/integration/test/authorization/AccessControlNotAllowIT.java diff --git a/clients/client-java/src/main/java/org/apache/gravitino/client/ErrorHandlers.java b/clients/client-java/src/main/java/org/apache/gravitino/client/ErrorHandlers.java index 18576e2775f..7414f4c4e88 100644 --- a/clients/client-java/src/main/java/org/apache/gravitino/client/ErrorHandlers.java +++ b/clients/client-java/src/main/java/org/apache/gravitino/client/ErrorHandlers.java @@ -564,6 +564,9 @@ public void accept(ErrorResponse errorResponse) { case ErrorConstants.ALREADY_EXISTS_CODE: throw new UserAlreadyExistsException(errorMessage); + case ErrorConstants.UNSUPPORTED_OPERATION_CODE: + throw new UnsupportedOperationException(errorMessage); + case ErrorConstants.INTERNAL_ERROR_CODE: throw new RuntimeException(errorMessage); @@ -599,6 +602,9 @@ public void accept(ErrorResponse errorResponse) { case ErrorConstants.ALREADY_EXISTS_CODE: throw new GroupAlreadyExistsException(errorMessage); + case ErrorConstants.UNSUPPORTED_OPERATION_CODE: + throw new UnsupportedOperationException(errorMessage); + case ErrorConstants.INTERNAL_ERROR_CODE: throw new RuntimeException(errorMessage); @@ -638,6 +644,9 @@ public void accept(ErrorResponse errorResponse) { case ErrorConstants.ALREADY_EXISTS_CODE: throw new RoleAlreadyExistsException(errorMessage); + case ErrorConstants.UNSUPPORTED_OPERATION_CODE: + throw new UnsupportedOperationException(errorMessage); + case ErrorConstants.INTERNAL_ERROR_CODE: throw new RuntimeException(errorMessage); @@ -675,6 +684,9 @@ public void accept(ErrorResponse errorResponse) { throw new NotFoundException(errorMessage); } + case ErrorConstants.UNSUPPORTED_OPERATION_CODE: + throw new UnsupportedOperationException(errorMessage); + case ErrorConstants.INTERNAL_ERROR_CODE: throw new RuntimeException(errorMessage); @@ -748,6 +760,9 @@ public void accept(ErrorResponse errorResponse) { throw new NotFoundException(errorMessage); } + case ErrorConstants.UNSUPPORTED_OPERATION_CODE: + throw new UnsupportedOperationException(errorMessage); + case ErrorConstants.INTERNAL_ERROR_CODE: throw new RuntimeException(errorMessage); diff --git a/integration-test/src/test/java/org/apache/gravitino/integration/test/authorization/AccessControlNotAllowIT.java b/integration-test/src/test/java/org/apache/gravitino/integration/test/authorization/AccessControlNotAllowIT.java new file mode 100644 index 00000000000..22986458ba6 --- /dev/null +++ b/integration-test/src/test/java/org/apache/gravitino/integration/test/authorization/AccessControlNotAllowIT.java @@ -0,0 +1,153 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.integration.test.authorization; + +import com.google.common.collect.Lists; +import java.util.Collections; +import org.apache.gravitino.MetadataObject; +import org.apache.gravitino.MetadataObjects; +import org.apache.gravitino.authorization.Owner; +import org.apache.gravitino.authorization.Privileges; +import org.apache.gravitino.authorization.SecurableObjects; +import org.apache.gravitino.client.GravitinoMetalake; +import org.apache.gravitino.integration.test.util.AbstractIT; +import org.apache.gravitino.utils.RandomNameUtils; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +public class AccessControlNotAllowIT extends AbstractIT { + + public static String metalakeTestName = RandomNameUtils.genRandomName("test"); + + @Test + public void testNotAllowFilter() { + GravitinoMetalake metalake = + client.createMetalake(metalakeTestName, "metalake test", Collections.emptyMap()); + + Exception e = + Assertions.assertThrows( + UnsupportedOperationException.class, () -> metalake.addUser("test")); + Assertions.assertTrue( + e.getMessage().contains("You should set 'gravitino.authorization.enable'")); + + e = + Assertions.assertThrows( + UnsupportedOperationException.class, () -> metalake.removeUser("test")); + Assertions.assertTrue( + e.getMessage().contains("You should set 'gravitino.authorization.enable'")); + + e = + Assertions.assertThrows( + UnsupportedOperationException.class, () -> metalake.getUser("test")); + Assertions.assertTrue( + e.getMessage().contains("You should set 'gravitino.authorization.enable'")); + + e = + Assertions.assertThrows( + UnsupportedOperationException.class, () -> metalake.addGroup("test")); + Assertions.assertTrue( + e.getMessage().contains("You should set 'gravitino.authorization.enable'")); + + e = + Assertions.assertThrows( + UnsupportedOperationException.class, () -> metalake.getGroup("test")); + Assertions.assertTrue( + e.getMessage().contains("You should set 'gravitino.authorization.enable'")); + + e = + Assertions.assertThrows( + UnsupportedOperationException.class, () -> metalake.removeGroup("test")); + Assertions.assertTrue( + e.getMessage().contains("You should set 'gravitino.authorization.enable'")); + + e = + Assertions.assertThrows( + UnsupportedOperationException.class, + () -> + metalake.createRole( + "test", + Collections.emptyMap(), + Lists.newArrayList( + SecurableObjects.ofMetalake( + "test", Lists.newArrayList(Privileges.SelectTable.allow()))))); + Assertions.assertTrue( + e.getMessage().contains("You should set 'gravitino.authorization.enable'")); + + e = + Assertions.assertThrows( + UnsupportedOperationException.class, () -> metalake.getRole("test")); + Assertions.assertTrue( + e.getMessage().contains("You should set 'gravitino.authorization.enable'")); + + e = + Assertions.assertThrows( + UnsupportedOperationException.class, () -> metalake.deleteRole("test")); + Assertions.assertTrue( + e.getMessage().contains("You should set 'gravitino.authorization.enable'")); + + e = + Assertions.assertThrows( + UnsupportedOperationException.class, + () -> metalake.grantRolesToGroup(Lists.newArrayList("test"), "test")); + Assertions.assertTrue( + e.getMessage().contains("You should set 'gravitino.authorization.enable'")); + + e = + Assertions.assertThrows( + UnsupportedOperationException.class, + () -> metalake.grantRolesToUser(Lists.newArrayList("test"), "test")); + Assertions.assertTrue( + e.getMessage().contains("You should set 'gravitino.authorization.enable'")); + + e = + Assertions.assertThrows( + UnsupportedOperationException.class, + () -> metalake.revokeRolesFromGroup(Lists.newArrayList("test"), "test")); + Assertions.assertTrue( + e.getMessage().contains("You should set 'gravitino.authorization.enable'")); + + e = + Assertions.assertThrows( + UnsupportedOperationException.class, + () -> metalake.revokeRolesFromUser(Lists.newArrayList("test"), "test")); + Assertions.assertTrue( + e.getMessage().contains("You should set 'gravitino.authorization.enable'")); + + e = + Assertions.assertThrows( + UnsupportedOperationException.class, + () -> + metalake.setOwner( + MetadataObjects.of(null, "test", MetadataObject.Type.METALAKE), + "test", + Owner.Type.USER)); + Assertions.assertTrue( + e.getMessage().contains("You should set 'gravitino.authorization.enable'")); + + e = + Assertions.assertThrows( + UnsupportedOperationException.class, + () -> + metalake.getOwner(MetadataObjects.of(null, "test", MetadataObject.Type.METALAKE))); + Assertions.assertTrue( + e.getMessage().contains("You should set 'gravitino.authorization.enable'")); + + client.dropMetalake(metalakeTestName); + } +} diff --git a/server/src/main/java/org/apache/gravitino/server/web/filter/AccessControlNotAllowedFilter.java b/server/src/main/java/org/apache/gravitino/server/web/filter/AccessControlNotAllowedFilter.java index 130827371c7..42c001f7667 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/filter/AccessControlNotAllowedFilter.java +++ b/server/src/main/java/org/apache/gravitino/server/web/filter/AccessControlNotAllowedFilter.java @@ -19,16 +19,13 @@ package org.apache.gravitino.server.web.filter; -import static javax.servlet.http.HttpServletResponse.SC_METHOD_NOT_ALLOWED; - import java.io.IOException; -import java.util.Collections; import javax.ws.rs.container.ContainerRequestContext; import javax.ws.rs.container.ContainerRequestFilter; -import javax.ws.rs.core.Response; import javax.ws.rs.ext.Provider; import org.apache.gravitino.Configs; import org.apache.gravitino.server.authorization.NameBindings; +import org.apache.gravitino.server.web.Utils; /** * AccessControlNotAllowedFilter is used for filter the requests related to access control if Apache @@ -43,13 +40,10 @@ public class AccessControlNotAllowedFilter implements ContainerRequestFilter { @Override public void filter(ContainerRequestContext requestContext) throws IOException { requestContext.abortWith( - Response.status( - SC_METHOD_NOT_ALLOWED, - String.format( - "You should set '%s' to true in the server side `gravitino.conf`" - + " to enable the authorization of the system, otherwise these interfaces can't work.", - Configs.ENABLE_AUTHORIZATION.getKey())) - .allow(Collections.emptySet()) - .build()); + Utils.unsupportedOperation( + String.format( + "You should set '%s' to true in the server side `gravitino.conf`" + + " to enable the authorization of the system, otherwise these interfaces can't work.", + Configs.ENABLE_AUTHORIZATION.getKey()))); } }