sijie closed pull request #2382: Issue 2283: Improve error message if
authorization is not enabled
URL: https://github.com/apache/incubator-pulsar/pull/2382
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java
b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java
index c4202c68db..0d7e0eeab9 100644
---
a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java
+++
b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java
@@ -50,6 +50,7 @@
import org.apache.pulsar.broker.PulsarServerException;
import org.apache.pulsar.broker.ServiceConfiguration;
import org.apache.pulsar.broker.admin.AdminResource;
+import org.apache.pulsar.broker.authorization.AuthorizationService;
import
org.apache.pulsar.broker.service.BrokerServiceException.SubscriptionBusyException;
import org.apache.pulsar.broker.service.Subscription;
import org.apache.pulsar.broker.service.Topic;
@@ -297,9 +298,13 @@ protected void internalGrantPermissionOnNamespace(String
role, Set<AuthAction> a
validateAdminAccessForTenant(namespaceName.getTenant());
try {
- pulsar().getBrokerService().getAuthorizationService()
- .grantPermissionAsync(namespaceName, actions, role,
null/*additional auth-data json*/)
+ AuthorizationService authService =
pulsar().getBrokerService().getAuthorizationService();
+ if (null != authService) {
+ authService.grantPermissionAsync(namespaceName, actions, role,
null/*additional auth-data json*/)
.get();
+ } else {
+ throw new RestException(Status.NOT_IMPLEMENTED, "Authorization
is not enabled");
+ }
} catch (InterruptedException e) {
log.error("[{}] Failed to get permissions for namespace {}",
clientAppId(), namespaceName, e);
throw new RestException(e);
diff --git
a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/Namespaces.java
b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/Namespaces.java
index 6b607ab3e2..b08bfb5763 100644
---
a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/Namespaces.java
+++
b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/Namespaces.java
@@ -218,7 +218,8 @@ public void deleteNamespaceBundle(@PathParam("property")
String property,
@ApiOperation(hidden = true, value = "Grant a new permission to a role on
a namespace.")
@ApiResponses(value = { @ApiResponse(code = 403, message = "Don't have
admin permission"),
@ApiResponse(code = 404, message = "Property or cluster or
namespace doesn't exist"),
- @ApiResponse(code = 409, message = "Concurrent modification") })
+ @ApiResponse(code = 409, message = "Concurrent modification"),
+ @ApiResponse(code = 501, message = "Authorization is not
enabled")})
public void grantPermissionOnNamespace(@PathParam("property") String
property, @PathParam("cluster") String cluster,
@PathParam("namespace") String namespace, @PathParam("role")
String role, Set<AuthAction> actions) {
validateNamespaceName(property, cluster, namespace);
diff --git
a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/Namespaces.java
b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/Namespaces.java
index 3a306e49d1..f58ec1916c 100644
---
a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/Namespaces.java
+++
b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/Namespaces.java
@@ -160,7 +160,8 @@ public void deleteNamespaceBundle(@PathParam("tenant")
String tenant, @PathParam
@ApiOperation(value = "Grant a new permission to a role on a namespace.")
@ApiResponses(value = { @ApiResponse(code = 403, message = "Don't have
admin permission"),
@ApiResponse(code = 404, message = "Tenant or cluster or namespace
doesn't exist"),
- @ApiResponse(code = 409, message = "Concurrent modification") })
+ @ApiResponse(code = 409, message = "Concurrent modification"),
+ @ApiResponse(code = 501, message = "Authorization is not
enabled")})
public void grantPermissionOnNamespace(@PathParam("tenant") String tenant,
@PathParam("namespace") String namespace, @PathParam("role")
String role, Set<AuthAction> actions) {
validateNamespaceName(tenant, namespace);
diff --git
a/pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/PulsarAdminException.java
b/pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/PulsarAdminException.java
index 143e84846e..9961454283 100644
---
a/pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/PulsarAdminException.java
+++
b/pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/PulsarAdminException.java
@@ -47,25 +47,25 @@ private static String
getReasonFromServer(WebApplicationException e) {
public PulsarAdminException(ClientErrorException e) {
super(getReasonFromServer(e), e);
- this.httpError = e.getMessage();
+ this.httpError = getReasonFromServer(e);
this.statusCode = e.getResponse().getStatus();
}
public PulsarAdminException(ClientErrorException e, String message) {
super(message, e);
- this.httpError = e.getMessage();
+ this.httpError = getReasonFromServer(e);
this.statusCode = e.getResponse().getStatus();
}
public PulsarAdminException(ServerErrorException e) {
super(getReasonFromServer(e), e);
- this.httpError = e.getMessage();
+ this.httpError = getReasonFromServer(e);
this.statusCode = e.getResponse().getStatus();
}
public PulsarAdminException(ServerErrorException e, String message) {
super(message, e);
- this.httpError = e.getMessage();
+ this.httpError = getReasonFromServer(e);
this.statusCode = e.getResponse().getStatus();
}
@@ -75,6 +75,12 @@ public PulsarAdminException(Throwable t) {
statusCode = DEFAULT_STATUS_CODE;
}
+ public PulsarAdminException(WebApplicationException e) {
+ super(getReasonFromServer(e), e);
+ this.httpError = getReasonFromServer(e);
+ this.statusCode = e.getResponse().getStatus();
+ }
+
public PulsarAdminException(String message, Throwable t) {
super(message, t);
httpError = null;
@@ -124,6 +130,10 @@ public PreconditionFailedException(ClientErrorException e)
{
}
public static class ServerSideErrorException extends PulsarAdminException {
+ public ServerSideErrorException(ServerErrorException e, String msg) {
+ super(e, msg);
+ }
+
public ServerSideErrorException(ServerErrorException e) {
super(e, "Some error occourred on the server");
}
diff --git
a/pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/BaseResource.java
b/pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/BaseResource.java
index 96671ec3e3..25d622bdfd 100644
---
a/pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/BaseResource.java
+++
b/pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/BaseResource.java
@@ -159,7 +159,7 @@ public PulsarAdminException getApiException(Throwable e) {
// Handle 5xx exceptions
if (e instanceof ServerErrorException) {
ServerErrorException see = (ServerErrorException) e;
- return new ServerSideErrorException(see);
+ return new ServerSideErrorException(see, e.getMessage());
} else if (e instanceof ClientErrorException) {
// Handle 4xx exceptions
ClientErrorException cee = (ClientErrorException) e;
@@ -176,12 +176,11 @@ public PulsarAdminException getApiException(Throwable e) {
return new ConflictException(cee);
case 412:
return new PreconditionFailedException(cee);
-
default:
return new PulsarAdminException(cee);
}
} else {
- return new PulsarAdminException(e);
+ return new PulsarAdminException((WebApplicationException) e);
}
} else {
return new PulsarAdminException(e);
diff --git
a/tests/integration/src/test/java/org/apache/pulsar/tests/integration/cli/CLITest.java
b/tests/integration/src/test/java/org/apache/pulsar/tests/integration/cli/CLITest.java
index e30a1e3305..8526ded2e2 100644
---
a/tests/integration/src/test/java/org/apache/pulsar/tests/integration/cli/CLITest.java
+++
b/tests/integration/src/test/java/org/apache/pulsar/tests/integration/cli/CLITest.java
@@ -18,6 +18,7 @@
*/
package org.apache.pulsar.tests.integration.cli;
+import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertTrue;
import static org.testng.Assert.fail;
@@ -208,4 +209,27 @@ public void testSetInfiniteRetention() throws Exception {
result.getStdout());
}
+ // authorization related tests
+
+ @Test
+ public void testGrantPermissionsAuthorizationDisabled() throws Exception {
+ ContainerExecResult result;
+
+ String namespace = "grant-permissions-" + randomName(8);
+ result = pulsarCluster.createNamespace(namespace);
+ assertEquals(0, result.getExitCode());
+
+ String[] grantCommand = {
+ "namespaces", "grant-permission", "public/" + namespace,
+ "--actions", "produce",
+ "--role", "test-role"
+ };
+ try {
+ pulsarCluster.runAdminCommandOnAnyBroker(grantCommand);
+ } catch (ContainerExecException cee) {
+ result = cee.getResult();
+ assertTrue(result.getStderr().contains("HTTP 501 Not
Implemented"), result.getStderr());
+ }
+ }
+
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
