[pulsar] branch branch-2.11 updated: [fix][broker] Only validate superuser access if authz enabled (#19989)

[mmarshall]

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

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


The following commit(s) were added to refs/heads/branch-2.11 by this push:
     new 0ea8dd8cf09 [fix][broker] Only validate superuser access if authz 
enabled (#19989)
0ea8dd8cf09 is described below

commit 0ea8dd8cf092e47da937bc8da1cf638d2d2f47c7
Author: Michael Marshall <mmarsh...@apache.org>
AuthorDate: Wed Apr 5 18:10:55 2023 -0500

    [fix][broker] Only validate superuser access if authz enabled (#19989)
    
    ### Motivation
    
    In #19455, I added a requirement that only the proxy role could supply an 
original principal. That check is only supposed to apply when the broker has 
authorization enabled. However, in one case, that was not the case. This PR 
does a check and returns early when authorization is not enabled in the broker.
    
    See https://github.com/apache/pulsar/pull/19830#issuecomment-1492262201 for 
additional motivation.
    
    ### Modifications
    
    * Update the `PulsarWebResource#validateSuperUserAccessAsync` to only 
validate when authentication and authorization are enabled in the configuration.
    
    ### Verifying this change
    
    This is a trivial change. It'd be good to add tests, but I didn't include 
them here because this is a somewhat urgent fix. There was one test that broke 
because of this change, so there is at least some existing coverage.
    
    ### Documentation
    
    - [x] `doc-not-needed`
    
    ### Matching PR in forked repository
    
    PR in forked repository: https://github.com/michaeljmarshall/pulsar/pull/39
    
    (cherry picked from commit 1a6c28dd0072de05a544dbc9243bfbe6bccea5db)
---
 .../pulsar/broker/web/PulsarWebResource.java       | 29 ++++++++--------------
 .../broker/admin/v3/AdminApiTransactionTest.java   |  1 +
 2 files changed, 12 insertions(+), 18 deletions(-)

diff --git 
a/pulsar-broker/src/main/java/org/apache/pulsar/broker/web/PulsarWebResource.java
 
b/pulsar-broker/src/main/java/org/apache/pulsar/broker/web/PulsarWebResource.java
index e87f337e5f7..ceb25a7e003 100644
--- 
a/pulsar-broker/src/main/java/org/apache/pulsar/broker/web/PulsarWebResource.java
+++ 
b/pulsar-broker/src/main/java/org/apache/pulsar/broker/web/PulsarWebResource.java
@@ -185,8 +185,8 @@ public abstract class PulsarWebResource {
         return true;
     }
 
-    public CompletableFuture<Void> validateSuperUserAccessAsync(){
-        if (!config().isAuthenticationEnabled()) {
+    public CompletableFuture<Void> validateSuperUserAccessAsync() {
+        if (!config().isAuthenticationEnabled() || 
!config().isAuthorizationEnabled()) {
             return CompletableFuture.completedFuture(null);
         }
         String appId = clientAppId();
@@ -221,22 +221,15 @@ public abstract class PulsarWebResource {
                         }
                     });
         } else {
-            if (config().isAuthorizationEnabled()) {
-                return pulsar.getBrokerService()
-                        .getAuthorizationService()
-                        .isSuperUser(appId, clientAuthData())
-                        .thenAccept(proxyAuthorizationSuccess -> {
-                            if (!proxyAuthorizationSuccess) {
-                                throw new RestException(Status.UNAUTHORIZED,
-                                        "This operation requires super-user 
access");
-                            }
-                        });
-            }
-            if (log.isDebugEnabled()) {
-                log.debug("Successfully authorized {} as super-user",
-                        appId);
-            }
-            return CompletableFuture.completedFuture(null);
+            return pulsar.getBrokerService()
+                    .getAuthorizationService()
+                    .isSuperUser(appId, clientAuthData())
+                    .thenAccept(proxyAuthorizationSuccess -> {
+                        if (!proxyAuthorizationSuccess) {
+                            throw new RestException(Status.UNAUTHORIZED,
+                                    "This operation requires super-user 
access");
+                        }
+                    });
         }
     }
 
diff --git 
a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/v3/AdminApiTransactionTest.java
 
b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/v3/AdminApiTransactionTest.java
index 4afb2030eca..5291475e929 100644
--- 
a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/v3/AdminApiTransactionTest.java
+++ 
b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/v3/AdminApiTransactionTest.java
@@ -610,6 +610,7 @@ public class AdminApiTransactionTest extends 
MockedPulsarServiceBaseTest {
         Awaitility.await().until(() -> 
pulsar.getTransactionMetadataStoreService().getStores().size() ==
                         coordinatorSize * 2);
         pulsar.getConfiguration().setAuthenticationEnabled(true);
+        pulsar.getConfiguration().setAuthorizationEnabled(true);
         Set<String> proxyRoles = spy(Set.class);
         doReturn(true).when(proxyRoles).contains(any());
         pulsar.getConfiguration().setProxyRoles(proxyRoles);