From 751d47c2babdaf80cf33d4eb7af8367c44993970 Mon Sep 17 00:00:00 2001
From: ken <1647023764@qq.com>
Date: Tue, 13 Sep 2022 15:02:03 +0800
Subject: [PATCH] fix can not revoke permission after update topic partition

---
 .../admin/impl/PersistentTopicsBase.java      | 12 ++++++----
 .../server/ProxyWithJwtAuthorizationTest.java | 22 +++++++++++++++++++
 2 files changed, 30 insertions(+), 4 deletions(-)

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 4ff236af5ac27..b4106ea15e76a 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
@@ -310,7 +310,7 @@ protected void internalDeleteTopicForcefully(boolean authoritative, boolean dele
         }
     }
 
-    private void revokePermissions(String topicUri, String role) {
+    private void revokePermissions(String topicUri, String role, boolean force) {
         Policies policies;
         try {
             policies = namespaceResources().getPolicies(namespaceName)
@@ -323,7 +323,11 @@ private void revokePermissions(String topicUri, String role) {
                 || !policies.auth_policies.getTopicAuthentication().get(topicUri).containsKey(role)) {
             log.warn("[{}] Failed to revoke permission from role {} on topic: Not set at topic level {}", clientAppId(),
                     role, topicUri);
-            throw new RestException(Status.PRECONDITION_FAILED, "Permissions are not set at the topic level");
+            if (force) {
+                return;
+            } else {
+                throw new RestException(Status.PRECONDITION_FAILED, "Permissions are not set at the topic level");
+            }
         }
         try {
             // Write the new policies to metadata store
@@ -349,10 +353,10 @@ protected void internalRevokePermissionsOnTopic(String role) {
         if (numPartitions > 0) {
             for (int i = 0; i < numPartitions; i++) {
                 TopicName topicNamePartition = topicName.getPartition(i);
-                revokePermissions(topicNamePartition.toString(), role);
+                revokePermissions(topicNamePartition.toString(), role, true);
             }
         }
-        revokePermissions(topicName.toString(), role);
+        revokePermissions(topicName.toString(), role, false);
     }
 
     protected void internalCreateNonPartitionedTopic(boolean authoritative) {
diff --git a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyWithJwtAuthorizationTest.java b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyWithJwtAuthorizationTest.java
index 6178454dd1900..4a4a35ece69b1 100644
--- a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyWithJwtAuthorizationTest.java
+++ b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyWithJwtAuthorizationTest.java
@@ -210,6 +210,8 @@ public void testProxyAuthorization() throws Exception {
      * 2. Update the topic partition number to 4.
      * 3. Use new producer/consumer with client role to process the topic.
      * 4. Broker should authorize producer/consumer normally.
+     * 5. revoke produce/consumer permission of topic
+     * 6. new producer/consumer should not be authorized
      * </pre>
      */
     @Test
@@ -291,6 +293,26 @@ public void testUpdatePartitionNumAndReconnect() throws Exception {
         Assert.assertEquals(messageSet, receivedMessageSet);
         consumer.close();
         producer.close();
+
+        // revoke produce/consume permission
+        admin.topics().revokePermissions(topicName, CLIENT_ROLE);
+
+        // produce/consume the topic should fail
+        try {
+            consumer = proxyClient.newConsumer()
+                    .topic(topicName)
+                    .subscriptionName(subscriptionName).subscribe();
+            Assert.fail("Should not pass");
+        } catch (PulsarClientException.AuthorizationException ex) {
+            // ok
+        }
+        try {
+            producer = proxyClient.newProducer(Schema.BYTES)
+                    .topic(topicName).create();
+            Assert.fail("Should not pass");
+        } catch (PulsarClientException.AuthorizationException ex) {
+            // ok
+        }
         log.info("-- Exiting {} test --", methodName);
     }