diff options
15 files changed, 190 insertions, 604 deletions
diff --git a/qpid/java/broker-plugins/firewall/src/test/java/org/apache/qpid/server/security/access/FirewallConfigurationTest.java b/qpid/java/broker-plugins/firewall/src/test/java/org/apache/qpid/server/security/access/FirewallConfigurationTest.java index ab8957e7ef..ebede414f4 100644 --- a/qpid/java/broker-plugins/firewall/src/test/java/org/apache/qpid/server/security/access/FirewallConfigurationTest.java +++ b/qpid/java/broker-plugins/firewall/src/test/java/org/apache/qpid/server/security/access/FirewallConfigurationTest.java @@ -93,7 +93,6 @@ public class FirewallConfigurationTest extends InternalBrokerBaseCase out.write("\t\t\t</principal-database>\n"); out.write("\t\t</principal-databases>\n"); out.write("\t\t<jmx>\n"); - out.write("\t\t\t<access>/dev/null</access>\n"); out.write("\t\t\t<principal-database>passwordfile</principal-database>\n"); out.write("\t\t</jmx>\n"); out.write("\t\t<firewall>\n"); @@ -194,7 +193,6 @@ public class FirewallConfigurationTest extends InternalBrokerBaseCase out.write("\t\t\t</principal-database>\n"); out.write("\t\t</principal-databases>\n"); out.write("\t\t<jmx>\n"); - out.write("\t\t\t<access>/dev/null</access>\n"); out.write("\t\t\t<principal-database>passwordfile</principal-database>\n"); out.write("\t\t</jmx>\n"); out.write("\t\t<firewall>\n"); @@ -304,7 +302,6 @@ public class FirewallConfigurationTest extends InternalBrokerBaseCase out.write("\t\t\t</principal-database>\n"); out.write("\t\t</principal-databases>\n"); out.write("\t\t<jmx>\n"); - out.write("\t\t\t<access>/dev/null</access>\n"); out.write("\t\t\t<principal-database>passwordfile</principal-database>\n"); out.write("\t\t</jmx>\n"); out.write("\t\t<firewall>\n"); diff --git a/qpid/java/broker/etc/config.xml b/qpid/java/broker/etc/config.xml index 14b9456067..ec386ab669 100644 --- a/qpid/java/broker/etc/config.xml +++ b/qpid/java/broker/etc/config.xml @@ -88,7 +88,6 @@ <msg-auth>false</msg-auth> <jmx> - <access>${conf}/jmxremote.access</access> <principal-database>passwordfile</principal-database> </jmx> </security> diff --git a/qpid/java/broker/etc/jmxremote.access b/qpid/java/broker/etc/jmxremote.access deleted file mode 100644 index 1a51a6991b..0000000000 --- a/qpid/java/broker/etc/jmxremote.access +++ /dev/null @@ -1,23 +0,0 @@ -#
-# 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.
-
-#Generated by JMX Console : Last edited by user:admin
-#Tue Jun 12 16:46:39 BST 2007
-admin=admin
-guest=readonly
-user=readwrite
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/ServerConfiguration.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/ServerConfiguration.java index 43be0611a5..7fc91c817b 100644 --- a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/ServerConfiguration.java +++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/ServerConfiguration.java @@ -205,7 +205,15 @@ public class ServerConfiguration extends ConfigurationPlugin implements SignalHa @Override public void validateConfiguration() throws ConfigurationException { - //Currently doesn't do validation + // Support for security.jmx.access was removed when JMX access rights were incorporated into the main ACL. + // This ensure that users remove the element from their configuration file. + + if (getListValue("security.jmx.access").size() > 0) + { + String message = "Validation error : security/jmx/access is no longer a supported element within the configuration xml." + + (_configFile == null ? "" : " Configuration file : " + _configFile); + throw new ConfigurationException(message); + } } /* @@ -530,11 +538,6 @@ public class ServerConfiguration extends ConfigurationPlugin implements SignalHa return getListValue("security.jmx.principal-database"); } - public List<String> getManagementAccessList() - { - return getListValue("security.jmx.access"); - } - public int getFrameSize() { return getIntValue("advanced.framesize", DEFAULT_FRAME_SIZE); diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/ConfigurationFilePrincipalDatabaseManager.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/ConfigurationFilePrincipalDatabaseManager.java index 5cebb7d2d8..e9276e1b0e 100644 --- a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/ConfigurationFilePrincipalDatabaseManager.java +++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/ConfigurationFilePrincipalDatabaseManager.java @@ -184,33 +184,6 @@ public class ConfigurationFilePrincipalDatabaseManager implements PrincipalDatab } _mbean.setPrincipalDatabase(database); - - List<String> jmxaccesslist = config.getManagementAccessList(); - if (jmxaccesslist.isEmpty()) - { - throw new ConfigurationException("No access control files specified for jmx security"); - } - - String jmxaccesssFile = null; - - try - { - jmxaccesssFile = PropertyUtils.replaceProperties(jmxaccesslist.get(0)); - } - catch (PropertyException e) - { - throw new ConfigurationException("Unable to parse access control filename '" + jmxaccesssFile + "'"); - } - - try - { - _mbean.setAccessFile(jmxaccesssFile); - } - catch (IOException e) - { - _logger.warn("Unable to load access file:" + jmxaccesssFile); - } - _mbean.register(); } catch (JMException e) diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/management/AMQUserManagementMBean.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/management/AMQUserManagementMBean.java index ee4336055b..a839315bcc 100644 --- a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/management/AMQUserManagementMBean.java +++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/management/AMQUserManagementMBean.java @@ -20,19 +20,9 @@ */ package org.apache.qpid.server.security.auth.management; -import java.io.File; -import java.io.FileInputStream; -import java.io.FileOutputStream; import java.io.IOException; -import java.security.AccessControlContext; -import java.security.AccessController; import java.security.Principal; -import java.util.Enumeration; import java.util.List; -import java.util.Properties; -import java.util.Random; -import java.util.Set; -import java.util.concurrent.locks.ReentrantLock; import javax.management.JMException; import javax.management.openmbean.CompositeData; @@ -44,17 +34,13 @@ import javax.management.openmbean.SimpleType; import javax.management.openmbean.TabularData; import javax.management.openmbean.TabularDataSupport; import javax.management.openmbean.TabularType; -import javax.management.remote.JMXPrincipal; -import javax.security.auth.Subject; import javax.security.auth.login.AccountNotFoundException; -import org.apache.commons.configuration.ConfigurationException; import org.apache.log4j.Logger; import org.apache.qpid.management.common.mbeans.UserManagement; import org.apache.qpid.management.common.mbeans.annotations.MBeanDescription; import org.apache.qpid.management.common.mbeans.annotations.MBeanOperation; import org.apache.qpid.server.management.AMQManagedObject; -import org.apache.qpid.server.management.MBeanInvocationHandlerImpl; import org.apache.qpid.server.security.auth.database.PrincipalDatabase; import org.apache.qpid.server.security.auth.sasl.UsernamePrincipal; @@ -65,22 +51,18 @@ public class AMQUserManagementMBean extends AMQManagedObject implements UserMana private static final Logger _logger = Logger.getLogger(AMQUserManagementMBean.class); private PrincipalDatabase _principalDatabase; - private Properties _accessRights; - private File _accessFile; - - private ReentrantLock _accessRightsUpdate = new ReentrantLock(); // Setup for the TabularType - static TabularType _userlistDataType; // Datatype for representing User Lists - static CompositeType _userDataType; // Composite type for representing User + private static final TabularType _userlistDataType; // Datatype for representing User Lists + private static final CompositeType _userDataType; // Composite type for representing User static { OpenType[] userItemTypes = new OpenType[4]; // User item types. userItemTypes[0] = SimpleType.STRING; // For Username - userItemTypes[1] = SimpleType.BOOLEAN; // For Rights - Read - userItemTypes[2] = SimpleType.BOOLEAN; // For Rights - Write - userItemTypes[3] = SimpleType.BOOLEAN; // For Rights - Admin + userItemTypes[1] = SimpleType.BOOLEAN; // For Rights - Read - No longer in use + userItemTypes[2] = SimpleType.BOOLEAN; // For Rights - Write - No longer in use + userItemTypes[3] = SimpleType.BOOLEAN; // For Rights - Admin - No longer is use try { @@ -92,12 +74,11 @@ public class AMQUserManagementMBean extends AMQManagedObject implements UserMana } catch (OpenDataException e) { - _logger.error("Tabular data setup for viewing users incorrect."); - _userlistDataType = null; + _logger.error("Tabular data setup for viewing users incorrect.", e); + throw new ExceptionInInitializerError("Tabular data setup for viewing users incorrect"); } } - public AMQUserManagementMBean() throws JMException { super(UserManagement.class, UserManagement.TYPE); @@ -122,141 +103,45 @@ public class AMQUserManagementMBean extends AMQManagedObject implements UserMana } catch (AccountNotFoundException e) { - _logger.warn("Attempt to set password of non-existant user'" + username + "'"); + _logger.warn("Attempt to set password of non-existent user'" + username + "'"); return false; } } public boolean setRights(String username, boolean read, boolean write, boolean admin) { - - Object oldRights = null; - if ((oldRights =_accessRights.get(username)) == null) - { - // If the user doesn't exist in the access rights file check that they at least have an account. - if (_principalDatabase.getUser(username) == null) - { - return false; - } - } - - try - { - _accessRightsUpdate.lock(); - - // Update the access rights - if (admin) - { - _accessRights.put(username, MBeanInvocationHandlerImpl.ADMIN); - } - else - { - if (read | write) - { - if (read) - { - _accessRights.put(username, MBeanInvocationHandlerImpl.READONLY); - } - if (write) - { - _accessRights.put(username, MBeanInvocationHandlerImpl.READWRITE); - } - } - else - { - _accessRights.remove(username); - } - } - - //save the rights file - try - { - saveAccessFile(); - } - catch (IOException e) - { - _logger.warn("Problem occured saving '" + _accessFile + "', the access right changes will not be preserved: " + e); - - //the rights file was not successfully saved, restore user rights to previous value - _logger.warn("Reverting attempted rights update for user'" + username + "'"); - if (oldRights != null) - { - _accessRights.put(username, oldRights); - } - else - { - _accessRights.remove(username); - } - - return false; - } - } - finally + throw new UnsupportedOperationException("Support for setting access rights no longer supported."); + } + + public boolean createUser(String username, String password) + { + if (_principalDatabase.createPrincipal(new UsernamePrincipal(username), password.toCharArray())) { - _accessRightsUpdate.unlock(); + return true; } - return true; + return false; } public boolean createUser(String username, String password, boolean read, boolean write, boolean admin) { - return createUser(username, password.toCharArray(), read, write, admin); + if (read || write || admin) + { + throw new UnsupportedOperationException("Support for setting access rights to true no longer supported."); + } + return createUser(username, password); } public boolean createUser(String username, char[] password, boolean read, boolean write, boolean admin) { - if (_principalDatabase.createPrincipal(new UsernamePrincipal(username), password)) - { - if (!setRights(username, read, write, admin)) - { - //unable to set rights for user, remove account - try - { - _principalDatabase.deletePrincipal(new UsernamePrincipal(username)); - } - catch (AccountNotFoundException e) - { - //ignore - } - return false; - } - else - { - return true; - } - } - - return false; + return createUser(username, new String(password), read, write, admin); } public boolean deleteUser(String username) { try { - if (_principalDatabase.deletePrincipal(new UsernamePrincipal(username))) - { - try - { - _accessRightsUpdate.lock(); - - _accessRights.remove(username); - - try - { - saveAccessFile(); - } - catch (IOException e) - { - _logger.warn("Problem occured saving '" + _accessFile + "', the access right changes will not be preserved: " + e); - return false; - } - } - finally - { - _accessRightsUpdate.unlock(); - } - } + _principalDatabase.deletePrincipal(new UsernamePrincipal(username)); } catch (AccountNotFoundException e) { @@ -269,38 +154,23 @@ public class AMQUserManagementMBean extends AMQManagedObject implements UserMana public boolean reloadData() { - try - { - loadAccessFile(); - _principalDatabase.reload(); - } - catch (ConfigurationException e) - { - _logger.warn("Reload failed due to:" + e); - return false; - } - catch (IOException e) - { - _logger.warn("Reload failed due to:" + e); - return false; - } - // Reload successful - return true; + try + { + _principalDatabase.reload(); + } + catch (IOException e) + { + _logger.warn("Reload failed due to:", e); + return false; + } + // Reload successful + return true; } - @MBeanOperation(name = "viewUsers", description = "All users with access rights to the system.") + @MBeanOperation(name = "viewUsers", description = "All users that are currently available to the system.") public TabularData viewUsers() { - // Table of users - // Username(string), Access rights Read,Write,Admin(bool,bool,bool) - - if (_userlistDataType == null) - { - _logger.warn("TabluarData not setup correctly"); - return null; - } - List<Principal> users = _principalDatabase.getUsers(); TabularDataSupport userList = new TabularDataSupport(_userlistDataType); @@ -311,29 +181,16 @@ public class AMQUserManagementMBean extends AMQManagedObject implements UserMana for (Principal user : users) { // Create header attributes list - - String rights = (String) _accessRights.get(user.getName()); - - Boolean read = false; - Boolean write = false; - Boolean admin = false; - - if (rights != null) - { - read = rights.equals(MBeanInvocationHandlerImpl.READONLY) - || rights.equals(MBeanInvocationHandlerImpl.READWRITE); - write = rights.equals(MBeanInvocationHandlerImpl.READWRITE); - admin = rights.equals(MBeanInvocationHandlerImpl.ADMIN); - } - - Object[] itemData = {user.getName(), read, write, admin}; + + // Read,Write,Admin items are depcreated and we return always false. + Object[] itemData = {user.getName(), false, false, false}; CompositeData messageData = new CompositeDataSupport(_userDataType, COMPOSITE_ITEM_NAMES.toArray(new String[COMPOSITE_ITEM_NAMES.size()]), itemData); userList.put(messageData); } } catch (OpenDataException e) { - _logger.warn("Unable to create user list due to :" + e); + _logger.warn("Unable to create user list due to :", e); return null; } @@ -351,187 +208,4 @@ public class AMQUserManagementMBean extends AMQManagedObject implements UserMana { _principalDatabase = database; } - - /** - * setAccessFile - * - * @param accessFile the file to use for updating. - * - * @throws java.io.IOException If the file cannot be accessed - * @throws org.apache.commons.configuration.ConfigurationException - * if checks on the file fail. - */ - public void setAccessFile(String accessFile) throws IOException, ConfigurationException - { - if (accessFile != null) - { - _accessFile = new File(accessFile); - if (!_accessFile.exists()) - { - throw new ConfigurationException("'" + _accessFile + "' does not exist"); - } - - if (!_accessFile.canRead()) - { - throw new ConfigurationException("Cannot read '" + _accessFile + "'."); - } - - if (!_accessFile.canWrite()) - { - _logger.warn("Unable to write to access rights file '" + _accessFile + "', changes will not be preserved."); - } - - loadAccessFile(); - } - else - { - _logger.warn("Access rights file specified is null. Access rights not changed."); - } - } - - private void loadAccessFile() throws IOException, ConfigurationException - { - if(_accessFile == null) - { - _logger.error("No jmx access rights file has been specified."); - return; - } - - if(_accessFile.exists()) - { - try - { - _accessRightsUpdate.lock(); - - Properties accessRights = new Properties(); - FileInputStream inStream = new FileInputStream(_accessFile); - try - { - accessRights.load(inStream); - } - finally - { - inStream.close(); - } - - checkAccessRights(accessRights); - setAccessRights(accessRights); - } - finally - { - _accessRightsUpdate.unlock(); - } - } - else - { - _logger.error("Specified jmxaccess rights file '" + _accessFile + "' does not exist."); - } - } - - private void checkAccessRights(Properties accessRights) - { - Enumeration values = accessRights.propertyNames(); - - while (values.hasMoreElements()) - { - String user = (String) values.nextElement(); - - if (_principalDatabase.getUser(user) == null) - { - _logger.warn("Access rights contains user '" + user + "' but there is no authentication data for that user"); - } - } - } - - private void saveAccessFile() throws IOException - { - try - { - _accessRightsUpdate.lock(); - - // Create temporary file - Random r = new Random(); - File tmp; - do - { - tmp = new File(_accessFile.getPath() + r.nextInt() + ".tmp"); - } - while(tmp.exists()); - - tmp.deleteOnExit(); - - FileOutputStream output = new FileOutputStream(tmp); - _accessRights.store(output, "Generated by AMQUserManagementMBean Console : Last edited by user:" + getCurrentJMXUser()); - output.close(); - - // Swap temp file to main rights file. - File old = new File(_accessFile.getAbsoluteFile() + ".old"); - if (old.exists()) - { - old.delete(); - } - - if(!_accessFile.renameTo(old)) - { - //unable to rename the existing file to the backup name - _logger.error("Could not backup the existing management rights file"); - throw new IOException("Could not backup the existing management rights file"); - } - - if(!tmp.renameTo(_accessFile)) - { - //failed to rename the new file to the required filename - - if(!old.renameTo(_accessFile)) - { - //unable to return the backup to required filename - _logger.error("Could not rename the new management rights file into place, and unable to restore original file"); - throw new IOException("Could not rename the new management rights file into place, and unable to restore original file"); - } - - _logger.error("Could not rename the new management rights file into place"); - throw new IOException("Could not rename the new management rights file into place"); - } - } - finally - { - _accessRightsUpdate.unlock(); - } - - } - - private String getCurrentJMXUser() - { - AccessControlContext acc = AccessController.getContext(); - - Subject subject = Subject.getSubject(acc); - if (subject == null) - { - return "Unknown user, authentication Subject was null"; - } - - // Retrieve JMXPrincipal from Subject - Set<JMXPrincipal> principals = subject.getPrincipals(JMXPrincipal.class); - if (principals == null || principals.isEmpty()) - { - return "Unknown user principals were null"; - } - - Principal principal = principals.iterator().next(); - return principal.getName(); - } - - /** - * user=read user=write user=readwrite user=admin - * - * @param accessRights The properties list of access rights to process - */ - private void setAccessRights(Properties accessRights) - { - _logger.debug("Setting Access Rights:" + accessRights); - _accessRights = accessRights; - - // TODO check where this is used - // MBeanInvocationHandlerImpl.setAccessRights(_accessRights); - } } diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/ServerConfigurationTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/ServerConfigurationTest.java index 718874cf69..43540c88a1 100644 --- a/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/ServerConfigurationTest.java +++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/ServerConfigurationTest.java @@ -177,23 +177,7 @@ public class ServerConfigurationTest extends InternalBrokerBaseCase assertEquals("b", dbs.get(1)); } - public void testGetManagementAccessList() throws ConfigurationException - { - // Check default - ServerConfiguration serverConfig = new ServerConfiguration(_config); - serverConfig.initialise(); - assertEquals(0, serverConfig.getManagementAccessList().size()); - // Check value we set - _config.setProperty("security.jmx.access(0)", "a"); - _config.setProperty("security.jmx.access(1)", "b"); - serverConfig = new ServerConfiguration(_config); - serverConfig.initialise(); - List<String> dbs = serverConfig.getManagementAccessList(); - assertEquals(2, dbs.size()); - assertEquals("a", dbs.get(0)); - assertEquals("b", dbs.get(1)); - } public void testGetFrameSize() throws ConfigurationException { @@ -848,7 +832,6 @@ public class ServerConfigurationTest extends InternalBrokerBaseCase out.write("\t\t\t</principal-database>\n"); out.write("\t\t</principal-databases>\n"); out.write("\t\t<jmx>\n"); - out.write("\t\t\t<access>/dev/null</access>\n"); out.write("\t\t\t<principal-database>passwordfile</principal-database>\n"); out.write("\t\t</jmx>\n"); out.write("\t\t<firewall>\n"); @@ -899,7 +882,6 @@ public class ServerConfigurationTest extends InternalBrokerBaseCase out.write("\t\t\t</principal-database>\n"); out.write("\t\t</principal-databases>\n"); out.write("\t\t<jmx>\n"); - out.write("\t\t\t<access>/dev/null</access>\n"); out.write("\t\t\t<principal-database>passwordfile</principal-database>\n"); out.write("\t\t</jmx>\n"); out.write("\t\t<firewall>\n"); @@ -1005,7 +987,6 @@ public class ServerConfigurationTest extends InternalBrokerBaseCase out.write("\t\t\t</principal-database>\n"); out.write("\t\t</principal-databases>\n"); out.write("\t\t<jmx>\n"); - out.write("\t\t\t<access>/dev/null</access>\n"); out.write("\t\t\t<principal-database>passwordfile</principal-database>\n"); out.write("\t\t</jmx>\n"); out.write("\t\t<firewall>\n"); @@ -1481,4 +1462,31 @@ public class ServerConfigurationTest extends InternalBrokerBaseCase fail("Should throw a ConfigurationException"); } } + + /* + * Tests that the old element security.jmx.access (that used to be used + * to define JMX access rights) is rejected. + */ + public void testManagementAccessRejected() throws ConfigurationException + { + // Check default + ServerConfiguration serverConfig = new ServerConfiguration(_config); + serverConfig.initialise(); + + // Check value we set + _config.setProperty("security.jmx.access(0)", "jmxremote.access"); + serverConfig = new ServerConfiguration(_config); + + try + { + serverConfig.initialise(); + fail("Exception not thrown"); + } + catch (ConfigurationException ce) + { + assertEquals("Incorrect error message", + "Validation error : security/jmx/access is no longer a supported element within the configuration xml.", + ce.getMessage()); + } + } } diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/management/AMQUserManagementMBeanTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/management/AMQUserManagementMBeanTest.java index a6c17e042e..21f79e4b69 100644 --- a/qpid/java/broker/src/test/java/org/apache/qpid/server/management/AMQUserManagementMBeanTest.java +++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/management/AMQUserManagementMBeanTest.java @@ -21,22 +21,26 @@ package org.apache.qpid.server.management; -import java.io.BufferedReader; import java.io.BufferedWriter; import java.io.File; -import java.io.FileReader; import java.io.FileWriter; import java.io.IOException; -import org.apache.commons.configuration.ConfigurationException; +import javax.management.openmbean.CompositeData; +import javax.management.openmbean.TabularData; + + +import org.apache.commons.lang.NotImplementedException; +import org.apache.qpid.management.common.mbeans.UserManagement; import org.apache.qpid.server.security.auth.database.PlainPasswordFilePrincipalDatabase; import org.apache.qpid.server.security.auth.management.AMQUserManagementMBean; import org.apache.qpid.server.util.InternalBrokerBaseCase; -/* Note: The main purpose is to test the jmx access rights file manipulation - * within AMQUserManagementMBean. The Principal Databases are tested by their own tests, - * this test just exercises their usage in AMQUserManagementMBean. +/** + * + * Tests the AMQUserManagementMBean and its interaction with the PrincipalDatabase. + * */ public class AMQUserManagementMBeanTest extends InternalBrokerBaseCase { @@ -44,7 +48,6 @@ public class AMQUserManagementMBeanTest extends InternalBrokerBaseCase private AMQUserManagementMBean _amqumMBean; private File _passwordFile; - private File _accessFile; private static final String TEST_USERNAME = "testuser"; private static final String TEST_PASSWORD = "password"; @@ -57,7 +60,6 @@ public class AMQUserManagementMBeanTest extends InternalBrokerBaseCase _database = new PlainPasswordFilePrincipalDatabase(); _amqumMBean = new AMQUserManagementMBean(); loadFreshTestPasswordFile(); - loadFreshTestAccessFile(); } @Override @@ -65,142 +67,101 @@ public class AMQUserManagementMBeanTest extends InternalBrokerBaseCase { //clean up test password/access files File _oldPasswordFile = new File(_passwordFile.getAbsolutePath() + ".old"); - File _oldAccessFile = new File(_accessFile.getAbsolutePath() + ".old"); _oldPasswordFile.delete(); - _oldAccessFile.delete(); _passwordFile.delete(); - _accessFile.delete(); super.tearDown(); } public void testDeleteUser() { - loadFreshTestPasswordFile(); - loadFreshTestAccessFile(); + assertEquals("Unexpected number of users before test", 1,_amqumMBean.viewUsers().size()); + assertTrue("Delete should return true to flag successful delete", _amqumMBean.deleteUser(TEST_USERNAME)); + assertEquals("Unexpected number of users after test", 0,_amqumMBean.viewUsers().size()); + } + + public void testDeleteUserWhereUserDoesNotExist() + { + assertEquals("Unexpected number of users before test", 1,_amqumMBean.viewUsers().size()); + assertFalse("Delete should return false to flag unsuccessful delete", _amqumMBean.deleteUser("made.up.username")); + assertEquals("Unexpected number of users after test", 1,_amqumMBean.viewUsers().size()); - //try deleting a non existant user - assertFalse(_amqumMBean.deleteUser("made.up.username")); - - assertTrue(_amqumMBean.deleteUser(TEST_USERNAME)); } - public void testDeleteUserIsSavedToAccessFile() + public void testCreateUser() { - loadFreshTestPasswordFile(); - loadFreshTestAccessFile(); - - assertTrue(_amqumMBean.deleteUser(TEST_USERNAME)); - - //check the access rights were actually deleted from the file - try{ - BufferedReader reader = new BufferedReader(new FileReader(_accessFile)); + assertEquals("Unexpected number of users before test", 1,_amqumMBean.viewUsers().size()); + assertTrue("Create should return true to flag successful create", _amqumMBean.createUser("newuser", "mypass")); + assertEquals("Unexpected number of users before test", 2,_amqumMBean.viewUsers().size()); + } - //check the 'generated by' comment line is present - assertTrue("File has no content", reader.ready()); - assertTrue("'Generated by' comment line was missing",reader.readLine().contains("Generated by " + - "AMQUserManagementMBean Console : Last edited by user:")); + public void testCreateUserWhereUserAlreadyExists() + { + assertEquals("Unexpected number of users before test", 1,_amqumMBean.viewUsers().size()); + assertFalse("Create should return false to flag unsuccessful create", _amqumMBean.createUser(TEST_USERNAME, "mypass")); + assertEquals("Unexpected number of users before test", 1,_amqumMBean.viewUsers().size()); + } - //there should also be a modified date/time comment line - assertTrue("File has no modified date/time comment line", reader.ready()); - assertTrue("Modification date/time comment line was missing",reader.readLine().startsWith("#")); - - //the access file should not contain any further data now as we just deleted the only user - assertFalse("User access data was present when it should have been deleted", reader.ready()); - } - catch (IOException e) - { - fail("Unable to valdate file contents due to:" + e.getMessage()); - } - + public void testFiveArgCreateUserWithNegativeRightsRemainsSupported() + { + assertEquals("Unexpected number of users before test", 1,_amqumMBean.viewUsers().size()); + assertTrue("Create should return true to flag successful create", _amqumMBean.createUser("newuser", "mypass".toCharArray(), false, false, false)); + assertEquals("Unexpected number of users before test", 2,_amqumMBean.viewUsers().size()); } - public void testSetRights() + public void testSetPassword() { - loadFreshTestPasswordFile(); - loadFreshTestAccessFile(); - - assertFalse(_amqumMBean.setRights("made.up.username", true, false, false)); - - assertTrue(_amqumMBean.setRights(TEST_USERNAME, true, false, false)); - assertTrue(_amqumMBean.setRights(TEST_USERNAME, false, true, false)); - assertTrue(_amqumMBean.setRights(TEST_USERNAME, false, false, true)); + assertTrue("Set password should return true to flag successful change", _amqumMBean.setPassword(TEST_USERNAME, "newpassword")); } - public void testSetRightsIsSavedToAccessFile() + public void testSetPasswordWhereUserDoesNotExist() { - loadFreshTestPasswordFile(); - loadFreshTestAccessFile(); - - assertTrue(_amqumMBean.setRights(TEST_USERNAME, false, false, true)); - - //check the access rights were actually updated in the file - try{ - BufferedReader reader = new BufferedReader(new FileReader(_accessFile)); + assertFalse("Set password should return false to flag successful change", _amqumMBean.setPassword("made.up.username", "newpassword")); + } - //check the 'generated by' comment line is present - assertTrue("File has no content", reader.ready()); - assertTrue("'Generated by' comment line was missing",reader.readLine().contains("Generated by " + - "AMQUserManagementMBean Console : Last edited by user:")); + public void testViewUsers() + { + TabularData userList = _amqumMBean.viewUsers(); - //there should also be a modified date/time comment line - assertTrue("File has no modified date/time comment line", reader.ready()); - assertTrue("Modification date/time comment line was missing",reader.readLine().startsWith("#")); - - //the access file should not contain any further data now as we just deleted the only user - assertTrue("User access data was not updated in the access file", - reader.readLine().equals(TEST_USERNAME + "=" + MBeanInvocationHandlerImpl.ADMIN)); - - //the access file should not contain any further data now as we just deleted the only user - assertFalse("Additional user access data was present when there should be no more", reader.ready()); - } - catch (IOException e) - { - fail("Unable to valdate file contents due to:" + e.getMessage()); - } + assertNotNull(userList); + assertEquals("Unexpected number of users in user list", 1, userList.size()); + assertTrue(userList.containsKey(new Object[]{TEST_USERNAME})); + + // Check the deprecated read, write and admin items continue to exist but return false. + CompositeData userRec = userList.get(new Object[]{TEST_USERNAME}); + assertTrue(userRec.containsKey(UserManagement.RIGHTS_READ_ONLY)); + assertEquals(false, userRec.get(UserManagement.RIGHTS_READ_ONLY)); + assertEquals(false, userRec.get(UserManagement.RIGHTS_READ_WRITE)); + assertTrue(userRec.containsKey(UserManagement.RIGHTS_READ_WRITE)); + assertTrue(userRec.containsKey(UserManagement.RIGHTS_ADMIN)); + assertEquals(false, userRec.get(UserManagement.RIGHTS_ADMIN)); } - public void testSetAccessFileWithMissingFile() + // TEST DEPRECATED METHODS + public void testFiveArgCreateUserWithPositiveRightsThrowsUnsupportedOperation() { - try - { - _amqumMBean.setAccessFile("made.up.filename"); - } - catch (IOException e) + try { - fail("Should not have been an IOE." + e.getMessage()); + _amqumMBean.createUser(TEST_USERNAME, "mypass", true, false, false); + fail("Exception not thrown"); } - catch (ConfigurationException e) + catch (UnsupportedOperationException uoe) { - assertTrue(e.getMessage(), e.getMessage().endsWith("does not exist")); + // PASS } } - public void testSetAccessFileWithReadOnlyFile() + public void testSetRightsThrowsUnsupportedOperation() { - File testFile = null; - try - { - testFile = File.createTempFile(this.getClass().getName(),".access.readonly"); - BufferedWriter passwordWriter = new BufferedWriter(new FileWriter(testFile, false)); - passwordWriter.write(TEST_USERNAME + ":" + TEST_PASSWORD); - passwordWriter.newLine(); - passwordWriter.flush(); - passwordWriter.close(); - - testFile.setReadOnly(); - _amqumMBean.setAccessFile(testFile.getPath()); - } - catch (IOException e) + try { - fail("Access file was not created." + e.getMessage()); + _amqumMBean.setRights("", false, false, false); + fail("Exception not thrown"); } - catch (ConfigurationException e) + catch(UnsupportedOperationException nie) { - fail("There should not have been a configuration exception." + e.getMessage()); + // PASS } - - testFile.delete(); } // ============================ Utility methods ========================= @@ -227,37 +188,4 @@ public class AMQUserManagementMBeanTest extends InternalBrokerBaseCase fail("Unable to create test password file: " + e.getMessage()); } } - - private void loadFreshTestAccessFile() - { - try - { - if(_accessFile == null) - { - _accessFile = File.createTempFile(this.getClass().getName(),".access"); - } - - BufferedWriter accessWriter = new BufferedWriter(new FileWriter(_accessFile,false)); - accessWriter.write("#Last Updated By comment"); - accessWriter.newLine(); - accessWriter.write("#Date/time comment"); - accessWriter.newLine(); - accessWriter.write(TEST_USERNAME + "=" + MBeanInvocationHandlerImpl.READONLY); - accessWriter.newLine(); - accessWriter.flush(); - accessWriter.close(); - } - catch (IOException e) - { - fail("Unable to create test access file: " + e.getMessage()); - } - - try{ - _amqumMBean.setAccessFile(_accessFile.toString()); - } - catch (Exception e) - { - fail("Unable to set access file: " + e.getMessage()); - } - } } diff --git a/qpid/java/management/common/src/main/java/org/apache/qpid/management/common/mbeans/ServerInformation.java b/qpid/java/management/common/src/main/java/org/apache/qpid/management/common/mbeans/ServerInformation.java index 91961580f2..12ae69571e 100644 --- a/qpid/java/management/common/src/main/java/org/apache/qpid/management/common/mbeans/ServerInformation.java +++ b/qpid/java/management/common/src/main/java/org/apache/qpid/management/common/mbeans/ServerInformation.java @@ -30,7 +30,7 @@ import org.apache.qpid.management.common.mbeans.annotations.MBeanOperation; /** * Interface for the ServerInformation MBean * - * @version Qpid JMX API 2.2 + * @version Qpid JMX API 2.3 * @since Qpid JMX API 1.3 */ public interface ServerInformation @@ -47,7 +47,7 @@ public interface ServerInformation * Qpid JMX API 1.1 can be assumed. */ int QPID_JMX_API_MAJOR_VERSION = 2; - int QPID_JMX_API_MINOR_VERSION = 2; + int QPID_JMX_API_MINOR_VERSION = 3; /** diff --git a/qpid/java/management/common/src/main/java/org/apache/qpid/management/common/mbeans/UserManagement.java b/qpid/java/management/common/src/main/java/org/apache/qpid/management/common/mbeans/UserManagement.java index 2bd00d5802..194bd83752 100644 --- a/qpid/java/management/common/src/main/java/org/apache/qpid/management/common/mbeans/UserManagement.java +++ b/qpid/java/management/common/src/main/java/org/apache/qpid/management/common/mbeans/UserManagement.java @@ -38,15 +38,17 @@ public interface UserManagement //TabularType and contained CompositeType key/description information. //For compatibility reasons, DONT MODIFY the existing key values if expanding the set. String USERNAME = "Username"; - String RIGHTS_READ_ONLY = "read"; - String RIGHTS_READ_WRITE = "write"; - String RIGHTS_ADMIN = "admin"; + String RIGHTS_READ_ONLY = "read"; // item deprecated + String RIGHTS_READ_WRITE = "write"; // item deprecated + String RIGHTS_ADMIN = "admin"; // item deprecated + List<String> COMPOSITE_ITEM_NAMES = Collections.unmodifiableList(Arrays.asList(USERNAME, RIGHTS_READ_ONLY, RIGHTS_READ_WRITE, RIGHTS_ADMIN)); List<String> COMPOSITE_ITEM_DESCRIPTIONS = Collections.unmodifiableList( Arrays.asList("Broker Login username", - "Management Console Read Permission", - "Management Console Write Permission", - "Management Console Admin Permission")); + "Item no longer used", + "Item no longer used", + "Item no longer used")); + List<String> TABULAR_UNIQUE_INDEX = Collections.unmodifiableList(Arrays.asList(USERNAME)); //********** Operations *****************// @@ -57,7 +59,7 @@ public interface UserManagement * * @deprecated since Qpid JMX API 1.7 * - * @param username The username to create + * @param username The username for which the password is to be set * @param password The password for the user * * @return The result of the operation @@ -85,7 +87,11 @@ public interface UserManagement @MBeanOperationParameter(name = "password", description = "Password")String password); /** - * set rights for users with given details + * Set rights for users with given details. + * Since Qpid JMX API 2.3 all invocations will cause an exception to be thrown + * as access rights can no longer be maintain via this interface. + * + * @deprecated since Qpid JMX API 2.3 * * @param username The username to create * @param read The set of permission to give the new user @@ -94,6 +100,7 @@ public interface UserManagement * * @return The result of the operation */ + @Deprecated @MBeanOperation(name = "setRights", description = "Set access rights for user.", impact = MBeanOperationInfo.ACTION) boolean setRights(@MBeanOperationParameter(name = "username", description = "Username")String username, @@ -102,7 +109,9 @@ public interface UserManagement @MBeanOperationParameter(name = "admin", description = "Administration rights")boolean admin); /** - * Create users with given details + * Create users with given details. + * Since Qpid JMX API 2.3 if the user passes true for parameters read, write, or admin, a + * exception will be thrown as access rights can no longer be maintain via this interface. * * Since Qpid JMX API 1.2 this operation expects plain text passwords to be provided. Prior to this, MD5 hashed passwords were supplied. * @@ -114,7 +123,7 @@ public interface UserManagement * @param write The set of permission to give the new user * @param admin The set of permission to give the new user * - * @return The result of the operation + * @return true if the user was created successfully, or false otherwise */ @Deprecated @MBeanOperation(name = "createUser", description = "Create new user from system.", @@ -128,7 +137,10 @@ public interface UserManagement /** * Create users with given details. + * Since Qpid JMX API 2.3 if the user passes true for parameters read, write, or admin, a + * exception will be thrown as access rights can no longer be maintain via this interface. * + * @deprecated since Qpid JMX API 2.3 * @since Qpid JMX API 1.7 * * @param username The username to create @@ -137,8 +149,9 @@ public interface UserManagement * @param write The set of permission to give the new user * @param admin The set of permission to give the new user * - * @return The result of the operation + * @return true if the user was created successfully, or false otherwise */ + @Deprecated @MBeanOperation(name = "createUser", description = "Create a new user.", impact = MBeanOperationInfo.ACTION) boolean createUser(@MBeanOperationParameter(name = "username", description = "Username")String username, @@ -146,6 +159,21 @@ public interface UserManagement @MBeanOperationParameter(name = "read", description = "Administration read")boolean read, @MBeanOperationParameter(name = "readAndWrite", description = "Administration write")boolean write, @MBeanOperationParameter(name = "admin", description = "Administration rights")boolean admin); + + /** + * Create users with given details. + * + * @since Qpid JMX API 2.3 + * + * @param username The username to create + * @param password The password for the user + * + * @return true if the user was created successfully, or false otherwise + */ + @MBeanOperation(name = "createUser", description = "Create a new user.", + impact = MBeanOperationInfo.ACTION) + boolean createUser(@MBeanOperationParameter(name = "username", description = "Username")String username, + @MBeanOperationParameter(name = "password", description = "Password")String password); /** * View users returns all the users that are currently available to the system. @@ -172,10 +200,13 @@ public interface UserManagement /** * View users returns all the users that are currently available to the system. + * + * Since Qpid JMX API 2.3 the items that corresponded to read, write and admin flags + * are deprecated and always return false. * * @return a table of users data (Username, read, write, admin) */ - @MBeanOperation(name = "viewUsers", description = "All users with access rights to the system.", + @MBeanOperation(name = "viewUsers", description = "All users that are currently available to the system.", impact = MBeanOperationInfo.INFO) TabularData viewUsers(); diff --git a/qpid/java/management/eclipse-plugin/src/main/java/org/apache/qpid/management/ui/ApplicationRegistry.java b/qpid/java/management/eclipse-plugin/src/main/java/org/apache/qpid/management/ui/ApplicationRegistry.java index b02289ee7c..4a59176374 100644 --- a/qpid/java/management/eclipse-plugin/src/main/java/org/apache/qpid/management/ui/ApplicationRegistry.java +++ b/qpid/java/management/eclipse-plugin/src/main/java/org/apache/qpid/management/ui/ApplicationRegistry.java @@ -47,7 +47,7 @@ public abstract class ApplicationRegistry //max supported broker management interface supported by this release of the management console public static final int SUPPORTED_QPID_JMX_API_MAJOR_VERSION = 2; - public static final int SUPPORTED_QPID_JMX_API_MINOR_VERSION = 2; + public static final int SUPPORTED_QPID_JMX_API_MINOR_VERSION = 3; public static final String DATA_DIR = System.getProperty("user.home") + File.separator + ".qpidmc"; diff --git a/qpid/java/systests/etc/config-systests-ServerConfigurationTest-New.xml b/qpid/java/systests/etc/config-systests-ServerConfigurationTest-New.xml index d1dce019dd..39805cbc48 100644 --- a/qpid/java/systests/etc/config-systests-ServerConfigurationTest-New.xml +++ b/qpid/java/systests/etc/config-systests-ServerConfigurationTest-New.xml @@ -59,7 +59,6 @@ </principal-databases> <jmx> - <access>${passwordDir}/jmxremote.access</access> <principal-database>passwordfile</principal-database> </jmx> </security> diff --git a/qpid/java/systests/etc/config-systests-ServerConfigurationTest-Old.xml b/qpid/java/systests/etc/config-systests-ServerConfigurationTest-Old.xml index 1de0389533..e87be87154 100644 --- a/qpid/java/systests/etc/config-systests-ServerConfigurationTest-Old.xml +++ b/qpid/java/systests/etc/config-systests-ServerConfigurationTest-Old.xml @@ -57,7 +57,6 @@ </principal-database> </principal-databases> <jmx> -<access>${passwordDir}/jmxremote.access</access> <principal-database>passwordfile</principal-database> </jmx> </security> diff --git a/qpid/java/systests/etc/config-systests-firewall-2.xml b/qpid/java/systests/etc/config-systests-firewall-2.xml index 38ff9ddbb0..05c3eaff9f 100644 --- a/qpid/java/systests/etc/config-systests-firewall-2.xml +++ b/qpid/java/systests/etc/config-systests-firewall-2.xml @@ -85,7 +85,6 @@ <msg-auth>false</msg-auth> <jmx> - <access>${conf}/jmxremote.access</access> <principal-database>passwordfile</principal-database> </jmx> diff --git a/qpid/java/systests/etc/config-systests-firewall-3.xml b/qpid/java/systests/etc/config-systests-firewall-3.xml index 6c0ee7ee3c..861a3b33a3 100644 --- a/qpid/java/systests/etc/config-systests-firewall-3.xml +++ b/qpid/java/systests/etc/config-systests-firewall-3.xml @@ -85,7 +85,6 @@ <msg-auth>false</msg-auth> <jmx> - <access>${conf}/jmxremote.access</access> <principal-database>passwordfile</principal-database> </jmx> |