From d622adb4ef724c2a01478f53a8126e5601358580 Mon Sep 17 00:00:00 2001 From: Martin Crawford Date: Sun, 26 Jun 2016 10:01:15 -0400 Subject: [PATCH] Add clientID parameter to the IAuthenticator interface (breaks compatibility). --- .../moquette/spi/impl/ProtocolProcessor.java | 4 +- .../impl/security/AcceptAllAuthenticator.java | 2 +- .../spi/impl/security/DBAuthenticator.java | 2 +- .../spi/impl/security/FileAuthenticator.java | 5 ++- .../moquette/spi/security/IAuthenticator.java | 2 +- .../server/ConfigurationClassLoaderTest.java | 2 +- .../moquette/spi/impl/MockAuthenticator.java | 12 ++++-- .../spi/impl/ProtocolProcessorTest.java | 7 +++- .../impl/ProtocolProcessor_CONNECT_Test.java | 40 ++++++++++--------- .../impl/security/DBAuthenticatorTest.java | 6 +-- .../impl/security/FileAuthenticatorTest.java | 6 +-- 11 files changed, 51 insertions(+), 37 deletions(-) diff --git a/broker/src/main/java/io/moquette/spi/impl/ProtocolProcessor.java b/broker/src/main/java/io/moquette/spi/impl/ProtocolProcessor.java index 92f935b5..dd5fa28f 100644 --- a/broker/src/main/java/io/moquette/spi/impl/ProtocolProcessor.java +++ b/broker/src/main/java/io/moquette/spi/impl/ProtocolProcessor.java @@ -58,7 +58,7 @@ import org.slf4j.LoggerFactory; /** * Class responsible to handle the logic of MQTT protocol it's the director of - * the protocol execution. + * the protocol execution. * * Used by the front facing class SimpleMessaging. * @@ -167,7 +167,7 @@ public class ProtocolProcessor { failedCredentials(channel); return; } - if (!m_authenticator.checkValid(msg.getUsername(), pwd)) { + if (!m_authenticator.checkValid(msg.getClientID(), msg.getUsername(), pwd)) { failedCredentials(channel); channel.close(); return; diff --git a/broker/src/main/java/io/moquette/spi/impl/security/AcceptAllAuthenticator.java b/broker/src/main/java/io/moquette/spi/impl/security/AcceptAllAuthenticator.java index 0e39dd6d..39898665 100644 --- a/broker/src/main/java/io/moquette/spi/impl/security/AcceptAllAuthenticator.java +++ b/broker/src/main/java/io/moquette/spi/impl/security/AcceptAllAuthenticator.java @@ -6,7 +6,7 @@ import io.moquette.spi.security.IAuthenticator; * Created by andrea on 8/23/14. */ public class AcceptAllAuthenticator implements IAuthenticator { - public boolean checkValid(String username, byte[] password) { + public boolean checkValid(String clientId, String username, byte[] password) { return true; } } diff --git a/broker/src/main/java/io/moquette/spi/impl/security/DBAuthenticator.java b/broker/src/main/java/io/moquette/spi/impl/security/DBAuthenticator.java index 0d92faa0..e2c8e849 100644 --- a/broker/src/main/java/io/moquette/spi/impl/security/DBAuthenticator.java +++ b/broker/src/main/java/io/moquette/spi/impl/security/DBAuthenticator.java @@ -63,7 +63,7 @@ public class DBAuthenticator implements IAuthenticator { } @Override - public synchronized boolean checkValid(String username, byte[] password) { + public synchronized boolean checkValid(String clientId, String username, byte[] password) { // Check Username / Password in DB using sqlQuery if (username == null || password == null) { LOG.info("username or password was null"); diff --git a/broker/src/main/java/io/moquette/spi/impl/security/FileAuthenticator.java b/broker/src/main/java/io/moquette/spi/impl/security/FileAuthenticator.java index e2423417..7825e916 100644 --- a/broker/src/main/java/io/moquette/spi/impl/security/FileAuthenticator.java +++ b/broker/src/main/java/io/moquette/spi/impl/security/FileAuthenticator.java @@ -106,8 +106,9 @@ public class FileAuthenticator implements IAuthenticator { throw new ParseException("Failed to read", 1); } } - - public boolean checkValid(String username, byte[] password) { + + @Override + public boolean checkValid(String clientId, String username, byte[] password) { if (username == null || password == null) { LOG.info("username or password was null"); return false; diff --git a/broker/src/main/java/io/moquette/spi/security/IAuthenticator.java b/broker/src/main/java/io/moquette/spi/security/IAuthenticator.java index e760a3d2..0bd4f118 100644 --- a/broker/src/main/java/io/moquette/spi/security/IAuthenticator.java +++ b/broker/src/main/java/io/moquette/spi/security/IAuthenticator.java @@ -22,5 +22,5 @@ package io.moquette.spi.security; */ public interface IAuthenticator { - boolean checkValid(String username, byte[] password); + boolean checkValid(String clientId, String username, byte[] password); } diff --git a/broker/src/test/java/io/moquette/server/ConfigurationClassLoaderTest.java b/broker/src/test/java/io/moquette/server/ConfigurationClassLoaderTest.java index 68ac1f18..91636841 100644 --- a/broker/src/test/java/io/moquette/server/ConfigurationClassLoaderTest.java +++ b/broker/src/test/java/io/moquette/server/ConfigurationClassLoaderTest.java @@ -72,7 +72,7 @@ public class ConfigurationClassLoaderTest implements IAuthenticator, IAuthorizat } @Override - public boolean checkValid(String username, byte[] password) { + public boolean checkValid(String clientID, String username, byte[] password) { return true; } diff --git a/broker/src/test/java/io/moquette/spi/impl/MockAuthenticator.java b/broker/src/test/java/io/moquette/spi/impl/MockAuthenticator.java index 892b835e..296a4c90 100644 --- a/broker/src/test/java/io/moquette/spi/impl/MockAuthenticator.java +++ b/broker/src/test/java/io/moquette/spi/impl/MockAuthenticator.java @@ -16,6 +16,7 @@ package io.moquette.spi.impl; import java.util.Map; +import java.util.Set; import io.moquette.spi.security.IAuthenticator; @@ -25,14 +26,19 @@ import io.moquette.spi.security.IAuthenticator; * @author andrea */ class MockAuthenticator implements IAuthenticator { - + + private Set m_clientIds; private Map m_userPwds; - MockAuthenticator(Map userPwds) { + MockAuthenticator(Set clientIds, Map userPwds) { + m_clientIds = clientIds; m_userPwds = userPwds; } - public boolean checkValid(String username, byte[] password) { + public boolean checkValid(String clientId, String username, byte[] password) { + if (!m_clientIds.contains(clientId)) { + return false; + } if (!m_userPwds.containsKey(username)) { return false; } diff --git a/broker/src/test/java/io/moquette/spi/impl/ProtocolProcessorTest.java b/broker/src/test/java/io/moquette/spi/impl/ProtocolProcessorTest.java index 9792ebdb..983dece9 100644 --- a/broker/src/test/java/io/moquette/spi/impl/ProtocolProcessorTest.java +++ b/broker/src/test/java/io/moquette/spi/impl/ProtocolProcessorTest.java @@ -83,10 +83,13 @@ public class ProtocolProcessorTest { m_messagesStore = memStorage.messagesStore(); m_sessionStore = memStorage.sessionsStore(); //m_messagesStore.initStore(); - + + Set clientIds = new HashSet<>(); + clientIds.add(FAKE_CLIENT_ID); + clientIds.add(FAKE_CLIENT_ID2); Map users = new HashMap<>(); users.put(TEST_USER, TEST_PWD); - m_mockAuthenticator = new MockAuthenticator(users); + m_mockAuthenticator = new MockAuthenticator(clientIds, users); subscriptions = new SubscriptionsStore(); subscriptions.init(memStorage.sessionsStore()); diff --git a/broker/src/test/java/io/moquette/spi/impl/ProtocolProcessor_CONNECT_Test.java b/broker/src/test/java/io/moquette/spi/impl/ProtocolProcessor_CONNECT_Test.java index 17692691..27d5b5ee 100644 --- a/broker/src/test/java/io/moquette/spi/impl/ProtocolProcessor_CONNECT_Test.java +++ b/broker/src/test/java/io/moquette/spi/impl/ProtocolProcessor_CONNECT_Test.java @@ -30,7 +30,9 @@ import org.junit.Before; import org.junit.Test; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; +import java.util.Set; import static io.moquette.parser.netty.Utils.VERSION_3_1_1; import static io.moquette.parser.proto.messages.ConnAckMessage.BAD_USERNAME_OR_PASSWORD; @@ -71,9 +73,11 @@ public class ProtocolProcessor_CONNECT_Test { m_sessionStore = memStorage.sessionsStore(); //m_messagesStore.initStore(); + Set clientIds = new HashSet<>(); + clientIds.add(ProtocolProcessorTest.FAKE_CLIENT_ID); Map users = new HashMap<>(); users.put(ProtocolProcessorTest.TEST_USER, ProtocolProcessorTest.TEST_PWD); - m_mockAuthenticator = new MockAuthenticator(users); + m_mockAuthenticator = new MockAuthenticator(clientIds, users); subscriptions = new SubscriptionsStore(); subscriptions.init(m_sessionStore); @@ -106,7 +110,7 @@ public class ProtocolProcessor_CONNECT_Test { @Test public void testWill() { - connMsg.setClientID("123"); + connMsg.setClientID(ProtocolProcessorTest.FAKE_CLIENT_ID); connMsg.setWillFlag(true); connMsg.setWillTopic("topic"); connMsg.setWillMessage("Topic message".getBytes()); @@ -125,7 +129,7 @@ public class ProtocolProcessor_CONNECT_Test { @Test public void validAuthentication() { - connMsg.setClientID("123"); + connMsg.setClientID(ProtocolProcessorTest.FAKE_CLIENT_ID); connMsg.setUserFlag(true); connMsg.setPasswordFlag(true); connMsg.setUsername(ProtocolProcessorTest.TEST_USER); @@ -140,7 +144,7 @@ public class ProtocolProcessor_CONNECT_Test { @Test public void noPasswdAuthentication() { - connMsg.setClientID("123"); + connMsg.setClientID(ProtocolProcessorTest.FAKE_CLIENT_ID); connMsg.setUserFlag(true); connMsg.setPasswordFlag(false); connMsg.setUsername(ProtocolProcessorTest.TEST_USER); @@ -154,7 +158,7 @@ public class ProtocolProcessor_CONNECT_Test { @Test public void invalidAuthentication() { - connMsg.setClientID("123"); + connMsg.setClientID(ProtocolProcessorTest.FAKE_CLIENT_ID); connMsg.setUserFlag(true); connMsg.setPasswordFlag(true); connMsg.setUsername(ProtocolProcessorTest.TEST_USER + "_fake"); @@ -169,7 +173,7 @@ public class ProtocolProcessor_CONNECT_Test { @Test public void prohibitAnonymousClient() { - connMsg.setClientID("123"); + connMsg.setClientID(ProtocolProcessorTest.FAKE_CLIENT_ID); m_processor.init(subscriptions, m_messagesStore, m_sessionStore, m_mockAuthenticator, false, new PermitAllAuthorizator(), ProtocolProcessorTest.NO_OBSERVERS_INTERCEPTOR); @@ -182,7 +186,7 @@ public class ProtocolProcessor_CONNECT_Test { @Test public void prohibitAnonymousClient_providingUsername() { - connMsg.setClientID("123"); + connMsg.setClientID(ProtocolProcessorTest.FAKE_CLIENT_ID); connMsg.setUserFlag(true); connMsg.setUsername(ProtocolProcessorTest.TEST_USER + "_fake"); m_processor.init(subscriptions, m_messagesStore, m_sessionStore, m_mockAuthenticator, false, @@ -197,7 +201,7 @@ public class ProtocolProcessor_CONNECT_Test { @Test public void acceptAnonymousClient() { - connMsg.setClientID("123"); + connMsg.setClientID(ProtocolProcessorTest.FAKE_CLIENT_ID); m_processor.init(subscriptions, m_messagesStore, m_sessionStore, m_mockAuthenticator, true, new PermitAllAuthorizator(), ProtocolProcessorTest.NO_OBSERVERS_INTERCEPTOR); @@ -214,7 +218,7 @@ public class ProtocolProcessor_CONNECT_Test { new PermitAllAuthorizator(), ProtocolProcessorTest.NO_OBSERVERS_INTERCEPTOR); //first connect with clean session true - connMsg.setClientID("123"); + connMsg.setClientID(ProtocolProcessorTest.FAKE_CLIENT_ID); connMsg.setCleanSession(true); m_processor.processConnect(m_session, connMsg); assertEqualsConnAck(CONNECTION_ACCEPTED, m_session.readOutbound()); @@ -225,12 +229,12 @@ public class ProtocolProcessor_CONNECT_Test { m_session = new EmbeddedChannel(); ConnectMessage secondConnMsg = new ConnectMessage(); secondConnMsg.setProtocolVersion((byte) 0x03); - secondConnMsg.setClientID("123"); + secondConnMsg.setClientID(ProtocolProcessorTest.FAKE_CLIENT_ID); m_processor.processConnect(m_session, secondConnMsg); assertEqualsConnAck(CONNECTION_ACCEPTED, m_session.readOutbound()); //Verify client session is clean false - ClientSession session = m_sessionStore.sessionForClient("123"); + ClientSession session = m_sessionStore.sessionForClient(ProtocolProcessorTest.FAKE_CLIENT_ID); assertFalse(session.isCleanSession()); //Verify @@ -240,7 +244,7 @@ public class ProtocolProcessor_CONNECT_Test { @Test public void connectWithSameClientIDBadCredentialsDoesntDropExistingClient() { //Connect a client1 - connMsg.setClientID("Client1"); + connMsg.setClientID(ProtocolProcessorTest.FAKE_CLIENT_ID); connMsg.setUserFlag(true); connMsg.setPasswordFlag(true); connMsg.setUsername(ProtocolProcessorTest.TEST_USER); @@ -251,7 +255,7 @@ public class ProtocolProcessor_CONNECT_Test { //create another connect same clientID but with bad credentials ConnectMessage evilClientConnMsg = new ConnectMessage(); evilClientConnMsg.setProtocolVersion((byte) 0x03); - evilClientConnMsg.setClientID("Client1"); + evilClientConnMsg.setClientID(ProtocolProcessorTest.FAKE_CLIENT_ID); evilClientConnMsg.setUserFlag(true); evilClientConnMsg.setPasswordFlag(true); evilClientConnMsg.setUsername(ProtocolProcessorTest.EVIL_TEST_USER); @@ -274,9 +278,9 @@ public class ProtocolProcessor_CONNECT_Test { public void testConnAckContainsSessionPresentFlag() throws InterruptedException { connMsg = new ConnectMessage(); connMsg.setProtocolVersion(VERSION_3_1_1); - connMsg.setClientID("CliID"); + connMsg.setClientID(ProtocolProcessorTest.FAKE_CLIENT_ID); connMsg.setCleanSession(false); - NettyUtils.clientID(m_session, "CliID"); + NettyUtils.clientID(m_session, ProtocolProcessorTest.FAKE_CLIENT_ID); NettyUtils.cleanSession(m_session, false); //Connect a first time @@ -298,7 +302,7 @@ public class ProtocolProcessor_CONNECT_Test { //connect with clean a false and subscribe to a topic connMsg = new ConnectMessage(); connMsg.setProtocolVersion(VERSION_3_1_1); - connMsg.setClientID("CliID"); + connMsg.setClientID(ProtocolProcessorTest.FAKE_CLIENT_ID); connMsg.setCleanSession(false); m_processor.processConnect(m_session, connMsg); assertEqualsConnAck(CONNECTION_ACCEPTED, m_session.readOutbound()); @@ -307,10 +311,10 @@ public class ProtocolProcessor_CONNECT_Test { SubscribeMessage subscribeMsg = new SubscribeMessage(); subscribeMsg.addSubscription(new SubscribeMessage.Couple((byte) AbstractMessage.QOSType.MOST_ONE.ordinal(), ProtocolProcessorTest.FAKE_TOPIC)); - NettyUtils.clientID(m_session, "CliID"); + NettyUtils.clientID(m_session, ProtocolProcessorTest.FAKE_CLIENT_ID); NettyUtils.cleanSession(m_session, false); m_processor.processSubscribe(m_session, subscribeMsg); - Subscription expectedSubscription = new Subscription("CliID", ProtocolProcessorTest.FAKE_TOPIC, + Subscription expectedSubscription = new Subscription(ProtocolProcessorTest.FAKE_CLIENT_ID, ProtocolProcessorTest.FAKE_TOPIC, AbstractMessage.QOSType.MOST_ONE); assertTrue(subscriptions.contains(expectedSubscription)); assertEqualsSubAck(m_session.readOutbound()); diff --git a/broker/src/test/java/io/moquette/spi/impl/security/DBAuthenticatorTest.java b/broker/src/test/java/io/moquette/spi/impl/security/DBAuthenticatorTest.java index 1db62da6..885825d2 100644 --- a/broker/src/test/java/io/moquette/spi/impl/security/DBAuthenticatorTest.java +++ b/broker/src/test/java/io/moquette/spi/impl/security/DBAuthenticatorTest.java @@ -57,19 +57,19 @@ public class DBAuthenticatorTest { @Test public void Db_verifyValid() { final DBAuthenticator dbAuthenticator = new DBAuthenticator(ORG_H2_DRIVER, JDBC_H2_MEM_TEST, "SELECT PASSWORD FROM ACCOUNT WHERE LOGIN=?", SHA_256); - assertTrue(dbAuthenticator.checkValid("dbuser","password".getBytes())); + assertTrue(dbAuthenticator.checkValid(null, "dbuser","password".getBytes())); } @Test public void Db_verifyInvalidLogin(){ final DBAuthenticator dbAuthenticator = new DBAuthenticator(ORG_H2_DRIVER, JDBC_H2_MEM_TEST, "SELECT PASSWORD FROM ACCOUNT WHERE LOGIN=?", SHA_256); - assertFalse(dbAuthenticator.checkValid("dbuser2","password".getBytes())); + assertFalse(dbAuthenticator.checkValid(null, "dbuser2","password".getBytes())); } @Test public void Db_verifyInvalidPassword(){ final DBAuthenticator dbAuthenticator = new DBAuthenticator(ORG_H2_DRIVER, JDBC_H2_MEM_TEST, "SELECT PASSWORD FROM ACCOUNT WHERE LOGIN=?", SHA_256); - assertFalse(dbAuthenticator.checkValid("dbuser","wrongPassword".getBytes())); + assertFalse(dbAuthenticator.checkValid(null, "dbuser","wrongPassword".getBytes())); } diff --git a/broker/src/test/java/io/moquette/spi/impl/security/FileAuthenticatorTest.java b/broker/src/test/java/io/moquette/spi/impl/security/FileAuthenticatorTest.java index f18e2673..0e707e8e 100644 --- a/broker/src/test/java/io/moquette/spi/impl/security/FileAuthenticatorTest.java +++ b/broker/src/test/java/io/moquette/spi/impl/security/FileAuthenticatorTest.java @@ -32,7 +32,7 @@ public class FileAuthenticatorTest { String file = getClass().getResource("/password_file.conf").getPath(); IAuthenticator auth = new FileAuthenticator(null, file); - assertTrue(auth.checkValid("testuser", "passwd".getBytes())); + assertTrue(auth.checkValid(null, "testuser", "passwd".getBytes())); } @Test @@ -40,14 +40,14 @@ public class FileAuthenticatorTest { String file = getClass().getResource("/password_file.conf").getPath(); IAuthenticator auth = new FileAuthenticator(null, file); - assertFalse(auth.checkValid("testuser2", "passwd".getBytes())); + assertFalse(auth.checkValid(null, "testuser2", "passwd".getBytes())); } @Test public void loadPasswordFile_verifyDirectoryRef() { IAuthenticator auth = new FileAuthenticator("", ""); - assertFalse(auth.checkValid("testuser2", "passwd".getBytes())); + assertFalse(auth.checkValid(null, "testuser2", "passwd".getBytes())); } } \ No newline at end of file