From 3b76dd435d1bb7f5f933dbc15307255c7c928ca4 Mon Sep 17 00:00:00 2001 From: Michael Marshall Date: Sat, 1 Apr 2023 01:16:26 -0500 Subject: [PATCH 1/5] [fix][broker] Only validate superuser access if authz enabled --- .../pulsar/broker/web/PulsarWebResource.java | 29 +++++++------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/web/PulsarWebResource.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/web/PulsarWebResource.java index c5713ebddaa2f..01198c3ec60c9 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/web/PulsarWebResource.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/web/PulsarWebResource.java @@ -185,8 +185,8 @@ protected boolean hasSuperUserAccess() { return true; } - public CompletableFuture validateSuperUserAccessAsync(){ - if (!config().isAuthenticationEnabled()) { + public CompletableFuture validateSuperUserAccessAsync() { + if (!config().isAuthorizationEnabled()) { return CompletableFuture.completedFuture(null); } String appId = clientAppId(); @@ -221,22 +221,15 @@ public CompletableFuture validateSuperUserAccessAsync(){ } }); } else { - if (config().isAuthorizationEnabled()) { - return pulsar.getBrokerService() - .getAuthorizationService() - .isSuperUser(appId, clientAuthData()) - .thenAccept(proxyAuthorizationSuccess -> { - if (!proxyAuthorizationSuccess) { - throw new RestException(Status.UNAUTHORIZED, - "This operation requires super-user access"); - } - }); - } - if (log.isDebugEnabled()) { - log.debug("Successfully authorized {} as super-user", - appId); - } - return CompletableFuture.completedFuture(null); + return pulsar.getBrokerService() + .getAuthorizationService() + .isSuperUser(appId, clientAuthData()) + .thenAccept(proxyAuthorizationSuccess -> { + if (!proxyAuthorizationSuccess) { + throw new RestException(Status.UNAUTHORIZED, + "This operation requires super-user access"); + } + }); } } From bff583aed630bd14966bd7de1792382682bf54ac Mon Sep 17 00:00:00 2001 From: Michael Marshall Date: Tue, 4 Apr 2023 01:31:07 -0500 Subject: [PATCH 2/5] Fix testUpdateTransactionCoordinatorNumber: enable authorization --- .../apache/pulsar/broker/admin/v3/AdminApiTransactionTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/v3/AdminApiTransactionTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/v3/AdminApiTransactionTest.java index 019b7c11fd579..0e51470da75a5 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/v3/AdminApiTransactionTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/v3/AdminApiTransactionTest.java @@ -630,6 +630,7 @@ public void testUpdateTransactionCoordinatorNumber() throws Exception { Awaitility.await().until(() -> pulsar.getTransactionMetadataStoreService().getStores().size() == coordinatorSize * 2); pulsar.getConfiguration().setAuthenticationEnabled(true); + pulsar.getConfiguration().setAuthorizationEnabled(true); Set proxyRoles = spy(Set.class); doReturn(true).when(proxyRoles).contains(any()); pulsar.getConfiguration().setProxyRoles(proxyRoles); From c083f8ebd6cb0d462d2c1bfa5beed28aefd3db7d Mon Sep 17 00:00:00 2001 From: Michael Marshall Date: Wed, 5 Apr 2023 00:54:46 -0500 Subject: [PATCH 3/5] Remove misleading authz config from test --- .../java/org/apache/pulsar/broker/service/ReplicatorTest.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ReplicatorTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ReplicatorTest.java index 901451c022bfe..67432a6e20bb2 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ReplicatorTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ReplicatorTest.java @@ -233,9 +233,7 @@ public Void call() throws Exception { @Test(timeOut = 10000) public void activeBrokerParse() throws Exception { - pulsar1.getConfiguration().setAuthorizationEnabled(true); //init clusterData - String cluster2ServiceUrls = String.format("%s,localhost:1234,localhost:5678,localhost:5677,localhost:5676", pulsar2.getWebServiceAddress()); ClusterData cluster2Data = ClusterData.builder().serviceUrl(cluster2ServiceUrls).build(); @@ -246,8 +244,6 @@ public void activeBrokerParse() throws Exception { List list = admin1.brokers().getActiveBrokers(cluster2); assertEquals(list.get(0), url2.toString().replace("http://", "")); - //restore configuration - pulsar1.getConfiguration().setAuthorizationEnabled(false); } @SuppressWarnings("unchecked") From 1bad34242d2329c91c70dbbd377ceedc00317675 Mon Sep 17 00:00:00 2001 From: Michael Marshall Date: Wed, 5 Apr 2023 15:33:15 -0500 Subject: [PATCH 4/5] Revert "Remove misleading authz config from test" This reverts commit c083f8ebd6cb0d462d2c1bfa5beed28aefd3db7d. --- .../java/org/apache/pulsar/broker/service/ReplicatorTest.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ReplicatorTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ReplicatorTest.java index 67432a6e20bb2..901451c022bfe 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ReplicatorTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ReplicatorTest.java @@ -233,7 +233,9 @@ public Void call() throws Exception { @Test(timeOut = 10000) public void activeBrokerParse() throws Exception { + pulsar1.getConfiguration().setAuthorizationEnabled(true); //init clusterData + String cluster2ServiceUrls = String.format("%s,localhost:1234,localhost:5678,localhost:5677,localhost:5676", pulsar2.getWebServiceAddress()); ClusterData cluster2Data = ClusterData.builder().serviceUrl(cluster2ServiceUrls).build(); @@ -244,6 +246,8 @@ public void activeBrokerParse() throws Exception { List list = admin1.brokers().getActiveBrokers(cluster2); assertEquals(list.get(0), url2.toString().replace("http://", "")); + //restore configuration + pulsar1.getConfiguration().setAuthorizationEnabled(false); } @SuppressWarnings("unchecked") From 04f9e980b2ac877f81e0c5cfe24eac5072ee5585 Mon Sep 17 00:00:00 2001 From: Michael Marshall Date: Wed, 5 Apr 2023 15:53:48 -0500 Subject: [PATCH 5/5] Include authentication check due to hack --- .../java/org/apache/pulsar/broker/web/PulsarWebResource.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/web/PulsarWebResource.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/web/PulsarWebResource.java index 01198c3ec60c9..a182e4733fdfe 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/web/PulsarWebResource.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/web/PulsarWebResource.java @@ -186,7 +186,7 @@ protected boolean hasSuperUserAccess() { } public CompletableFuture validateSuperUserAccessAsync() { - if (!config().isAuthorizationEnabled()) { + if (!config().isAuthenticationEnabled() || !config().isAuthorizationEnabled()) { return CompletableFuture.completedFuture(null); } String appId = clientAppId();