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

technoboy 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 0b0d15f9d27 [fix][broker] Fix MultiRoles token provider NPE when using 
anonymous clients (#21429)
0b0d15f9d27 is described below

commit 0b0d15f9d278df3fb006c5c43645337bf891e668
Author: Jiwei Guo <[email protected]>
AuthorDate: Wed Oct 25 23:02:19 2023 +0800

    [fix][broker] Fix MultiRoles token provider NPE when using anonymous 
clients (#21429)
---
 .../AuthenticationDataSubscription.java            |  4 ++++
 .../MultiRolesTokenAuthorizationProvider.java      | 24 ++++++++++++++--------
 .../MultiRolesTokenAuthorizationProviderTest.java  | 22 ++++++++++++++++++++
 3 files changed, 42 insertions(+), 8 deletions(-)

diff --git 
a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationDataSubscription.java
 
b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationDataSubscription.java
index f6723609908..9584044e354 100644
--- 
a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationDataSubscription.java
+++ 
b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationDataSubscription.java
@@ -71,4 +71,8 @@ public class AuthenticationDataSubscription implements 
AuthenticationDataSource
     public String getSubscription() {
         return subscription;
     }
+
+    public AuthenticationDataSource getAuthData() {
+        return authData;
+    }
 }
diff --git 
a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/MultiRolesTokenAuthorizationProvider.java
 
b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/MultiRolesTokenAuthorizationProvider.java
index 279e975214e..3df625894b1 100644
--- 
a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/MultiRolesTokenAuthorizationProvider.java
+++ 
b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/MultiRolesTokenAuthorizationProvider.java
@@ -35,6 +35,7 @@ import javax.ws.rs.core.Response;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.pulsar.broker.ServiceConfiguration;
 import org.apache.pulsar.broker.authentication.AuthenticationDataSource;
+import org.apache.pulsar.broker.authentication.AuthenticationDataSubscription;
 import org.apache.pulsar.broker.resources.PulsarResources;
 import org.apache.pulsar.common.naming.NamespaceName;
 import org.apache.pulsar.common.naming.TopicName;
@@ -144,7 +145,8 @@ public class MultiRolesTokenAuthorizationProvider extends 
PulsarAuthorizationPro
     }
 
     private Set<String> getRoles(String role, AuthenticationDataSource 
authData) {
-        if (authData == null) {
+        if (authData == null || (authData instanceof 
AuthenticationDataSubscription
+                && ((AuthenticationDataSubscription) authData).getAuthData() 
== null)) {
             return Collections.singleton(role);
         }
 
@@ -198,13 +200,19 @@ public class MultiRolesTokenAuthorizationProvider extends 
PulsarAuthorizationPro
 
     public CompletableFuture<Boolean> authorize(String role, 
AuthenticationDataSource authenticationData,
                                                 Function<String, 
CompletableFuture<Boolean>> authorizeFunc) {
-        Set<String> roles = getRoles(role, authenticationData);
-        if (roles.isEmpty()) {
-            return CompletableFuture.completedFuture(false);
-        }
-        List<CompletableFuture<Boolean>> futures = new 
ArrayList<>(roles.size());
-        roles.forEach(r -> futures.add(authorizeFunc.apply(r)));
-        return FutureUtil.waitForAny(futures, ret -> (boolean) 
ret).thenApply(v -> v.isPresent());
+        return isSuperUser(role, authenticationData, conf)
+                .thenCompose(superUser -> {
+                    if (superUser) {
+                        return CompletableFuture.completedFuture(true);
+                    }
+                    Set<String> roles = getRoles(role, authenticationData);
+                    if (roles.isEmpty()) {
+                        return CompletableFuture.completedFuture(false);
+                    }
+                    List<CompletableFuture<Boolean>> futures = new 
ArrayList<>(roles.size());
+                    roles.forEach(r -> futures.add(authorizeFunc.apply(r)));
+                    return FutureUtil.waitForAny(futures, ret -> (boolean) 
ret).thenApply(v -> v.isPresent());
+                });
     }
 
     /**
diff --git 
a/pulsar-broker-common/src/test/java/org/apache/pulsar/broker/authorization/MultiRolesTokenAuthorizationProviderTest.java
 
b/pulsar-broker-common/src/test/java/org/apache/pulsar/broker/authorization/MultiRolesTokenAuthorizationProviderTest.java
index ee6854703b6..2e5cdcead05 100644
--- 
a/pulsar-broker-common/src/test/java/org/apache/pulsar/broker/authorization/MultiRolesTokenAuthorizationProviderTest.java
+++ 
b/pulsar-broker-common/src/test/java/org/apache/pulsar/broker/authorization/MultiRolesTokenAuthorizationProviderTest.java
@@ -28,6 +28,7 @@ import java.util.function.Function;
 import lombok.Cleanup;
 import org.apache.pulsar.broker.ServiceConfiguration;
 import org.apache.pulsar.broker.authentication.AuthenticationDataSource;
+import org.apache.pulsar.broker.authentication.AuthenticationDataSubscription;
 import org.apache.pulsar.broker.authentication.utils.AuthTokenUtils;
 import org.apache.pulsar.broker.resources.PulsarResources;
 import org.testng.annotations.Test;
@@ -46,6 +47,8 @@ public class MultiRolesTokenAuthorizationProviderTest {
         String token = Jwts.builder().claim("sub", new String[]{userA, 
userB}).signWith(secretKey).compact();
 
         MultiRolesTokenAuthorizationProvider provider = new 
MultiRolesTokenAuthorizationProvider();
+        ServiceConfiguration conf = new ServiceConfiguration();
+        provider.initialize(conf, mock(PulsarResources.class));
 
         AuthenticationDataSource ads = new AuthenticationDataSource() {
             @Override
@@ -85,6 +88,8 @@ public class MultiRolesTokenAuthorizationProviderTest {
         String token = Jwts.builder().claim("sub", new 
String[]{}).signWith(secretKey).compact();
 
         MultiRolesTokenAuthorizationProvider provider = new 
MultiRolesTokenAuthorizationProvider();
+        ServiceConfiguration conf = new ServiceConfiguration();
+        provider.initialize(conf, mock(PulsarResources.class));
 
         AuthenticationDataSource ads = new AuthenticationDataSource() {
             @Override
@@ -112,6 +117,8 @@ public class MultiRolesTokenAuthorizationProviderTest {
         String token = Jwts.builder().claim("sub", 
testRole).signWith(secretKey).compact();
 
         MultiRolesTokenAuthorizationProvider provider = new 
MultiRolesTokenAuthorizationProvider();
+        ServiceConfiguration conf = new ServiceConfiguration();
+        provider.initialize(conf, mock(PulsarResources.class));
 
         AuthenticationDataSource ads = new AuthenticationDataSource() {
             @Override
@@ -141,6 +148,9 @@ public class MultiRolesTokenAuthorizationProviderTest {
     public void testMultiRolesAuthzWithAnonymousUser() throws Exception {
         @Cleanup
         MultiRolesTokenAuthorizationProvider provider = new 
MultiRolesTokenAuthorizationProvider();
+        ServiceConfiguration conf = new ServiceConfiguration();
+
+        provider.initialize(conf, mock(PulsarResources.class));
 
         Function<String, CompletableFuture<Boolean>> authorizeFunc = (String 
role) -> {
             if (role.equals("test-role")) {
@@ -150,6 +160,7 @@ public class MultiRolesTokenAuthorizationProviderTest {
         };
         assertTrue(provider.authorize("test-role", null, authorizeFunc).get());
         assertFalse(provider.authorize("test-role-x", null, 
authorizeFunc).get());
+        assertTrue(provider.authorize("test-role", new 
AuthenticationDataSubscription(null, "test-sub"), authorizeFunc).get());
     }
 
     @Test
@@ -157,6 +168,8 @@ public class MultiRolesTokenAuthorizationProviderTest {
         String token = "a-non-jwt-token";
 
         MultiRolesTokenAuthorizationProvider provider = new 
MultiRolesTokenAuthorizationProvider();
+        ServiceConfiguration conf = new ServiceConfiguration();
+        provider.initialize(conf, mock(PulsarResources.class));
 
         AuthenticationDataSource ads = new AuthenticationDataSource() {
             @Override
@@ -246,5 +259,14 @@ public class MultiRolesTokenAuthorizationProviderTest {
         };
 
         assertTrue(provider.isSuperUser(testAdminRole, ads, conf).get());
+        Function<String, CompletableFuture<Boolean>> authorizeFunc = (String 
role) -> {
+            if (role.equals("admin1")) {
+                return CompletableFuture.completedFuture(true);
+            }
+            return CompletableFuture.completedFuture(false);
+        };
+        assertTrue(provider.authorize(testAdminRole, ads, (String role) -> 
CompletableFuture.completedFuture(false)).get());
+        assertTrue(provider.authorize("admin1", null, authorizeFunc).get());
+        assertFalse(provider.authorize("admin2", null, authorizeFunc).get());
     }
 }

Reply via email to