From 043a95571366ed31e56f9e7ff7fd08440f8b4b33 Mon Sep 17 00:00:00 2001 From: guangning Date: Mon, 7 Aug 2023 10:35:08 +0800 Subject: [PATCH 1/5] Fixed super user role checked --- .../MultiRolesTokenAuthorizationProvider.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 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 fa613245cfa27..a4ce0d300a0f0 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 @@ -89,15 +89,18 @@ public void initialize(ServiceConfiguration conf, PulsarResources pulsarResource @Override public CompletableFuture isSuperUser(String role, AuthenticationDataSource authenticationData, ServiceConfiguration serviceConfiguration) { - Set roles = getRoles(authenticationData); - if (roles.isEmpty()) { - return CompletableFuture.completedFuture(false); - } + // if role contains in config, return true. Set superUserRoles = serviceConfiguration.getSuperUserRoles(); if (superUserRoles.isEmpty()) { return CompletableFuture.completedFuture(false); } - + if (role != null && superUserRoles.contains(role)) { + return CompletableFuture.completedFuture(true); + } + Set roles = getRoles(authenticationData); + if (roles.isEmpty()) { + return CompletableFuture.completedFuture(false); + } return CompletableFuture.completedFuture(roles.stream().anyMatch(superUserRoles::contains)); } From bae6af411a769429b204acb8665a15cd50cf2b30 Mon Sep 17 00:00:00 2001 From: guangning Date: Thu, 10 Aug 2023 07:30:53 +0800 Subject: [PATCH 2/5] Add test for super user role check --- ...tiRolesTokenAuthorizationProviderTest.java | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) 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 7e329d14307c7..f0a857bdd695d 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 @@ -31,6 +31,7 @@ import org.testng.annotations.Test; import javax.crypto.SecretKey; +import java.util.Set; import java.util.concurrent.CompletableFuture; public class MultiRolesTokenAuthorizationProviderTest { @@ -198,4 +199,35 @@ public String getHttpHeader(String name) { return CompletableFuture.completedFuture(false); }).get()); } + + @Test + public void testMultiRolesAuthzWithSuperUser() throws Exception { + SecretKey secretKey = AuthTokenUtils.createSecretKey(SignatureAlgorithm.HS256); + String testAdminRole = "admin"; + String token = Jwts.builder().claim("sub", testAdminRole).signWith(secretKey).compact(); + + ServiceConfiguration conf = new ServiceConfiguration(); + conf.setSuperUserRoles(Set.of(testAdminRole)); + + MultiRolesTokenAuthorizationProvider provider = new MultiRolesTokenAuthorizationProvider(); + provider.initialize(conf, mock(PulsarResources.class)); + + 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"); + } + } + }; + + assertTrue(provider.isSuperUser(testAdminRole, ads, conf).get()); + } } From 211ede3139b6db4fe834dfc36d3fe54d3e092d27 Mon Sep 17 00:00:00 2001 From: guangning Date: Thu, 10 Aug 2023 07:33:02 +0800 Subject: [PATCH 3/5] Fixed super user role check --- .../authorization/MultiRolesTokenAuthorizationProvider.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 a4ce0d300a0f0..db5f4f18e8cc3 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 @@ -89,7 +89,7 @@ public void initialize(ServiceConfiguration conf, PulsarResources pulsarResource @Override public CompletableFuture isSuperUser(String role, AuthenticationDataSource authenticationData, ServiceConfiguration serviceConfiguration) { - // if role contains in config, return true. + // if superUser role contains in config, return true. Set superUserRoles = serviceConfiguration.getSuperUserRoles(); if (superUserRoles.isEmpty()) { return CompletableFuture.completedFuture(false); From 6cf14f28b261ee5f91a53501bd4355f161b3f4ea Mon Sep 17 00:00:00 2001 From: guangning Date: Fri, 11 Aug 2023 17:32:32 +0800 Subject: [PATCH 4/5] Update timeout to 30s --- .../apache/pulsar/client/impl/KeySharedSubscriptionTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/KeySharedSubscriptionTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/KeySharedSubscriptionTest.java index 6f7d8d9c3f181..1d534176e8d61 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/KeySharedSubscriptionTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/KeySharedSubscriptionTest.java @@ -146,7 +146,7 @@ public void testCanRecoverConsumptionWhenLiftMaxUnAckedMessagesRestriction(Subsc // Wait for all consumers to continue receiving messages. Awaitility.await() - .atMost(15, TimeUnit.SECONDS) + .atMost(30, TimeUnit.SECONDS) .pollDelay(5, TimeUnit.SECONDS) .until(() -> (System.currentTimeMillis() - lastActiveTime.get()) > TimeUnit.SECONDS.toMillis(5)); From 1314d9051421dc571d4b37fcb65e47eb52e0ca42 Mon Sep 17 00:00:00 2001 From: guangning Date: Fri, 11 Aug 2023 18:40:36 +0800 Subject: [PATCH 5/5] Revert timeout --- .../apache/pulsar/client/impl/KeySharedSubscriptionTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/KeySharedSubscriptionTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/KeySharedSubscriptionTest.java index 1d534176e8d61..6f7d8d9c3f181 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/KeySharedSubscriptionTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/KeySharedSubscriptionTest.java @@ -146,7 +146,7 @@ public void testCanRecoverConsumptionWhenLiftMaxUnAckedMessagesRestriction(Subsc // Wait for all consumers to continue receiving messages. Awaitility.await() - .atMost(30, TimeUnit.SECONDS) + .atMost(15, TimeUnit.SECONDS) .pollDelay(5, TimeUnit.SECONDS) .until(() -> (System.currentTimeMillis() - lastActiveTime.get()) > TimeUnit.SECONDS.toMillis(5));