This is an automated email from the ASF dual-hosted git repository.
mchades 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 79742f364 [#4699] fix(server): The error response of addUser should
meet the REST specification (#4704)
79742f364 is described below
commit 79742f364f9fccf189e6333dc5c7539c409b76dc
Author: roryqi <[email protected]>
AuthorDate: Tue Aug 27 17:49:15 2024 +0800
[#4699] fix(server): The error response of addUser should meet the REST
specification (#4704)
### 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."}
```
---
.../org/apache/gravitino/client/ErrorHandlers.java | 15 ++
.../authorization/AccessControlNotAllowIT.java | 153 +++++++++++++++++++++
.../web/filter/AccessControlNotAllowedFilter.java | 18 +--
3 files changed, 174 insertions(+), 12 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 18576e277..7414f4c4e 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 class ErrorHandlers {
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 class ErrorHandlers {
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 class ErrorHandlers {
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 class ErrorHandlers {
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 class ErrorHandlers {
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 000000000..22986458b
--- /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 130827371..42c001f76 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())));
}
}
