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 567e6fd8e..47a6106a8 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 19f001e77..954b469c5 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 469a88da0..6be063eb4 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 28b988b9a..6f6519fd3 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