diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/CertificateLoginModule.java b/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/CertificateLoginModule.java index c67a036b34..e21fc39fc4 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/CertificateLoginModule.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/CertificateLoginModule.java @@ -44,7 +44,6 @@ public abstract class CertificateLoginModule extends PropertiesLoader implements private X509Certificate[] certificates; private String username; - private Set roles; private Set principals = new HashSet<>(); /** @@ -82,8 +81,6 @@ public abstract class CertificateLoginModule extends PropertiesLoader implements throw new FailedLoginException("No user for client certificate: " + getDistinguishedName(certificates)); } - roles = getUserRoles(username); - if (debug) { ActiveMQServerLogger.LOGGER.debug("Certificate for user: " + username); } @@ -97,7 +94,7 @@ public abstract class CertificateLoginModule extends PropertiesLoader implements public boolean commit() throws LoginException { principals.add(new UserPrincipal(username)); - for (String role : roles) { + for (String role : getUserRoles(username)) { principals.add(new RolePrincipal(role)); } @@ -142,8 +139,8 @@ public abstract class CertificateLoginModule extends PropertiesLoader implements * Helper method. */ private void clear() { - roles.clear(); certificates = null; + username = null; } /** diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/PropertiesLoginModule.java b/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/PropertiesLoginModule.java index 038823fb9b..61e1aa540d 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/PropertiesLoginModule.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/PropertiesLoginModule.java @@ -43,7 +43,7 @@ public class PropertiesLoginModule extends PropertiesLoader implements LoginModu private CallbackHandler callbackHandler; private Properties users; - private Properties roles; + private Map> roles; private String user; private final Set principals = new HashSet<>(); private boolean loginSucceeded; @@ -59,7 +59,7 @@ public class PropertiesLoginModule extends PropertiesLoader implements LoginModu init(options); users = load(USER_FILE_PROP_NAME, "user", options).getProps(); - roles = load(ROLE_FILE_PROP_NAME, "role", options).getProps(); + roles = load(ROLE_FILE_PROP_NAME, "role", options).invertedPropertiesValuesMap(); } @Override @@ -107,17 +107,10 @@ public class PropertiesLoginModule extends PropertiesLoader implements LoginModu if (result) { principals.add(new UserPrincipal(user)); - for (Map.Entry entry : roles.entrySet()) { - String name = (String) entry.getKey(); - String[] userList = ((String) entry.getValue()).split(","); - if (debug) { - ActiveMQServerLogger.LOGGER.debug("Inspecting role '" + name + "' with user(s): " + entry.getValue()); - } - for (int i = 0; i < userList.length; i++) { - if (user.equals(userList[i])) { - principals.add(new RolePrincipal(name)); - break; - } + Set matchedRoles = roles.get(user); + if (matchedRoles != null) { + for (String entry : matchedRoles) { + principals.add(new RolePrincipal(entry)); } } diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/ReloadableProperties.java b/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/ReloadableProperties.java index 9fc9435e66..da4c651326 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/ReloadableProperties.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/ReloadableProperties.java @@ -20,8 +20,10 @@ import java.io.File; import java.io.FileInputStream; import java.io.IOException; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; import java.util.Properties; +import java.util.Set; import org.apache.activemq.artemis.core.server.ActiveMQServerLogger; @@ -29,6 +31,7 @@ public class ReloadableProperties { private Properties props = new Properties(); private Map invertedProps; + private Map> invertedValueProps; private long reloadTime = -1; private final PropertiesLoader.FileNameKey key; @@ -46,6 +49,7 @@ public class ReloadableProperties { try { load(key.file(), props); invertedProps = null; + invertedValueProps = null; if (key.isDebug()) { ActiveMQServerLogger.LOGGER.debug("Load of: " + key); } @@ -71,6 +75,24 @@ public class ReloadableProperties { return invertedProps; } + public synchronized Map> invertedPropertiesValuesMap() { + if (invertedValueProps == null) { + invertedValueProps = new HashMap<>(props.size()); + for (Map.Entry val : props.entrySet()) { + String[] userList = ((String)val.getValue()).split(","); + for (String user : userList) { + Set set = invertedValueProps.get(user); + if (set == null) { + set = new HashSet<>(); + invertedValueProps.put(user, set); + } + set.add((String)val.getKey()); + } + } + } + return invertedValueProps; + } + private void load(final File source, Properties props) throws IOException { try (FileInputStream in = new FileInputStream(source)) { diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/TextFileCertificateLoginModule.java b/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/TextFileCertificateLoginModule.java index 404d45d578..dda073ae3b 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/TextFileCertificateLoginModule.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/TextFileCertificateLoginModule.java @@ -20,10 +20,8 @@ import javax.security.auth.Subject; import javax.security.auth.callback.CallbackHandler; import javax.security.auth.login.LoginException; import javax.security.cert.X509Certificate; -import java.util.Enumeration; -import java.util.HashSet; +import java.util.Collections; import java.util.Map; -import java.util.Properties; import java.util.Set; /** @@ -42,7 +40,7 @@ public class TextFileCertificateLoginModule extends CertificateLoginModule { private static final String USER_FILE_PROP_NAME = "org.apache.activemq.jaas.textfiledn.user"; private static final String ROLE_FILE_PROP_NAME = "org.apache.activemq.jaas.textfiledn.role"; - private Properties roles; + private Map> rolesByUser; private Map usersByDn; /** @@ -52,7 +50,7 @@ public class TextFileCertificateLoginModule extends CertificateLoginModule { public void initialize(Subject subject, CallbackHandler callbackHandler, Map sharedState, Map options) { super.initialize(subject, callbackHandler, sharedState, options); usersByDn = load(USER_FILE_PROP_NAME, "", options).invertedPropertiesMap(); - roles = load(ROLE_FILE_PROP_NAME, "", options).getProps(); + rolesByUser = load(ROLE_FILE_PROP_NAME, "", options).invertedPropertiesValuesMap(); } /** @@ -84,16 +82,9 @@ public class TextFileCertificateLoginModule extends CertificateLoginModule { */ @Override protected Set getUserRoles(String username) throws LoginException { - Set userRoles = new HashSet<>(); - for (Enumeration enumeration = roles.keys(); enumeration.hasMoreElements(); ) { - String groupName = (String) enumeration.nextElement(); - String[] userList = (roles.getProperty(groupName) + "").split(","); - for (int i = 0; i < userList.length; i++) { - if (username.equals(userList[i])) { - userRoles.add(groupName); - break; - } - } + Set userRoles = rolesByUser.get(username); + if (userRoles == null) { + userRoles = Collections.emptySet(); } return userRoles; diff --git a/artemis-server/src/test/java/org/apache/activemq/artemis/core/security/jaas/PropertiesLoginModuleTest.java b/artemis-server/src/test/java/org/apache/activemq/artemis/core/security/jaas/PropertiesLoginModuleTest.java index b1f08a6d92..8d5347258e 100644 --- a/artemis-server/src/test/java/org/apache/activemq/artemis/core/security/jaas/PropertiesLoginModuleTest.java +++ b/artemis-server/src/test/java/org/apache/activemq/artemis/core/security/jaas/PropertiesLoginModuleTest.java @@ -25,11 +25,13 @@ import javax.security.auth.callback.UnsupportedCallbackException; import javax.security.auth.login.FailedLoginException; import javax.security.auth.login.LoginContext; import javax.security.auth.login.LoginException; +import java.io.File; import java.io.IOException; import java.net.URL; import org.apache.activemq.artemis.spi.core.security.jaas.RolePrincipal; import org.apache.activemq.artemis.spi.core.security.jaas.UserPrincipal; +import org.apache.commons.io.FileUtils; import org.junit.Assert; import org.junit.Test; @@ -48,22 +50,8 @@ public class PropertiesLoginModuleTest extends Assert { @Test public void testLogin() throws LoginException { - LoginContext context = new LoginContext("PropertiesLogin", new CallbackHandler() { - @Override - public void handle(Callback[] callbacks) throws IOException, UnsupportedCallbackException { - for (int i = 0; i < callbacks.length; i++) { - if (callbacks[i] instanceof NameCallback) { - ((NameCallback) callbacks[i]).setName("first"); - } - else if (callbacks[i] instanceof PasswordCallback) { - ((PasswordCallback) callbacks[i]).setPassword("secret".toCharArray()); - } - else { - throw new UnsupportedCallbackException(callbacks[i]); - } - } - } - }); + LoginContext context = new LoginContext("PropertiesLogin", new UserPassHandler("first", "secret")); + context.login(); Subject subject = context.getSubject(); @@ -77,24 +65,55 @@ public class PropertiesLoginModuleTest extends Assert { assertEquals("Should have zero principals", 0, subject.getPrincipals().size()); } + @Test + public void testLoginReload() throws Exception { + File targetPropDir = new File("target/loginReloadTest"); + File usersFile = new File(targetPropDir, "users.properties"); + File rolesFile = new File(targetPropDir, "roles.properties"); + + //Set up initial properties + FileUtils.copyFile(new File(getClass().getResource("/users.properties").toURI()), usersFile); + FileUtils.copyFile(new File(getClass().getResource("/roles.properties").toURI()), rolesFile); + + LoginContext context = new LoginContext("PropertiesLoginReload", new UserPassHandler("first", "secret")); + context.login(); + Subject subject = context.getSubject(); + + //test initial principals + assertEquals("Should have three principals", 3, subject.getPrincipals().size()); + assertEquals("Should have one user principal", 1, subject.getPrincipals(UserPrincipal.class).size()); + assertEquals("Should have two group principals", 2, subject.getPrincipals(RolePrincipal.class).size()); + + context.logout(); + + assertEquals("Should have zero principals", 0, subject.getPrincipals().size()); + + //Modify the file and test that the properties are reloaded + Thread.sleep(1000); + FileUtils.copyFile(new File(getClass().getResource("/usersReload.properties").toURI()), usersFile); + FileUtils.copyFile(new File(getClass().getResource("/rolesReload.properties").toURI()), rolesFile); + FileUtils.touch(usersFile); + FileUtils.touch(rolesFile); + + //Use new password to verify users file was reloaded + context = new LoginContext("PropertiesLoginReload", new UserPassHandler("first", "secrets")); + context.login(); + subject = context.getSubject(); + + //Check that the principals changed + assertEquals("Should have three principals", 2, subject.getPrincipals().size()); + assertEquals("Should have one user principal", 1, subject.getPrincipals(UserPrincipal.class).size()); + assertEquals("Should have one group principals", 1, subject.getPrincipals(RolePrincipal.class).size()); + + context.logout(); + + assertEquals("Should have zero principals", 0, subject.getPrincipals().size()); + } + @Test public void testBadUseridLogin() throws Exception { - LoginContext context = new LoginContext("PropertiesLogin", new CallbackHandler() { - @Override - public void handle(Callback[] callbacks) throws IOException, UnsupportedCallbackException { - for (int i = 0; i < callbacks.length; i++) { - if (callbacks[i] instanceof NameCallback) { - ((NameCallback) callbacks[i]).setName("BAD"); - } - else if (callbacks[i] instanceof PasswordCallback) { - ((PasswordCallback) callbacks[i]).setPassword("secret".toCharArray()); - } - else { - throw new UnsupportedCallbackException(callbacks[i]); - } - } - } - }); + LoginContext context = new LoginContext("PropertiesLogin", new UserPassHandler("BAD", "secret")); + try { context.login(); fail("Should have thrown a FailedLoginException"); @@ -106,22 +125,8 @@ public class PropertiesLoginModuleTest extends Assert { @Test public void testBadPWLogin() throws Exception { - LoginContext context = new LoginContext("PropertiesLogin", new CallbackHandler() { - @Override - public void handle(Callback[] callbacks) throws IOException, UnsupportedCallbackException { - for (int i = 0; i < callbacks.length; i++) { - if (callbacks[i] instanceof NameCallback) { - ((NameCallback) callbacks[i]).setName("first"); - } - else if (callbacks[i] instanceof PasswordCallback) { - ((PasswordCallback) callbacks[i]).setPassword("BAD".toCharArray()); - } - else { - throw new UnsupportedCallbackException(callbacks[i]); - } - } - } - }); + LoginContext context = new LoginContext("PropertiesLogin", new UserPassHandler("first", "BAD")); + try { context.login(); fail("Should have thrown a FailedLoginException"); @@ -130,4 +135,30 @@ public class PropertiesLoginModuleTest extends Assert { } } + + private static class UserPassHandler implements CallbackHandler { + + private final String user; + private final String pass; + + public UserPassHandler(final String user, final String pass) { + this.user = user; + this.pass = pass; + } + + @Override + public void handle(Callback[] callbacks) throws IOException, UnsupportedCallbackException { + for (int i = 0; i < callbacks.length; i++) { + if (callbacks[i] instanceof NameCallback) { + ((NameCallback) callbacks[i]).setName(user); + } + else if (callbacks[i] instanceof PasswordCallback) { + ((PasswordCallback) callbacks[i]).setPassword(pass.toCharArray()); + } + else { + throw new UnsupportedCallbackException(callbacks[i]); + } + } + } + } } diff --git a/artemis-server/src/test/resources/login.config b/artemis-server/src/test/resources/login.config index 9b1e1c003b..997bfe5bd6 100644 --- a/artemis-server/src/test/resources/login.config +++ b/artemis-server/src/test/resources/login.config @@ -21,6 +21,15 @@ PropertiesLogin { org.apache.activemq.jaas.properties.role="roles.properties"; }; +PropertiesLoginReload { + org.apache.activemq.artemis.spi.core.security.jaas.PropertiesLoginModule required + debug=true + reload=true + baseDir="target/loginReloadTest/" + org.apache.activemq.jaas.properties.user="users.properties" + org.apache.activemq.jaas.properties.role="roles.properties"; +}; + LDAPLogin { org.apache.activemq.artemis.spi.core.security.jaas.LDAPLoginModule required debug=true diff --git a/artemis-server/src/test/resources/rolesReload.properties b/artemis-server/src/test/resources/rolesReload.properties new file mode 100644 index 0000000000..890e06110a --- /dev/null +++ b/artemis-server/src/test/resources/rolesReload.properties @@ -0,0 +1,19 @@ +## --------------------------------------------------------------------------- +## Licensed to the Apache Software Foundation (ASF) under one or more +## contributor license agreements. See the NOTICE file distributed with +## this work for additional information regarding copyright ownership. +## The ASF licenses this file to You under the Apache License, Version 2.0 +## (the "License"); you may not use this file except in compliance with +## the License. You may obtain a copy of the License at +## +## http://www.apache.org/licenses/LICENSE-2.0 +## +## Unless required by applicable law or agreed to in writing, software +## distributed under the License is distributed on an "AS IS" BASIS, +## WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +## See the License for the specific language governing permissions and +## limitations under the License. +## --------------------------------------------------------------------------- + +programmers=first +accounting=second \ No newline at end of file diff --git a/artemis-server/src/test/resources/usersReload.properties b/artemis-server/src/test/resources/usersReload.properties new file mode 100644 index 0000000000..6fa6205cae --- /dev/null +++ b/artemis-server/src/test/resources/usersReload.properties @@ -0,0 +1,20 @@ +## --------------------------------------------------------------------------- +## Licensed to the Apache Software Foundation (ASF) under one or more +## contributor license agreements. See the NOTICE file distributed with +## this work for additional information regarding copyright ownership. +## The ASF licenses this file to You under the Apache License, Version 2.0 +## (the "License"); you may not use this file except in compliance with +## the License. You may obtain a copy of the License at +## +## http://www.apache.org/licenses/LICENSE-2.0 +## +## Unless required by applicable law or agreed to in writing, software +## distributed under the License is distributed on an "AS IS" BASIS, +## WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +## See the License for the specific language governing permissions and +## limitations under the License. +## --------------------------------------------------------------------------- + +#different password than users.properties +first=secrets +second=password \ No newline at end of file