You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by GitBox <gi...@apache.org> on 2018/12/24 10:37:24 UTC
[GitHub] duhenglucky closed pull request #628: [ISSUE#403] fix some bugs and
Optimization code for rocketmq's acl feature.
duhenglucky closed pull request #628: [ISSUE#403] fix some bugs and Optimization code for rocketmq's acl feature.
URL: https://github.com/apache/rocketmq/pull/628
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/acl/src/main/java/org/apache/rocketmq/acl/common/SessionCredentials.java b/acl/src/main/java/org/apache/rocketmq/acl/common/SessionCredentials.java
index a637e3680..33a8a3435 100644
--- a/acl/src/main/java/org/apache/rocketmq/acl/common/SessionCredentials.java
+++ b/acl/src/main/java/org/apache/rocketmq/acl/common/SessionCredentials.java
@@ -30,7 +30,7 @@
public static final String SECURITY_TOKEN = "SecurityToken";
public static final String KEY_FILE = System.getProperty("rocketmq.client.keyFile",
- System.getProperty("user.home") + File.separator + "onskey");
+ System.getProperty("user.home") + File.separator + "key");
private String accessKey;
private String secretKey;
diff --git a/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionLoader.java b/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionLoader.java
index 01161d0cf..9c36ecf71 100644
--- a/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionLoader.java
+++ b/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionLoader.java
@@ -81,8 +81,8 @@ public void initialize() {
}
JSONArray accounts = accessControlTransport.getJSONArray("accounts");
- List<PlainAccessConfig> plainAccessList = accounts.toJavaList(PlainAccessConfig.class);
- if (plainAccessList != null && !plainAccessList.isEmpty()) {
+ if (accounts != null && !accounts.isEmpty()) {
+ List<PlainAccessConfig> plainAccessList = accounts.toJavaList(PlainAccessConfig.class);
for (PlainAccessConfig plainAccess : plainAccessList) {
this.addPlainAccessResource(getPlainAccessResource(plainAccess));
}
@@ -168,6 +168,11 @@ void checkPerm(PlainAccessResource needCheckedAccess, PlainAccessResource ownedA
Map<String, Byte> needCheckedPermMap = needCheckedAccess.getResourcePermMap();
Map<String, Byte> ownedPermMap = ownedAccess.getResourcePermMap();
+ if (needCheckedPermMap == null) {
+ //if the needCheckedPermMap is null,then return
+ return;
+ }
+
for (Map.Entry<String, Byte> needCheckedEntry : needCheckedPermMap.entrySet()) {
String resource = needCheckedEntry.getKey();
Byte neededPerm = needCheckedEntry.getValue();
@@ -223,16 +228,14 @@ private void addGlobalWhiteRemoteAddress(String remoteAddresses) {
public void validate(PlainAccessResource plainAccessResource) {
//Step 1, check the global white remote addr
- if (plainAccessResource.getAccessKey() == null) {
- if (globalWhiteRemoteAddressStrategy.isEmpty()) {
- throw new AclException(String.format("No accessKey is configured and no global white remote addr is configured"));
+ for (RemoteAddressStrategy remoteAddressStrategy : globalWhiteRemoteAddressStrategy) {
+ if (remoteAddressStrategy.match(plainAccessResource)) {
+ return;
}
- for (RemoteAddressStrategy remoteAddressStrategy : globalWhiteRemoteAddressStrategy) {
- if (remoteAddressStrategy.match(plainAccessResource)) {
- return;
- }
- }
- throw new AclException(String.format("No accessKey is configured and no global white remote addr is matched"));
+ }
+
+ if (plainAccessResource.getAccessKey() == null) {
+ throw new AclException(String.format("No accessKey is configured"));
}
if (!plainAccessResourceMap.containsKey(plainAccessResource.getAccessKey())) {
diff --git a/acl/src/main/java/org/apache/rocketmq/acl/plain/RemoteAddressStrategyFactory.java b/acl/src/main/java/org/apache/rocketmq/acl/plain/RemoteAddressStrategyFactory.java
index b82d79388..10b473458 100644
--- a/acl/src/main/java/org/apache/rocketmq/acl/plain/RemoteAddressStrategyFactory.java
+++ b/acl/src/main/java/org/apache/rocketmq/acl/plain/RemoteAddressStrategyFactory.java
@@ -21,19 +21,26 @@
import org.apache.commons.lang3.StringUtils;
import org.apache.rocketmq.acl.common.AclException;
import org.apache.rocketmq.acl.common.AclUtils;
+import org.apache.rocketmq.common.constant.LoggerName;
+import org.apache.rocketmq.logging.InternalLogger;
+import org.apache.rocketmq.logging.InternalLoggerFactory;
public class RemoteAddressStrategyFactory {
+ private static final InternalLogger log = InternalLoggerFactory.getLogger(LoggerName.ACL_PLUG_LOGGER_NAME);
+
public static final NullRemoteAddressStrategy NULL_NET_ADDRESS_STRATEGY = new NullRemoteAddressStrategy();
+ public static final BlankRemoteAddressStrategy BLANK_NET_ADDRESS_STRATEGY = new BlankRemoteAddressStrategy();
+
public RemoteAddressStrategy getRemoteAddressStrategy(PlainAccessResource plainAccessResource) {
return getRemoteAddressStrategy(plainAccessResource.getWhiteRemoteAddress());
-
}
public RemoteAddressStrategy getRemoteAddressStrategy(String remoteAddr) {
if (StringUtils.isBlank(remoteAddr)) {
- throw new AclException("Must fill in the white list address");
+ log.warn("white list address is null");
+ return BLANK_NET_ADDRESS_STRATEGY;
}
if ("*".equals(remoteAddr)) {
return NULL_NET_ADDRESS_STRATEGY;
@@ -62,6 +69,14 @@ public boolean match(PlainAccessResource plainAccessResource) {
}
+ public static class BlankRemoteAddressStrategy implements RemoteAddressStrategy {
+ @Override
+ public boolean match(PlainAccessResource plainAccessResource) {
+ return false;
+ }
+
+ }
+
public static class MultipleRemoteAddressStrategy implements RemoteAddressStrategy {
private final Set<String> multipleSet = new HashSet<>();
diff --git a/acl/src/test/java/org/apache/rocketmq/acl/plain/PlainPermissionLoaderTest.java b/acl/src/test/java/org/apache/rocketmq/acl/plain/PlainPermissionLoaderTest.java
index 4f5ae5b52..2bd5b8cea 100644
--- a/acl/src/test/java/org/apache/rocketmq/acl/plain/PlainPermissionLoaderTest.java
+++ b/acl/src/test/java/org/apache/rocketmq/acl/plain/PlainPermissionLoaderTest.java
@@ -227,6 +227,7 @@ public void watchTest() throws IOException, IllegalAccessException {
File file = new File("src/test/resources/watch/conf");
file.mkdirs();
File transport = new File("src/test/resources/watch/conf/plain_acl.yml");
+ transport.delete();
transport.createNewFile();
FileWriter writer = new FileWriter(transport);
@@ -258,11 +259,6 @@ public void watchTest() throws IOException, IllegalAccessException {
plainAccessResourceMap = (Map<String, List<PlainAccessResource>>) FieldUtils.readDeclaredField(plainPermissionLoader, "plainAccessResourceMap", true);
Assert.assertNotNull(plainAccessResourceMap.get("rokcet1"));
- transport.delete();
- file.delete();
- file = new File("src/test/resources/watch");
- file.delete();
-
}
@Test(expected = AclException.class)
diff --git a/acl/src/test/java/org/apache/rocketmq/acl/plain/RemoteAddressStrategyTest.java b/acl/src/test/java/org/apache/rocketmq/acl/plain/RemoteAddressStrategyTest.java
index a390c604f..53391f411 100644
--- a/acl/src/test/java/org/apache/rocketmq/acl/plain/RemoteAddressStrategyTest.java
+++ b/acl/src/test/java/org/apache/rocketmq/acl/plain/RemoteAddressStrategyTest.java
@@ -24,10 +24,12 @@
RemoteAddressStrategyFactory remoteAddressStrategyFactory = new RemoteAddressStrategyFactory();
- @Test(expected = AclException.class)
+ @Test
public void netaddressStrategyFactoryExceptionTest() {
PlainAccessResource plainAccessResource = new PlainAccessResource();
remoteAddressStrategyFactory.getRemoteAddressStrategy(plainAccessResource);
+ Assert.assertEquals(remoteAddressStrategyFactory.getRemoteAddressStrategy(plainAccessResource).getClass(),
+ RemoteAddressStrategyFactory.BlankRemoteAddressStrategy.class);
}
@Test
@@ -61,6 +63,10 @@ public void netaddressStrategyFactoryTest() {
plainAccessResource.setWhiteRemoteAddress("127.0.1-20.*");
remoteAddressStrategy = remoteAddressStrategyFactory.getRemoteAddressStrategy(plainAccessResource);
Assert.assertEquals(remoteAddressStrategy.getClass(), RemoteAddressStrategyFactory.RangeRemoteAddressStrategy.class);
+
+ plainAccessResource.setWhiteRemoteAddress("");
+ remoteAddressStrategy = remoteAddressStrategyFactory.getRemoteAddressStrategy(plainAccessResource);
+ Assert.assertEquals(remoteAddressStrategy.getClass(), RemoteAddressStrategyFactory.BlankRemoteAddressStrategy.class);
}
@Test(expected = AclException.class)
@@ -78,6 +84,12 @@ public void nullNetaddressStrategyTest() {
Assert.assertTrue(isMatch);
}
+ @Test
+ public void blankNetaddressStrategyTest() {
+ boolean isMatch = RemoteAddressStrategyFactory.BLANK_NET_ADDRESS_STRATEGY.match(new PlainAccessResource());
+ Assert.assertFalse(isMatch);
+ }
+
public void oneNetaddressStrategyTest() {
PlainAccessResource plainAccessResource = new PlainAccessResource();
plainAccessResource.setWhiteRemoteAddress("127.0.0.1");
diff --git a/broker/src/main/java/org/apache/rocketmq/broker/BrokerController.java b/broker/src/main/java/org/apache/rocketmq/broker/BrokerController.java
index e649665ad..73ed7eb4c 100644
--- a/broker/src/main/java/org/apache/rocketmq/broker/BrokerController.java
+++ b/broker/src/main/java/org/apache/rocketmq/broker/BrokerController.java
@@ -499,6 +499,7 @@ private void initialAcl() {
List<AccessValidator> accessValidators = ServiceProvider.load(ServiceProvider.ACL_VALIDATOR_ID, AccessValidator.class);
if (accessValidators == null || accessValidators.isEmpty()) {
+ log.info("The broker dose not load the AccessValidator");
return;
}
diff --git a/broker/src/main/resources/META-INF/service/org.apache.rocketmq.acl.AccessValidator b/broker/src/main/resources/META-INF/service/org.apache.rocketmq.acl.AccessValidator
new file mode 100644
index 000000000..1abc92e01
--- /dev/null
+++ b/broker/src/main/resources/META-INF/service/org.apache.rocketmq.acl.AccessValidator
@@ -0,0 +1 @@
+org.apache.rocketmq.acl.plain.PlainAccessValidator
\ No newline at end of file
diff --git a/broker/src/test/java/org/apache/rocketmq/broker/BrokerControllerTest.java b/broker/src/test/java/org/apache/rocketmq/broker/BrokerControllerTest.java
index 56abf084a..71bbe0696 100644
--- a/broker/src/test/java/org/apache/rocketmq/broker/BrokerControllerTest.java
+++ b/broker/src/test/java/org/apache/rocketmq/broker/BrokerControllerTest.java
@@ -42,6 +42,21 @@ public void testBrokerRestart() throws Exception {
brokerController.shutdown();
}
+ @Test
+ public void testBrokerStartAclEnabled() throws Exception {
+ BrokerConfig brokerConfigAclEnabled = new BrokerConfig();
+ brokerConfigAclEnabled.setEnableAcl(true);
+
+ BrokerController brokerController = new BrokerController(
+ brokerConfigAclEnabled,
+ new NettyServerConfig(),
+ new NettyClientConfig(),
+ new MessageStoreConfig());
+ assertThat(brokerController.initialize());
+ brokerController.start();
+ brokerController.shutdown();
+ }
+
@After
public void destroy() {
UtilAll.deleteFile(new File(new MessageStoreConfig().getStorePathRootDir()));
diff --git a/broker/src/test/resources/META-INF/service/org.apache.rocketmq.acl.AccessValidator b/broker/src/test/resources/META-INF/service/org.apache.rocketmq.acl.AccessValidator
index bbf21d376..1abc92e01 100644
--- a/broker/src/test/resources/META-INF/service/org.apache.rocketmq.acl.AccessValidator
+++ b/broker/src/test/resources/META-INF/service/org.apache.rocketmq.acl.AccessValidator
@@ -1 +1 @@
-org.apache.rocketmq.acl.DefaultAclRemotingServiceImpl
\ No newline at end of file
+org.apache.rocketmq.acl.plain.PlainAccessValidator
\ No newline at end of file
diff --git a/common/src/main/java/org/apache/rocketmq/common/BrokerConfig.java b/common/src/main/java/org/apache/rocketmq/common/BrokerConfig.java
index 60bd7ce41..07242b377 100644
--- a/common/src/main/java/org/apache/rocketmq/common/BrokerConfig.java
+++ b/common/src/main/java/org/apache/rocketmq/common/BrokerConfig.java
@@ -171,7 +171,11 @@
@ImportantField
private long transactionCheckInterval = 60 * 1000;
- private boolean enableAcl;
+ /**
+ * Acl feature switch
+ */
+ @ImportantField
+ private boolean enableAcl = false;
public static String localHostName() {
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services