fix(Auth): Fix multi roles authz cannot handle empty roles case (apac…

…he#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 <[email protected]>
lujp · Dec 30, 2021 · 4f942d7 · 4f942d7
1 parent 05a68bf
commit 4f942d7
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 2 deletions.
diff --git a/...ain/java/org/apache/pulsar/broker/authorization/MultiRolesTokenAuthorizationProvider.java b/...ain/java/org/apache/pulsar/broker/authorization/MultiRolesTokenAuthorizationProvider.java
@@ -127,6 +127,9 @@ private List<String> getRoles(AuthenticationDataSource authData) {
 
     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/...java/org/apache/pulsar/broker/authorization/MultiRolesTokenAuthorizationProviderTest.java b/...java/org/apache/pulsar/broker/authorization/MultiRolesTokenAuthorizationProviderTest.java
@@ -56,7 +56,9 @@ public String getHttpHeader(String name) {
         };
 
         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 String getHttpHeader(String name) {
         }).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());
+    }
 }