From 3401ba5783ae86e9d8d890935d7178aba5849658 Mon Sep 17 00:00:00 2001 From: zhangyang21 Date: Tue, 29 Sep 2020 20:11:04 +0800 Subject: [PATCH] [ACL] Parameter verification Signed-off-by: zhangyang21 --- .../rocketmq/acl/common/AclConstants.java | 10 +++++++ .../rocketmq/acl/common/Permission.java | 29 +++++++++++++++---- .../acl/plain/PlainPermissionManager.java | 5 +++- .../rocketmq/acl/common/PermissionTest.java | 24 +++++++++++++++ .../acl/plain/PlainAccessValidatorTest.java | 20 +++++++++++++ pom.xml | 2 +- 6 files changed, 83 insertions(+), 7 deletions(-) diff --git a/acl/src/main/java/org/apache/rocketmq/acl/common/AclConstants.java b/acl/src/main/java/org/apache/rocketmq/acl/common/AclConstants.java index bfe96f53..d129c66d 100644 --- a/acl/src/main/java/org/apache/rocketmq/acl/common/AclConstants.java +++ b/acl/src/main/java/org/apache/rocketmq/acl/common/AclConstants.java @@ -44,6 +44,16 @@ public class AclConstants { public static final String CONFIG_TIME_STAMP = "timestamp"; + public static final String PUB = "PUB"; + + public static final String SUB = "SUB"; + + public static final String DENY = "DENY"; + + public static final String PUB_SUB = "PUB|SUB"; + + public static final String SUB_PUB = "SUB|PUB"; + public static final int ACCESS_KEY_MIN_LENGTH = 6; public static final int SECRET_KEY_MIN_LENGTH = 6; diff --git a/acl/src/main/java/org/apache/rocketmq/acl/common/Permission.java b/acl/src/main/java/org/apache/rocketmq/acl/common/Permission.java index 8ceb135f..dadcaa30 100644 --- a/acl/src/main/java/org/apache/rocketmq/acl/common/Permission.java +++ b/acl/src/main/java/org/apache/rocketmq/acl/common/Permission.java @@ -60,14 +60,14 @@ public class Permission { return Permission.DENY; } switch (permString.trim()) { - case "PUB": + case AclConstants.PUB: return Permission.PUB; - case "SUB": + case AclConstants.SUB: return Permission.SUB; - case "PUB|SUB": - case "SUB|PUB": + case AclConstants.PUB_SUB: + case AclConstants.SUB_PUB: return Permission.PUB | Permission.SUB; - case "DENY": + case AclConstants.DENY: return Permission.DENY; default: return Permission.DENY; @@ -89,6 +89,25 @@ public class Permission { } } + public static void checkResourcePerms(List resources) { + if (resources == null || resources.isEmpty()) { + return; + } + + for (String resource : resources) { + String[] items = StringUtils.split(resource, "="); + if (items.length != 2) { + throw new AclException(String.format("Parse Resource format error for %s.\n" + + "The expected resource format is 'Res=Perm'. For example: topicA=SUB", resource)); + } + + if (!AclConstants.DENY.equals(items[1].trim()) && Permission.DENY == Permission.parsePermFromString(items[1].trim())) { + throw new AclException(String.format("Parse resource permission error for %s.\n" + + "The expected permissions are 'SUB' or 'PUB' or 'SUB|PUB' or 'PUB|SUB'.", resource)); + } + } + } + public static boolean needAdminPerm(Integer code) { return ADMIN_CODE.contains(code); } diff --git a/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java b/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java index c182d7eb..65149197 100644 --- a/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java +++ b/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java @@ -128,9 +128,12 @@ public class PlainPermissionManager { if (plainAccessConfig == null) { log.error("Parameter value plainAccessConfig is null,Please check your parameter"); - return false; + throw new AclException("Parameter value plainAccessConfig is null, Please check your parameter"); } + Permission.checkResourcePerms(plainAccessConfig.getTopicPerms()); + Permission.checkResourcePerms(plainAccessConfig.getGroupPerms()); + Map aclAccessConfigMap = AclUtils.getYamlDataObject(fileHome + File.separator + fileName, Map.class); diff --git a/acl/src/test/java/org/apache/rocketmq/acl/common/PermissionTest.java b/acl/src/test/java/org/apache/rocketmq/acl/common/PermissionTest.java index 253b5b24..c824065f 100644 --- a/acl/src/test/java/org/apache/rocketmq/acl/common/PermissionTest.java +++ b/acl/src/test/java/org/apache/rocketmq/acl/common/PermissionTest.java @@ -17,6 +17,7 @@ package org.apache.rocketmq.acl.common; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -165,4 +166,27 @@ public class PermissionTest { aclException.setStatus("netaddress examine scope Exception netaddress"); Assert.assertEquals(aclException.getStatus(),"netaddress examine scope Exception netaddress"); } + + @Test + public void checkResourcePermsNormalTest() { + Permission.checkResourcePerms(null); + Permission.checkResourcePerms(new ArrayList<>()); + Permission.checkResourcePerms(Arrays.asList("topicA=PUB")); + Permission.checkResourcePerms(Arrays.asList("topicA=PUB", "topicB=SUB", "topicC=PUB|SUB")); + } + + @Test(expected = AclException.class) + public void checkResourcePermsExceptionTest1() { + Permission.checkResourcePerms(Arrays.asList("topicA")); + } + + @Test(expected = AclException.class) + public void checkResourcePermsExceptionTest2() { + Permission.checkResourcePerms(Arrays.asList("topicA=")); + } + + @Test(expected = AclException.class) + public void checkResourcePermsExceptionTest3() { + Permission.checkResourcePerms(Arrays.asList("topicA=DENY1")); + } } diff --git a/acl/src/test/java/org/apache/rocketmq/acl/plain/PlainAccessValidatorTest.java b/acl/src/test/java/org/apache/rocketmq/acl/plain/PlainAccessValidatorTest.java index 645e5227..c3d41919 100644 --- a/acl/src/test/java/org/apache/rocketmq/acl/plain/PlainAccessValidatorTest.java +++ b/acl/src/test/java/org/apache/rocketmq/acl/plain/PlainAccessValidatorTest.java @@ -546,6 +546,26 @@ public class PlainAccessValidatorTest { Assert.assertEquals(plainAccessValidator.updateAccessConfig(plainAccessConfig), false); } + @Test(expected = AclException.class) + public void createAndUpdateAccessAclYamlConfigExceptionTest() { + System.setProperty("rocketmq.home.dir", "src/test/resources"); + System.setProperty("rocketmq.acl.plain.file", "/conf/plain_acl_update_create.yml"); + + PlainAccessConfig plainAccessConfig = new PlainAccessConfig(); + plainAccessConfig.setAccessKey("RocketMQ33"); + plainAccessConfig.setSecretKey("123456789111"); + List topicPerms = new ArrayList(); + topicPerms.add("topicB=PUB"); + plainAccessConfig.setTopicPerms(topicPerms); + List groupPerms = new ArrayList(); + groupPerms.add("groupC=DENY1"); + plainAccessConfig.setGroupPerms(groupPerms); + + PlainAccessValidator plainAccessValidator = new PlainAccessValidator(); + // Create element in the acl access yaml config file + plainAccessValidator.updateAccessConfig(plainAccessConfig); + } + @Test public void updateGlobalWhiteAddrsNormalTest() { System.setProperty("rocketmq.home.dir", "src/test/resources"); diff --git a/pom.xml b/pom.xml index 37ea2ebc..9370ecfe 100644 --- a/pom.xml +++ b/pom.xml @@ -549,7 +549,7 @@ com.alibaba fastjson - 1.2.69 + 1.2.70 org.javassist -- GitLab