diff --git a/api/src/org/labkey/api/security/AuthenticationConfigurationCache.java b/api/src/org/labkey/api/security/AuthenticationConfigurationCache.java index 16f9895bab1..a1f153740f3 100644 --- a/api/src/org/labkey/api/security/AuthenticationConfigurationCache.java +++ b/api/src/org/labkey/api/security/AuthenticationConfigurationCache.java @@ -82,8 +82,6 @@ protected Set> createCollection() private AuthenticationConfigurationCollections() { - boolean acceptOnlyFicamProviders = AuthenticationManager.isAcceptOnlyFicamProviders(); - // 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. @@ -99,13 +97,13 @@ private AuthenticationConfigurationCollections() configs .map(this::getAuthenticationConfiguration) .filter(Objects::nonNull) - .filter(c->!acceptOnlyFicamProviders || c.getAuthenticationProvider().isFicamApproved()) .forEach(this::addConfiguration); // MultiValuedMap of domains to AuthenticationConfigurations that claim them _activeDomainMap = getActive(PrimaryAuthenticationConfiguration.class).stream() .filter(config -> null != config.getDomain()) .filter(config -> !AuthenticationManager.ALL_DOMAINS.equals(config.getDomain())) + .map(config -> (AuthenticationConfiguration) config) .collect(LabKeyCollectors.toMultiValuedMap(AuthenticationConfiguration::getDomain, config -> config)); List activeDomains = new ArrayList<>(_activeDomainMap.keySet()); @@ -113,15 +111,18 @@ private AuthenticationConfigurationCollections() _activeDomains = Collections.unmodifiableCollection(activeDomains); } - // Little helper method simplifies the stream handling above + // Little helper method simplifies the stream handling above. Filters out configurations based on FICAM-only setting. private @Nullable AuthenticationConfiguration getAuthenticationConfiguration(Map map) { String providerName = (String)map.get("Provider"); AuthenticationProvider provider = AuthenticationProviderCache.getProvider(AuthenticationProvider.class, providerName); if (null == provider) { - String description = (String)map.get("Description"); - LOG.warn("A saved authentication configuration requires the \"{}\" authentication provider, but that provider is not present in this deployment. Authentication via {} will not be available.", providerName, null != description ? "\"" + description + "\"" : "this mechanism"); + if (!AuthenticationManager.isAcceptOnlyFicamProviders()) // Don't warn if FICAM-only is checked + { + String description = (String)map.get("Description"); + LOG.warn("A saved authentication configuration requires the \"{}\" authentication provider, but that provider is not present in this deployment. Authentication via {} will not be available.", providerName, null != description ? "\"" + description + "\"" : "this mechanism"); + } return null; } @@ -162,7 +163,7 @@ private void addToMap(SetValuedMap, return null != configurations ? configurations : Collections.emptyList(); } - private @NotNull Collection getActiveConfigurationsForDomain(String domain) + private @NotNull Collection> getActiveConfigurationsForDomain(String domain) { return new ArrayList<>(_activeDomainMap.get(domain)); } @@ -233,7 +234,7 @@ public static void clear() /** * Return a collection of authentication configurations that claim the specified domain */ - public static @NotNull Collection getActiveConfigurationsForDomain(String domain) + public static @NotNull Collection> getActiveConfigurationsForDomain(String domain) { return CACHE.get(CACHE_KEY).getActiveConfigurationsForDomain(domain); } diff --git a/api/src/org/labkey/api/security/AuthenticationManager.java b/api/src/org/labkey/api/security/AuthenticationManager.java index 9f35e3b36a1..f35db4daff3 100644 --- a/api/src/org/labkey/api/security/AuthenticationManager.java +++ b/api/src/org/labkey/api/security/AuthenticationManager.java @@ -662,6 +662,12 @@ public static void deleteConfiguration(User user, int rowId) { // Delete any logos attached to the configuration AuthenticationConfiguration configuration = AuthenticationConfigurationCache.getConfiguration(AuthenticationConfiguration.class, rowId); + + if (null == configuration) + { + throw new NotFoundException("Unable to delete authentication configuration"); + } + AttachmentService.get().deleteAttachments(configuration); // Delete configuration @@ -746,6 +752,7 @@ public static void setAcceptOnlyFicamProviders(User user, boolean enable) if (isAcceptOnlyFicamProviders() != enable) { saveAuthSetting(user, ACCEPT_ONLY_FICAM_PROVIDERS_KEY, enable); + AuthenticationProviderCache.clear(); AuthenticationConfigurationCache.clear(); } } diff --git a/api/src/org/labkey/api/security/AuthenticationProvider.java b/api/src/org/labkey/api/security/AuthenticationProvider.java index b293a5ddf02..07df797aaef 100644 --- a/api/src/org/labkey/api/security/AuthenticationProvider.java +++ b/api/src/org/labkey/api/security/AuthenticationProvider.java @@ -234,6 +234,12 @@ interface ResetPasswordProvider extends AuthenticationProvider * @param isAdminCopy true for sending admin a copy of reset password email */ @Nullable SecurityMessage getAPIResetPasswordMessage(User user, boolean isAdminCopy); + + @Override + default boolean isFicamApproved() + { + return true; + } } interface SecondaryAuthenticationProvider> extends ConfigurableAuthenticationProvider @@ -286,11 +292,23 @@ interface DisableLoginProvider extends AuthenticationProvider void addUserDelay(HttpServletRequest request, String id, int addCount); void resetUserDelay(String id); + + @Override + default boolean isFicamApproved() + { + return true; + } } interface ExpireAccountProvider extends AuthenticationProvider { boolean isEnabled(); + + @Override + default boolean isFicamApproved() + { + return true; + } } class AuthenticationResponse diff --git a/api/src/org/labkey/api/security/AuthenticationProviderCache.java b/api/src/org/labkey/api/security/AuthenticationProviderCache.java index 974898bd7eb..fac6f6ed008 100644 --- a/api/src/org/labkey/api/security/AuthenticationProviderCache.java +++ b/api/src/org/labkey/api/security/AuthenticationProviderCache.java @@ -27,9 +27,6 @@ import java.util.LinkedHashSet; import java.util.Set; -/** - * Created by adam on 5/20/2016. - */ public class AuthenticationProviderCache { // We have just a single object to cache (a global AuthenticationProviderCollection), but use standard cache (blocking cache wrapping the @@ -56,13 +53,15 @@ protected Set createCollection() private AuthenticationProviderCollections() { - for (AuthenticationProvider provider : AuthenticationManager.getAllProviders()) - { - AuthenticationProvider.ALL_PROVIDER_INTERFACES + boolean acceptOnlyFicamProviders = AuthenticationManager.isAcceptOnlyFicamProviders(); + + AuthenticationManager.getAllProviders().stream() + .filter(provider -> !acceptOnlyFicamProviders || provider.isFicamApproved()) + .forEach(provider -> AuthenticationProvider.ALL_PROVIDER_INTERFACES .stream() .filter(providerClass -> providerClass.isInstance(provider)) - .forEach(providerClass -> _map.put(providerClass, provider)); - } + .forEach(providerClass -> _map.put(providerClass, provider)) + ); } private Collection get(Class clazz) diff --git a/api/src/org/labkey/api/security/SaveConfigurationAction.java b/api/src/org/labkey/api/security/SaveConfigurationAction.java index 3f14a00bcef..0d0c7f55b66 100644 --- a/api/src/org/labkey/api/security/SaveConfigurationAction.java +++ b/api/src/org/labkey/api/security/SaveConfigurationAction.java @@ -127,7 +127,8 @@ public static AuthenticationConfiguration s return StringUtilsLabKey.getMapDifference( null != oldConfiguration ? oldConfiguration.getLoggingProperties() : null, null != newConfiguration ? newConfiguration.getLoggingProperties() : null, - 50); + 50 + ); } protected AC getFromCache(int rowId) @@ -136,9 +137,20 @@ protected AC getFromCache(int rowId) return (AC)AuthenticationConfigurationCache.getConfiguration(AuthenticationConfiguration.class, rowId); } - protected Map getConfigurationMap(int rowId) + protected final Map getConfigurationMap(int rowId) { AC configuration = getFromCache(rowId); + + if (null == configuration) + { + throw new NotFoundException("Unable to save configuration"); + } + + return getConfigurationMap(configuration); + } + + protected Map getConfigurationMap(@NotNull AC configuration) + { return AuthenticationManager.getConfigurationMap(configuration); } } diff --git a/api/src/org/labkey/api/security/SsoSaveConfigurationAction.java b/api/src/org/labkey/api/security/SsoSaveConfigurationAction.java index cd3a798bedf..c3b5c27f941 100644 --- a/api/src/org/labkey/api/security/SsoSaveConfigurationAction.java +++ b/api/src/org/labkey/api/security/SsoSaveConfigurationAction.java @@ -120,9 +120,8 @@ public static void logLogoAction(User user, SSOAuthenticationConfiguration co } @Override - protected Map getConfigurationMap(int rowId) + protected Map getConfigurationMap(@NotNull AC configuration) { - AC configuration = getFromCache(rowId); return AuthenticationManager.getSsoConfigurationMap(configuration); } diff --git a/core/src/client/components/AuthRow.tsx b/core/src/client/components/AuthRow.tsx index 1362a830f22..6f225c1fe50 100644 --- a/core/src/client/components/AuthRow.tsx +++ b/core/src/client/components/AuthRow.tsx @@ -145,7 +145,15 @@ export default class AuthRow extends PureComponent> { this.onToggleModal('deleteModalOpen', this.state.deleteModalOpen); toggleModalOpen(false); }} - onConfirm={onDelete} + onConfirm={() => { + // Close the confirmation modal as soon as the user confirms, regardless of whether the + // delete succeeds. On success the row unmounts anyway; on failure (e.g. the configuration + // was already removed, or FICAM-only mode was enabled in another tab and the provider is no + // longer available) the error is surfaced in the main window and the modal should not linger. + this.onToggleModal('deleteModalOpen', this.state.deleteModalOpen); + toggleModalOpen(false); + onDelete?.(); + }} title={`Permanently delete ${authConfig.provider} configuration?`} >