From a7bc96c05a80dc3c98599c79f1a97eca5820741b Mon Sep 17 00:00:00 2001 From: mattisonchao Date: Mon, 20 Apr 2026 11:47:38 +0800 Subject: [PATCH] [fix][broker] Use explicit auth ops for namespace properties endpoints --- .../PulsarAuthorizationProvider.java | 3 ++ .../broker/admin/impl/NamespacesBase.java | 49 ++++++++++++++++--- .../pulsar/broker/web/PulsarWebResource.java | 1 + .../broker/admin/NamespaceAuthZTest.java | 14 ++++++ .../policies/data/NamespaceOperation.java | 3 ++ 5 files changed, 64 insertions(+), 6 deletions(-) diff --git a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/PulsarAuthorizationProvider.java b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/PulsarAuthorizationProvider.java index ab5399c34eb82..6a9f0659924f7 100644 --- a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/PulsarAuthorizationProvider.java +++ b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/PulsarAuthorizationProvider.java @@ -606,6 +606,9 @@ public CompletableFuture allowNamespaceOperationAsync(NamespaceName nam case GRANT_PERMISSION: case GET_PERMISSION: case REVOKE_PERMISSION: + case GET_PROPERTIES: + case UPDATE_PROPERTIES: + case DELETE_PROPERTIES: return CompletableFuture.completedFuture(false); default: return FutureUtil.failedFuture(new IllegalStateException( diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java index 1cded999be527..b948bf7a866d3 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java @@ -2759,7 +2759,7 @@ protected void internalSetMaxTopicsPerNamespace(Integer maxTopicsPerNamespace) { } protected void internalSetProperty(String key, String value, AsyncResponse asyncResponse) { - validateAdminAccessForTenantAsync(namespaceName.getTenant()) + validateBothAdminAccessForTenantAndNamespaceOperationAsync(namespaceName, NamespaceOperation.UPDATE_PROPERTIES) .thenCompose(__ -> validatePoliciesReadOnlyAccessAsync()) .thenCompose(__ -> updatePoliciesAsync(namespaceName, policies -> { policies.properties.put(key, value); @@ -2779,7 +2779,7 @@ protected void internalSetProperty(String key, String value, AsyncResponse async } protected void internalSetProperties(Map properties, AsyncResponse asyncResponse) { - validateAdminAccessForTenantAsync(namespaceName.getTenant()) + validateBothAdminAccessForTenantAndNamespaceOperationAsync(namespaceName, NamespaceOperation.UPDATE_PROPERTIES) .thenCompose(__ -> validatePoliciesReadOnlyAccessAsync()) .thenCompose(__ -> updatePoliciesAsync(namespaceName, policies -> { policies.properties.putAll(properties); @@ -2799,7 +2799,7 @@ protected void internalSetProperties(Map properties, AsyncRespon } protected void internalGetProperty(String key, AsyncResponse asyncResponse) { - validateAdminAccessForTenantAsync(namespaceName.getTenant()) + validateBothAdminAccessForTenantAndNamespaceOperationAsync(namespaceName, NamespaceOperation.GET_PROPERTIES) .thenCompose(__ -> getNamespacePoliciesAsync(namespaceName)) .thenAccept(policies -> asyncResponse.resume(policies.properties.get(key))) .exceptionally(ex -> { @@ -2812,7 +2812,7 @@ protected void internalGetProperty(String key, AsyncResponse asyncResponse) { } protected void internalGetProperties(AsyncResponse asyncResponse) { - validateAdminAccessForTenantAsync(namespaceName.getTenant()) + validateBothAdminAccessForTenantAndNamespaceOperationAsync(namespaceName, NamespaceOperation.GET_PROPERTIES) .thenCompose(__ -> getNamespacePoliciesAsync(namespaceName)) .thenAccept(policies -> asyncResponse.resume(policies.properties)) .exceptionally(ex -> { @@ -2825,7 +2825,7 @@ protected void internalGetProperties(AsyncResponse asyncResponse) { protected void internalRemoveProperty(String key, AsyncResponse asyncResponse) { AtomicReference oldVal = new AtomicReference<>(null); - validateAdminAccessForTenantAsync(namespaceName.getTenant()) + validateBothAdminAccessForTenantAndNamespaceOperationAsync(namespaceName, NamespaceOperation.DELETE_PROPERTIES) .thenCompose(__ -> validatePoliciesReadOnlyAccessAsync()) .thenCompose(__ -> updatePoliciesAsync(namespaceName, policies -> { oldVal.set(policies.properties.remove(key)); @@ -2845,7 +2845,7 @@ protected void internalRemoveProperty(String key, AsyncResponse asyncResponse) { protected void internalClearProperties(AsyncResponse asyncResponse) { AtomicReference clearedCount = new AtomicReference<>(0); - validateAdminAccessForTenantAsync(namespaceName.getTenant()) + validateBothAdminAccessForTenantAndNamespaceOperationAsync(namespaceName, NamespaceOperation.DELETE_PROPERTIES) .thenCompose(__ -> validatePoliciesReadOnlyAccessAsync()) .thenCompose(__ -> updatePoliciesAsync(namespaceName, policies -> { clearedCount.set(policies.properties.size()); @@ -3206,4 +3206,41 @@ private CompletableFuture getDefaultBundleDataAsync() { getBundles(config().getDefaultNumberOfNamespaceBundles())); } + + protected CompletableFuture validateBothAdminAccessForTenantAndNamespaceOperationAsync( + NamespaceName namespaceName, NamespaceOperation operation) { + final var tenantAdminValidation = validateAdminAccessForTenantAsync(namespaceName.getTenant()); + final var namespaceOperationValidation = validateNamespaceOperationAsync(namespaceName, operation); + return FutureUtil.waitForAll(List.of(tenantAdminValidation, namespaceOperationValidation)) + .handle((result, err) -> { + if (!tenantAdminValidation.isCompletedExceptionally() + || !namespaceOperationValidation.isCompletedExceptionally()) { + return null; + } + if (log.isDebugEnabled()) { + Throwable tenantAdminValidationException = null; + try { + tenantAdminValidation.join(); + } catch (Throwable ex) { + tenantAdminValidationException = FutureUtil.unwrapCompletionException(ex); + } + Throwable namespaceOperationValidationException = null; + try { + namespaceOperationValidation.join(); + } catch (Throwable ex) { + namespaceOperationValidationException = FutureUtil.unwrapCompletionException(ex); + } + log.debug("validateBothAdminAccessForTenantAndNamespaceOperationAsync failed." + + " originalPrincipal={} clientAppId={} operation={} namespace={} " + + "tenantAdminValidationError={} namespaceOperationValidationError={}", + originalPrincipal(), clientAppId(), operation.toString(), namespaceName, + tenantAdminValidationException, namespaceOperationValidationException); + } + throw new RestException(Status.UNAUTHORIZED, + String.format("Unauthorized to validateBothAdminAccessForTenantAndNamespaceOperationAsync" + + " for originalPrincipal [%s] and clientAppId [%s] about operation [%s]" + + " on namespace [%s]", + originalPrincipal(), clientAppId(), operation.toString(), namespaceName)); + }); + } } 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 b702e1b4a1780..6dfc6d274040b 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 @@ -31,6 +31,7 @@ import java.net.URI; import java.net.URL; import java.time.Duration; +import java.util.List; import java.util.Optional; import java.util.Set; import java.util.concurrent.CompletableFuture; diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespaceAuthZTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespaceAuthZTest.java index ca9f07c474bcf..1df5ad14b253b 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespaceAuthZTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespaceAuthZTest.java @@ -242,24 +242,38 @@ public void testProperties() { tenantManagerAdmin.namespaces().clearProperties(namespace); // test nobody + AtomicBoolean execFlag = setAuthorizationOperationChecker(subject, NamespaceOperation.UPDATE_PROPERTIES); Assert.assertThrows(PulsarAdminException.NotAuthorizedException.class, () -> subAdmin.namespaces().setProperties(namespace, properties)); + Assert.assertTrue(execFlag.get()); + execFlag = setAuthorizationOperationChecker(subject, NamespaceOperation.UPDATE_PROPERTIES); Assert.assertThrows(PulsarAdminException.NotAuthorizedException.class, () -> subAdmin.namespaces().setProperty(namespace, "key2", "value2")); + Assert.assertTrue(execFlag.get()); + execFlag = setAuthorizationOperationChecker(subject, NamespaceOperation.GET_PROPERTIES); Assert.assertThrows(PulsarAdminException.NotAuthorizedException.class, () -> subAdmin.namespaces().getProperties(namespace)); + Assert.assertTrue(execFlag.get()); + execFlag = setAuthorizationOperationChecker(subject, NamespaceOperation.GET_PROPERTIES); Assert.assertThrows(PulsarAdminException.NotAuthorizedException.class, () -> subAdmin.namespaces().getProperty(namespace, "key2")); + Assert.assertTrue(execFlag.get()); + execFlag = setAuthorizationOperationChecker(subject, NamespaceOperation.DELETE_PROPERTIES); Assert.assertThrows(PulsarAdminException.NotAuthorizedException.class, () -> subAdmin.namespaces().removeProperty(namespace, "key2")); + Assert.assertTrue(execFlag.get()); + execFlag = setAuthorizationOperationChecker(subject, NamespaceOperation.DELETE_PROPERTIES); Assert.assertThrows(PulsarAdminException.NotAuthorizedException.class, () -> subAdmin.namespaces().clearProperties(namespace)); + Assert.assertTrue(execFlag.get()); + + clearAuthorizationOperationChecker(); for (AuthAction action : AuthAction.values()) { superUserAdmin.namespaces().grantPermissionOnNamespace(namespace, subject, Set.of(action)); diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/NamespaceOperation.java b/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/NamespaceOperation.java index 732072747a417..34132f5944f24 100644 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/NamespaceOperation.java +++ b/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/NamespaceOperation.java @@ -34,6 +34,9 @@ public enum NamespaceOperation { GET_PERMISSION, GRANT_PERMISSION, REVOKE_PERMISSION, + GET_PROPERTIES, + UPDATE_PROPERTIES, + DELETE_PROPERTIES, CLEAR_BACKLOG, UNSUBSCRIBE,