[pulsar] 02/16: fix(Auth): Fix multi roles authz cannot handle empty roles case (#13477)

[pe...@apache.org]

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

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

commit 395bd73b3684602fbdcca40b8e09972632cefb4a
Author: Zike Yang <zk...@streamnative.io>
AuthorDate: Thu Dec 30 19:13:41 2021 +0800

    fix(Auth): Fix multi roles authz cannot handle empty roles case (#13477)
    
    Motivation
    Currently, if the roles in the token are empty, then he `MultiRolesTokenAuthorizationProvider` will have problems processing it. It will keep waiting for an empty list of futures. Eventually causing the operation to time out.
    
    Modification
    * In `MultiRolesTokenAuthorizationProvider.authorize`, return false immediately when the roles are empty.
    
    Signed-off-by: Zike Yang <zk...@streamnative.io>
    (cherry picked from commit 4f942d71263424f12d9be88f39b79c41485d0f32)
---
 .../MultiRolesTokenAuthorizationProvider.java      |  3 ++
 .../MultiRolesTokenAuthorizationProviderTest.java  | 32 ++++++++++++++++++++--
 2 files changed, 33 insertions(+), 2 deletions(-)

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 dcdf779..8a91d7f 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
@@ -128,6 +128,9 @@ public class MultiRolesTokenAuthorizationProvider extends PulsarAuthorizationPro
 
     public CompletableFuture<Boolean> authorize(AuthenticationDataSource authenticationData, Function<String, CompletableFuture<Boolean>> authorizeFunc) {
         List<String> roles = getRoles(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 CompletableFuture.supplyAsync(() -> {
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 fdedf86..edd0baa 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
@@ -56,7 +56,9 @@ public class MultiRolesTokenAuthorizationProviderTest {
         };
 
         Assert.assertTrue(provider.authorize(ads, role -> {
-            if (role.equals(userB)) return CompletableFuture.completedFuture(true); // only userB has permission
+            if (role.equals(userB)) {
+                return CompletableFuture.completedFuture(true); // only userB has permission
+            }
             return CompletableFuture.completedFuture(false);
         }).get());
 
@@ -65,7 +67,33 @@ public class MultiRolesTokenAuthorizationProviderTest {
         }).get());
 
         Assert.assertFalse(provider.authorize(ads, role -> {
-            return CompletableFuture.completedFuture(false); // only users has no permission
+            return CompletableFuture.completedFuture(false); // all users has no permission
         }).get());
     }
+
+    @Test
+    public void testMultiRolesAuthzWithEmptyRoles() throws Exception {
+        SecretKey secretKey = AuthTokenUtils.createSecretKey(SignatureAlgorithm.HS256);
+        String token = Jwts.builder().claim("sub", new String[]{}).signWith(secretKey).compact();
+
+        MultiRolesTokenAuthorizationProvider provider = new MultiRolesTokenAuthorizationProvider();
+
+        AuthenticationDataSource ads = new AuthenticationDataSource() {
+            @Override
+            public boolean hasDataFromHttp() {
+                return true;
+            }
+
+            @Override
+            public String getHttpHeader(String name) {
+                if (name.equals("Authorization")) {
+                    return "Bearer " + token;
+                } else {
+                    throw new IllegalArgumentException("Wrong HTTP header");
+                }
+            }
+        };
+
+        Assert.assertFalse(provider.authorize(ads, role -> CompletableFuture.completedFuture(false)).get());
+    }
 }