From 269b024d0d85495e0b8ad816f04e8e2b24ee6f39 Mon Sep 17 00:00:00 2001 From: Ruguo Yu Date: Sun, 5 Dec 2021 14:18:20 +0800 Subject: [PATCH 1/3] [Broker] Remove tenant permission verification when list partitioned-topic --- .../pulsar/broker/admin/impl/PersistentTopicsBase.java | 1 - .../client/api/AuthorizationProducerConsumerTest.java | 10 ++++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java index c221f67b5e33e..968fec2057c94 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java @@ -172,7 +172,6 @@ protected List internalGetList() { } protected List internalGetPartitionedTopicList() { - validateAdminAccessForTenant(namespaceName.getTenant()); validateNamespaceOperation(namespaceName, NamespaceOperation.GET_TOPICS); // Validate that namespace exists, throws 404 if it doesn't exist try { diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/client/api/AuthorizationProducerConsumerTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/client/api/AuthorizationProducerConsumerTest.java index b0c9a093cfed5..2f600361b1bdb 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/client/api/AuthorizationProducerConsumerTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/client/api/AuthorizationProducerConsumerTest.java @@ -24,6 +24,7 @@ import static org.testng.Assert.assertNotNull; import static org.testng.Assert.assertTrue; import static org.testng.Assert.fail; +import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.collect.Sets; import java.io.IOException; @@ -416,6 +417,13 @@ public void testClearBacklogPermission() throws Exception { .get(subscriptionName).getMsgBacklog(), 10); // subscriptionRole doesn't have namespace-level authorization, so it will fail to clear backlog + try { + sub1Admin.topics().getPartitionedTopicList(namespace); + fail("should have failed with authorization exception"); + } catch (Exception e) { + assertTrue(e.getMessage().startsWith( + "Unauthorized to validateNamespaceOperation for operation [GET_TOPICS]")); + } try { sub1Admin.namespaces().clearNamespaceBundleBacklog(namespace, "0x00000000_0xffffffff"); fail("should have failed with authorization exception"); @@ -427,6 +435,8 @@ public void testClearBacklogPermission() throws Exception { superAdmin.namespaces().grantPermissionOnNamespace(namespace, subscriptionRole, Sets.newHashSet(AuthAction.consume)); // now, subscriptionRole have consume authorization on namespace, so it will successfully clear backlog + assertEquals(sub1Admin.topics().getPartitionedTopicList(namespace), + Lists.newArrayList(topicName)); sub1Admin.namespaces().clearNamespaceBundleBacklog(namespace, "0x00000000_0xffffffff"); assertEquals(sub1Admin.topics().getStats(topicName + "-partition-0").getSubscriptions() .get(subscriptionName).getMsgBacklog(), 0); From fe7ac306fc22cc0922a9333276acb3ea67dcd86d Mon Sep 17 00:00:00 2001 From: Ruguo Yu Date: Mon, 6 Dec 2021 14:08:05 +0800 Subject: [PATCH 2/3] Improve test --- .../client/api/AuthorizationProducerConsumerTest.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/client/api/AuthorizationProducerConsumerTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/client/api/AuthorizationProducerConsumerTest.java index 2f600361b1bdb..5ea9765f2527a 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/client/api/AuthorizationProducerConsumerTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/client/api/AuthorizationProducerConsumerTest.java @@ -366,6 +366,7 @@ public void testClearBacklogPermission() throws Exception { conf.setAuthorizationProvider(PulsarAuthorizationProvider.class.getName()); setup(); + final String tenantRole = "tenant-role"; final String subscriptionRole = "sub-role"; final String subscriptionName = "sub1"; final String namespace = "my-property/my-ns-sub-auth"; @@ -378,6 +379,11 @@ public void testClearBacklogPermission() throws Exception { PulsarAdmin superAdmin = spy(PulsarAdmin.builder().serviceHttpUrl(brokerUrl.toString()) .authentication(adminAuthentication).build()); + Authentication tenantAdminAuthentication = new ClientAuthentication(tenantRole); + @Cleanup + PulsarAdmin tenantAdmin = spy(PulsarAdmin.builder().serviceHttpUrl(brokerUrl.toString()) + .authentication(tenantAdminAuthentication).build()); + Authentication subAdminAuthentication = new ClientAuthentication(subscriptionRole); @Cleanup PulsarAdmin sub1Admin = spy(PulsarAdmin.builder().serviceHttpUrl(brokerUrl.toString()) @@ -386,9 +392,11 @@ public void testClearBacklogPermission() throws Exception { superAdmin.clusters().createCluster("test", ClusterData.builder().serviceUrl(brokerUrl.toString()).build()); superAdmin.tenants().createTenant("my-property", - new TenantInfoImpl(Sets.newHashSet(), Sets.newHashSet("test"))); + new TenantInfoImpl(Sets.newHashSet(tenantRole), Sets.newHashSet("test"))); superAdmin.namespaces().createNamespace(namespace, Sets.newHashSet("test")); superAdmin.topics().createPartitionedTopic(topicName, 1); + assertEquals(tenantAdmin.topics().getPartitionedTopicList(namespace), + Lists.newArrayList(topicName)); // grant topic consume&produce authorization to the subscriptionRole superAdmin.topics().grantPermission(topicName, subscriptionRole, From 12c3d5e6f37eebc3bc849e0279f67163ae0c7f18 Mon Sep 17 00:00:00 2001 From: Ruguo Yu Date: Wed, 8 Dec 2021 08:12:35 +0800 Subject: [PATCH 3/3] Update annotation --- .../org/apache/pulsar/broker/admin/v1/PersistentTopics.java | 4 ++-- .../org/apache/pulsar/broker/admin/v2/PersistentTopics.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/PersistentTopics.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/PersistentTopics.java index fd8688be84416..709b9a08c1160 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/PersistentTopics.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/PersistentTopics.java @@ -69,7 +69,7 @@ public class PersistentTopics extends PersistentTopicsBase { @Path("/{property}/{cluster}/{namespace}") @ApiOperation(hidden = true, value = "Get the list of topics under a namespace.", response = String.class, responseContainer = "List") - @ApiResponses(value = {@ApiResponse(code = 403, message = "Don't have admin permission"), + @ApiResponses(value = {@ApiResponse(code = 403, message = "Don't have admin or consume permission on namespace"), @ApiResponse(code = 404, message = "Namespace doesn't exist")}) public void getList(@Suspended final AsyncResponse asyncResponse, @PathParam("property") String property, @PathParam("cluster") String cluster, @PathParam("namespace") String namespace) { @@ -87,7 +87,7 @@ public void getList(@Suspended final AsyncResponse asyncResponse, @PathParam("pr @Path("/{property}/{cluster}/{namespace}/partitioned") @ApiOperation(hidden = true, value = "Get the list of partitioned topics under a namespace.", response = String.class, responseContainer = "List") - @ApiResponses(value = {@ApiResponse(code = 403, message = "Don't have admin permission"), + @ApiResponses(value = {@ApiResponse(code = 403, message = "Don't have admin or consume permission on namespace"), @ApiResponse(code = 404, message = "Namespace doesn't exist")}) public List getPartitionedTopicList(@PathParam("property") String property, @PathParam("cluster") String cluster, @PathParam("namespace") String namespace) { diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java index 344c9fa6858ad..a489ab5fb0bb4 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java @@ -85,7 +85,7 @@ public class PersistentTopics extends PersistentTopicsBase { response = String.class, responseContainer = "List") @ApiResponses(value = { @ApiResponse(code = 401, message = "Don't have permission to administrate resources on this tenant"), - @ApiResponse(code = 403, message = "Don't have admin permission"), + @ApiResponse(code = 403, message = "Don't have admin or consume permission on namespace"), @ApiResponse(code = 404, message = "tenant/namespace/topic doesn't exit"), @ApiResponse(code = 412, message = "Namespace name is not valid"), @ApiResponse(code = 500, message = "Internal server error")}) @@ -111,7 +111,7 @@ public void getList( response = String.class, responseContainer = "List") @ApiResponses(value = { @ApiResponse(code = 401, message = "Don't have permission to administrate resources on this tenant"), - @ApiResponse(code = 403, message = "Don't have admin permission"), + @ApiResponse(code = 403, message = "Don't have admin or consume permission on namespace"), @ApiResponse(code = 404, message = "tenant/namespace/topic doesn't exit"), @ApiResponse(code = 412, message = "Namespace name is not valid"), @ApiResponse(code = 500, message = "Internal server error")})