Fix the authorization of the packages

zymap · Jan 10, 2021 · 0c4b335 · 0c4b335
1 parent 421c399
commit 0c4b335
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 11 deletions.
diff --git a/...mon/src/main/java/org/apache/pulsar/broker/authorization/PulsarAuthorizationProvider.java b/...mon/src/main/java/org/apache/pulsar/broker/authorization/PulsarAuthorizationProvider.java
@@ -538,7 +538,22 @@ public CompletableFuture<Boolean> allowNamespaceOperationAsync(NamespaceName nam
                                                                    String role,
                                                                    NamespaceOperation operation,
                                                                    AuthenticationDataSource authData) {
-        return validateTenantAdminAccess(namespaceName.getTenant(), role, authData);
+        CompletableFuture<Boolean> future = new CompletableFuture<>();
+        configCache.policiesCache().getAsync(POLICY_ROOT + namespaceName.toString())
+            .thenAccept(policies -> {
+                if (!policies.isPresent()) {
+                    future.complete(false);
+                    if (log.isDebugEnabled()) {
+                        log.debug("Policies node couldn't be found for namespace : {}", namespaceName);
+                    }
+                } else {
+                    Set<AuthAction> namespacesActions = policies.get().auth_policies.namespace_auth.get(role);
+                    future.complete(namespacesActions.stream().anyMatch(authAction ->
+                        authAction.toString().equals(operation.toString().toLowerCase())));
+                }
+            });
+        return validateTenantAdminAccess(namespaceName.getTenant(), role, authData)
+            .thenCombine(future, (isTenantAdmin, hasPermission) -> isTenantAdmin || hasPermission);
     }
 
     @Override

diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PackagesBase.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PackagesBase.java
@@ -166,16 +166,16 @@ protected void internalListPackages(String type, String tenant, String namespace
     private CompletableFuture<Void> checkPermissions(String tenant, String namespace) {
         CompletableFuture<Void> future = new CompletableFuture<>();
         if (config().isAuthenticationEnabled()) {
-            String role = clientAppId();
             NamespaceName namespaceName;
             try {
                 namespaceName = NamespaceName.get(tenant, namespace);
             } catch (Exception e) {
                 future.completeExceptionally(e);
                 return future;
             }
-            getAuthorizationService().allowNamespaceOperationAsync(namespaceName, NamespaceOperation.PACKAGES,
-                    originalPrincipal(), role, clientAuthData())
+            getAuthorizationService()
+                .allowNamespaceOperationAsync(namespaceName, NamespaceOperation.PACKAGES,
+                    originalPrincipal(), clientAppId(), clientAuthData())
                 .whenComplete((hasPermission, throwable) -> {
                     if (throwable != null) {
                         future.completeExceptionally(throwable);
@@ -184,8 +184,8 @@ private CompletableFuture<Void> checkPermissions(String tenant, String namespace
                     if (hasPermission) {
                         future.complete(null);
                     } else {
-                        future.completeExceptionally(new RestException(Response.Status.UNAUTHORIZED, String.format(
-                            "Role %s has not the 'package' permission to do the packages operations.", role)));
+                        future.completeExceptionally(new RestException(Response.Status.FORBIDDEN, String.format(
+                            "Role %s has not the 'package' permission to do the packages operations.", clientAppId())));
                     }
                 });
         } else {

diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/TopicOperation.java b/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/TopicOperation.java
@@ -47,4 +47,6 @@ public enum TopicOperation {
     SUBSCRIBE,
     GET_SUBSCRIPTIONS,
     UNSUBSCRIBE,
+
+    PACKAGES,
 }
diff --git a/...src/test/java/org/apache/pulsar/tests/integration/auth/admin/PackagesOpsWithAuthTest.java b/...src/test/java/org/apache/pulsar/tests/integration/auth/admin/PackagesOpsWithAuthTest.java
@@ -197,14 +197,12 @@ public void testPackagesOps() throws Exception {
             clientAdmin.packages().listPackages("function", "public/default");
             fail("list package operation should fail because the client hasn't permission to do");
         } catch (PulsarAdminException e) {
-            assertEquals(e.getStatusCode(), 401);
+            assertEquals(e.getStatusCode(), 403);
         }
 
         // grant package permission to the role
-        TenantInfo tenantInfo = new TenantInfo();
-        tenantInfo.setAdminRoles(new HashSet<>(Arrays.asList(SUPER_USER_ROLE, PROXY_ROLE, REGULAR_USER_ROLE)));
-        tenantInfo.setAllowedClusters(new HashSet<>(superUserAdmin.clusters().getClusters()));
-        superUserAdmin.tenants().updateTenant("public", tenantInfo);
+        superUserAdmin.namespaces().grantPermissionOnNamespace("public/default",
+            REGULAR_USER_ROLE, Set.of(AuthAction.packages));
 
         // then do some package operations again, it should success
         List<String> packagesName = clientAdmin.packages().listPackages("function", "public/default");