From 9d76d0588dd7d4eb12e1191368e1ca725da95b2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Andr=C3=A9=20Pearce?= Date: Tue, 9 Oct 2018 00:35:52 +0100 Subject: [PATCH] ARTEMIS-2094 Capture SecurityRepository changes to server config This fixes SecurityFailoverTest and any other test that only updates security repository and not the config. --- .../core/server/impl/ActiveMQServerImpl.java | 43 +++++++++++++++---- .../core/settings/HierarchicalRepository.java | 9 ++++ .../impl/HierarchicalObjectRepository.java | 25 +++++++++++ 3 files changed, 69 insertions(+), 8 deletions(-) diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java index a5591d06b3d..f315df20ef7 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java @@ -164,6 +164,7 @@ import org.apache.activemq.artemis.core.server.reload.ReloadManagerImpl; import org.apache.activemq.artemis.core.server.transformer.Transformer; import org.apache.activemq.artemis.core.settings.HierarchicalRepository; +import org.apache.activemq.artemis.core.settings.HierarchicalRepositoryChangeListener; import org.apache.activemq.artemis.core.settings.impl.AddressFullMessagePolicy; import org.apache.activemq.artemis.core.settings.impl.AddressSettings; import org.apache.activemq.artemis.core.settings.impl.DeletionPolicy; @@ -257,6 +258,8 @@ public class ActiveMQServerImpl implements ActiveMQServer { private final HierarchicalRepository> securityRepository; + private final SecurityRepositoryChangeListener securityRepositoryChangeListener = new SecurityRepositoryChangeListener(); + private volatile ResourceManager resourceManager; private volatile ActiveMQServerControlImpl messagingServerControl; @@ -421,7 +424,8 @@ public ActiveMQServerImpl(Configuration configuration, addressSettingsRepository.setDefault(new AddressSettings()); securityRepository = new HierarchicalObjectRepository<>(configuration.getWildcardConfiguration()); - + securityRepository.registerListener(securityRepositoryChangeListener); + securityRepositoryChangeListener.enable(); securityRepository.setDefault(new HashSet()); this.parentServer = parentServer; @@ -2672,9 +2676,8 @@ public void completeActivation() throws Exception { } private void deploySecurityFromConfiguration() { - for (Map.Entry> entry : configuration.getSecurityRoles().entrySet()) { - securityRepository.addMatch(entry.getKey(), entry.getValue(), true); - } + securityRepository.addMatches(configuration.getSecurityRoles(), true); + for (SecuritySettingPlugin securitySettingPlugin : configuration.getSecuritySettingPlugins()) { securitySettingPlugin.setSecurityRepository(securityRepository); @@ -2891,12 +2894,17 @@ private void recoverStoredConfigs() throws Exception { } List roles = storageManager.recoverPersistedRoles(); + if (!roles.isEmpty()) { + Map> matches = new HashMap<>(roles.size()); + for (PersistedRoles roleItem : roles) { + Set setRoles = SecurityFormatter.createSecurity(roleItem.getSendRoles(), roleItem.getConsumeRoles(), roleItem.getCreateDurableQueueRoles(), roleItem.getDeleteDurableQueueRoles(), roleItem.getCreateNonDurableQueueRoles(), roleItem.getDeleteNonDurableQueueRoles(), roleItem.getManageRoles(), roleItem.getBrowseRoles(), roleItem.getCreateAddressRoles(), roleItem.getDeleteAddressRoles()); - for (PersistedRoles roleItem : roles) { - Set setRoles = SecurityFormatter.createSecurity(roleItem.getSendRoles(), roleItem.getConsumeRoles(), roleItem.getCreateDurableQueueRoles(), roleItem.getDeleteDurableQueueRoles(), roleItem.getCreateNonDurableQueueRoles(), roleItem.getDeleteNonDurableQueueRoles(), roleItem.getManageRoles(), roleItem.getBrowseRoles(), roleItem.getCreateAddressRoles(), roleItem.getDeleteAddressRoles()); - - securityRepository.addMatch(roleItem.getAddressMatch().toString(), setRoles); + matches.put(roleItem.getAddressMatch().toString(), setRoles); + } + securityRepository.addMatches(matches, false); } + + } @Override @@ -3517,4 +3525,23 @@ public List getExternalComponents() { return externalComponents; } + private class SecurityRepositoryChangeListener implements HierarchicalRepositoryChangeListener { + + private volatile boolean enabled; + + @Override + public void onChange() { + if (enabled) { + configuration.setSecurityRoles(securityRepository.map()); + } + } + + public synchronized void enable() { + enabled = true; + } + + public synchronized void disable() { + enabled = false; + } + } } diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/settings/HierarchicalRepository.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/settings/HierarchicalRepository.java index cb2054a9226..e8ef08ceb29 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/settings/HierarchicalRepository.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/settings/HierarchicalRepository.java @@ -55,6 +55,13 @@ public interface HierarchicalRepository { */ List values(); + /** + * Return a map of match and values being added + * + * @return + */ + Map map(); + /** * set the default value to fallback to if none found * @@ -100,5 +107,7 @@ public interface HierarchicalRepository { */ void clearCache(); + void addMatches(Map matches, boolean immutableMatch); + int getCacheSize(); } diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/settings/impl/HierarchicalObjectRepository.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/settings/impl/HierarchicalObjectRepository.java index e83b68570df..32e5e7044c7 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/settings/impl/HierarchicalObjectRepository.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/settings/impl/HierarchicalObjectRepository.java @@ -150,6 +150,22 @@ public List values() { } } + @Override + public Map map() { + lock.readLock().lock(); + try { + Map values = new HashMap<>(matches.size()); + + for (Entry> entry : matches.entrySet()) { + values.put(entry.getKey(), entry.getValue().getValue()); + } + + return values; + } finally { + lock.readLock().unlock(); + } + } + /** * Add a new match to the repository * @@ -161,6 +177,15 @@ public void addMatch(final String match, final T value, final boolean immutableM addMatch(match, value, immutableMatch, true); } + @Override + public void addMatches(final Map matches, final boolean immutableMatch) { + for (Map.Entry entry : matches.entrySet()) { + addMatch(entry.getKey(), entry.getValue(), immutableMatch, false); + } + + onChange(); + } + private void addMatch(final String match, final T value, final boolean immutableMatch, boolean notifyListeners) { lock.writeLock().lock(); try {