pulsar-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] sijie closed pull request #2382: Issue 2283: Improve error message if authorization is not enabled
Date Fri, 17 Aug 2018 15:48:55 GMT
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:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message