[fix][broker] Fix MultiRoles token provider NPE when using anonymous …

…clients (apache#21429)
streamnative · Oct 30, 2023 · 8e1180a · 8e1180a
1 parent 80713a8
commit 8e1180a
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 8 deletions.
diff --git a/...src/main/java/org/apache/pulsar/broker/authentication/AuthenticationDataSubscription.java b/...src/main/java/org/apache/pulsar/broker/authentication/AuthenticationDataSubscription.java
@@ -71,4 +71,8 @@ public boolean hasSubscription() {
     public String getSubscription() {
         return subscription;
     }
+
+    public AuthenticationDataSource getAuthData() {
+        return authData;
+    }
 }
diff --git a/...ain/java/org/apache/pulsar/broker/authorization/MultiRolesTokenAuthorizationProvider.java b/...ain/java/org/apache/pulsar/broker/authorization/MultiRolesTokenAuthorizationProvider.java
@@ -35,6 +35,7 @@
 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 CompletableFuture<Boolean> validateTenantAdminAccess(String tenantName, S
     }
 
     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 @@ private Set<String> getRoles(String role, AuthenticationDataSource authData) {
 
     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/...java/org/apache/pulsar/broker/authorization/MultiRolesTokenAuthorizationProviderTest.java b/...java/org/apache/pulsar/broker/authorization/MultiRolesTokenAuthorizationProviderTest.java
@@ -31,6 +31,7 @@
 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;
@@ -49,6 +50,8 @@ public void testMultiRolesAuthz() throws Exception {
         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
@@ -88,6 +91,8 @@ public void testMultiRolesAuthzWithEmptyRoles() throws Exception {
         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
@@ -115,6 +120,8 @@ public void testMultiRolesAuthzWithSingleRole() throws Exception {
         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
@@ -144,6 +151,9 @@ public String getHttpHeader(String name) {
     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")) {
@@ -153,13 +163,16 @@ public void testMultiRolesAuthzWithAnonymousUser() throws Exception {
         };
         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
     public void testMultiRolesNotFailNonJWT() throws Exception {
         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
@@ -250,5 +263,14 @@ public String getHttpHeader(String name) {
         };
 
         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());
     }
 }