This is an automated email from the ASF dual-hosted git repository.
roryqi pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new c545a0567f [#7972] fix(authz): Fix NPE when create table failed by not
auth (#7971)
c545a0567f is described below
commit c545a0567f9ae5529d369962528429034b42d1e4
Author: yangyang zhong <[email protected]>
AuthorDate: Mon Aug 11 15:20:21 2025 +0800
[#7972] fix(authz): Fix NPE when create table failed by not auth (#7971)
### What changes were proposed in this pull request?
Fix NPE when create table failed by not auth
### Why are the changes needed?
Fix: #7972
### Does this PR introduce _any_ user-facing change?
None
### How was this patch tested?
Existing test
---
.../org/apache/gravitino/client/ErrorHandlers.java | 3 ++
.../org/apache/gravitino/client/HTTPClient.java | 2 -
.../test/authorization/CatalogAuthorizationIT.java | 7 +--
.../test/authorization/FilesetAuthorizationIT.java | 27 +++++++-----
.../test/authorization/GroupAuthorizationIT.java | 5 ++-
.../authorization/MetalakeAuthorizationIT.java | 17 ++++----
.../test/authorization/ModelAuthorizationIT.java | 23 +++++-----
.../test/authorization/OwnerAuthorizationIT.java | 15 ++++---
.../authorization/PermissionAuthorizationIT.java | 13 +++---
.../test/authorization/RoleAuthorizationIT.java | 15 ++++---
.../test/authorization/SchemaAuthorizationIT.java | 11 ++---
.../test/authorization/TableAuthorizationIT.java | 40 ++++++++++++++---
.../test/authorization/TopicAuthorizationIT.java | 11 ++---
.../test/authorization/UserAuthorizationIT.java | 9 ++--
.../AuthorizationExpressionConverter.java | 2 +-
.../authorization/jcasbin/JcasbinAuthorizer.java | 50 +++++++++++++++++-----
.../web/filter/GravitinoInterceptionService.java | 17 +++++---
.../server/web/rest/CatalogOperations.java | 2 +-
.../server/web/rest/FilesetOperations.java | 2 +-
.../gravitino/server/web/rest/TableOperations.java | 2 +-
.../filter/TestGravitinoInterceptionService.java | 2 +-
21 files changed, 175 insertions(+), 100 deletions(-)
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 b9d5627e7e..787a654754 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
@@ -1150,6 +1150,9 @@ public class ErrorHandlers {
@Override
public void accept(ErrorResponse errorResponse) {
+ if (errorResponse.getCode() == ErrorConstants.FORBIDDEN_CODE) {
+ throw new ForbiddenException("Forbidden error :%s",
errorResponse.getMessage());
+ }
throw new RESTException("Unable to process: %s",
formatErrorMessage(errorResponse));
}
}
diff --git
a/clients/client-java/src/main/java/org/apache/gravitino/client/HTTPClient.java
b/clients/client-java/src/main/java/org/apache/gravitino/client/HTTPClient.java
index 54ed56707e..915e9c07a9 100644
---
a/clients/client-java/src/main/java/org/apache/gravitino/client/HTTPClient.java
+++
b/clients/client-java/src/main/java/org/apache/gravitino/client/HTTPClient.java
@@ -204,7 +204,6 @@ public class HTTPClient implements RESTClient {
private void throwFailure(
CloseableHttpResponse response, String responseBody,
Consumer<ErrorResponse> errorHandler) {
ErrorResponse errorResponse = null;
-
if (responseBody != null) {
try {
if (errorHandler instanceof ErrorHandler) {
@@ -233,7 +232,6 @@ public class HTTPClient implements RESTClient {
}
errorHandler.accept(errorResponse);
-
// Throw an exception in case the provided error handler does not throw.
throw new RESTException("Unhandled error: %s", errorResponse);
}
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/CatalogAuthorizationIT.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/CatalogAuthorizationIT.java
index 8da9efa068..2e1a698386 100644
---
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/CatalogAuthorizationIT.java
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/CatalogAuthorizationIT.java
@@ -24,6 +24,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
import com.google.common.collect.Maps;
import java.util.Map;
import org.apache.gravitino.Catalog;
+import org.apache.gravitino.exceptions.ForbiddenException;
import org.apache.gravitino.integration.test.container.ContainerSuite;
import org.apache.gravitino.integration.test.container.HiveContainer;
import org.junit.jupiter.api.BeforeAll;
@@ -63,7 +64,7 @@ public class CatalogAuthorizationIT extends
BaseRestApiAuthorizationIT {
properties.put("metastore.uris", hmsUri);
assertThrows(
"Can not access metadata {" + catalog1 + "}.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
normalUserClient
.loadMetalake(METALAKE)
@@ -77,7 +78,7 @@ public class CatalogAuthorizationIT extends
BaseRestApiAuthorizationIT {
.createCatalog(catalog2, Catalog.Type.RELATIONAL, "hive", "comment",
properties);
assertThrows(
"Can not access metadata {" + catalog1 + "}.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
normalUserClient
.loadMetalake(METALAKE)
@@ -119,7 +120,7 @@ public class CatalogAuthorizationIT extends
BaseRestApiAuthorizationIT {
assertArrayEquals(new String[] {catalog1, catalog2}, catalogs);
assertThrows(
"Can not access metadata {" + catalog1 + "}.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
normalUserClient.loadMetalake(METALAKE).dropCatalog(catalog1, true);
});
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/FilesetAuthorizationIT.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/FilesetAuthorizationIT.java
index f43e03b540..bbffa30902 100644
---
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/FilesetAuthorizationIT.java
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/FilesetAuthorizationIT.java
@@ -23,6 +23,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
@@ -36,6 +37,7 @@ import org.apache.gravitino.authorization.Privileges;
import org.apache.gravitino.authorization.SecurableObject;
import org.apache.gravitino.authorization.SecurableObjects;
import org.apache.gravitino.client.GravitinoMetalake;
+import org.apache.gravitino.exceptions.ForbiddenException;
import org.apache.gravitino.file.Fileset;
import org.apache.gravitino.file.FilesetCatalog;
import org.apache.gravitino.file.FilesetChange;
@@ -74,7 +76,7 @@ public class FilesetAuthorizationIT extends
BaseRestApiAuthorizationIT {
// try to load the schema as normal user, expect failure
assertThrows(
"Can not access metadata {" + CATALOG + "." + SCHEMA + "}.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
normalUserClient
.loadMetalake(METALAKE)
@@ -95,7 +97,7 @@ public class FilesetAuthorizationIT extends
BaseRestApiAuthorizationIT {
assertEquals(CATALOG, catalogLoadByNormalUser.name());
assertThrows(
"Can not access metadata {" + CATALOG + "." + SCHEMA + "}.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
catalogLoadByNormalUser.asSchemas().loadSchema(SCHEMA);
});
@@ -120,7 +122,7 @@ public class FilesetAuthorizationIT extends
BaseRestApiAuthorizationIT {
normalUserClient.loadMetalake(METALAKE).loadCatalog(CATALOG).asFilesetCatalog();
assertThrows(
"Can not access metadata {" + CATALOG + "." + SCHEMA + "}.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
filesetCatalogNormalUser.createFileset(
// NameIdentifier.of(SCHEMA, "fileset2"),
@@ -186,9 +188,9 @@ public class FilesetAuthorizationIT extends
BaseRestApiAuthorizationIT {
// normal user can load fileset2 and fileset3, but not fileset1
assertThrows(
String.format("Can not access metadata {%s.%s.%s}.", CATALOG, SCHEMA,
"fileset1"),
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
- filesetCatalogNormalUser.loadFileset(NameIdentifier.of(CATALOG,
SCHEMA, "fileset1"));
+ filesetCatalogNormalUser.loadFileset(NameIdentifier.of(SCHEMA,
"fileset1"));
});
Fileset fileset2 =
filesetCatalogNormalUser.loadFileset(NameIdentifier.of(SCHEMA, "fileset2"));
assertEquals("fileset2", fileset2.name());
@@ -216,7 +218,7 @@ public class FilesetAuthorizationIT extends
BaseRestApiAuthorizationIT {
// normal user cannot alter fileset1 (no privilege)
assertThrows(
String.format("Can not access metadata {%s.%s.%s}.", CATALOG, SCHEMA,
"fileset1"),
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
filesetCatalogNormalUser.alterFileset(
NameIdentifier.of(SCHEMA, "fileset1"),
FilesetChange.setProperty("key", "value"));
@@ -237,14 +239,19 @@ public class FilesetAuthorizationIT extends
BaseRestApiAuthorizationIT {
GravitinoMetalake gravitinoMetalake = client.loadMetalake(METALAKE);
FilesetCatalog filesetCatalogNormalUser =
normalUserClient.loadMetalake(METALAKE).loadCatalog(CATALOG).asFilesetCatalog();
+ // reset privilege
+ gravitinoMetalake.revokePrivilegesFromRole(
+ role,
+ MetadataObjects.of(
+ ImmutableList.of(CATALOG, SCHEMA, "fileset1"),
MetadataObject.Type.FILESET),
+ ImmutableSet.of(Privileges.WriteFileset.allow(),
Privileges.ReadFileset.allow()));
// normal user cannot alter fileset1 (no privilege)
assertThrows(
String.format("Can not access metadata {%s.%s.%s}.", CATALOG, SCHEMA,
"fileset1"),
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
- filesetCatalogNormalUser.getFileLocation(
- NameIdentifier.of(METALAKE, CATALOG, SCHEMA, "fileset1"),
"/test");
+ filesetCatalogNormalUser.getFileLocation(NameIdentifier.of(SCHEMA,
"fileset1"), "/test");
});
// grant normal user owner privilege on fileset4
gravitinoMetalake.setOwner(
@@ -270,7 +277,7 @@ public class FilesetAuthorizationIT extends
BaseRestApiAuthorizationIT {
// normal user cannot drop fileset1
assertThrows(
String.format("Can not access metadata {%s.%s.%s}.", CATALOG, SCHEMA,
"fileset1"),
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
filesetCatalogNormalUser.dropFileset(NameIdentifier.of(SCHEMA,
"fileset1"));
});
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/GroupAuthorizationIT.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/GroupAuthorizationIT.java
index ea995ad86e..deb5d8bf44 100644
---
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/GroupAuthorizationIT.java
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/GroupAuthorizationIT.java
@@ -26,6 +26,7 @@ import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.MetadataObjects;
import org.apache.gravitino.authorization.Privileges;
import org.apache.gravitino.client.GravitinoMetalake;
+import org.apache.gravitino.exceptions.ForbiddenException;
import org.junit.jupiter.api.MethodOrderer;
import org.junit.jupiter.api.Order;
import org.junit.jupiter.api.Tag;
@@ -41,7 +42,7 @@ public class GroupAuthorizationIT extends
BaseRestApiAuthorizationIT {
public void testCreateGroup() {
assertThrows(
"Current user access metadata {testMetalake}",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
normalUserClient.loadMetalake(METALAKE).addGroup("group1");
});
@@ -54,7 +55,7 @@ public class GroupAuthorizationIT extends
BaseRestApiAuthorizationIT {
public void testRemoveGroup() {
assertThrows(
"Current user access metadata {testMetalake}",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
normalUserClient.loadMetalake(METALAKE).removeGroup("group1");
});
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/MetalakeAuthorizationIT.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/MetalakeAuthorizationIT.java
index cf994b3491..2b72b80e1e 100644
---
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/MetalakeAuthorizationIT.java
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/MetalakeAuthorizationIT.java
@@ -27,6 +27,7 @@ import org.apache.gravitino.MetalakeChange;
import org.apache.gravitino.client.GravitinoAdminClient;
import org.apache.gravitino.client.GravitinoMetalake;
import org.apache.gravitino.dto.MetalakeDTO;
+import org.apache.gravitino.exceptions.ForbiddenException;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.MethodOrderer;
@@ -69,7 +70,7 @@ public class MetalakeAuthorizationIT extends
BaseRestApiAuthorizationIT {
public void testCreateMetalake() {
assertThrows(
"Only service admins can create metalakes, current user can't create
the metalake, you should configure it in the server configuration first",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
normalUserClient.createMetalake("testMetalake2", "", new
HashMap<>());
});
@@ -102,13 +103,13 @@ public class MetalakeAuthorizationIT extends
BaseRestApiAuthorizationIT {
normalUserClient.loadMetalake(METALAKE);
assertThrows(
"Current user access metadata {testMetalake2}",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
normalUserClient.loadMetalake("testMetalake2");
});
assertThrows(
"Current user access metadata {testMetalake3}",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
normalUserClient.loadMetalake("testMetalake3");
});
@@ -116,7 +117,7 @@ public class MetalakeAuthorizationIT extends
BaseRestApiAuthorizationIT {
serviceAdminButNotOwnerClient.loadMetalake("testMetalake2");
assertThrows(
"Current user access metadata {testMetalake3}",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
serviceAdminButNotOwnerClient.loadMetalake("testMetalake3");
});
@@ -127,14 +128,14 @@ public class MetalakeAuthorizationIT extends
BaseRestApiAuthorizationIT {
public void testSetMetalake() {
assertThrows(
"Current user access metadata {testMetalake2}",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
serviceAdminButNotOwnerClient.alterMetalake(
"testMetalake2", MetalakeChange.setProperty("key1", "value1"));
});
assertThrows(
"Current user access metadata {testMetalake2}",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
normalUserClient.alterMetalake(
"testMetalake2", MetalakeChange.setProperty("key1", "value1"));
@@ -147,13 +148,13 @@ public class MetalakeAuthorizationIT extends
BaseRestApiAuthorizationIT {
public void testDropMetalake() {
assertThrows(
"Current user access metadata {testMetalake2}",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
serviceAdminButNotOwnerClient.dropMetalake("testMetalake3", true);
});
assertThrows(
"Current user access metadata {testMetalake2}",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
normalUserClient.dropMetalake("testMetalake3", true);
});
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/ModelAuthorizationIT.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/ModelAuthorizationIT.java
index c66f417920..e75e4f514c 100644
---
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/ModelAuthorizationIT.java
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/ModelAuthorizationIT.java
@@ -37,6 +37,7 @@ import org.apache.gravitino.authorization.Privileges;
import org.apache.gravitino.authorization.SecurableObject;
import org.apache.gravitino.authorization.SecurableObjects;
import org.apache.gravitino.client.GravitinoMetalake;
+import org.apache.gravitino.exceptions.ForbiddenException;
import org.apache.gravitino.model.Model;
import org.apache.gravitino.model.ModelCatalog;
import org.apache.gravitino.model.ModelChange;
@@ -70,7 +71,7 @@ public class ModelAuthorizationIT extends
BaseRestApiAuthorizationIT {
// try to load the schema as normal user, expect failure
assertThrows(
"Can not access metadata {" + CATALOG + "." + SCHEMA + "}.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
normalUserClient
.loadMetalake(METALAKE)
@@ -91,7 +92,7 @@ public class ModelAuthorizationIT extends
BaseRestApiAuthorizationIT {
assertEquals(CATALOG, catalogLoadByNormalUser.name());
assertThrows(
"Can not access metadata {" + CATALOG + "." + SCHEMA + "}.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
catalogLoadByNormalUser.asSchemas().loadSchema(SCHEMA);
});
@@ -106,7 +107,7 @@ public class ModelAuthorizationIT extends
BaseRestApiAuthorizationIT {
normalUserClient.loadMetalake(METALAKE).loadCatalog(CATALOG).asModelCatalog();
assertThrows(
"Can not access metadata {" + METALAKE + "," + CATALOG + "." + SCHEMA
+ "}.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
normalUserCatalog.registerModel(NameIdentifier.of(SCHEMA, "model2"),
"", new HashMap<>());
});
@@ -155,7 +156,7 @@ public class ModelAuthorizationIT extends
BaseRestApiAuthorizationIT {
ModelCatalog modelCatalog = catalogEntityLoadByNormalUser.asModelCatalog();
assertThrows(
"Can not access metadata {" + METALAKE + "," + CATALOG + "." + SCHEMA
+ "}.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
modelCatalog.getModel(NameIdentifier.of(SCHEMA, "model1"));
});
@@ -182,7 +183,7 @@ public class ModelAuthorizationIT extends
BaseRestApiAuthorizationIT {
ModelCatalog modelCatalogLoadByNormalUser =
catalogEntityLoadByNormalUser.asModelCatalog();
assertThrows(
"Can not access metadata {" + METALAKE + "," + CATALOG + "." + SCHEMA
+ "}.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
modelCatalogLoadByNormalUser.alterModel(
NameIdentifier.of(SCHEMA, "model1"), new
ModelChange.RenameModel("model5"));
@@ -204,7 +205,7 @@ public class ModelAuthorizationIT extends
BaseRestApiAuthorizationIT {
ModelCatalog modelCatalogLoadByNormalUser =
catalogEntityLoadByNormalUser.asModelCatalog();
assertThrows(
"Can not access metadata {" + METALAKE + "," + CATALOG + "." + SCHEMA
+ "}.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
modelCatalogLoadByNormalUser.deleteModel(NameIdentifier.of(SCHEMA,
"model5"));
});
@@ -224,7 +225,7 @@ public class ModelAuthorizationIT extends
BaseRestApiAuthorizationIT {
NameIdentifier.of(SCHEMA, "model1"), "uri2", new String[] {"alias2"},
"comment2", null);
assertThrows(
"Can not access metadata {" + METALAKE + "," + CATALOG + "." + SCHEMA
+ "model1" + "}.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
modelCatalogLoadByNormalUser.linkModelVersion(
NameIdentifier.of(SCHEMA, "model1"),
@@ -246,7 +247,7 @@ public class ModelAuthorizationIT extends
BaseRestApiAuthorizationIT {
assertEquals(2, versions.length);
assertThrows(
"Can not access metadata {" + METALAKE + "," + CATALOG + "." + SCHEMA
+ "model1" + "}.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
modelCatalogLoadByNormalUser.linkModelVersion(
NameIdentifier.of(SCHEMA, "model1"),
@@ -268,7 +269,7 @@ public class ModelAuthorizationIT extends
BaseRestApiAuthorizationIT {
assertEquals(1, version.version());
assertThrows(
"Can not access metadata {" + METALAKE + "," + CATALOG + "." + SCHEMA
+ "model1" + "}.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
modelCatalogLoadByNormalUser.getModelVersion(NameIdentifier.of(SCHEMA,
"model1"), 1);
});
@@ -287,7 +288,7 @@ public class ModelAuthorizationIT extends
BaseRestApiAuthorizationIT {
assertEquals("value", version.properties().get("key"));
assertThrows(
"Can not access metadata {" + METALAKE + "," + CATALOG + "." + SCHEMA
+ "model1" + "}.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
modelCatalogLoadByNormalUser.alterModelVersion(
NameIdentifier.of(SCHEMA, "model1"),
@@ -305,7 +306,7 @@ public class ModelAuthorizationIT extends
BaseRestApiAuthorizationIT {
ModelCatalog modelCatalogLoadByNormalUser =
catalogEntityLoadByNormalUser.asModelCatalog();
assertThrows(
"Can not access metadata {" + METALAKE + "," + CATALOG + "." + SCHEMA
+ "model1" + "}.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
modelCatalogLoadByNormalUser.deleteModelVersion(NameIdentifier.of(SCHEMA,
"model1"), 1);
});
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/OwnerAuthorizationIT.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/OwnerAuthorizationIT.java
index 11a18d734d..a3781441fb 100644
---
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/OwnerAuthorizationIT.java
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/OwnerAuthorizationIT.java
@@ -32,6 +32,7 @@ import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.authorization.Owner;
import org.apache.gravitino.authorization.Privileges;
import org.apache.gravitino.client.GravitinoMetalake;
+import org.apache.gravitino.exceptions.ForbiddenException;
import org.apache.gravitino.integration.test.container.ContainerSuite;
import org.apache.gravitino.integration.test.container.HiveContainer;
import org.apache.gravitino.rel.Column;
@@ -107,7 +108,7 @@ public class OwnerAuthorizationIT extends
BaseRestApiAuthorizationIT {
GravitinoMetalake gravitinoMetalakeLoadByNormalUser =
normalUserClient.loadMetalake(METALAKE);
assertThrows(
"Current user can not set owner",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
gravitinoMetalakeLoadByNormalUser.setOwner(
MetadataObjects.of(
@@ -133,7 +134,7 @@ public class OwnerAuthorizationIT extends
BaseRestApiAuthorizationIT {
GravitinoMetalake gravitinoMetalakeLoadByNormalUser =
normalUserClient.loadMetalake(METALAKE);
assertThrows(
"Current user can not set owner",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
gravitinoMetalakeLoadByNormalUser.setOwner(
MetadataObjects.of(
@@ -179,7 +180,7 @@ public class OwnerAuthorizationIT extends
BaseRestApiAuthorizationIT {
GravitinoMetalake gravitinoMetalakeLoadByNormalUser =
normalUserClient.loadMetalake(METALAKE);
assertThrows(
"Current user can not set owner",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
gravitinoMetalakeLoadByNormalUser.setOwner(
MetadataObjects.of(
@@ -195,7 +196,7 @@ public class OwnerAuthorizationIT extends
BaseRestApiAuthorizationIT {
// normal user can set owner
assertThrows(
"Current user can not set owner",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
// NORMAL_USER has not USE_CATALOG
gravitinoMetalakeLoadByNormalUser.setOwner(
@@ -239,7 +240,7 @@ public class OwnerAuthorizationIT extends
BaseRestApiAuthorizationIT {
GravitinoMetalake gravitinoMetalakeLoadByNormalUser =
normalUserClient.loadMetalake(METALAKE);
assertThrows(
"Current user can not set owner",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
gravitinoMetalakeLoadByNormalUser.setOwner(
MetadataObjects.of(
@@ -254,7 +255,7 @@ public class OwnerAuthorizationIT extends
BaseRestApiAuthorizationIT {
Owner.Type.USER);
assertThrows(
"Current user can not set owner",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
// NORMAL_USER has not USE_SCHEMA
gravitinoMetalakeLoadByNormalUser.setOwner(
@@ -290,7 +291,7 @@ public class OwnerAuthorizationIT extends
BaseRestApiAuthorizationIT {
GravitinoMetalake gravitinoMetalakeLoadByNormalUser =
normalUserClient.loadMetalake(METALAKE);
assertThrows(
"Current user can not set owner",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
gravitinoMetalakeLoadByNormalUser.setOwner(
MetadataObjects.of(ImmutableList.of(tempRole),
MetadataObject.Type.ROLE),
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/PermissionAuthorizationIT.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/PermissionAuthorizationIT.java
index d353415be1..4f586d669a 100644
---
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/PermissionAuthorizationIT.java
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/PermissionAuthorizationIT.java
@@ -26,6 +26,7 @@ import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.MetadataObjects;
import org.apache.gravitino.authorization.Privileges;
import org.apache.gravitino.client.GravitinoMetalake;
+import org.apache.gravitino.exceptions.ForbiddenException;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
import org.testcontainers.shaded.com.google.common.collect.ImmutableList;
@@ -52,26 +53,26 @@ public class PermissionAuthorizationIT extends
BaseRestApiAuthorizationIT {
GravitinoMetalake gravitinoMetalakeLoadByNormalUser =
normalUserClient.loadMetalake(METALAKE);
assertThrows(
"Current user can not grant.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
gravitinoMetalakeLoadByNormalUser.grantRolesToUser(
ImmutableList.of("role1"), NORMAL_USER);
});
assertThrows(
"Current user can not grant.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
gravitinoMetalakeLoadByNormalUser.grantRolesToGroup(ImmutableList.of("role2"),
"group1");
});
assertThrows(
"Current user can not grant.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
gravitinoMetalakeLoadByNormalUser.grantRolesToGroup(ImmutableList.of("role2"),
"group1");
});
assertThrows(
"Current user can not grant.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
gravitinoMetalakeLoadByNormalUser.revokePrivilegesFromRole(
"role1",
@@ -80,14 +81,14 @@ public class PermissionAuthorizationIT extends
BaseRestApiAuthorizationIT {
});
assertThrows(
"Current user can not grant.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
gravitinoMetalakeLoadByNormalUser.revokeRolesFromGroup(
ImmutableList.of("role2"), "group1");
});
assertThrows(
"Current user can not grant.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
gravitinoMetalakeLoadByNormalUser.revokeRolesFromUser(
ImmutableList.of("role1"), NORMAL_USER);
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/RoleAuthorizationIT.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/RoleAuthorizationIT.java
index 542d3dc44a..3b7f4818ca 100644
---
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/RoleAuthorizationIT.java
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/RoleAuthorizationIT.java
@@ -25,6 +25,7 @@ import java.util.HashMap;
import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.MetadataObjects;
import org.apache.gravitino.authorization.Privileges;
+import org.apache.gravitino.exceptions.ForbiddenException;
import org.junit.jupiter.api.MethodOrderer;
import org.junit.jupiter.api.Order;
import org.junit.jupiter.api.Tag;
@@ -45,7 +46,7 @@ public class RoleAuthorizationIT extends
BaseRestApiAuthorizationIT {
// normal user can not create role
assertThrows(
"Current user can not create role.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
normalUserClient
.loadMetalake(METALAKE)
@@ -84,13 +85,13 @@ public class RoleAuthorizationIT extends
BaseRestApiAuthorizationIT {
// normal user can not get role
assertThrows(
"Current user can not create role.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
normalUserClient.loadMetalake(METALAKE).getRole("role2");
});
assertThrows(
"Current user can not create role.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
normalUserClient.loadMetalake(METALAKE).getRole("role3");
});
@@ -102,19 +103,19 @@ public class RoleAuthorizationIT extends
BaseRestApiAuthorizationIT {
// normal user can not delete role
assertThrows(
"Current user can not create role.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
normalUserClient.loadMetalake(METALAKE).deleteRole("role1");
});
assertThrows(
"Current user can not create role.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
normalUserClient.loadMetalake(METALAKE).deleteRole("role2");
});
assertThrows(
"Current user can not create role.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
normalUserClient.loadMetalake(METALAKE).deleteRole("role3");
});
@@ -125,7 +126,7 @@ public class RoleAuthorizationIT extends
BaseRestApiAuthorizationIT {
// normal user can not create role after delete role
assertThrows(
"Current user can not create role.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
normalUserClient
.loadMetalake(METALAKE)
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/SchemaAuthorizationIT.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/SchemaAuthorizationIT.java
index e6cae76f7c..27c4983f37 100644
---
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/SchemaAuthorizationIT.java
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/SchemaAuthorizationIT.java
@@ -38,6 +38,7 @@ import org.apache.gravitino.authorization.Privileges;
import org.apache.gravitino.authorization.SecurableObject;
import org.apache.gravitino.authorization.SecurableObjects;
import org.apache.gravitino.client.GravitinoMetalake;
+import org.apache.gravitino.exceptions.ForbiddenException;
import org.apache.gravitino.integration.test.container.ContainerSuite;
import org.apache.gravitino.integration.test.container.HiveContainer;
import org.junit.jupiter.api.BeforeAll;
@@ -103,7 +104,7 @@ public class SchemaAuthorizationIT extends
BaseRestApiAuthorizationIT {
normalUserClient.loadMetalake(METALAKE).loadCatalog(CATALOG);
assertThrows(
"Can not access metadata {" + CATALOG + "}.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
catalogEntityLoadByTester2.asSchemas().createSchema("schema2",
"test2", new HashMap<>());
});
@@ -140,7 +141,7 @@ public class SchemaAuthorizationIT extends
BaseRestApiAuthorizationIT {
assertArrayEquals(new String[] {"schema2", "schema3"},
schemasLoadByTester2);
assertThrows(
String.format("Can not access metadata {%s.%s}.", CATALOG, "schema1"),
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
catalogEntityLoadByTester2.asSchemas().loadSchema("schema1");
});
@@ -173,7 +174,7 @@ public class SchemaAuthorizationIT extends
BaseRestApiAuthorizationIT {
normalUserClient.loadMetalake(METALAKE).loadCatalog(CATALOG);
assertThrows(
String.format("Can not access metadata {%s.%s}.", CATALOG, "schema1"),
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
catalogEntityLoadByTester2
.asSchemas()
@@ -205,7 +206,7 @@ public class SchemaAuthorizationIT extends
BaseRestApiAuthorizationIT {
normalUserClient.loadMetalake(METALAKE).loadCatalog(CATALOG);
assertThrows(
String.format("Can not access metadata {%s.%s}.", CATALOG, "schema1"),
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
catalogEntityLoadByTester2.asSchemas().dropSchema("schema1", true);
});
@@ -221,7 +222,7 @@ public class SchemaAuthorizationIT extends
BaseRestApiAuthorizationIT {
// test catalog owner
assertThrows(
String.format("Can not access metadata {%s.%s}.", CATALOG, "schema1"),
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
catalogEntityLoadByTester2.asSchemas().dropSchema("schema1", true);
});
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/TableAuthorizationIT.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/TableAuthorizationIT.java
index 6c9f260594..f18023f1c8 100644
---
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/TableAuthorizationIT.java
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/TableAuthorizationIT.java
@@ -37,7 +37,9 @@ import org.apache.gravitino.authorization.Owner;
import org.apache.gravitino.authorization.Privileges;
import org.apache.gravitino.authorization.SecurableObject;
import org.apache.gravitino.authorization.SecurableObjects;
+import org.apache.gravitino.client.GravitinoAdminClient;
import org.apache.gravitino.client.GravitinoMetalake;
+import org.apache.gravitino.exceptions.ForbiddenException;
import org.apache.gravitino.integration.test.container.ContainerSuite;
import org.apache.gravitino.integration.test.container.HiveContainer;
import org.apache.gravitino.rel.Column;
@@ -102,7 +104,7 @@ public class TableAuthorizationIT extends
BaseRestApiAuthorizationIT {
assertEquals(CATALOG, catalogLoadByNormalUser.name());
assertThrows(
"Can not access metadata {" + CATALOG + "." + SCHEMA + "}.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
catalogLoadByNormalUser.asSchemas().loadSchema(SCHEMA);
});
@@ -124,7 +126,7 @@ public class TableAuthorizationIT extends
BaseRestApiAuthorizationIT {
normalUserClient.loadMetalake(METALAKE).loadCatalog(CATALOG).asTableCatalog();
assertThrows(
"Can not access metadata {" + CATALOG + "." + SCHEMA + "}.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
tableCatalogNormalUser.createTable(
NameIdentifier.of(SCHEMA, "table2"), createColumns(), "test2",
new HashMap<>());
@@ -140,6 +142,30 @@ public class TableAuthorizationIT extends
BaseRestApiAuthorizationIT {
NameIdentifier.of(SCHEMA, "table2"), createColumns(), "test2", new
HashMap<>());
tableCatalogNormalUser.createTable(
NameIdentifier.of(SCHEMA, "table3"), createColumns(), "test2", new
HashMap<>());
+ String tempUser = "tempUser";
+ gravitinoMetalake.addUser(tempUser);
+ GravitinoAdminClient tempUserClient =
+
GravitinoAdminClient.builder(serverUri).withSimpleAuth(tempUser).build();
+ String tempRole = "tempRole";
+ gravitinoMetalake.createRole(tempRole, new HashMap<>(), new ArrayList<>());
+ gravitinoMetalake.grantPrivilegesToRole(
+ tempRole,
+ MetadataObjects.of(ImmutableList.of(CATALOG),
MetadataObject.Type.CATALOG),
+ ImmutableList.of(Privileges.UseCatalog.allow()));
+ gravitinoMetalake.grantRolesToUser(ImmutableList.of(tempRole), tempUser);
+ TableCatalog catalogLoadByTmpUser =
+
tempUserClient.loadMetalake(METALAKE).loadCatalog(CATALOG).asTableCatalog();
+ assertThrows(
+ "Can not access metadata {" + CATALOG + "." + SCHEMA + "}.",
+ ForbiddenException.class,
+ () -> {
+ catalogLoadByTmpUser.createTable(
+ NameIdentifier.of(SCHEMA, "table2"), createColumns(), "test2",
new HashMap<>());
+ });
+ gravitinoMetalake.grantRolesToUser(ImmutableList.of(role), tempUser);
+ catalogLoadByTmpUser.createTable(
+ NameIdentifier.of(SCHEMA, "table4"), createColumns(), "test2", new
HashMap<>());
+ tableCatalog.dropTable(NameIdentifier.of(SCHEMA, "table4"));
}
@Test
@@ -173,9 +199,9 @@ public class TableAuthorizationIT extends
BaseRestApiAuthorizationIT {
// normal user can load table2 and table3, but not table1
assertThrows(
String.format("Can not access metadata {%s.%s.%s}.", CATALOG, SCHEMA,
"table1"),
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
- tableCatalogNormalUser.loadTable(NameIdentifier.of(CATALOG, SCHEMA,
"table1"));
+ tableCatalogNormalUser.loadTable(NameIdentifier.of(SCHEMA,
"table1"));
});
Table table2 = tableCatalogNormalUser.loadTable(NameIdentifier.of(SCHEMA,
"table2"));
assertEquals("table2", table2.name());
@@ -195,7 +221,7 @@ public class TableAuthorizationIT extends
BaseRestApiAuthorizationIT {
ImmutableList.of(Privileges.SelectTable.deny()));
assertThrows(
String.format("Can not access metadata {%s.%s.%s}.", CATALOG, SCHEMA,
"table1"),
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
tableCatalogNormalUser.loadTable(NameIdentifier.of(SCHEMA,
"table1"));
});
@@ -211,7 +237,7 @@ public class TableAuthorizationIT extends
BaseRestApiAuthorizationIT {
// normal user cannot alter table1 (no privilege)
assertThrows(
String.format("Can not access metadata {%s.%s.%s}.", CATALOG, SCHEMA,
"table1"),
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
tableCatalogNormalUser.alterTable(
NameIdentifier.of(SCHEMA, "table1"),
TableChange.setProperty("key", "value"));
@@ -239,7 +265,7 @@ public class TableAuthorizationIT extends
BaseRestApiAuthorizationIT {
// normal user cannot drop table1
assertThrows(
String.format("Can not access metadata {%s.%s.%s}.", CATALOG, SCHEMA,
"table1"),
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
tableCatalogNormalUser.dropTable(NameIdentifier.of(SCHEMA,
"table1"));
});
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/TopicAuthorizationIT.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/TopicAuthorizationIT.java
index 5d28f8a8bc..2bf775267e 100644
---
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/TopicAuthorizationIT.java
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/TopicAuthorizationIT.java
@@ -38,6 +38,7 @@ import org.apache.gravitino.authorization.Privileges;
import org.apache.gravitino.authorization.SecurableObject;
import org.apache.gravitino.authorization.SecurableObjects;
import org.apache.gravitino.client.GravitinoMetalake;
+import org.apache.gravitino.exceptions.ForbiddenException;
import org.apache.gravitino.integration.test.container.ContainerSuite;
import org.apache.gravitino.integration.test.container.KafkaContainer;
import org.apache.gravitino.messaging.Topic;
@@ -98,7 +99,7 @@ public class TopicAuthorizationIT extends
BaseRestApiAuthorizationIT {
assertEquals(CATALOG, catalogLoadByNormalUser.name());
assertThrows(
"Can not access metadata {" + CATALOG + "." + SCHEMA + "}.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
catalogLoadByNormalUser.asSchemas().loadSchema(SCHEMA);
});
@@ -115,7 +116,7 @@ public class TopicAuthorizationIT extends
BaseRestApiAuthorizationIT {
normalUserClient.loadMetalake(METALAKE).loadCatalog(CATALOG).asTopicCatalog();
assertThrows(
"Can not access metadata {" + CATALOG + "." + SCHEMA + "}.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
topicCatalogNormalUser.createTopic(
NameIdentifier.of(SCHEMA, "topic2"), "test2", null, new
HashMap<>());
@@ -164,7 +165,7 @@ public class TopicAuthorizationIT extends
BaseRestApiAuthorizationIT {
// normal user can load topic2 and topic3, but not topic1
assertThrows(
String.format("Can not access metadata {%s.%s.%s}.", CATALOG, SCHEMA,
"topic1"),
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
topicCatalogNormalUser.loadTopic(NameIdentifier.of(SCHEMA,
"topic1"));
});
@@ -192,7 +193,7 @@ public class TopicAuthorizationIT extends
BaseRestApiAuthorizationIT {
// normal user cannot alter topic1 (no privilege)
assertThrows(
String.format("Can not access metadata {%s.%s.%s}.", CATALOG, SCHEMA,
"topic1"),
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
topicCatalogNormalUser.alterTopic(
NameIdentifier.of(SCHEMA, "topic1"),
TopicChange.updateComment("new comment"));
@@ -220,7 +221,7 @@ public class TopicAuthorizationIT extends
BaseRestApiAuthorizationIT {
// normal user cannot drop topic1
assertThrows(
String.format("Can not access metadata {%s.%s.%s}.", CATALOG, SCHEMA,
"topic1"),
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
topicCatalogNormalUser.dropTopic(NameIdentifier.of(SCHEMA,
"topic1"));
});
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/UserAuthorizationIT.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/UserAuthorizationIT.java
index 41b9725831..4e39fe559b 100644
---
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/UserAuthorizationIT.java
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/UserAuthorizationIT.java
@@ -31,6 +31,7 @@ import org.apache.gravitino.authorization.Privileges;
import org.apache.gravitino.authorization.User;
import org.apache.gravitino.client.GravitinoAdminClient;
import org.apache.gravitino.client.GravitinoMetalake;
+import org.apache.gravitino.exceptions.ForbiddenException;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.MethodOrderer;
import org.junit.jupiter.api.Order;
@@ -47,7 +48,7 @@ public class UserAuthorizationIT extends
BaseRestApiAuthorizationIT {
public void testCreateUser() {
assertThrows(
"Current user can not access metadata {testMetalake}",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
normalUserClient.loadMetalake(METALAKE).addUser("user1");
});
@@ -83,7 +84,7 @@ public class UserAuthorizationIT extends
BaseRestApiAuthorizationIT {
user1Client.loadMetalake(METALAKE).getUser("user1");
assertThrows(
"Current user can not get user.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
user1Client.loadMetalake(METALAKE).getUser("user2");
});
@@ -97,13 +98,13 @@ public class UserAuthorizationIT extends
BaseRestApiAuthorizationIT {
GravitinoAdminClient user1Client = getClientByUser("user1");
assertThrows(
"Current user can not get user",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
user1Client.loadMetalake(METALAKE).removeUser("user2");
});
assertThrows(
"Current user can not get user.",
- RuntimeException.class,
+ ForbiddenException.class,
() -> {
user1Client.loadMetalake(METALAKE).removeUser("user1");
});
diff --git
a/server-common/src/main/java/org/apache/gravitino/server/authorization/expression/AuthorizationExpressionConverter.java
b/server-common/src/main/java/org/apache/gravitino/server/authorization/expression/AuthorizationExpressionConverter.java
index a090edbb77..71bbacbcc0 100644
---
a/server-common/src/main/java/org/apache/gravitino/server/authorization/expression/AuthorizationExpressionConverter.java
+++
b/server-common/src/main/java/org/apache/gravitino/server/authorization/expression/AuthorizationExpressionConverter.java
@@ -245,7 +245,7 @@ public class AuthorizationExpressionConverter {
expression =
expression.replaceAll(
CAN_OPERATE_METADATA_PRIVILEGE,
-
"authorizer.hasMetadataPrivilegePermission(p_metalake,p_metadataObjectType,p_fullName)");
+
"authorizer.hasMetadataPrivilegePermission(p_metalake,p_type,p_fullName)");
return expression;
}
}
diff --git
a/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinAuthorizer.java
b/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinAuthorizer.java
index 8ec23469f5..7db3f3f3fc 100644
---
a/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinAuthorizer.java
+++
b/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinAuthorizer.java
@@ -102,8 +102,17 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
String metalake,
MetadataObject metadataObject,
Privilege.Name privilege) {
- return allowInternalAuthorizer.authorizeInternal(
- principal, metalake, metadataObject, privilege.name());
+ boolean result =
+ allowInternalAuthorizer.authorizeInternal(
+ principal, metalake, metadataObject, privilege.name());
+ LOG.debug(
+ "principal {},metalake {},metadata object {},privilege {}, result {}",
+ principal,
+ metalake,
+ metadataObject,
+ privilege,
+ result);
+ return result;
}
@Override
@@ -112,14 +121,32 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
String metalake,
MetadataObject metadataObject,
Privilege.Name privilege) {
- return denyInternalAuthorizer.authorizeInternal(
- principal, metalake, metadataObject, privilege.name());
+ boolean result =
+ denyInternalAuthorizer.authorizeInternal(
+ principal, metalake, metadataObject, privilege.name());
+ LOG.debug(
+ "principal {},metalake {},metadata object {},privilege {},deny result
{}",
+ principal,
+ metalake,
+ metadataObject,
+ privilege,
+ result);
+ return result;
}
@Override
public boolean isOwner(Principal principal, String metalake, MetadataObject
metadataObject) {
- return allowInternalAuthorizer.authorizeInternal(
- principal, metalake, metadataObject, AuthConstants.OWNER);
+ boolean result =
+ allowInternalAuthorizer.authorizeInternal(
+ principal, metalake, metadataObject, AuthConstants.OWNER);
+ LOG.debug(
+ "principal {},metalake {},metadata object {},privilege {},deny result
{}",
+ principal,
+ metalake,
+ metadataObject,
+ "OWNER",
+ result);
+ return result;
}
@Override
@@ -331,16 +358,17 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
for (RoleEntity role : entities) {
Long roleId = role.id();
+ allowEnforcer.addRoleForUser(String.valueOf(userId),
String.valueOf(roleId));
+ denyEnforcer.addRoleForUser(String.valueOf(userId),
String.valueOf(roleId));
+ if (loadedRoles.contains(roleId)) {
+ continue;
+ }
role =
entityStore.get(
NameIdentifierUtil.ofRole(metalake, role.name()),
Entity.EntityType.ROLE,
RoleEntity.class);
- if (loadedRoles.contains(roleId)) {
- continue;
- }
- allowEnforcer.addRoleForUser(String.valueOf(userId),
String.valueOf(roleId));
- denyEnforcer.addRoleForUser(String.valueOf(userId),
String.valueOf(roleId));
+
loadPolicyByRoleEntity(role);
loadedRoles.add(roleId);
}
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/filter/GravitinoInterceptionService.java
b/server/src/main/java/org/apache/gravitino/server/web/filter/GravitinoInterceptionService.java
index 811fcd8271..1bd192db86 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/filter/GravitinoInterceptionService.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/filter/GravitinoInterceptionService.java
@@ -157,9 +157,8 @@ public class GravitinoInterceptionService implements
InterceptionService {
currentUser,
methodName,
ex);
- return Utils.forbidden(
- "Authorization failed due to system internal error. Please contact
administrator.",
- null);
+ return Utils.internalError(
+ "Authorization failed due to system internal error. Please contact
administrator.", ex);
}
}
@@ -169,16 +168,20 @@ public class GravitinoInterceptionService implements
InterceptionService {
String currentUser,
String methodName) {
String contextualMessage;
+ String accessMetadataMessage =
+ accessMetadataName != null
+ ? String.format("on metadata '%s'", accessMetadataName.name())
+ : "";
if (StringUtils.isNotBlank(errorMessage)) {
contextualMessage =
String.format(
- "User '%s' is not authorized to perform operation '%s' on
metadata '%s': %s",
- currentUser, methodName, accessMetadataName.name(),
errorMessage);
+ "User '%s' is not authorized to perform operation '%s' %s: %s",
+ currentUser, methodName, accessMetadataMessage, errorMessage);
} else {
contextualMessage =
String.format(
- "User '%s' is not authorized to perform operation '%s' on
metadata '%s'",
- currentUser, methodName, accessMetadataName.name());
+ "User '%s' is not authorized to perform operation '%s' %s",
+ currentUser, methodName, accessMetadataMessage);
}
return Utils.forbidden(contextualMessage, null);
}
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/CatalogOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/CatalogOperations.java
index d7af80e7fe..f7461bdf56 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/CatalogOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/CatalogOperations.java
@@ -136,7 +136,7 @@ public class CatalogOperations {
@ResponseMetered(name = "create-catalog", absolute = true)
@AuthorizationExpression(
expression = "METALAKE::CREATE_CATALOG || METALAKE::OWNER",
- accessMetadataType = MetadataObject.Type.CATALOG)
+ accessMetadataType = MetadataObject.Type.METALAKE)
public Response createCatalog(
@PathParam("metalake") @AuthorizationMetadata(type =
Entity.EntityType.METALAKE)
String metalake,
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/FilesetOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/FilesetOperations.java
index b0165b27e6..bc06646bad 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/FilesetOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/FilesetOperations.java
@@ -140,7 +140,7 @@ public class FilesetOperations {
"ANY(OWNER, METALAKE, CATALOG) || "
+ "SCHEMA_OWNER_WITH_USE_CATALOG || "
+ "ANY_USE_CATALOG && ANY_USE_SCHEMA && ANY_CREATE_FILESET",
- accessMetadataType = MetadataObject.Type.FILESET)
+ accessMetadataType = MetadataObject.Type.SCHEMA)
public Response createFileset(
@PathParam("metalake") @AuthorizationMetadata(type =
Entity.EntityType.METALAKE)
String metalake,
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/TableOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/TableOperations.java
index 4302b737a9..5df8577199 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/TableOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/TableOperations.java
@@ -117,7 +117,7 @@ public class TableOperations {
"ANY(OWNER, METALAKE, CATALOG) || "
+ "SCHEMA_OWNER_WITH_USE_CATALOG || "
+ "ANY_USE_CATALOG && ANY_USE_SCHEMA && ANY_CREATE_TABLE",
- accessMetadataType = MetadataObject.Type.TABLE)
+ accessMetadataType = MetadataObject.Type.SCHEMA)
public Response createTable(
@PathParam("metalake") @AuthorizationMetadata(type =
Entity.EntityType.METALAKE)
String metalake,
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/filter/TestGravitinoInterceptionService.java
b/server/src/test/java/org/apache/gravitino/server/web/filter/TestGravitinoInterceptionService.java
index ba09e09f1d..a50b8b3a94 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/filter/TestGravitinoInterceptionService.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/filter/TestGravitinoInterceptionService.java
@@ -122,7 +122,7 @@ public class TestGravitinoInterceptionService {
errorResponse.getMessage());
// Verify correct HTTP status
- assertEquals(Response.Status.FORBIDDEN.getStatusCode(),
response.getStatus());
+ assertEquals(Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(),
response.getStatus());
}
}
