diff --git a/api/src/org/labkey/api/dataiterator/StatementDataIterator.java b/api/src/org/labkey/api/dataiterator/StatementDataIterator.java index be6fca826e9..313d1ca3d36 100644 --- a/api/src/org/labkey/api/dataiterator/StatementDataIterator.java +++ b/api/src/org/labkey/api/dataiterator/StatementDataIterator.java @@ -489,7 +489,7 @@ else if (_useAsynchronousExecute && _stmts.length > 1 && _txSize==-1) private void log(String message) { if (null != _log) - _log.debug(message); + _log.trace(message); } private void log(String message, Exception e) diff --git a/api/src/org/labkey/api/security/AuthenticationConfiguration.java b/api/src/org/labkey/api/security/AuthenticationConfiguration.java index 2efca4ba4b3..db8714664bf 100644 --- a/api/src/org/labkey/api/security/AuthenticationConfiguration.java +++ b/api/src/org/labkey/api/security/AuthenticationConfiguration.java @@ -32,7 +32,6 @@ public interface AuthenticationConfiguration int getRowId(); @NotNull String getDescription(); - int getSortOrder(); default @Nullable String getDetails() { return null; diff --git a/api/src/org/labkey/api/security/AuthenticationConfigurationCache.java b/api/src/org/labkey/api/security/AuthenticationConfigurationCache.java index cbb3699591b..8be116822bf 100644 --- a/api/src/org/labkey/api/security/AuthenticationConfigurationCache.java +++ b/api/src/org/labkey/api/security/AuthenticationConfigurationCache.java @@ -2,28 +2,31 @@ import org.apache.commons.collections4.SetValuedMap; import org.apache.commons.collections4.multimap.AbstractSetValuedMap; +import org.apache.log4j.LogManager; +import org.apache.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.labkey.api.cache.BlockingCache; import org.labkey.api.cache.CacheManager; import org.labkey.api.data.CoreSchema; +import org.labkey.api.data.Sort; import org.labkey.api.data.TableSelector; import org.labkey.api.security.AuthenticationConfiguration.PrimaryAuthenticationConfiguration; -import org.labkey.api.security.AuthenticationProvider.PrimaryAuthenticationProvider; import java.util.Collection; import java.util.Collections; -import java.util.Comparator; import java.util.LinkedHashMap; import java.util.LinkedHashSet; -import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; +import java.util.stream.Stream; public class AuthenticationConfigurationCache { + private static final Logger LOG = LogManager.getLogger(AuthenticationConfigurationCache.class); + // We have just a single object to cache (a global AuthenticationConfigurationCollections), but use standard cache (blocking cache wrapping the // shared cache) for convenience and to ensure that configuration changes will get propagated once multiple application servers are supported. private static final BlockingCache CACHE = new BlockingCache<>(CacheManager.getSharedCache(), (key, argument) -> new AuthenticationConfigurationCollections()); @@ -61,29 +64,23 @@ private AuthenticationConfigurationCollections() { boolean acceptOnlyFicamProviders = AuthenticationManager.isAcceptOnlyFicamProviders(); - // Select all the configurations listed in the core.AuthenticationConfigurations table and group by provider. - // We group them so we can make a single call to each AuthenticationProvider to convert all of its maps into - // AuthenticationConfigurations in a single operation. This allows the provider to definitively record current - // state, for example, the LDAP provider can stash all the email domains tied to LDAP authentication, to tailor - // messages on administration pages. - Map>> configurationMap = - new TableSelector(CoreSchema.getInstance().getTableInfoAuthenticationConfigurations()) // Don't bother sorting since we're grouping by provider - .mapStream() - .filter(m->{ - AuthenticationProvider provider = AuthenticationProviderCache.getProvider(AuthenticationProvider.class, (String)m.get("Provider")); - return (null != provider && (!acceptOnlyFicamProviders || provider.isFicamApproved())); - }) - .collect(Collectors.groupingBy(this::getAuthenticationConfigurationFactory)); - - // Add each group of configurations - addConfigurations(configurationMap); - - // Gather and add all the "permanent" configurations -- this should be just a single configuration for Database authentication - Map>> permanentMap = AuthenticationManager.getAllPrimaryProviders().stream() - .filter(AuthenticationProvider::isPermanent) - .collect(Collectors.toMap(p->p, p->Collections.emptyList())); - - addConfigurations(permanentMap); + // Select the configurations stored in the core.AuthenticationConfigurations table, add the database + // authentication configuration, map each to the appropriate AuthenticationConfiguration, and add to the maps. + + Stream> configs = Stream.concat( + new TableSelector(CoreSchema.getInstance().getTableInfoAuthenticationConfigurations(), null, new Sort("SortOrder, RowId")).mapStream(), + + // Gather the "permanent" configurations -- this should be just a single configuration for Database authentication + AuthenticationManager.getAllPrimaryProviders().stream() + .filter(AuthenticationProvider::isPermanent) + .map(p->Map.of("Provider", p.getName())) + ); + + configs + .map(this::getAuthenticationConfiguration) + .filter(Objects::nonNull) + .filter(c->!acceptOnlyFicamProviders || c.getAuthenticationProvider().isFicamApproved()) + .forEach(this::addConfiguration); _activeDomains = getActive(PrimaryAuthenticationConfiguration.class).stream() .map(AuthenticationConfiguration::getDomain) @@ -93,37 +90,21 @@ private AuthenticationConfigurationCollections() } // Little helper method simplifies the stream handling above - private @NotNull AuthenticationConfigurationFactory getAuthenticationConfigurationFactory(Map map) + private @Nullable AuthenticationConfiguration getAuthenticationConfiguration(Map map) { String providerName = (String)map.get("Provider"); AuthenticationProvider provider = AuthenticationProviderCache.getProvider(AuthenticationProvider.class, providerName); - if (provider instanceof AuthenticationConfigurationFactory) - return (AuthenticationConfigurationFactory)provider; - - throw new IllegalStateException("AuthenticationProvider does not implement AuthenticationConfigurationFactory: " + providerName); - } - - // Add all the configurations, one provider group at a time - private void addConfigurations(Map>> configurationMap) - { - configurationMap.entrySet().stream() - .map(e->getConfigurations(e.getKey(), e.getValue())) - .flatMap(Collection::stream) - .sorted(AUTHENTICATION_CONFIGURATION_COMPARATOR) - .forEach(this::addConfiguration); - } - - // Order by SortOrder & RowId - private static final Comparator AUTHENTICATION_CONFIGURATION_COMPARATOR = Comparator.comparingInt(AuthenticationConfiguration::getSortOrder).thenComparingInt(AuthenticationConfiguration::getRowId); + if (null == provider) + { + String description = (String)map.get("Description"); + LOG.warn("A saved authentication configuration requires the \"" + providerName + "\" authentication provider, but that provider is not present in this deployment. Authentication via " + (null != description ? "\"" + description + "\"" : "this mechanism") + " will not be available."); + return null; + } - // Translate a provider's maps into ConfigurationSettings and then ask the provider to convert these into AuthenticationConfigurations - private List getConfigurations(AuthenticationConfigurationFactory factory, List> list) - { - List settings = list.stream() - .map(ConfigurationSettings::new) - .collect(Collectors.toList()); + if (!(provider instanceof AuthenticationConfigurationFactory)) + throw new IllegalStateException("AuthenticationProvider does not implement AuthenticationConfigurationFactory: " + provider.getClass().getName()); - return factory.getAuthenticationConfigurations(settings); + return ((AuthenticationConfigurationFactory)provider).getAuthenticationConfiguration(new ConfigurationSettings(map)); } private void addConfiguration(AuthenticationConfiguration configuration) diff --git a/api/src/org/labkey/api/security/AuthenticationConfigurationFactory.java b/api/src/org/labkey/api/security/AuthenticationConfigurationFactory.java index 5f80459e79a..75cbe5743f8 100644 --- a/api/src/org/labkey/api/security/AuthenticationConfigurationFactory.java +++ b/api/src/org/labkey/api/security/AuthenticationConfigurationFactory.java @@ -2,22 +2,8 @@ import org.jetbrains.annotations.NotNull; -import java.util.List; -import java.util.stream.Collectors; - public interface AuthenticationConfigurationFactory> { - // Providers that need to do special batch-wide processing can override this method - default List getAuthenticationConfigurations(@NotNull List configurations) - { - return configurations.stream() - .map(this::getAuthenticationConfiguration) - .collect(Collectors.toList()); - } - - // Most providers need to override this method to translate a single ConfigurationSettings into an AuthenticationConfiguration - default AC getAuthenticationConfiguration(@NotNull ConfigurationSettings cs) - { - throw new IllegalStateException("Shouldn't invoke this method for " + getClass().getName()); - } + // Translate a single ConfigurationSettings into an AuthenticationConfiguration + AC getAuthenticationConfiguration(@NotNull ConfigurationSettings cs); } diff --git a/api/src/org/labkey/api/security/BaseAuthenticationConfiguration.java b/api/src/org/labkey/api/security/BaseAuthenticationConfiguration.java index b4ca0c2db10..94f0b7f4f89 100644 --- a/api/src/org/labkey/api/security/BaseAuthenticationConfiguration.java +++ b/api/src/org/labkey/api/security/BaseAuthenticationConfiguration.java @@ -14,7 +14,6 @@ public abstract class BaseAuthenticationConfiguration standardSettings) { @@ -23,7 +22,6 @@ public BaseAuthenticationConfiguration(AP provider, Map standard _entityId = (String)standardSettings.get("EntityId"); _description = (String)standardSettings.get("Description"); _enabled = (Boolean)standardSettings.get("Enabled"); - _sortOrder = (Integer)standardSettings.get("SortOrder"); } @Override @@ -69,12 +67,6 @@ public boolean isEnabled() return _enabled; } - @Override - public int getSortOrder() - { - return _sortOrder; - } - @Override public @NotNull Map getCustomProperties() { diff --git a/core/src/org/labkey/core/login/DbLoginAuthenticationProvider.java b/core/src/org/labkey/core/login/DbLoginAuthenticationProvider.java index f4e8c1aac3b..4d8c41bbacd 100644 --- a/core/src/org/labkey/core/login/DbLoginAuthenticationProvider.java +++ b/core/src/org/labkey/core/login/DbLoginAuthenticationProvider.java @@ -39,10 +39,8 @@ import javax.servlet.http.HttpServletRequest; import java.util.Collection; -import java.util.Collections; import java.util.EmptyStackException; import java.util.LinkedList; -import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; @@ -56,7 +54,7 @@ public class DbLoginAuthenticationProvider implements LoginFormAuthenticationProvider { @Override - public List getAuthenticationConfigurations(@NotNull List ignored) + public DbLoginConfiguration getAuthenticationConfiguration(@NotNull ConfigurationSettings ignored) { Map properties = Map.of( "RowId", 0, @@ -66,7 +64,7 @@ public List getAuthenticationConfigurations(@NotNull List< Map stringProperties = DbLoginManager.getProperties(); - return Collections.singletonList(new DbLoginConfiguration(this, stringProperties, properties)); + return new DbLoginConfiguration(this, stringProperties, properties); } @Override