[#225] Bind identity provider configs through the DS bind methods

.github/workflows/build.yml

@@ -60,6 +60,16 @@ jobs:
         env:
           MAVEN_OPTS: -Dhttps.protocols=TLSv1.2 -Dmaven.wagon.httpconnectionManager.ttlSeconds=120 -Dmaven.wagon.http.retryHandler.requestSentEnabled=true -Dmaven.wagon.http.retryHandler.count=10
         run: mvn --batch-mode --errors --update-snapshots verify --file pom.xml
+      - name: Check DS descriptors
+        if: runner.os != 'Windows'
+        # The identity provider references must be bound through their bind methods (#225). The descriptors
+        # only exist in the jars and no shipped sample configures an identity provider, so nothing else notices
+        # a @Reference moved back onto the field.
+        run: |
+          unzip -p openidm-identity-provider/target/openidm-identity-provider-*[0-9T].jar \
+              OSGI-INF/org.forgerock.openidm.identityProviders.xml | grep -q 'bind="bindIdentityProviderConfig"'
+          unzip -p openidm-authnfilter/target/openidm-authnfilter-*[0-9T].jar \
+              OSGI-INF/org.forgerock.openidm.authentication.xml | grep -q 'bind="bindIdentityProviderService"'
       - name: Test on Unix
         if: runner.os != 'Windows'
         run: |

openidm-authnfilter/src/main/java/org/forgerock/openidm/auth/AuthenticationService.java

@@ -12,7 +12,7 @@
  * information: "Portions copyright [year] [name of copyright owner]".
  *
  * Copyright 2013-2016 ForgeRock AS
- * Portions copyright 2024-2025 3A Systems LLC.
+ * Portions copyright 2024-2026 3A Systems LLC.
  */
 
 package org.forgerock.openidm.auth;
@@ -214,10 +214,10 @@
      * all the associated auth modules (OAUTH and OPENID_CONNECT) and remove the SOCIAL_PROVIDERS
      * authentication module in memory only so that it does not get initialized.
      */
-    private JsonValue amendedConfig;
+    private volatile JsonValue amendedConfig;
 
     /** The authenticators to delegate to.*/
-    private List<Authenticator> authenticators = new ArrayList<>();
+    private volatile List<Authenticator> authenticators = new ArrayList<>();
 
     // ----- Declarative Service Implementation
 
@@ -243,17 +243,28 @@
     @Reference(policy = ReferencePolicy.DYNAMIC, target="(service.pid=org.forgerock.openidm.auth.config)")
     private volatile AuthFilterWrapper authFilterWrapper;
 
-    @Reference(policy = ReferencePolicy.DYNAMIC, cardinality = ReferenceCardinality.OPTIONAL)
     private volatile IdentityProviderService identityProviderService;
 
-    void bindIdentityProviderService(IdentityProviderService identityProviderService) {
+    @Reference(
+            name = "identityProviderService",
+            policy = ReferencePolicy.DYNAMIC,
+            cardinality = ReferenceCardinality.OPTIONAL,
+            unbind = "unbindIdentityProviderService")
+    void bindIdentityProviderService(IdentityProviderService identityProviderService)
+            throws IdentityProviderServiceException {
         this.identityProviderService = identityProviderService;
         identityProviderService.registerIdentityProviderListener(this);
+        // no-op until activated; rebuilds the social auth modules if the service arrives later
+        identityProviderConfigChanged();
     }
 
-    void unbindIdentityProviderService() {
+    void unbindIdentityProviderService(IdentityProviderService identityProviderService)
+            throws IdentityProviderServiceException {
         identityProviderService.unregisterIdentityProviderListener(this);
-        identityProviderService = null;
+        if (this.identityProviderService == identityProviderService) {
+            this.identityProviderService = null;
+            identityProviderConfigChanged();
+        }
     }
 
     /** An on-demand Provider for the ConnectionFactory */
@@ -413,11 +424,30 @@
         if (identityProviderService != null) {
             authModuleConfig.asList().addAll(
                     FluentIterable.from(identityProviderService.getIdentityProviders())
+                            .filter(socialAuthModuleTypes)
                             .transform(new SocialAuthModuleConfigFactory(socialAuthTemplate))
                             .toList());
         }
     }
 
+    /**
+     * A {@link Predicate} that keeps the identity providers an auth module can be generated for, so that one
+     * provider with an unsupported type does not fail the whole authentication configuration.
+     */
+    private static final Predicate<ProviderConfig> socialAuthModuleTypes =
+            new Predicate<ProviderConfig>() {
+                @Override
+                public boolean apply(ProviderConfig providerConfig) {
+                    if (IDMAuthModule.OPENID_CONNECT.name().equals(providerConfig.getType())
+                            || IDMAuthModule.OAUTH.name().equals(providerConfig.getType())) {
+                        return true;
+                    }
+                    logger.warn("Identity provider {} has unsupported type {}, no auth module is generated for it",
+                            providerConfig.getName(), providerConfig.getType());
+                    return false;
+                }
+            };
+
     /**
      * Factory used to create OPENID_CONNECT and OAUTH auth module configurations.
      */
@@ -483,32 +513,34 @@
     }
 
     @Override
-    public void identityProviderConfigChanged() throws IdentityProviderServiceException {
+    public synchronized void identityProviderConfigChanged() throws IdentityProviderServiceException {
         if (config == null) {
             logger.debug("No configuration for Authentication Service");
             return;
         }
-        amendedConfig = config.copy();
+        final JsonValue newAmendedConfig = config.copy();
         // the auth module list config lives under at /serverAuthConfig/authModule
-        final JsonValue authModuleConfig = amendedConfig.get(SERVER_AUTH_CONTEXT_KEY).get(AUTH_MODULES_KEY);
+        final JsonValue authModuleConfig = newAmendedConfig.get(SERVER_AUTH_CONTEXT_KEY).get(AUTH_MODULES_KEY);
         amendAuthConfig(authModuleConfig);
 
         try {
-            authFilterWrapper.setFilter(configureAuthenticationFilter(amendedConfig));
+            authFilterWrapper.setFilter(configureAuthenticationFilter(newAmendedConfig));
         } catch (AuthenticationException e) {
             logger.debug("Error in configuration for Authentication Service. Filter not set.", e);
             throw new IdentityProviderServiceException(e.getMessage(), e);
         }
 
+        // this now runs on DS bind threads while request threads read both fields without a lock,
+        // so publish complete values only
+        amendedConfig = newAmendedConfig;
         // filter enabled module configs and get their properties;
         // then filter those with valid auth properties, and build an authenticator
-        authenticators.clear();
-        authenticators.addAll(FluentIterable.from(authModuleConfig)
+        authenticators = FluentIterable.from(authModuleConfig)
                 .filter(enabledAuthModules)
                 .transform(toModuleProperties)
                 .filter(authModulesThatHaveValidAuthenticatorProperties)
                 .transform(toAuthenticatorFromProperties)
-                .toList());
+                .toList();
     }
 
     /**
@@ -517,7 +549,7 @@
      * @param context The ComponentContext
      */
     @Activate
-    public void activate(final ComponentContext context)
+    public synchronized void activate(final ComponentContext context)
             throws AuthenticationException, IdentityProviderServiceException {
         logger.info("Activating Authentication Service with configuration {}", context.getProperties());
         config = enhancedConfig.getConfigurationAsJson(context);
@@ -531,10 +563,10 @@
      * @param context The ComponentContext.
      */
     @Deactivate
-    public void deactivate(ComponentContext context) {
+    public synchronized void deactivate(ComponentContext context) {
         logger.debug("OpenIDM Config for Authentication {} is deactivated.", config.get(Constants.SERVICE_PID));
         config = null;
-        authenticators.clear();
+        authenticators = new ArrayList<>();
 
         // remove CAF filter from CHF filter wrapper
         if (authFilterWrapper != null) {

openidm-authnfilter/src/test/java/org/forgerock/openidm/auth/AuthenticationServiceTest.java

@@ -21,7 +21,11 @@
 import static org.forgerock.json.resource.Requests.newActionRequest;
 import static org.forgerock.json.resource.Requests.newReadRequest;
 import static org.forgerock.openidm.auth.AuthenticationService.Action;
+import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import javax.security.auth.message.MessageInfo;
@@ -86,7 +90,6 @@ public void setUp() throws Exception {
                 OBJECT_MAPPER.readValue(getClass().getResource("/config/authentication.json"), Map.class));
         // Instantiate the object to be used with proper mocked IdentityProviderService
         authenticationService = new AuthenticationService();
-        authenticationService.setConfig(authenticationJson);
     }
 
     @AfterMethod
@@ -111,6 +114,8 @@ public void testAmendAuthConfig() throws Exception {
 
         // Instantiate the object to be used with proper mocked IdentityProviderService
         authenticationService.bindIdentityProviderService(identityProviderService);
+        // the reference is bound before the component is activated with its configuration
+        authenticationService.setConfig(authenticationJson);
 
         // Call the amendAuthConfig to see the configuration of authentication.json be modified with
         // the injected identityProvider config from the IdentityProviderService
@@ -154,6 +159,8 @@ public void testAmendAuthConfigWithTwoAuthTypes() throws Exception {
 
         // Instantiate the object to be used with proper mocked IdentityProviderService
         authenticationService.bindIdentityProviderService(identityProviderService);
+        // the reference is bound before the component is activated with its configuration
+        authenticationService.setConfig(authenticationJson);
 
         // Call the amendAuthConfig to see the configuration of authentication.json be modified with
         // the injected identityProvider config from the IdentityProviderService
@@ -183,6 +190,8 @@ public void testNoProviderConfigsToInject() throws Exception {
         when(identityProviderService.getIdentityProviders()).thenReturn(providerConfigs);
 
         authenticationService.bindIdentityProviderService(identityProviderService);
+        // the reference is bound before the component is activated with its configuration
+        authenticationService.setConfig(authenticationJson);
 
         // Call the amendAuthConfig to see the configuration of authentication.json be modified with
         // the injected identityProvider config from the IdentityProviderService; in this test case
@@ -234,6 +243,73 @@ public void amendAuthConfigShouldRemoveSocialProvidersModuleWhenIdentityProvider
         assertThat(authenticationJson.get(AUTH_MODULES).size()).isEqualTo(1);
     }
 
+    @Test
+    public void bindIdentityProviderServiceShouldRegisterListener() throws Exception {
+        final IdentityProviderService identityProviderService = mock(IdentityProviderService.class);
+
+        authenticationService.bindIdentityProviderService(identityProviderService);
+
+        verify(identityProviderService).registerIdentityProviderListener(authenticationService);
+    }
+
+    @Test
+    public void unbindIdentityProviderServiceShouldUnregisterListenerAndStopInjectingProviders() throws Exception {
+        final IdentityProviderService identityProviderService = mock(IdentityProviderService.class);
+        final List<ProviderConfig> openIdProviderConfigs = new ArrayList<>();
+        openIdProviderConfigs.add(ProviderConfigMapper.toProviderConfig(googleIdentityProvider));
+        when(identityProviderService.getIdentityProviders()).thenReturn(openIdProviderConfigs);
+
+        authenticationService.bindIdentityProviderService(identityProviderService);
+        authenticationService.unbindIdentityProviderService(identityProviderService);
+        authenticationService.setConfig(authenticationJson);
+        authenticationService.amendAuthConfig(authenticationJson.get(AUTH_MODULES));
+
+        verify(identityProviderService).unregisterIdentityProviderListener(authenticationService);
+        // only the stand-alone OPENID_CONNECT module is left, no module was generated from the provider
+        assertThat(authenticationJson.get(AUTH_MODULES).size()).isEqualTo(1);
+    }
+
+    @Test
+    public void identityProviderServiceBindAndUnbindShouldRebuildAuthModules() throws Exception {
+        final AuthenticationService service = spy(new AuthenticationService());
+        doNothing().when(service).identityProviderConfigChanged();
+        final IdentityProviderService first = mock(IdentityProviderService.class);
+        final IdentityProviderService second = mock(IdentityProviderService.class);
+
+        service.bindIdentityProviderService(first);
+        verify(service, times(1)).identityProviderConfigChanged();
+
+        // DS replaces a dynamic 0..1 reference by binding the new service before unbinding the old one
+        service.bindIdentityProviderService(second);
+        service.unbindIdentityProviderService(first);
+        verify(service, times(2)).identityProviderConfigChanged();
+
+        service.unbindIdentityProviderService(second);
+        verify(service, times(3)).identityProviderConfigChanged();
+    }
+
+    @Test
+    public void amendAuthConfigShouldSkipProvidersOfUnsupportedType() throws Exception {
+        final IdentityProviderService identityProviderService = mock(IdentityProviderService.class);
+        final List<ProviderConfig> providerConfigs = new ArrayList<>();
+        providerConfigs.add(ProviderConfigMapper.toProviderConfig(googleIdentityProvider));
+        // not an IDMAuthModule name at all
+        providerConfigs.add(ProviderConfigMapper.toProviderConfig(
+                googleIdentityProvider.copy().put("name", "unknown").put("type", "UNKNOWN")));
+        // an IDMAuthModule name, but not one a social auth module can be generated for
+        providerConfigs.add(ProviderConfigMapper.toProviderConfig(
+                googleIdentityProvider.copy().put("name", "managed").put("type", "MANAGED_USER")));
+        when(identityProviderService.getIdentityProviders()).thenReturn(providerConfigs);
+
+        authenticationService.bindIdentityProviderService(identityProviderService);
+        authenticationService.setConfig(authenticationJson);
+        authenticationService.amendAuthConfig(authenticationJson.get(AUTH_MODULES));
+
+        // the stand-alone OPENID_CONNECT module plus the one generated from the supported provider
+        assertThat(authenticationJson.get(AUTH_MODULES).size()).isEqualTo(2);
+        assertThat(authenticationJson.get(AUTH_MODULES).get(1).get("name").asString()).isEqualTo(OPENID_CONNECT);
+    }
+
     /**
      * Tests that the attribute that {@link JwtSessionModule#isLogoutRequest(MessageInfo)} expects is present in the
      * attributesContext.

openidm-identity-provider/src/main/java/org/forgerock/openidm/idp/impl/IdentityProviderService.java

@@ -12,14 +12,16 @@
  * information: "Portions copyright [year] [name of copyright owner]".
  *
  * Copyright 2016 ForgeRock AS.
- * Portions Copyrighted 2024 3A Systems LLC.
+ * Portions Copyrighted 2024-2026 3A Systems LLC.
  */
 package org.forgerock.openidm.idp.impl;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.CopyOnWriteArrayList;
 
 import static org.forgerock.http.handler.HttpClientHandler.OPTION_LOADER;
 import static org.forgerock.json.JsonValue.field;
@@ -143,24 +145,18 @@ private enum Action { availableProviders, getProfile }
      * The String param in Map is referring to the
      * type of auth the identity provider supports.
      */
+    private final Map<String, List<IdentityProviderConfig>> identityProviders = new ConcurrentHashMap<>();
+
     @Reference(
+            name = "identityProviders",
             service = IdentityProviderConfig.class,
             cardinality = ReferenceCardinality.MULTIPLE,
-            policy = ReferencePolicy.DYNAMIC)
-    private final Map<String, List<IdentityProviderConfig>> identityProviders = new ConcurrentHashMap<>();
-
+            policy = ReferencePolicy.DYNAMIC,
+            unbind = "unbindIdentityProviderConfig")
     protected void bindIdentityProviderConfig(final IdentityProviderConfig config)
             throws IdentityProviderServiceException {
-        // for this to be true, we do not have any identityProviders of this type
-        if (!identityProviders.containsKey(config.getIdentityProviderConfig().getType())) {
-            // initialize new array list to store providers of this type
-            List<IdentityProviderConfig> providers = new ArrayList<>();
-            providers.add(config);
-            identityProviders.put(config.getIdentityProviderConfig().getType(), providers);
-        } else {
-            // we currently have existing configs of this type, just add to it
-            identityProviders.get(config.getIdentityProviderConfig().getType()).add(config);
-        }
+        identityProviders.computeIfAbsent(config.getIdentityProviderConfig().getType(),
+                type -> new CopyOnWriteArrayList<>()).add(config);
         notifyListeners();
     }
 
@@ -201,11 +197,12 @@ public void deactivate(ComponentContext context) {
      */
     public List<ProviderConfig> getIdentityProviderByType(final String type) {
         final List<ProviderConfig> providers = new ArrayList<>();
-        if (identityProviders == null || identityProviders.size() == 0) {
+        if (identityProviders.isEmpty()) {
             logger.debug("No Identity Providers have been configured.");
             return providers;
         }
-        for (final IdentityProviderConfig config : identityProviders.get(type)) {
+        for (final IdentityProviderConfig config
+                : identityProviders.getOrDefault(type, Collections.<IdentityProviderConfig>emptyList())) {
             providers.add(config.getIdentityProviderConfig());
         }
         return providers;
@@ -360,8 +357,21 @@ public void unregisterIdentityProviderListener(IdentityProviderListener listener
      * on any identity provider configuration.
      */
     public void notifyListeners() throws IdentityProviderServiceException {
+        IdentityProviderServiceException failure = null;
         for (IdentityProviderListener listener : identityProviderListeners.values()) {
-            listener.identityProviderConfigChanged();
+            try {
+                listener.identityProviderConfigChanged();
+            } catch (IdentityProviderServiceException | RuntimeException e) {
+                // keep notifying the other listeners; one failing listener must not leave them stale
+                logger.warn("Listener {} failed to apply the identity provider change",
+                        listener.getListenerName(), e);
+                if (failure == null) {
+                    failure = new IdentityProviderServiceException(e.getMessage(), e);
+                }
+            }
+        }
+        if (failure != null) {
+            throw failure;
         }
     }