You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by ma...@apache.org on 2022/06/13 03:14:14 UTC
[pulsar] branch branch-2.9 updated (213b72f245b -> 6e7bd706cca)
This is an automated email from the ASF dual-hosted git repository.
mattisonchao pushed a change to branch branch-2.9
in repository https://gitbox.apache.org/repos/asf/pulsar.git
from 213b72f245b [cleanup][function] refine file io connector (#15250)
new e1b207e698e Fix wrong prompt exception when get non-persistent topic list without GET_BUDNLE permission (#14638)
new 6e7bd706cca Fix grant all permissions but can't list topic. (#15501)
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../authorization/PulsarAuthorizationProvider.java | 2 +-
.../broker/admin/v1/NonPersistentTopics.java | 25 +++++++++--------
.../broker/admin/v2/NonPersistentTopics.java | 31 ++++++++++------------
.../pulsar/broker/auth/AuthorizationTest.java | 27 ++++++++++++++++---
4 files changed, 51 insertions(+), 34 deletions(-)
[pulsar] 02/02: Fix grant all permissions but can't list topic. (#15501)
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
mattisonchao pushed a commit to branch branch-2.9
in repository https://gitbox.apache.org/repos/asf/pulsar.git
commit 6e7bd706ccaa9a4ef5afe8a55c4e208d7c585237
Author: Jiwei Guo <te...@apache.org>
AuthorDate: Mon May 9 22:05:07 2022 +0800
Fix grant all permissions but can't list topic. (#15501)
(cherry picked from commit 5155b1df876bd98d173e87753cca642b82b6595a)
---
.../authorization/PulsarAuthorizationProvider.java | 2 +-
.../apache/pulsar/broker/auth/AuthorizationTest.java | 18 +++---------------
2 files changed, 4 insertions(+), 16 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 9aea1261cf2..097464bfb5f 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
@@ -542,6 +542,7 @@ public class PulsarAuthorizationProvider implements AuthorizationProvider {
namespaceName, role, authData, AuthAction.packages);
case GET_TOPIC:
case GET_TOPICS:
+ case GET_BUNDLE:
return allowConsumeOrProduceOpsAsync(namespaceName, role, authData);
case UNSUBSCRIBE:
case CLEAR_BACKLOG:
@@ -550,7 +551,6 @@ public class PulsarAuthorizationProvider implements AuthorizationProvider {
case CREATE_TOPIC:
case DELETE_TOPIC:
case ADD_BUNDLE:
- case GET_BUNDLE:
case DELETE_BUNDLE:
case GRANT_PERMISSION:
case GET_PERMISSION:
diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/AuthorizationTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/AuthorizationTest.java
index 4b18791fce0..2596d243a9f 100644
--- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/AuthorizationTest.java
+++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/AuthorizationTest.java
@@ -19,7 +19,6 @@
package org.apache.pulsar.broker.auth;
import static org.mockito.Mockito.when;
-import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertTrue;
import static org.testng.Assert.fail;
@@ -27,7 +26,6 @@ import java.util.EnumSet;
import org.apache.pulsar.broker.authorization.AuthorizationService;
import org.apache.pulsar.client.admin.PulsarAdmin;
import org.apache.pulsar.client.admin.PulsarAdminBuilder;
-import org.apache.pulsar.client.admin.PulsarAdminException;
import org.apache.pulsar.common.naming.TopicDomain;
import org.apache.pulsar.common.naming.TopicName;
import org.apache.pulsar.common.policies.data.AuthAction;
@@ -233,7 +231,7 @@ public class AuthorizationTest extends MockedPulsarServiceBaseTest {
}
@Test
- public void testGetListWithoutGetBundleOp() throws Exception {
+ public void testGetListWithGetBundleOp() throws Exception {
String tenant = "p1";
String namespaceV1 = "p1/global/ns1";
String namespaceV2 = "p1/ns2";
@@ -249,18 +247,8 @@ public class AuthorizationTest extends MockedPulsarServiceBaseTest {
.authentication(new MockAuthentication("pass.pass2"))
.build();
when(pulsar.getAdminClient()).thenReturn(admin2);
- try {
- admin2.topics().getList(namespaceV1, TopicDomain.non_persistent);
- } catch (Exception ex) {
- assertTrue(ex instanceof PulsarAdminException.NotAuthorizedException);
- assertEquals(ex.getMessage(), "Unauthorized to validateNamespaceOperation for operation [GET_BUNDLE] on namespace [p1/global/ns1]");
- }
- try {
- admin2.topics().getList(namespaceV2, TopicDomain.non_persistent);
- } catch (Exception ex) {
- assertTrue(ex instanceof PulsarAdminException.NotAuthorizedException);
- assertEquals(ex.getMessage(), "Unauthorized to validateNamespaceOperation for operation [GET_BUNDLE] on namespace [p1/ns2]");
- }
+ Assert.assertEquals(admin2.topics().getList(namespaceV1, TopicDomain.non_persistent).size(), 0);
+ Assert.assertEquals(admin2.topics().getList(namespaceV2, TopicDomain.non_persistent).size(), 0);
}
private static void waitForChange() {
[pulsar] 01/02: Fix wrong prompt exception when get non-persistent topic list without GET_BUDNLE permission (#14638)
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
mattisonchao pushed a commit to branch branch-2.9
in repository https://gitbox.apache.org/repos/asf/pulsar.git
commit e1b207e698e1bc09fc08656f36dc2cd7fbda323a
Author: Jiwei Guo <te...@apache.org>
AuthorDate: Sat Mar 12 11:20:57 2022 +0800
Fix wrong prompt exception when get non-persistent topic list without GET_BUDNLE permission (#14638)
Fixes #14191
### Motivation
We have some big issues with the permission part. We only have the permission with [doc](https://pulsar.apache.org/docs/en/admin-api-permissions/) mentioned. But if user do it according to the doc, they will face the same issue that #14191 described. We don't have GET_BUNDLE in the grant interface but given the prompt message to the user. And currently, only the admin role could have the permission.
This pr is not solving the permission issue but fixing the prompt message first, not giving 500 error to the user. Then I open an issue https://github.com/apache/pulsar/issues/14639 to discuss refactoring the permission part.
### Modification
- Return 403 to the user when permission is denied.
(cherry picked from commit ca6e82422ecbc5272d3dda2c90873c5a493764e1)
---
.../broker/admin/v1/NonPersistentTopics.java | 25 +++++++-------
.../broker/admin/v2/NonPersistentTopics.java | 31 ++++++++---------
.../pulsar/broker/auth/AuthorizationTest.java | 39 ++++++++++++++++++++--
3 files changed, 62 insertions(+), 33 deletions(-)
diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/NonPersistentTopics.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/NonPersistentTopics.java
index 2a5ab779097..68a6c719942 100644
--- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/NonPersistentTopics.java
+++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/NonPersistentTopics.java
@@ -198,6 +198,7 @@ public class NonPersistentTopics extends PersistentTopics {
Policies policies = null;
NamespaceName nsName = null;
try {
+ validateNamespaceName(property, cluster, namespace);
validateNamespaceOperation(namespaceName, NamespaceOperation.GET_TOPICS);
policies = getNamespacePolicies(property, cluster, namespace);
nsName = NamespaceName.get(property, cluster, namespace);
@@ -232,22 +233,19 @@ public class NonPersistentTopics extends PersistentTopics {
}
}
- final List<String> topics = Lists.newArrayList();
- FutureUtil.waitForAll(futures).handle((result, exception) -> {
- for (int i = 0; i < futures.size(); i++) {
- try {
- if (futures.get(i).isDone() && futures.get(i).get() != null) {
- topics.addAll(futures.get(i).get());
+ FutureUtil.waitForAll(futures).whenComplete((result, ex) -> {
+ if (ex != null) {
+ resumeAsyncResponseExceptionally(asyncResponse, ex);
+ } else {
+ final List<String> topics = Lists.newArrayList();
+ for (int i = 0; i < futures.size(); i++) {
+ List<String> topicList = futures.get(i).join();
+ if (topicList != null) {
+ topics.addAll(topicList);
}
- } catch (InterruptedException | ExecutionException e) {
- log.error("[{}] Failed to get list of topics under namespace {}/{}/{}", clientAppId(), property,
- cluster, namespace, e);
- asyncResponse.resume(new RestException(e instanceof ExecutionException ? e.getCause() : e));
- return null;
}
+ asyncResponse.resume(topics);
}
- asyncResponse.resume(topics);
- return null;
});
}
@@ -264,6 +262,7 @@ public class NonPersistentTopics extends PersistentTopics {
@PathParam("bundle") String bundleRange) {
log.info("[{}] list of topics on namespace bundle {}/{}/{}/{}", clientAppId(), property, cluster, namespace,
bundleRange);
+ validateNamespaceName(property, cluster, namespace);
validateNamespaceOperation(namespaceName, NamespaceOperation.GET_BUNDLE);
Policies policies = getNamespacePolicies(property, cluster, namespace);
if (!cluster.equals(Constants.GLOBAL_CLUSTER)) {
diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/NonPersistentTopics.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/NonPersistentTopics.java
index fa52cb45997..38b1d725a70 100644
--- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/NonPersistentTopics.java
+++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/NonPersistentTopics.java
@@ -403,26 +403,23 @@ public class NonPersistentTopics extends PersistentTopics {
}
}
- final List<String> topics = Lists.newArrayList();
- FutureUtil.waitForAll(futures).handle((result, exception) -> {
- for (int i = 0; i < futures.size(); i++) {
- try {
- if (futures.get(i).isDone() && futures.get(i).get() != null) {
- topics.addAll(futures.get(i).get());
+ FutureUtil.waitForAll(futures).whenComplete((result, ex) -> {
+ if (ex != null) {
+ resumeAsyncResponseExceptionally(asyncResponse, ex);
+ } else {
+ final List<String> topics = Lists.newArrayList();
+ for (int i = 0; i < futures.size(); i++) {
+ List<String> topicList = futures.get(i).join();
+ if (topicList != null) {
+ topics.addAll(topicList);
}
- } catch (InterruptedException | ExecutionException e) {
- log.error("[{}] Failed to get list of topics under namespace {}", clientAppId(), namespaceName, e);
- asyncResponse.resume(new RestException(e instanceof ExecutionException ? e.getCause() : e));
- return null;
}
+ final List<String> nonPersistentTopics =
+ topics.stream()
+ .filter(name -> !TopicName.get(name).isPersistent())
+ .collect(Collectors.toList());
+ asyncResponse.resume(nonPersistentTopics);
}
-
- final List<String> nonPersistentTopics =
- topics.stream()
- .filter(name -> !TopicName.get(name).isPersistent())
- .collect(Collectors.toList());
- asyncResponse.resume(nonPersistentTopics);
- return null;
});
}
diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/AuthorizationTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/AuthorizationTest.java
index b7a54d137fd..4b18791fce0 100644
--- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/AuthorizationTest.java
+++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/AuthorizationTest.java
@@ -18,14 +18,17 @@
*/
package org.apache.pulsar.broker.auth;
+import static org.mockito.Mockito.when;
+import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertTrue;
import static org.testng.Assert.fail;
-
import java.util.EnumSet;
-
import org.apache.pulsar.broker.authorization.AuthorizationService;
+import org.apache.pulsar.client.admin.PulsarAdmin;
import org.apache.pulsar.client.admin.PulsarAdminBuilder;
+import org.apache.pulsar.client.admin.PulsarAdminException;
+import org.apache.pulsar.common.naming.TopicDomain;
import org.apache.pulsar.common.naming.TopicName;
import org.apache.pulsar.common.policies.data.AuthAction;
import org.apache.pulsar.common.policies.data.ClusterData;
@@ -36,7 +39,6 @@ import org.testng.Assert;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
-
import com.google.common.collect.Sets;
@Test(groups = "flaky")
@@ -230,6 +232,37 @@ public class AuthorizationTest extends MockedPulsarServiceBaseTest {
admin.clusters().deleteCluster("c1");
}
+ @Test
+ public void testGetListWithoutGetBundleOp() throws Exception {
+ String tenant = "p1";
+ String namespaceV1 = "p1/global/ns1";
+ String namespaceV2 = "p1/ns2";
+ admin.clusters().createCluster("c1", ClusterData.builder().build());
+ admin.tenants().createTenant(tenant, new TenantInfoImpl(Sets.newHashSet("role1"), Sets.newHashSet("c1")));
+ admin.namespaces().createNamespace(namespaceV1, Sets.newHashSet("c1"));
+ admin.namespaces().grantPermissionOnNamespace(namespaceV1, "pass.pass2", EnumSet.of(AuthAction.produce));
+ admin.namespaces().createNamespace(namespaceV2, Sets.newHashSet("c1"));
+ admin.namespaces().grantPermissionOnNamespace(namespaceV2, "pass.pass2", EnumSet.of(AuthAction.produce));
+ PulsarAdmin admin2 = PulsarAdmin.builder().serviceHttpUrl(brokerUrl != null
+ ? brokerUrl.toString()
+ : brokerUrlTls.toString())
+ .authentication(new MockAuthentication("pass.pass2"))
+ .build();
+ when(pulsar.getAdminClient()).thenReturn(admin2);
+ try {
+ admin2.topics().getList(namespaceV1, TopicDomain.non_persistent);
+ } catch (Exception ex) {
+ assertTrue(ex instanceof PulsarAdminException.NotAuthorizedException);
+ assertEquals(ex.getMessage(), "Unauthorized to validateNamespaceOperation for operation [GET_BUNDLE] on namespace [p1/global/ns1]");
+ }
+ try {
+ admin2.topics().getList(namespaceV2, TopicDomain.non_persistent);
+ } catch (Exception ex) {
+ assertTrue(ex instanceof PulsarAdminException.NotAuthorizedException);
+ assertEquals(ex.getMessage(), "Unauthorized to validateNamespaceOperation for operation [GET_BUNDLE] on namespace [p1/ns2]");
+ }
+ }
+
private static void waitForChange() {
try {
Thread.sleep(100);