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 <zkyang@streamnative.io>
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 73f997f..e83c733 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
@@ -127,6 +127,9 @@
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 @@
};
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 @@
}).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());
+ }
}
