Filter available authentication providers for FICAM setting

api/src/org/labkey/api/security/AuthenticationConfigurationCache.java

@@ -82,8 +82,6 @@ protected Set<AuthenticationConfiguration<?>> 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,29 +97,32 @@ 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<String> activeDomains = new ArrayList<>(_activeDomainMap.keySet());
             Collections.sort(activeDomains);
             _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<String, Object> 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<Class<? extends AuthenticationConfiguration>,
             return null != configurations ? configurations : Collections.emptyList();
         }
 
-        private @NotNull Collection<AuthenticationConfiguration> getActiveConfigurationsForDomain(String domain)
+        private @NotNull Collection<AuthenticationConfiguration<?>> 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<AuthenticationConfiguration> getActiveConfigurationsForDomain(String domain)
+    public static @NotNull Collection<AuthenticationConfiguration<?>> getActiveConfigurationsForDomain(String domain)
     {
         return CACHE.get(CACHE_KEY).getActiveConfigurationsForDomain(domain);
     }

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();
         }
     }

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<SAC extends SecondaryAuthenticationConfiguration<?>> extends ConfigurableAuthenticationProvider<SAC>
@@ -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

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<AuthenticationProvider> 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 <T extends AuthenticationProvider> Collection<T> get(Class<T> clazz)

api/src/org/labkey/api/security/SaveConfigurationAction.java

@@ -127,7 +127,8 @@ public static <F extends SaveConfigurationForm> 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<String, Object> getConfigurationMap(int rowId)
+    protected final Map<String, Object> getConfigurationMap(int rowId)
     {
         AC configuration = getFromCache(rowId);
+
+        if (null == configuration)
+        {
+            throw new NotFoundException("Unable to save configuration");
+        }
+
+        return getConfigurationMap(configuration);
+    }
+
+    protected Map<String, Object> getConfigurationMap(@NotNull AC configuration)
+    {
         return AuthenticationManager.getConfigurationMap(configuration);
     }
 }

api/src/org/labkey/api/security/SsoSaveConfigurationAction.java

@@ -120,9 +120,8 @@ public static void logLogoAction(User user, SSOAuthenticationConfiguration<?> co
     }
 
     @Override
-    protected Map<String, Object> getConfigurationMap(int rowId)
+    protected Map<String, Object> getConfigurationMap(@NotNull AC configuration)
     {
-        AC configuration = getFromCache(rowId);
         return AuthenticationManager.getSsoConfigurationMap(configuration);
     }
 

core/src/client/components/AuthRow.tsx

@@ -145,7 +145,15 @@ export default class AuthRow extends PureComponent<Props, Partial<State>> {
                     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?`}
             >
                 <div className="auth-row__delete-modal__textBox">