Add missing permission checck, fail update on non-existent ResourceGr…

…oup (apache#11957) Add unit tests
mfleming · Sep 14, 2021 · e671932 · e671932
1 parent e2bc52d
commit e671932
Show file tree

Hide file tree

Showing 3 changed files with 72 additions and 1 deletion.
diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/ResourceGroupsBase.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/ResourceGroupsBase.java
@@ -50,6 +50,7 @@ protected List<String> internalGetResourceGroups() {
     protected ResourceGroup internalGetResourceGroup(String rgName) {
         try {
             final String resourceGroupPath = AdminResource.path(RESOURCEGROUPS, rgName);
+            validateSuperUserAccess();
             ResourceGroup resourceGroup = resourceGroupResources().get(resourceGroupPath)
                     .orElseThrow(() -> new RestException(Response.Status.NOT_FOUND, "ResourceGroup does not exist"));
             return resourceGroup;
@@ -161,6 +162,7 @@ protected void internalDeleteResourceGroup(String rgName) {
          * need to walk the namespaces and make sure it is not in use
          */
         try {
+            validateSuperUserAccess();
             /*
              * walk the namespaces and make sure it is not in use.
              */

diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiTlsAuthTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiTlsAuthTest.java
@@ -49,6 +49,7 @@
 import org.apache.pulsar.client.api.PulsarClient;
 import org.apache.pulsar.client.api.Schema;
 import org.apache.pulsar.common.policies.data.ClusterData;
+import org.apache.pulsar.common.policies.data.ResourceGroup;
 import org.apache.pulsar.common.tls.NoopHostnameVerifier;
 import org.apache.pulsar.common.policies.data.AuthAction;
 import org.apache.pulsar.common.policies.data.ClusterDataImpl;
@@ -174,6 +175,72 @@ public void testProxyRoleCantListTenants() throws Exception {
         }
     }
 
+    @Test
+    public void testSuperUserCanGetResourceGroups() throws Exception {
+        try (PulsarAdmin admin = buildAdminClient("admin")) {
+            admin.resourcegroups().createResourceGroup("test-resource-group",
+                    new ResourceGroup());
+            admin.resourcegroups().getResourceGroup("test-resource-group");
+            Assert.assertEquals(ImmutableSet.of("test-resource-group"),
+                            admin.resourcegroups().getResourceGroups());
+            admin.resourcegroups().getResourceGroup("test-resource-group");
+        }
+    }
+
+    @Test
+    public void testSuperUserCanDeleteResourceGroups() throws Exception {
+        try (PulsarAdmin admin = buildAdminClient("admin")) {
+            admin.resourcegroups().createResourceGroup("test-resource-group",
+                    new ResourceGroup());
+            admin.resourcegroups().deleteResourceGroup("test-resource-group");
+        }
+    }
+
+    @Test
+    public void testProxyRoleCantDeleteResourceGroups() throws Exception {
+        try (PulsarAdmin admin = buildAdminClient("admin")) {
+            admin.resourcegroups().createResourceGroup("test-resource-group",
+                    new ResourceGroup());
+        }
+        try (PulsarAdmin admin = buildAdminClient("proxy")) {
+            admin.resourcegroups().deleteResourceGroup("test-resource-group");
+            Assert.fail("Shouldn't be able to delete ResourceGroup");
+        } catch (PulsarAdminException.NotAuthorizedException e) {
+            // expected
+        }
+    }
+
+    @Test
+    public void testProxyRoleCantCreateResourceGroups() throws Exception {
+        try (PulsarAdmin admin = buildAdminClient("proxy")) {
+            admin.resourcegroups().createResourceGroup("test-resource-group",
+                    new ResourceGroup());
+            Assert.fail("Shouldn't be able to create ResourceGroup");
+        } catch (PulsarAdminException.NotAuthorizedException e) {
+            // expected
+        }
+    }
+
+    @Test
+    public void testProxyRoleCantGetResourceGroups() throws Exception {
+        try (PulsarAdmin admin = buildAdminClient("admin")) {
+            admin.resourcegroups().createResourceGroup("test-resource-group",
+                    new ResourceGroup());
+        }
+        try (PulsarAdmin admin = buildAdminClient("proxy")) {
+            admin.resourcegroups().getResourceGroups();
+            Assert.fail("Shouldn't be able to list ResourceGroups");
+        } catch (PulsarAdminException.NotAuthorizedException e) {
+            // expected
+        }
+        try (PulsarAdmin admin = buildAdminClient("proxy")) {
+            admin.resourcegroups().getResourceGroup("test-resource-group");
+            Assert.fail("Shouldn't be able to get ResourceGroup");
+        } catch (PulsarAdminException.NotAuthorizedException e) {
+            // expected
+        }
+    }
+
     @Test
     public void testProxyRoleCantListNamespacesEvenWithAccess() throws Exception {
         try (PulsarAdmin admin = buildAdminClient("admin")) {

diff --git a/...lient-admin/src/main/java/org/apache/pulsar/client/admin/internal/ResourceGroupsImpl.java b/...lient-admin/src/main/java/org/apache/pulsar/client/admin/internal/ResourceGroupsImpl.java
@@ -31,6 +31,7 @@
 import org.apache.pulsar.client.admin.ResourceGroups;
 import org.apache.pulsar.client.api.Authentication;
 import org.apache.pulsar.common.policies.data.ResourceGroup;
+import org.apache.pulsar.common.util.RestException;
 
 
 public class ResourceGroupsImpl extends BaseResource implements ResourceGroups {
@@ -130,8 +131,9 @@ public CompletableFuture<Void> createResourceGroupAsync(String name, ResourceGro
     @Override
     public void updateResourceGroup(String name, ResourceGroup resourcegroup) throws PulsarAdminException {
         try {
+            getResourceGroup(name);
             updateResourceGroupAsync(name, resourcegroup).get(this.readTimeoutMs, TimeUnit.MILLISECONDS);
-        } catch (ExecutionException e) {
+        } catch (ExecutionException | RestException e) {
             throw (PulsarAdminException) e.getCause();
         } catch (InterruptedException e) {
             Thread.currentThread().interrupt();