You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by je...@apache.org on 2019/05/01 08:57:04 UTC
[pulsar] branch master updated: fix function authorization bug
involving a tenant with no admin (#4186)
This is an automated email from the ASF dual-hosted git repository.
jerrypeng pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pulsar.git
The following commit(s) were added to refs/heads/master by this push:
new 2bc10e7 fix function authorization bug involving a tenant with no admin (#4186)
2bc10e7 is described below
commit 2bc10e74493d4688d13b66f8a74fdc82c8637590
Author: Boyang Jerry Peng <je...@gmail.com>
AuthorDate: Wed May 1 01:56:58 2019 -0700
fix function authorization bug involving a tenant with no admin (#4186)
* fix function authorization bug involving a tenant with no admin
* cleaning up comments
---
.../worker/PulsarFunctionE2ESecurityTest.java | 18 +++++
.../functions/worker/rest/api/ComponentImpl.java | 10 ++-
.../worker/rest/api/FunctionsImplTest.java | 90 ++++++++++++++++++++++
3 files changed, 114 insertions(+), 4 deletions(-)
diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/functions/worker/PulsarFunctionE2ESecurityTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/functions/worker/PulsarFunctionE2ESecurityTest.java
index b139457..88bb83e 100644
--- a/pulsar-broker/src/test/java/org/apache/pulsar/functions/worker/PulsarFunctionE2ESecurityTest.java
+++ b/pulsar-broker/src/test/java/org/apache/pulsar/functions/worker/PulsarFunctionE2ESecurityTest.java
@@ -85,6 +85,8 @@ public class PulsarFunctionE2ESecurityTest {
BrokerStats brokerStatsClient;
WorkerService functionsWorkerService;
final String TENANT = "external-repl-prop";
+ final String TENANT2 = "tenant2";
+
final String NAMESPACE = "test-ns";
String pulsarFunctionsNamespace = TENANT + "/use/pulsar-function-admin";
String primaryHost;
@@ -178,11 +180,18 @@ public class PulsarFunctionE2ESecurityTest {
propAdmin.setAllowedClusters(Sets.newHashSet(Lists.newArrayList("use")));
superUserAdmin.tenants().updateTenant(TENANT, propAdmin);
+
final String replNamespace = TENANT + "/" + NAMESPACE;
superUserAdmin.namespaces().createNamespace(replNamespace);
Set<String> clusters = Sets.newHashSet(Lists.newArrayList("use"));
superUserAdmin.namespaces().setNamespaceReplicationClusters(replNamespace, clusters);
+ // create another test tenant and namespace
+ propAdmin = new TenantInfo();
+ propAdmin.setAllowedClusters(Sets.newHashSet(Lists.newArrayList("use")));
+ superUserAdmin.tenants().createTenant(TENANT2, propAdmin);
+ superUserAdmin.namespaces().createNamespace( TENANT2 + "/" + NAMESPACE);
+
System.setProperty(JAVA_INSTANCE_JAR_PROPERTY, "");
Thread.sleep(100);
@@ -309,6 +318,15 @@ public class PulsarFunctionE2ESecurityTest {
}
+ // creating on another tenant should also fail
+ try {
+ admin2.functions().createFunctionWithUrl(createFunctionConfig(TENANT2, NAMESPACE, functionName,
+ sourceTopic, sinkTopic, subscriptionName), jarFilePathUrl);
+ fail("client admin shouldn't have permissions to create function");
+ } catch (PulsarAdminException.NotAuthorizedException e) {
+
+ }
+
retryStrategically((test) -> {
try {
return admin1.functions().getFunctionStatus(TENANT, NAMESPACE, functionName).getNumRunning() == 1
diff --git a/pulsar-functions/worker/src/main/java/org/apache/pulsar/functions/worker/rest/api/ComponentImpl.java b/pulsar-functions/worker/src/main/java/org/apache/pulsar/functions/worker/rest/api/ComponentImpl.java
index 87dc897..3ef5ec5 100644
--- a/pulsar-functions/worker/src/main/java/org/apache/pulsar/functions/worker/rest/api/ComponentImpl.java
+++ b/pulsar-functions/worker/src/main/java/org/apache/pulsar/functions/worker/rest/api/ComponentImpl.java
@@ -1692,10 +1692,12 @@ private FunctionDetails validateUpdateRequestParams(final String tenant,
if (isSuperUser(clientRole)) {
return true;
}
- TenantInfo tenantInfo = worker().getBrokerAdmin().tenants().getTenantInfo(tenant);
- if (clientRole != null && (tenantInfo.getAdminRoles() == null || tenantInfo.getAdminRoles().isEmpty()
- || tenantInfo.getAdminRoles().contains(clientRole))) {
- return true;
+
+ if (clientRole != null) {
+ TenantInfo tenantInfo = worker().getBrokerAdmin().tenants().getTenantInfo(tenant);
+ if (tenantInfo.getAdminRoles() != null && tenantInfo.getAdminRoles().contains(clientRole)) {
+ return true;
+ }
}
// check if role has permissions granted
diff --git a/pulsar-functions/worker/src/test/java/org/apache/pulsar/functions/worker/rest/api/FunctionsImplTest.java b/pulsar-functions/worker/src/test/java/org/apache/pulsar/functions/worker/rest/api/FunctionsImplTest.java
index 565033f..78248e0 100644
--- a/pulsar-functions/worker/src/test/java/org/apache/pulsar/functions/worker/rest/api/FunctionsImplTest.java
+++ b/pulsar-functions/worker/src/test/java/org/apache/pulsar/functions/worker/rest/api/FunctionsImplTest.java
@@ -19,8 +19,10 @@
package org.apache.pulsar.functions.worker.rest.api;
import org.apache.distributedlog.api.namespace.Namespace;
+import org.apache.pulsar.broker.authentication.AuthenticationDataSource;
import org.apache.pulsar.client.admin.Namespaces;
import org.apache.pulsar.client.admin.PulsarAdmin;
+import org.apache.pulsar.client.admin.PulsarAdminException;
import org.apache.pulsar.client.admin.Tenants;
import org.apache.pulsar.common.functions.FunctionConfig;
import org.apache.pulsar.common.policies.data.FunctionStats;
@@ -54,11 +56,13 @@ import org.testng.annotations.ObjectFactory;
import org.testng.annotations.Test;
import java.io.InputStream;
+import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.concurrent.CompletableFuture;
+import java.util.function.Supplier;
import static org.apache.pulsar.functions.utils.ComponentType.FUNCTION;
import static org.mockito.Matchers.any;
@@ -100,6 +104,7 @@ public class FunctionsImplTest {
}
private static final int parallelism = 1;
private static final String workerId = "worker-0";
+ private static final String superUser = "superUser";
private WorkerService mockedWorkerService;
private PulsarAdmin mockedPulsarAdmin;
@@ -225,6 +230,91 @@ public class FunctionsImplTest {
Assert.assertTrue(functionStats.calculateOverall() != null);
}
+ @Test
+ public void testIsAuthorizedRole() throws PulsarAdminException {
+
+
+ FunctionsImpl functionImpl = spy(new FunctionsImpl(() -> mockedWorkerService));
+ WorkerConfig workerConfig = new WorkerConfig();
+ workerConfig.setAuthorizationEnabled(true);
+ workerConfig.setSuperUserRoles(Collections.singleton(superUser));
+ doReturn(workerConfig).when(mockedWorkerService).getWorkerConfig();
+
+ // test super user
+ AuthenticationDataSource authenticationDataSource = mock(AuthenticationDataSource.class);
+ Assert.assertTrue(functionImpl.isAuthorizedRole("test-tenant", "test-ns", superUser, authenticationDataSource));
+
+ // test normal user
+ functionImpl = spy(new FunctionsImpl(() -> mockedWorkerService));
+ doReturn(false).when(functionImpl).allowFunctionOps(any(), any(), any());
+ Tenants tenants = mock(Tenants.class);
+ when(tenants.getTenantInfo(any())).thenReturn(new TenantInfo());
+ PulsarAdmin admin = mock(PulsarAdmin.class);
+ when(admin.tenants()).thenReturn(tenants);
+ when(this.mockedWorkerService.getBrokerAdmin()).thenReturn(admin);
+ Assert.assertFalse(functionImpl.isAuthorizedRole("test-tenant", "test-ns", "test-user", authenticationDataSource));
+
+ // if user is tenant admin
+ functionImpl = spy(new FunctionsImpl(() -> mockedWorkerService));
+ doReturn(false).when(functionImpl).allowFunctionOps(any(), any(), any());
+ tenants = mock(Tenants.class);
+ TenantInfo tenantInfo = new TenantInfo();
+ tenantInfo.setAdminRoles(Collections.singleton("test-user"));
+ when(tenants.getTenantInfo(any())).thenReturn(tenantInfo);
+
+ admin = mock(PulsarAdmin.class);
+ when(admin.tenants()).thenReturn(tenants);
+ when(this.mockedWorkerService.getBrokerAdmin()).thenReturn(admin);
+ Assert.assertTrue(functionImpl.isAuthorizedRole("test-tenant", "test-ns", "test-user", authenticationDataSource));
+
+ // test user allow function action
+ functionImpl = spy(new FunctionsImpl(() -> mockedWorkerService));
+ doReturn(true).when(functionImpl).allowFunctionOps(any(), any(), any());
+ tenants = mock(Tenants.class);
+ when(tenants.getTenantInfo(any())).thenReturn(new TenantInfo());
+
+ admin = mock(PulsarAdmin.class);
+ when(admin.tenants()).thenReturn(tenants);
+ when(this.mockedWorkerService.getBrokerAdmin()).thenReturn(admin);
+ Assert.assertTrue(functionImpl.isAuthorizedRole("test-tenant", "test-ns", "test-user", authenticationDataSource));
+
+ // test role is null
+ functionImpl = spy(new FunctionsImpl(() -> mockedWorkerService));
+ doReturn(true).when(functionImpl).allowFunctionOps(any(), any(), any());
+ tenants = mock(Tenants.class);
+ when(tenants.getTenantInfo(any())).thenReturn(new TenantInfo());
+
+ admin = mock(PulsarAdmin.class);
+ when(admin.tenants()).thenReturn(tenants);
+ when(this.mockedWorkerService.getBrokerAdmin()).thenReturn(admin);
+ Assert.assertFalse(functionImpl.isAuthorizedRole("test-tenant", "test-ns", null, authenticationDataSource));
+ }
+
+ @Test
+ public void testIsSuperUser() throws PulsarAdminException {
+
+ FunctionsImpl functionImpl = spy(new FunctionsImpl(() -> mockedWorkerService));
+ WorkerConfig workerConfig = new WorkerConfig();
+ workerConfig.setAuthorizationEnabled(true);
+ workerConfig.setSuperUserRoles(Collections.singleton(superUser));
+ doReturn(workerConfig).when(mockedWorkerService).getWorkerConfig();
+
+ AuthenticationDataSource authenticationDataSource = mock(AuthenticationDataSource.class);
+ Assert.assertTrue(functionImpl.isSuperUser(superUser));
+
+ Assert.assertFalse(functionImpl.isSuperUser("normal-user"));
+ Assert.assertFalse(functionImpl.isSuperUser( null));
+
+ // test super roles is null
+
+ functionImpl = spy(new FunctionsImpl(() -> mockedWorkerService));
+ workerConfig = new WorkerConfig();
+ workerConfig.setAuthorizationEnabled(true);
+ doReturn(workerConfig).when(mockedWorkerService).getWorkerConfig();
+
+ Assert.assertFalse(functionImpl.isSuperUser(superUser));
+ }
+
public static FunctionConfig createDefaultFunctionConfig() {
FunctionConfig functionConfig = new FunctionConfig();
functionConfig.setTenant(tenant);