[fix][broker] Use explicit auth ops for namespace properties endpoints

[mattisonchao]

Motivation

Namespace property / properties endpoints still authorize through validateAdminAccessForTenantAsync, so they do not map to explicit namespace operations. This makes it harder to introduce operation-specific authorization for namespace property reads, updates, and deletes while still keeping compatibility with the legacy tenant-admin authorization path.

Modifications

• Added GET_PROPERTIES, UPDATE_PROPERTIES, and DELETE_PROPERTIES to NamespaceOperation
• Updated PulsarAuthorizationProvider to recognize the new namespace property operations
• Added a compatibility helper that accepts either tenant-admin authorization or namespace-operation authorization
• Switched namespace property / properties endpoints to use explicit read, update, and delete operations
• Extended NamespaceAuthZTest to verify the expected operation is checked for each endpoint

Verifying this change

This change is already covered by existing tests, such as:

• ./gradlew --no-configuration-cache :pulsar-broker:test --tests org.apache.pulsar.broker.admin.NamespaceAuthZTest --rerun-tasks

Does this pull request potentially affect one of the following parts:

• Dependencies (add or upgrade a dependency)
• The public API
• The schema
• The default values of configurations
• The threading model
• The binary protocol
• The REST endpoints
• The admin CLI options
• The metrics
• Anything that affects deployment

Documentation

• doc
• doc-required
• doc-not-needed
• doc-complete

Matching PR in forked repository

PR in forked repository: mattisonchao#3

[Copilot]

Pull request overview

This PR introduces explicit namespace-authorization operations for namespace property/properties REST endpoints, while keeping compatibility with the legacy tenant-admin authorization path. It helps make future operation-specific authorization for namespace property reads/updates/deletes possible without breaking existing admin flows.

Changes:

• Added GET_PROPERTIES, UPDATE_PROPERTIES, and DELETE_PROPERTIES to NamespaceOperation.
• Switched namespace property endpoints to validate explicit namespace operations (with a compatibility helper that also accepts tenant-admin auth).
• Updated PulsarAuthorizationProvider and extended NamespaceAuthZTest to cover the new operations.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/NamespaceOperation.java	Adds new namespace operations for property endpoints.
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/PulsarAuthorizationProvider.java	Ensures the default provider recognizes the new operations.
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java	Switches property endpoints to explicit operations and adds a compatibility validation helper.
pulsar-broker/src/main/java/org/apache/pulsar/broker/web/PulsarWebResource.java	Updates imports (currently introduces an unused import).
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespaceAuthZTest.java	Adds assertions that the correct namespace operation is checked per endpoint.

[Copilot]

The added java.util.List import appears to be unused in this file (the only List usage is still fully-qualified as java.util.List), which will fail style checks (unused imports) and should be removed or the fully-qualified usages should be switched to the imported type.

Suggested change

import java.util.List;

[Copilot]

validateBothAdminAccessForTenantAndNamespaceOperationAsync always starts both authorization checks and waits for both to complete, even if the tenant-admin check succeeds. This adds extra auth traffic/latency to every namespace properties request and can make the endpoint as slow as the slower of the two checks. Consider short-circuiting on the first successful validation (and only running the second path if the first fails).

Suggested change

	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));
	});
	return validateAdminAccessForTenantAsync(namespaceName.getTenant())
	.handle((result, tenantAdminValidationError) -> {
	if (tenantAdminValidationError == null) {
	return CompletableFuture.<Void>completedFuture(null);
	}
	final Throwable tenantAdminValidationException =
	FutureUtil.unwrapCompletionException(tenantAdminValidationError);
	return validateNamespaceOperationAsync(namespaceName, operation)
	.handle((namespaceResult, namespaceOperationValidationError) -> {
	if (namespaceOperationValidationError == null) {
	return null;
	}
	final Throwable namespaceOperationValidationException =
	FutureUtil.unwrapCompletionException(namespaceOperationValidationError);
	if (log.isDebugEnabled()) {
	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));
	});
	}).thenCompose(Function.identity());

[Copilot]

When both validations fail, this helper throws a new RestException(Status.UNAUTHORIZED, ...), which can change the HTTP status and semantics compared to the underlying checks (e.g., validateNamespaceOperationAsync uses 403 FORBIDDEN; validateAdminAccessForTenantAsync can return 404 NOT_FOUND for missing tenants). This risks returning 401 for cases that should remain 403/404 and also discards the original error details. Prefer propagating one of the original exceptions (or selecting the most appropriate status) instead of always creating a new 401.

[lhotari]

Just wondering if adding new enums breaks existing 3rd party AuthorizationProviders which many Pulsar users use currently in production?