Skip to content

Commit

Permalink
[ISSUE apache#3281 ][acl] fix fail to delete topic perm list and glob…
Browse files Browse the repository at this point in the history 
…al white address(apache#3128) (apache#3280)

* fix fail to remove topic/group permissions and global white address of acl config

* update acl config for all brokers

* refactor rename UtilAll.string2List and list2String

* fix fail to remove whiteRemoteAddress of acl config
  • Loading branch information
@yuz10
yuz10 committed Sep 14, 2021
1 parent 8409adb commit 3a84fbc
Show file tree
Hide file tree
Showing 8 changed files with 77 additions and 35 deletions.
23 changes: 12 additions & 11 deletions acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
View file
Expand Up @@ -18,13 +18,6 @@

import com.alibaba.fastjson.JSONArray;
import com.alibaba.fastjson.JSONObject;
import java.io.File;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import org.apache.commons.lang3.StringUtils;
import org.apache.rocketmq.acl.common.AclConstants;
import org.apache.rocketmq.acl.common.AclException;
Expand All @@ -39,6 +32,14 @@
import org.apache.rocketmq.logging.InternalLoggerFactory;
import org.apache.rocketmq.srvutil.FileWatchService;

import java.io.File;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;

public class PlainPermissionManager {

private static final InternalLogger log = InternalLoggerFactory.getLogger(LoggerName.COMMON_LOGGER_NAME);
Expand Down Expand Up @@ -194,9 +195,9 @@ private Map<String, Object> createAclAccessConfigMap(Map<String, Object> existed
"The secretKey=%s value length should longer than 6",
plainAccessConfig.getSecretKey()));
}
newAccountsMap.put(AclConstants.CONFIG_SECRET_KEY, (String) plainAccessConfig.getSecretKey());
newAccountsMap.put(AclConstants.CONFIG_SECRET_KEY, plainAccessConfig.getSecretKey());
}
if (!StringUtils.isEmpty(plainAccessConfig.getWhiteRemoteAddress())) {
if (plainAccessConfig.getWhiteRemoteAddress() != null) {
newAccountsMap.put(AclConstants.CONFIG_WHITE_ADDR, plainAccessConfig.getWhiteRemoteAddress());
}
if (!StringUtils.isEmpty(String.valueOf(plainAccessConfig.isAdmin()))) {
Expand All @@ -208,10 +209,10 @@ private Map<String, Object> createAclAccessConfigMap(Map<String, Object> existed
if (!StringUtils.isEmpty(plainAccessConfig.getDefaultGroupPerm())) {
newAccountsMap.put(AclConstants.CONFIG_DEFAULT_GROUP_PERM, plainAccessConfig.getDefaultGroupPerm());
}
if (plainAccessConfig.getTopicPerms() != null && !plainAccessConfig.getTopicPerms().isEmpty()) {
if (plainAccessConfig.getTopicPerms() != null) {
newAccountsMap.put(AclConstants.CONFIG_TOPIC_PERMS, plainAccessConfig.getTopicPerms());
}
if (plainAccessConfig.getGroupPerms() != null && !plainAccessConfig.getGroupPerms().isEmpty()) {
if (plainAccessConfig.getGroupPerms() != null) {
newAccountsMap.put(AclConstants.CONFIG_GROUP_PERMS, plainAccessConfig.getGroupPerms());
}

Expand Down
43 changes: 42 additions & 1 deletion acl/src/test/java/org/apache/rocketmq/acl/plain/PlainAccessValidatorTest.java
View file
Expand Up @@ -19,6 +19,7 @@

import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -524,7 +525,7 @@ public void deleteAccessAclYamlConfigNormalTest() {
// Verify the dateversion element is correct or not
List<Map<String, Object>> dataVersions = (List<Map<String, Object>>) readableMap.get(AclConstants.CONFIG_DATA_VERSION);
Assert.assertEquals(1,dataVersions.get(0).get(AclConstants.CONFIG_COUNTER));

// Restore the backup file and flush to yaml file
AclUtils.writeDataObject(targetFileName, backUpAclConfigMap);
}
Expand Down Expand Up @@ -616,4 +617,44 @@ public void getAllAclConfigTest(){
Assert.assertEquals(aclConfig.getPlainAccessConfigs().size(), 2);
}


@Test
public void updateAccessConfigEmptyPermListTest(){
PlainAccessValidator plainAccessValidator = new PlainAccessValidator();
PlainAccessConfig plainAccessConfig = new PlainAccessConfig();
String accessKey = "updateAccessConfigEmptyPerm";
plainAccessConfig.setAccessKey(accessKey);
plainAccessConfig.setSecretKey("123456789111");
plainAccessConfig.setTopicPerms(Collections.singletonList("topicB=PUB"));
plainAccessValidator.updateAccessConfig(plainAccessConfig);

plainAccessConfig.setTopicPerms(new ArrayList<>());
plainAccessValidator.updateAccessConfig(plainAccessConfig);

PlainAccessConfig result = plainAccessValidator.getAllAclConfig().getPlainAccessConfigs()
.stream().filter(c->c.getAccessKey().equals(accessKey)).findFirst().orElse(null);
Assert.assertEquals(0, result.getTopicPerms().size());

plainAccessValidator.deleteAccessConfig(accessKey);
}

@Test
public void updateAccessConfigEmptyWhiteRemoteAddressTest(){
PlainAccessValidator plainAccessValidator = new PlainAccessValidator();
PlainAccessConfig plainAccessConfig = new PlainAccessConfig();
String accessKey = "updateAccessConfigEmptyWhiteRemoteAddress";
plainAccessConfig.setAccessKey(accessKey);
plainAccessConfig.setSecretKey("123456789111");
plainAccessConfig.setWhiteRemoteAddress("127.0.0.1");
plainAccessValidator.updateAccessConfig(plainAccessConfig);

plainAccessConfig.setWhiteRemoteAddress("");
plainAccessValidator.updateAccessConfig(plainAccessConfig);

PlainAccessConfig result = plainAccessValidator.getAllAclConfig().getPlainAccessConfigs()
.stream().filter(c->c.getAccessKey().equals(accessKey)).findFirst().orElse(null);
Assert.assertEquals("", result.getWhiteRemoteAddress());

plainAccessValidator.deleteAccessConfig(accessKey);
}
}
6 changes: 3 additions & 3 deletions broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java
View file
Expand Up @@ -316,8 +316,8 @@ private synchronized RemotingCommand updateAndCreateAccessConfig(ChannelHandlerC
accessConfig.setWhiteRemoteAddress(requestHeader.getWhiteRemoteAddress());
accessConfig.setDefaultTopicPerm(requestHeader.getDefaultTopicPerm());
accessConfig.setDefaultGroupPerm(requestHeader.getDefaultGroupPerm());
accessConfig.setTopicPerms(UtilAll.string2List(requestHeader.getTopicPerms(), ","));
accessConfig.setGroupPerms(UtilAll.string2List(requestHeader.getGroupPerms(), ","));
accessConfig.setTopicPerms(UtilAll.split(requestHeader.getTopicPerms(), ","));
accessConfig.setGroupPerms(UtilAll.split(requestHeader.getGroupPerms(), ","));
accessConfig.setAdmin(requestHeader.isAdmin());
try {

Expand Down Expand Up @@ -390,7 +390,7 @@ private synchronized RemotingCommand updateGlobalWhiteAddrsConfig(ChannelHandler

try {
AccessValidator accessValidator = this.brokerController.getAccessValidatorMap().get(PlainAccessValidator.class);
if (accessValidator.updateGlobalWhiteAddrsConfig(UtilAll.string2List(requestHeader.getGlobalWhiteAddrs(), ","))) {
if (accessValidator.updateGlobalWhiteAddrsConfig(UtilAll.split(requestHeader.getGlobalWhiteAddrs(), ","))) {
response.setCode(ResponseCode.SUCCESS);
response.setOpaque(request.getOpaque());
response.markResponseType();
Expand Down
4 changes: 2 additions & 2 deletions client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java
View file
Expand Up @@ -307,8 +307,8 @@ public void createPlainAccessConfig(final String addr, final PlainAccessConfig p
requestHeader.setDefaultGroupPerm(plainAccessConfig.getDefaultGroupPerm());
requestHeader.setDefaultTopicPerm(plainAccessConfig.getDefaultTopicPerm());
requestHeader.setWhiteRemoteAddress(plainAccessConfig.getWhiteRemoteAddress());
requestHeader.setTopicPerms(UtilAll.list2String(plainAccessConfig.getTopicPerms(), ","));
requestHeader.setGroupPerms(UtilAll.list2String(plainAccessConfig.getGroupPerms(), ","));
requestHeader.setTopicPerms(UtilAll.join(plainAccessConfig.getTopicPerms(), ","));
requestHeader.setGroupPerms(UtilAll.join(plainAccessConfig.getGroupPerms(), ","));

RemotingCommand request = RemotingCommand.createRequestCommand(RequestCode.UPDATE_AND_CREATE_ACL_CONFIG, requestHeader);

Expand Down
16 changes: 8 additions & 8 deletions common/src/main/java/org/apache/rocketmq/common/UtilAll.java
View file
Expand Up @@ -39,7 +39,6 @@
import java.util.zip.CRC32;
import java.util.zip.DeflaterOutputStream;
import java.util.zip.InflaterInputStream;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.validator.routines.InetAddressValidator;
import org.apache.rocketmq.common.constant.LoggerName;
import org.apache.rocketmq.logging.InternalLogger;
Expand Down Expand Up @@ -466,7 +465,7 @@ private static boolean ipCheck(byte[] ip) {
if (ip.length != 4) {
throw new RuntimeException("illegal ipv4 bytes");
}

InetAddressValidator validator = InetAddressValidator.getInstance();
return validator.isValidInet4Address(ipToIPv4Str(ip));
}
Expand Down Expand Up @@ -566,27 +565,28 @@ public static void deleteFile(File file) {
}
}

public static String list2String(List<String> list, String splitor) {
if (list == null || list.size() == 0) {
public static String join(List<String> list, String splitter) {
if (list == null) {
return null;
}

StringBuilder str = new StringBuilder();
for (int i = 0; i < list.size(); i++) {
str.append(list.get(i));
if (i == list.size() - 1) {
break;
}
str.append(splitor);
str.append(splitter);
}
return str.toString();
}

public static List<String> string2List(String str, String splitor) {
if (StringUtils.isEmpty(str)) {
public static List<String> split(String str, String splitter) {
if (str == null) {
return null;
}

String[] addrArray = str.split(splitor);
String[] addrArray = str.split(splitter);
return Arrays.asList(addrArray);
}
}
8 changes: 4 additions & 4 deletions common/src/test/java/org/apache/rocketmq/common/UtilAllTest.java
View file
Expand Up @@ -114,12 +114,12 @@ public void testIPv6Check() throws UnknownHostException {
}

@Test
public void testList2String() {
public void testJoin() {
List<String> list = Arrays.asList("groupA=DENY", "groupB=PUB|SUB", "groupC=SUB");
String comma = ",";
assertEquals("groupA=DENY,groupB=PUB|SUB,groupC=SUB", UtilAll.list2String(list, comma));
assertEquals(null, UtilAll.list2String(null, comma));
assertEquals(null, UtilAll.list2String(Collections.emptyList(), comma));
assertEquals("groupA=DENY,groupB=PUB|SUB,groupC=SUB", UtilAll.join(list, comma));
assertEquals(null, UtilAll.join(null, comma));
assertEquals("", UtilAll.join(Collections.emptyList(), comma));
}

static class DemoConfig {
Expand Down
6 changes: 3 additions & 3 deletions tools/src/main/java/org/apache/rocketmq/tools/command/acl/DeleteAccessConfigSubCommand.java
View file
Expand Up @@ -85,9 +85,9 @@ public Options buildCommandlineOptions(Options options) {

defaultMQAdminExt.start();

Set<String> masterSet =
CommandUtil.fetchMasterAddrByClusterName(defaultMQAdminExt, clusterName);
for (String addr : masterSet) {
Set<String> brokerAddrSet =
CommandUtil.fetchMasterAndSlaveAddrByClusterName(defaultMQAdminExt, clusterName);
for (String addr : brokerAddrSet) {
defaultMQAdminExt.deletePlainAccessConfig(addr, accessKey);
System.out.printf("delete plain access config account from %s success.%n", addr);
}
Expand Down
6 changes: 3 additions & 3 deletions .../src/main/java/org/apache/rocketmq/tools/command/acl/UpdateGlobalWhiteAddrSubCommand.java
View file
Expand Up @@ -82,9 +82,9 @@ public class UpdateGlobalWhiteAddrSubCommand implements SubCommand {
String clusterName = commandLine.getOptionValue('c').trim();

defaultMQAdminExt.start();
Set<String> masterSet =
CommandUtil.fetchMasterAddrByClusterName(defaultMQAdminExt, clusterName);
for (String addr : masterSet) {
Set<String> brokerAddrSet =
CommandUtil.fetchMasterAndSlaveAddrByClusterName(defaultMQAdminExt, clusterName);
for (String addr : brokerAddrSet) {
defaultMQAdminExt.updateGlobalWhiteAddrConfig(addr, globalWhiteRemoteAddresses);
System.out.printf("update global white remote addresses to %s success.%n", addr);
}
Expand Down

0 comments on commit 3a84fbc

Please sign in to comment.