From db4b7ca1921b5c14ae6e808b7b28565e3747a4b6 Mon Sep 17 00:00:00 2001 From: andrea Date: Tue, 26 May 2015 22:40:34 +0200 Subject: [PATCH] Added trivial implementation of ACL patterns --- .../moquette/spi/impl/ProtocolProcessor.java | 4 +- .../spi/impl/security/Authorization.java | 2 +- .../security/AuthorizationsCollector.java | 65 ++++++++++++------- .../impl/security/DenyAllAuthorizator.java | 4 +- .../spi/impl/security/IAuthorizator.java | 4 +- .../impl/security/PermitAllAuthorizator.java | 4 +- .../spi/impl/MapDBPersistentStoreTest.java | 2 +- .../spi/impl/security/ACLFileParserTest.java | 4 +- .../security/AuthorizationsCollectorTest.java | 26 +++++--- 9 files changed, 73 insertions(+), 42 deletions(-) diff --git a/broker/src/main/java/org/eclipse/moquette/spi/impl/ProtocolProcessor.java b/broker/src/main/java/org/eclipse/moquette/spi/impl/ProtocolProcessor.java index fe730e00..0222eb2c 100644 --- a/broker/src/main/java/org/eclipse/moquette/spi/impl/ProtocolProcessor.java +++ b/broker/src/main/java/org/eclipse/moquette/spi/impl/ProtocolProcessor.java @@ -299,7 +299,7 @@ class ProtocolProcessor implements EventHandler { boolean retain = msg.isRetainFlag(); //check if the topic can be wrote String user = (String) session.getAttribute(NettyChannel.ATTR_KEY_USERNAME); - if (m_authorizator.canWrite(topic, user)) { + if (m_authorizator.canWrite(topic, user, clientID)) { processPublish(clientID, topic, qos, message, retain, msg.getMessageID()); } else { LOG.debug("topic {} doesn't have write credentials", topic); @@ -439,7 +439,7 @@ class ProtocolProcessor implements EventHandler { LOG.debug("Session for clientId {} is {}", clientId, session); String user = (String) session.getAttribute(NettyChannel.ATTR_KEY_USERNAME); - if (!m_authorizator.canRead(topic, user)) { + if (!m_authorizator.canRead(topic, user, clientId)) { LOG.debug("topic {} doesn't have read credentials", topic); return; } diff --git a/broker/src/main/java/org/eclipse/moquette/spi/impl/security/Authorization.java b/broker/src/main/java/org/eclipse/moquette/spi/impl/security/Authorization.java index 1cb4400a..5cd844e6 100644 --- a/broker/src/main/java/org/eclipse/moquette/spi/impl/security/Authorization.java +++ b/broker/src/main/java/org/eclipse/moquette/spi/impl/security/Authorization.java @@ -29,7 +29,7 @@ public class Authorization { * */ enum Permission { READ, WRITE, READWRITE - }; + } Authorization(String topic) { this(topic, Permission.READWRITE); diff --git a/broker/src/main/java/org/eclipse/moquette/spi/impl/security/AuthorizationsCollector.java b/broker/src/main/java/org/eclipse/moquette/spi/impl/security/AuthorizationsCollector.java index 10c8c548..76ec126f 100644 --- a/broker/src/main/java/org/eclipse/moquette/spi/impl/security/AuthorizationsCollector.java +++ b/broker/src/main/java/org/eclipse/moquette/spi/impl/security/AuthorizationsCollector.java @@ -42,6 +42,7 @@ class AuthorizationsCollector implements IAuthorizator { private List m_patternAuthorizations = new ArrayList(); private Map> m_userAuthorizations = new HashMap(); private boolean m_parsingUsersSpecificSection = false; + private boolean m_parsingPatternSpecificSection = false; private String m_currentUser = ""; static final AuthorizationsCollector emptyImmutableCollector() { @@ -65,6 +66,8 @@ class AuthorizationsCollector implements IAuthorizator { } List userAuths = m_userAuthorizations.get(m_currentUser); userAuths.add(acl); + } else if (m_parsingPatternSpecificSection) { + m_patternAuthorizations.add(acl); } else { m_globalAuthorizations.add(acl); } @@ -75,43 +78,50 @@ class AuthorizationsCollector implements IAuthorizator { String keyword = tokens[0].toLowerCase(); switch (keyword) { case "topic": - if (tokens.length > 2) { - //if the tokenized lines has 3 token the second must be the permission - try { - Authorization.Permission permission = Authorization.Permission.valueOf(tokens[1].toUpperCase()); - //bring topic with all original spacing - String topic = line.substring(line.indexOf(tokens[2])); - - return new Authorization(topic, permission); - } catch (IllegalArgumentException iaex) { - throw new ParseException("invalid permission token", 1); - } - } - String topic = tokens[1]; - return new Authorization(topic); + return createAuthorization(line, tokens); case "user": m_parsingUsersSpecificSection = true; m_currentUser = tokens[1]; + m_parsingPatternSpecificSection = false; return null; case "pattern": - //TODO implement the part for patter matching - return null; + m_parsingUsersSpecificSection = false; + m_currentUser = ""; + m_parsingPatternSpecificSection = true; + return createAuthorization(line, tokens); default: throw new ParseException(String.format("invalid line definition found %s", line), 1); } } - @Override - public boolean canWrite(String topic, String user) { - return canDoOperation(topic, WRITE, user); + private Authorization createAuthorization(String line, String[] tokens) throws ParseException { + if (tokens.length > 2) { + //if the tokenized lines has 3 token the second must be the permission + try { + Authorization.Permission permission = Authorization.Permission.valueOf(tokens[1].toUpperCase()); + //bring topic with all original spacing + String topic = line.substring(line.indexOf(tokens[2])); + + return new Authorization(topic, permission); + } catch (IllegalArgumentException iaex) { + throw new ParseException("invalid permission token", 1); + } + } + String topic = tokens[1]; + return new Authorization(topic); } @Override - public boolean canRead(String topic, String user) { - return canDoOperation(topic, READ, user); + public boolean canWrite(String topic, String user, String client) { + return canDoOperation(topic, WRITE, user, client); } - private boolean canDoOperation(String topic, Authorization.Permission permission, String username) { + @Override + public boolean canRead(String topic, String user, String client) { + return canDoOperation(topic, READ, user, client); + } + + private boolean canDoOperation(String topic, Authorization.Permission permission, String username, String client) { for (Authorization auth : m_globalAuthorizations) { if (auth.permission == permission || auth.permission == READWRITE) { if (SubscriptionsStore.matchTopics(topic, auth.topic)) { @@ -120,6 +130,17 @@ class AuthorizationsCollector implements IAuthorizator { } } + if (client != null && !client.isEmpty()) { + for (Authorization auth : m_patternAuthorizations) { + String substitutedTopic = auth.topic.replace("%c", client); + if (auth.permission == permission || auth.permission == READWRITE) { + if (SubscriptionsStore.matchTopics(topic, substitutedTopic)) { + return true; + } + } + } + } + if (username != null && !username.isEmpty()) { if (m_userAuthorizations.containsKey(username)) { for (Authorization auth : m_userAuthorizations.get(username)) { diff --git a/broker/src/main/java/org/eclipse/moquette/spi/impl/security/DenyAllAuthorizator.java b/broker/src/main/java/org/eclipse/moquette/spi/impl/security/DenyAllAuthorizator.java index 0d96284a..f3cf4a72 100644 --- a/broker/src/main/java/org/eclipse/moquette/spi/impl/security/DenyAllAuthorizator.java +++ b/broker/src/main/java/org/eclipse/moquette/spi/impl/security/DenyAllAuthorizator.java @@ -20,12 +20,12 @@ package org.eclipse.moquette.spi.impl.security; */ public class DenyAllAuthorizator implements IAuthorizator { @Override - public boolean canWrite(String topic, String user) { + public boolean canWrite(String topic, String user, String client) { return true; } @Override - public boolean canRead(String topic, String user) { + public boolean canRead(String topic, String user, String client) { return true; } } \ No newline at end of file diff --git a/broker/src/main/java/org/eclipse/moquette/spi/impl/security/IAuthorizator.java b/broker/src/main/java/org/eclipse/moquette/spi/impl/security/IAuthorizator.java index 39fa5b1e..c3bc171e 100644 --- a/broker/src/main/java/org/eclipse/moquette/spi/impl/security/IAuthorizator.java +++ b/broker/src/main/java/org/eclipse/moquette/spi/impl/security/IAuthorizator.java @@ -29,7 +29,7 @@ public interface IAuthorizator { /** * Ask the implementation of the authorizator if the topic can be used in a publish. * */ - boolean canWrite(String topic, String user); + boolean canWrite(String topic, String user, String client); - boolean canRead(String topic, String user); + boolean canRead(String topic, String user, String client); } diff --git a/broker/src/main/java/org/eclipse/moquette/spi/impl/security/PermitAllAuthorizator.java b/broker/src/main/java/org/eclipse/moquette/spi/impl/security/PermitAllAuthorizator.java index 3f9987aa..5c112a68 100644 --- a/broker/src/main/java/org/eclipse/moquette/spi/impl/security/PermitAllAuthorizator.java +++ b/broker/src/main/java/org/eclipse/moquette/spi/impl/security/PermitAllAuthorizator.java @@ -20,12 +20,12 @@ package org.eclipse.moquette.spi.impl.security; */ public class PermitAllAuthorizator implements IAuthorizator { @Override - public boolean canWrite(String topic, String user) { + public boolean canWrite(String topic, String user, String client) { return true; } @Override - public boolean canRead(String topic, String user) { + public boolean canRead(String topic, String user, String client) { return true; } } diff --git a/broker/src/test/java/org/eclipse/moquette/spi/impl/MapDBPersistentStoreTest.java b/broker/src/test/java/org/eclipse/moquette/spi/impl/MapDBPersistentStoreTest.java index 7969fa3e..78aaf9db 100644 --- a/broker/src/test/java/org/eclipse/moquette/spi/impl/MapDBPersistentStoreTest.java +++ b/broker/src/test/java/org/eclipse/moquette/spi/impl/MapDBPersistentStoreTest.java @@ -39,7 +39,7 @@ public class MapDBPersistentStoreTest { @Before public void setUp() throws Exception { File dbFile = new File(DEFAULT_PERSISTENT_PATH); - assertFalse(String.format("The DB storagefile %s already exists", DEFAULT_PERSISTENT_PATH), dbFile.exists()); + assertFalse(String.format("The DB storage file %s already exists", DEFAULT_PERSISTENT_PATH), dbFile.exists()); m_storageService = new MapDBPersistentStore(DEFAULT_PERSISTENT_PATH); m_storageService.initStore(); diff --git a/broker/src/test/java/org/eclipse/moquette/spi/impl/security/ACLFileParserTest.java b/broker/src/test/java/org/eclipse/moquette/spi/impl/security/ACLFileParserTest.java index b43635f9..c6af1bc9 100644 --- a/broker/src/test/java/org/eclipse/moquette/spi/impl/security/ACLFileParserTest.java +++ b/broker/src/test/java/org/eclipse/moquette/spi/impl/security/ACLFileParserTest.java @@ -58,8 +58,8 @@ public class ACLFileParserTest { AuthorizationsCollector authorizations = ACLFileParser.parse(conf); //Verify - assertTrue(authorizations.canRead("/weather/italy/anemometer", "")); - assertTrue(authorizations.canWrite("/weather/italy/anemometer", "")); + assertTrue(authorizations.canRead("/weather/italy/anemometer", "", "")); + assertTrue(authorizations.canWrite("/weather/italy/anemometer", "", "")); } } \ No newline at end of file diff --git a/broker/src/test/java/org/eclipse/moquette/spi/impl/security/AuthorizationsCollectorTest.java b/broker/src/test/java/org/eclipse/moquette/spi/impl/security/AuthorizationsCollectorTest.java index 04f7efb2..1245e05a 100644 --- a/broker/src/test/java/org/eclipse/moquette/spi/impl/security/AuthorizationsCollectorTest.java +++ b/broker/src/test/java/org/eclipse/moquette/spi/impl/security/AuthorizationsCollectorTest.java @@ -18,6 +18,8 @@ package org.eclipse.moquette.spi.impl.security; import org.junit.Before; import org.junit.Test; +import java.io.Reader; +import java.io.StringReader; import java.text.ParseException; import static org.junit.Assert.*; @@ -90,7 +92,7 @@ public class AuthorizationsCollectorTest { authorizator.parse("topic write /sensors"); //verify - assertTrue(authorizator.canWrite("/sensors", "")); + assertTrue(authorizator.canWrite("/sensors", "", "")); } @Test @@ -98,7 +100,7 @@ public class AuthorizationsCollectorTest { authorizator.parse("topic read /sensors"); //verify - assertTrue(authorizator.canRead("/sensors", "")); + assertTrue(authorizator.canRead("/sensors", "", "")); } @Test @@ -107,8 +109,8 @@ public class AuthorizationsCollectorTest { authorizator.parse("topic read /sensors/anemometer"); //verify - assertTrue(authorizator.canWrite("/sensors", "")); - assertFalse(authorizator.canRead("/sensors", "")); + assertTrue(authorizator.canWrite("/sensors", "", "")); + assertFalse(authorizator.canRead("/sensors", "", "")); } @Test @@ -116,7 +118,7 @@ public class AuthorizationsCollectorTest { authorizator.parse("topic write /sensors/#"); //verify - assertTrue(authorizator.canWrite("/sensors/anemometer/wind", "")); + assertTrue(authorizator.canWrite("/sensors/anemometer/wind", "", "")); } @Test @@ -124,7 +126,7 @@ public class AuthorizationsCollectorTest { authorizator.parse("topic write /sensors/+"); //verify - assertTrue(authorizator.canWrite("/sensors/anemometer", "")); + assertTrue(authorizator.canWrite("/sensors/anemometer", "", "")); } @Test @@ -133,7 +135,15 @@ public class AuthorizationsCollectorTest { authorizator.parse("topic write /sensors"); //verify - assertTrue(authorizator.canWrite("/sensors", "john")); - assertFalse(authorizator.canWrite("/sensors", "jack")); + assertTrue(authorizator.canWrite("/sensors", "john", "")); + assertFalse(authorizator.canWrite("/sensors", "jack", "")); + } + + @Test + public void testParsePatternClientLineACL() throws ParseException { + authorizator.parse("pattern read /weather/italy/%c"); + + //Verify + assertTrue(authorizator.canRead("/weather/italy/anemometer1", "", "anemometer1")); } } \ No newline at end of file