(pulsar) branch branch-3.0 updated: [fix][admin] Verify is policies read only before revoke permissions on topic (#23730)

[lhotari]

This is an automated email from the ASF dual-hosted git repository.

lhotari pushed a commit to branch branch-3.0
in repository https://gitbox.apache.org/repos/asf/pulsar.git


The following commit(s) were added to refs/heads/branch-3.0 by this push:
     new 7df87f0cb60 [fix][admin] Verify is policies read only before revoke 
permissions on topic (#23730)
7df87f0cb60 is described below

commit 7df87f0cb60f021dd2fd0592d4ab6cd8873c9fb8
Author: 道君 <dao...@apache.org>
AuthorDate: Tue Dec 17 11:38:13 2024 +0800

    [fix][admin] Verify is policies read only before revoke permissions on 
topic (#23730)
    
    (cherry picked from commit 069cc3db7c741a7b1fc64c79733ada847dca873e)
---
 .../broker/admin/impl/PersistentTopicsBase.java    |  2 +-
 .../pulsar/broker/admin/PersistentTopicsTest.java  | 30 ++++++++++++++++++++++
 2 files changed, 31 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 08fe12178a8..8833d0c70cb 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
@@ -365,7 +365,7 @@ public class PersistentTopicsBase extends AdminResource {
         // This operation should be reading from zookeeper and it should be 
allowed without having admin privileges
         CompletableFuture<Void> validateAccessForTenantCf =
                 validateAdminAccessForTenantAsync(namespaceName.getTenant())
-                .thenCompose(__ -> validatePoliciesReadOnlyAccessAsync());
+                        .thenCompose(__ -> 
validatePoliciesReadOnlyAccessAsync());
 
         var checkIfTopicExists = 
!pulsar().getConfiguration().isAllowAclChangesOnNonExistentTopics();
         if (checkIfTopicExists) {
diff --git 
a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/PersistentTopicsTest.java
 
b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/PersistentTopicsTest.java
index a5063802cfe..f4a9d7c0643 100644
--- 
a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/PersistentTopicsTest.java
+++ 
b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/PersistentTopicsTest.java
@@ -1021,6 +1021,36 @@ public class PersistentTopicsTest extends 
MockedPulsarServiceBaseTest {
         }
     }
 
+    @Test
+    public void testRevokePartitionedTopicWithReadonlyPolicies() throws 
Exception {
+        final String partitionedTopicName = 
"testRevokePartitionedTopicWithReadonlyPolicies-topic";
+        final int numPartitions = 5;
+        AsyncResponse response = mock(AsyncResponse.class);
+        ArgumentCaptor<Response> responseCaptor = 
ArgumentCaptor.forClass(Response.class);
+        persistentTopics.createPartitionedTopic(
+                response, testTenant, testNamespace, partitionedTopicName, 
numPartitions, true);
+        verify(response, 
timeout(5000).times(1)).resume(responseCaptor.capture());
+        Assert.assertEquals(responseCaptor.getValue().getStatus(), 
Response.Status.NO_CONTENT.getStatusCode());
+        String role = "role";
+        Set<AuthAction> expectActions = new HashSet<>();
+        expectActions.add(AuthAction.produce);
+        response = mock(AsyncResponse.class);
+        responseCaptor = ArgumentCaptor.forClass(Response.class);
+        persistentTopics.grantPermissionsOnTopic(response, testTenant, 
testNamespace, partitionedTopicName, role,
+                expectActions);
+        verify(response, 
timeout(5000).times(1)).resume(responseCaptor.capture());
+        Assert.assertEquals(responseCaptor.getValue().getStatus(), 
Response.Status.NO_CONTENT.getStatusCode());
+        response = mock(AsyncResponse.class);
+        doReturn(CompletableFuture.failedFuture(
+                new RestException(Response.Status.FORBIDDEN,  "Broker is 
forbidden to do read-write operations"))
+        ).when(persistentTopics).validatePoliciesReadOnlyAccessAsync();
+        persistentTopics.revokePermissionsOnTopic(response, testTenant, 
testNamespace, partitionedTopicName, role);
+        ArgumentCaptor<RestException> exceptionCaptor = 
ArgumentCaptor.forClass(RestException.class);
+        verify(response, 
timeout(5000).times(1)).resume(exceptionCaptor.capture());
+        
Assert.assertEquals(exceptionCaptor.getValue().getResponse().getStatus(),
+                Response.Status.FORBIDDEN.getStatusCode());
+    }
+
     @Test
     public void testTriggerCompactionTopic() {
         final String partitionTopicName = "test-part";