From b4ddcca5c43e19546e095b6fad766ddc96bb05ea Mon Sep 17 00:00:00 2001 From: Alex Rudyy Date: Fri, 22 Mar 2013 10:15:35 +0000 Subject: QPID-4661: Improve broker attribute editing UI to avoid sending of unmodified attributes, improve UI look-&-feel, improve broker attribute changing functionality to avoid unnecessary modifications git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk/qpid@1459695 13f79535-47bb-0310-9956-ffa450edef68 --- .../startup/AuthenticationProviderRecoverer.java | 2 +- .../configuration/startup/BrokerRecoverer.java | 7 -- .../qpid/server/model/AuthenticationProvider.java | 2 - .../adapter/AuthenticationProviderAdapter.java | 15 +--- .../adapter/AuthenticationProviderFactory.java | 5 +- .../qpid/server/model/adapter/BrokerAdapter.java | 87 +++++++++++++++++++--- .../security/group/GroupPrincipalAccessor.java | 2 - .../configuration/startup/BrokerRecovererTest.java | 4 - .../adapter/AuthenticationProviderFactoryTest.java | 2 +- 9 files changed, 81 insertions(+), 45 deletions(-) (limited to 'java/broker') diff --git a/java/broker/src/main/java/org/apache/qpid/server/configuration/startup/AuthenticationProviderRecoverer.java b/java/broker/src/main/java/org/apache/qpid/server/configuration/startup/AuthenticationProviderRecoverer.java index 9b06a2b499..3290c827c9 100644 --- a/java/broker/src/main/java/org/apache/qpid/server/configuration/startup/AuthenticationProviderRecoverer.java +++ b/java/broker/src/main/java/org/apache/qpid/server/configuration/startup/AuthenticationProviderRecoverer.java @@ -47,7 +47,7 @@ public class AuthenticationProviderRecoverer implements ConfiguredObjectRecovere AuthenticationProvider authenticationProvider = _authenticationProviderFactory.create( configurationEntry.getId(), broker, - attributes, null); + attributes); return authenticationProvider; } diff --git a/java/broker/src/main/java/org/apache/qpid/server/configuration/startup/BrokerRecoverer.java b/java/broker/src/main/java/org/apache/qpid/server/configuration/startup/BrokerRecoverer.java index b76717802c..0d7be75a0b 100644 --- a/java/broker/src/main/java/org/apache/qpid/server/configuration/startup/BrokerRecoverer.java +++ b/java/broker/src/main/java/org/apache/qpid/server/configuration/startup/BrokerRecoverer.java @@ -18,7 +18,6 @@ import org.apache.qpid.server.model.adapter.BrokerAdapter; import org.apache.qpid.server.model.adapter.PortFactory; import org.apache.qpid.server.configuration.store.StoreConfigurationChangeListener; import org.apache.qpid.server.configuration.updater.TaskExecutor; -import org.apache.qpid.server.security.group.GroupPrincipalAccessor; import org.apache.qpid.server.stats.StatisticsGatherer; import org.apache.qpid.server.virtualhost.VirtualHostRegistry; @@ -103,12 +102,6 @@ public class BrokerRecoverer implements ConfiguredObjectRecoverer } broker.setDefaultAuthenticationProvider(defaultAuthenticationProvider); - GroupPrincipalAccessor groupPrincipalAccessor = new GroupPrincipalAccessor(broker.getGroupProviders()); - for (AuthenticationProvider authenticationProvider : authenticationProviders) - { - authenticationProvider.setGroupAccessor(groupPrincipalAccessor); - } - Collection ports = broker.getPorts(); for (Port port : ports) { diff --git a/java/broker/src/main/java/org/apache/qpid/server/model/AuthenticationProvider.java b/java/broker/src/main/java/org/apache/qpid/server/model/AuthenticationProvider.java index 2e5c3a0cc7..61ceae9cf0 100644 --- a/java/broker/src/main/java/org/apache/qpid/server/model/AuthenticationProvider.java +++ b/java/broker/src/main/java/org/apache/qpid/server/model/AuthenticationProvider.java @@ -25,7 +25,6 @@ import java.util.Collection; import java.util.Collections; import org.apache.qpid.server.security.SubjectCreator; -import org.apache.qpid.server.security.group.GroupPrincipalAccessor; public interface AuthenticationProvider extends ConfiguredObject { @@ -67,5 +66,4 @@ public interface AuthenticationProvider extends ConfiguredObject */ SubjectCreator getSubjectCreator(); - void setGroupAccessor(GroupPrincipalAccessor groupPrincipalAccessor); } diff --git a/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AuthenticationProviderAdapter.java b/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AuthenticationProviderAdapter.java index a6c5a74765..a7204d991c 100644 --- a/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AuthenticationProviderAdapter.java +++ b/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AuthenticationProviderAdapter.java @@ -36,6 +36,7 @@ import javax.security.auth.login.AccountNotFoundException; import org.apache.log4j.Logger; import org.apache.qpid.server.model.AuthenticationProvider; import org.apache.qpid.server.model.Broker; +import org.apache.qpid.server.model.ConfigurationChangeListener; import org.apache.qpid.server.model.ConfiguredObject; import org.apache.qpid.server.model.IllegalStateTransitionException; import org.apache.qpid.server.model.IntegrityViolationException; @@ -67,8 +68,6 @@ public abstract class AuthenticationProviderAdapter _supportedAttributes; Map _factories; @@ -238,10 +237,6 @@ public abstract class AuthenticationProviderAdapter * The configured {@link AuthenticationManagerFactory}'s are used to try to create the {@link AuthenticationProvider}. * The first non-null instance is returned. The factories are used in non-deterministic order. - * @param groupPrincipalAccessor TODO */ - public AuthenticationProvider create(UUID id, Broker broker, Map attributes, GroupPrincipalAccessor groupPrincipalAccessor) + public AuthenticationProvider create(UUID id, Broker broker, Map attributes) { for (AuthenticationManagerFactory factory : _factories) { @@ -77,7 +75,6 @@ public class AuthenticationProviderFactory { authenticationProvider = new SimpleAuthenticationProviderAdapter(id, broker, manager, attributes, factory.getAttributeNames()); } - authenticationProvider.setGroupAccessor(groupPrincipalAccessor); return authenticationProvider; } } diff --git a/java/broker/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java b/java/broker/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java index fd275b23bd..7e4282f4ee 100644 --- a/java/broker/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java +++ b/java/broker/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java @@ -63,7 +63,6 @@ import org.apache.qpid.server.security.auth.manager.Base64MD5PasswordFileAuthent import org.apache.qpid.server.security.auth.manager.PlainPasswordFileAuthenticationManagerFactory; import org.apache.qpid.server.security.group.FileGroupManager; import org.apache.qpid.server.security.group.GroupManager; -import org.apache.qpid.server.security.group.GroupPrincipalAccessor; import org.apache.qpid.server.security.SecurityManager; import org.apache.qpid.server.security.SubjectCreator; import org.apache.qpid.server.stats.StatisticsGatherer; @@ -208,6 +207,14 @@ public class BrokerAdapter extends AbstractAdapter implements Broker, Configurat * A temporary method to create broker children that can be only configured via broker attributes */ private void createBrokerChildrenFromAttributes() + { + createGroupProvider(); + createKeyStore(); + createTrustStore(); + createPeerStore(); + } + + private void createGroupProvider() { String groupFile = (String) getAttribute(GROUP_FILE); if (groupFile != null) @@ -222,6 +229,10 @@ public class BrokerAdapter extends AbstractAdapter implements Broker, Configurat { _groupProviders.remove(DEFAULT_GROUP_PROFIDER_NAME); } + } + + private void createKeyStore() + { Map actualAttributes = getActualAttributes(); String keyStorePath = (String) getAttribute(KEY_STORE_PATH); if (keyStorePath != null) @@ -240,6 +251,11 @@ public class BrokerAdapter extends AbstractAdapter implements Broker, Configurat { _keyStores.remove(_defaultKeyStoreId); } + } + + private void createTrustStore() + { + Map actualAttributes = getActualAttributes(); String trustStorePath = (String) getAttribute(TRUST_STORE_PATH); if (trustStorePath != null) { @@ -257,6 +273,11 @@ public class BrokerAdapter extends AbstractAdapter implements Broker, Configurat { _trustStores.remove(_defaultTrustStoreId); } + } + + private void createPeerStore() + { + Map actualAttributes = getActualAttributes(); String peerStorePath = (String) getAttribute(PEER_STORE_PATH); UUID peerStoreId = UUIDGenerator.generateBrokerChildUUID(TrustStore.class.getSimpleName(), DEFAULT_PEER_STORE_NAME); if (peerStorePath != null) @@ -539,11 +560,7 @@ public class BrokerAdapter extends AbstractAdapter implements Broker, Configurat } - // it's cheap to create the groupPrincipalAccessor on the fly - GroupPrincipalAccessor groupPrincipalAccessor = new GroupPrincipalAccessor(_groupProviders.values()); - - authenticationProvider = _authenticationProviderFactory.create(UUID.randomUUID(), this, attributes, - groupPrincipalAccessor); + authenticationProvider = _authenticationProviderFactory.create(UUID.randomUUID(), this, attributes); addAuthenticationProvider(authenticationProvider); } authenticationProvider.setDesiredState(State.INITIALISING, State.ACTIVE); @@ -1033,15 +1050,63 @@ public class BrokerAdapter extends AbstractAdapter implements Broker, Configurat //TODO: Add management mode check Map convertedAttributes = MapValueConverter.convert(attributes, ATTRIBUTE_TYPES); validateAttributes(convertedAttributes); - super.changeAttributes(convertedAttributes); + + boolean keyStoreChanged = false; + boolean trustStoreChanged = false; + boolean peerStoreChanged = false; + Collection names = AVAILABLE_ATTRIBUTES; + for (String name : names) + { + if (attributes.containsKey(name)) + { + Object desired = attributes.get(name); + Object expected = getAttribute(name); + if (changeAttribute(name, expected, desired)) + { + if (GROUP_FILE.equals(name)) + { + createGroupProvider(); + } + else if (DEFAULT_AUTHENTICATION_PROVIDER.equals(name)) + { + if (!_defaultAuthenticationProvider.getName().equals(desired)) + { + _defaultAuthenticationProvider = findAuthenticationProviderByName((String)desired); + } + } + else if (KEY_STORE_PATH.equals(name) || KEY_STORE_PASSWORD.equals(name) || KEY_STORE_CERT_ALIAS.equals(name)) + { + keyStoreChanged = true; + } + else if (TRUST_STORE_PATH.equals(name) || TRUST_STORE_PASSWORD.equals(name)) + { + trustStoreChanged = true; + } + else if (PEER_STORE_PATH.equals(name) || PEER_STORE_PASSWORD.equals(name)) + { + peerStoreChanged = true; + } + attributeSet(name, expected, desired); + } + } + } // the calls below are not thread safe but they should be fine in a management mode // as there will be no user connected - createBrokerChildrenFromAttributes(); - String defaultProviderName = (String)getAttribute(DEFAULT_AUTHENTICATION_PROVIDER); - if (!_defaultAuthenticationProvider.getName().equals(defaultProviderName)) + // The new keystore/trustore/peerstore will be only used with new ports + // At the moment we cannot restart ports with new keystore/trustore/peerstore + + if (keyStoreChanged) + { + createKeyStore(); + } + if (trustStoreChanged) + { + createTrustStore(); + } + if (peerStoreChanged) { - _defaultAuthenticationProvider = findAuthenticationProviderByName(defaultProviderName); + createPeerStore(); } } diff --git a/java/broker/src/main/java/org/apache/qpid/server/security/group/GroupPrincipalAccessor.java b/java/broker/src/main/java/org/apache/qpid/server/security/group/GroupPrincipalAccessor.java index d549b76aab..1b8cdc91bc 100644 --- a/java/broker/src/main/java/org/apache/qpid/server/security/group/GroupPrincipalAccessor.java +++ b/java/broker/src/main/java/org/apache/qpid/server/security/group/GroupPrincipalAccessor.java @@ -25,8 +25,6 @@ import java.util.HashSet; import java.util.Set; import org.apache.qpid.server.model.GroupProvider; -import org.apache.qpid.server.model.adapter.GroupProviderAdapter; - public class GroupPrincipalAccessor { diff --git a/java/broker/src/test/java/org/apache/qpid/server/configuration/startup/BrokerRecovererTest.java b/java/broker/src/test/java/org/apache/qpid/server/configuration/startup/BrokerRecovererTest.java index 8e0bfdaeb7..79dcf0cac4 100644 --- a/java/broker/src/test/java/org/apache/qpid/server/configuration/startup/BrokerRecovererTest.java +++ b/java/broker/src/test/java/org/apache/qpid/server/configuration/startup/BrokerRecovererTest.java @@ -53,7 +53,6 @@ import org.apache.qpid.server.model.VirtualHost; import org.apache.qpid.server.model.adapter.AuthenticationProviderFactory; import org.apache.qpid.server.model.adapter.PortFactory; import org.apache.qpid.server.configuration.updater.TaskExecutor; -import org.apache.qpid.server.security.group.GroupPrincipalAccessor; import org.apache.qpid.server.stats.StatisticsGatherer; import org.apache.qpid.server.virtualhost.VirtualHostRegistry; @@ -300,9 +299,6 @@ public class BrokerRecovererTest extends TestCase assertNotNull(broker); assertEquals("Unexpected number of authentication providers", 2, broker.getAuthenticationProviders().size()); - //verify that a GroupAcessor was added to the AuthenticationProviders - verify(_authenticationProvider1).setGroupAccessor(any(GroupPrincipalAccessor.class)); - verify(authenticationProvider2).setGroupAccessor(any(GroupPrincipalAccessor.class)); } public void testCreateBrokerWithGroupProvider() diff --git a/java/broker/src/test/java/org/apache/qpid/server/model/adapter/AuthenticationProviderFactoryTest.java b/java/broker/src/test/java/org/apache/qpid/server/model/adapter/AuthenticationProviderFactoryTest.java index 585fecae83..9bf80bb87e 100644 --- a/java/broker/src/test/java/org/apache/qpid/server/model/adapter/AuthenticationProviderFactoryTest.java +++ b/java/broker/src/test/java/org/apache/qpid/server/model/adapter/AuthenticationProviderFactoryTest.java @@ -74,7 +74,7 @@ public class AuthenticationProviderFactoryTest extends TestCase when(authenticationManagerFactory.createInstance(attributes)).thenReturn(authenticationManager); AuthenticationProviderFactory providerFactory = new AuthenticationProviderFactory(authManagerFactoryServiceLoader); - AuthenticationProvider provider = providerFactory.create(id, broker, attributes, null); + AuthenticationProvider provider = providerFactory.create(id, broker, attributes); assertNotNull("Provider is not created", provider); assertEquals("Unexpected ID", id, provider.getId()); -- cgit v1.2.1