From 5ac24678fc40ae7333495ecf6d21d404e6e867f6 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Wed, 23 Sep 2026 17:00:53 +0300 Subject: [PATCH] [#225] Bind identity provider configs through the DS bind methods The @Reference to IdentityProviderConfig sat on a Map field, a type Declarative Services cannot inject, so SCR rejected it and bindIdentityProviderConfig was never called: /identityProviders stayed empty and no social auth module was generated. Move the annotation to the bind method, and do the same for AuthenticationService, whose bindIdentityProviderService (which registers the provider listener) was bypassed by field injection too. Also make getIdentityProviderByType return an empty list for a type with no bound provider, and add providers atomically. Fixes #225 --- .../openidm/auth/AuthenticationService.java | 21 +++++++++--- .../auth/AuthenticationServiceTest.java | 34 ++++++++++++++++++- .../idp/impl/IdentityProviderService.java | 29 +++++++--------- .../idp/impl/IdentityProviderServiceTest.java | 34 +++++++++++++++++++ 4 files changed, 96 insertions(+), 22 deletions(-) diff --git a/openidm-authnfilter/src/main/java/org/forgerock/openidm/auth/AuthenticationService.java b/openidm-authnfilter/src/main/java/org/forgerock/openidm/auth/AuthenticationService.java index 567e6fd8ee..47a6106a82 100644 --- a/openidm-authnfilter/src/main/java/org/forgerock/openidm/auth/AuthenticationService.java +++ b/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; @@ -243,17 +243,28 @@ public class AuthenticationService implements SingletonResourceProvider, Identit @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 */ diff --git a/openidm-authnfilter/src/test/java/org/forgerock/openidm/auth/AuthenticationServiceTest.java b/openidm-authnfilter/src/test/java/org/forgerock/openidm/auth/AuthenticationServiceTest.java index 19f001e772..954b469c5a 100644 --- a/openidm-authnfilter/src/test/java/org/forgerock/openidm/auth/AuthenticationServiceTest.java +++ b/openidm-authnfilter/src/test/java/org/forgerock/openidm/auth/AuthenticationServiceTest.java @@ -22,6 +22,7 @@ import static org.forgerock.json.resource.Requests.newReadRequest; import static org.forgerock.openidm.auth.AuthenticationService.Action; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import javax.security.auth.message.MessageInfo; @@ -86,7 +87,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 +111,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 +156,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 +187,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 +240,32 @@ 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 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); + } + /** * Tests that the attribute that {@link JwtSessionModule#isLogoutRequest(MessageInfo)} expects is present in the * attributesContext. diff --git a/openidm-identity-provider/src/main/java/org/forgerock/openidm/idp/impl/IdentityProviderService.java b/openidm-identity-provider/src/main/java/org/forgerock/openidm/idp/impl/IdentityProviderService.java index 469a88da00..6be063eb4c 100644 --- a/openidm-identity-provider/src/main/java/org/forgerock/openidm/idp/impl/IdentityProviderService.java +++ b/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> identityProviders = new ConcurrentHashMap<>(); + @Reference( + name = "identityProviders", service = IdentityProviderConfig.class, cardinality = ReferenceCardinality.MULTIPLE, - policy = ReferencePolicy.DYNAMIC) - private final Map> 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 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 getIdentityProviderByType(final String type) { final List 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.emptyList())) { providers.add(config.getIdentityProviderConfig()); } return providers; diff --git a/openidm-identity-provider/src/test/java/org/forgerock/openidm/idp/impl/IdentityProviderServiceTest.java b/openidm-identity-provider/src/test/java/org/forgerock/openidm/idp/impl/IdentityProviderServiceTest.java index 28b988b9a5..6f6519fd3f 100644 --- a/openidm-identity-provider/src/test/java/org/forgerock/openidm/idp/impl/IdentityProviderServiceTest.java +++ b/openidm-identity-provider/src/test/java/org/forgerock/openidm/idp/impl/IdentityProviderServiceTest.java @@ -21,6 +21,8 @@ import static org.forgerock.json.resource.Requests.newReadRequest; import static org.forgerock.json.test.assertj.AssertJJsonValueAssert.assertThat; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.util.Map; @@ -96,4 +98,36 @@ public void testReadInstance() throws Exception { assertThat(google).doesNotContain("client_secret"); // it should be removed by readInstance assertThat(google.isEqualTo(expected)).isTrue(); } + + @Test + public void testGetIdentityProviderByType() throws Exception { + IdentityProviderConfig idpConfig = mock(IdentityProviderConfig.class); + when(idpConfig.getIdentityProviderConfig()).thenReturn(googleIdentityProvider); + + IdentityProviderService service = new IdentityProviderService(); + service.bindIdentityProviderConfig(idpConfig); + + assertThat(service.getIdentityProviderByType("OPENID_CONNECT")).containsExactly(googleIdentityProvider); + // a type with no bound provider yields an empty list rather than failing + assertThat(service.getIdentityProviderByType("OAUTH")).isEmpty(); + } + + @Test + public void testUnbindIdentityProviderConfig() throws Exception { + IdentityProviderConfig idpConfig = mock(IdentityProviderConfig.class); + when(idpConfig.getIdentityProviderConfig()).thenReturn(googleIdentityProvider); + IdentityProviderListener listener = mock(IdentityProviderListener.class); + when(listener.getListenerName()).thenReturn("listener"); + + IdentityProviderService service = new IdentityProviderService(); + service.registerIdentityProviderListener(listener); + service.bindIdentityProviderConfig(idpConfig); + assertThat(service.getIdentityProvider("google")).isSameAs(googleIdentityProvider); + + service.unbindIdentityProviderConfig(idpConfig); + + assertThat(service.getIdentityProviders()).isEmpty(); + assertThat(service.getIdentityProvider("google")).isNull(); + verify(listener, times(2)).identityProviderConfigChanged(); + } } \ No newline at end of file