diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index bfda1ce5fd..e0d29e0329 100644 --- a/.github/workflows/build.yml +++ b/.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: | 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..cd10913761 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; @@ -214,10 +214,10 @@ public class AuthenticationService implements SingletonResourceProvider, Identit * 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 authenticators = new ArrayList<>(); + private volatile List authenticators = new ArrayList<>(); // ----- Declarative Service Implementation @@ -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 */ @@ -413,11 +424,30 @@ void amendAuthConfig(final JsonValue authModuleConfig) { 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 socialAuthModuleTypes = + new Predicate() { + @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 @@ public String getListenerName() { } @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 @@ public void identityProviderConfigChanged() throws IdentityProviderServiceExcept * @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 @@ public void activate(final ComponentContext context) * @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) { 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..5ba518975a 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 @@ -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 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 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. diff --git a/openidm-doc/src/main/asciidoc/integrators-guide/appendix-auth-modules.adoc b/openidm-doc/src/main/asciidoc/integrators-guide/appendix-auth-modules.adoc index cccac0dd8a..b86d9e6fcb 100644 --- a/openidm-doc/src/main/asciidoc/integrators-guide/appendix-auth-modules.adoc +++ b/openidm-doc/src/main/asciidoc/integrators-guide/appendix-auth-modules.adoc @@ -294,11 +294,11 @@ In general, if you add a custom property, the Admin UI writes changes to the `au [#social-providers-module-details] === SOCIAL_PROVIDERS Module Configuration Options -The `SOCIAL_PROVIDERS` module is a meta-module (template) that dynamically generates `OPENID_CONNECT` and `OAUTH` authentication modules at startup for supported providers registered in the `IdentityProviderService`. The identity provider configurations themselves (client IDs, client secrets, authorization endpoints, etc.) are defined in `conf/identityProviders.json`, not inside the `SOCIAL_PROVIDERS` module entry. +The `SOCIAL_PROVIDERS` module is a meta-module (template) that dynamically generates `OPENID_CONNECT` and `OAUTH` authentication modules at startup for supported providers registered in the `IdentityProviderService`. The identity provider configurations themselves (client IDs, client secrets, authorization endpoints, etc.) are defined in one `conf/identityProvider-.json` file per provider, not inside the `SOCIAL_PROVIDERS` module entry. `conf/identityProviders.json` only holds the catalog of providers that the Admin UI offers; see xref:chap-auth.adoc#social-providers-module["SOCIAL_PROVIDERS"]. [NOTE] ====== -The `SOCIAL_PROVIDERS` entry is removed from the active authentication module list at startup — it is never initialized as an authenticator itself. Each `OPENID_CONNECT` or `OAUTH` provider entry in `conf/identityProviders.json` results in exactly one generated authentication module. +The `SOCIAL_PROVIDERS` entry is removed from the active authentication module list at startup — it is never initialized as an authenticator itself. Each `conf/identityProvider-.json` file results in exactly one generated `OPENID_CONNECT` or `OAUTH` authentication module, which carries the `enabled` flag of that provider. ====== [#social-providers-module-prop-basic] diff --git a/openidm-doc/src/main/asciidoc/integrators-guide/chap-auth.adoc b/openidm-doc/src/main/asciidoc/integrators-guide/chap-auth.adoc index c67d7ec701..38cd6b5fbe 100644 --- a/openidm-doc/src/main/asciidoc/integrators-guide/chap-auth.adoc +++ b/openidm-doc/src/main/asciidoc/integrators-guide/chap-auth.adoc @@ -481,17 +481,58 @@ IWA:: The IWA module enables users to authenticate by using Integrated Windows Authentication (IWA), rather than by providing a username and password. For information about configuring the IWA module with OpenIDM, see xref:#openidm-auth-kerberos["Configuring IWA Authentication"]. [[social-providers-module]]SOCIAL_PROVIDERS:: -The `SOCIAL_PROVIDERS` module is a __meta-module__ (template) that bridges the social identity provider configuration in `conf/identityProviders.json` and the OpenIDM authentication filter. It is not an authenticator itself — instead, at startup it scans all providers registered with the `IdentityProviderService`, removes itself from the active module list, and dynamically generates the corresponding `OPENID_CONNECT` or `OAUTH` authentication modules: +The `SOCIAL_PROVIDERS` module is a __meta-module__ (template) that bridges the social identity providers configured in `conf/identityProvider-.json` files and the OpenIDM authentication filter. It is not an authenticator itself — instead, at startup it scans all providers registered with the `IdentityProviderService`, removes itself from the active module list, and dynamically generates the corresponding `OPENID_CONNECT` or `OAUTH` authentication modules: + * For each provider of type `OPENID_CONNECT`, an `OPENID_CONNECT` auth module is generated with `openIdConnectHeader: "authToken"`. * For each provider of type `OAUTH`, an `OAUTH` auth module is generated with `authTokenHeader: "authToken"` and `authResolverHeader: "provider"`. + -The generated modules inherit the `augmentSecurityContext`, `propertyMapping`, and `defaultUserRoles` values from the `SOCIAL_PROVIDERS` template entry. +The generated modules inherit the `augmentSecurityContext`, `propertyMapping`, and `defaultUserRoles` values from the `SOCIAL_PROVIDERS` template entry. Each generated module also carries the `enabled` flag of its provider, so a provider with `"enabled" : false` produces no active module. + -Providers are configured separately in `conf/identityProviders.json`, or via the Admin UI under *Configure > Social ID Providers*. The `SOCIAL_PROVIDERS` module acts as a single configuration point so that operators do not need to add individual `OPENID_CONNECT` or `OAUTH` entries to `authentication.json` for every social provider. +Each provider is configured in a file of its own, `conf/identityProvider-.json`, for example `conf/identityProvider-google.json`. The provider name is taken from the file name. You can write the file by hand, or enable the provider in the Admin UI under *Configure > Social ID Providers*, which writes the same file. OpenIDM stores the `client_secret` of these files encrypted. The `SOCIAL_PROVIDERS` module acts as a single configuration point so that operators do not need to add individual `OPENID_CONNECT` or `OAUTH` entries to `authentication.json` for every social provider. + ++ +A sample `conf/identityProvider-google.json` is as follows: ++ +[source, json] +---- +{ + "name" : "google", + "type" : "OPENID_CONNECT", + "enabled" : true, + "icon" : "", + "authorization_endpoint" : "https://accounts.google.com/o/oauth2/v2/auth", + "token_endpoint" : "https://oauth2.googleapis.com/token", + "userinfo_endpoint" : "https://openidconnect.googleapis.com/v1/userinfo", + "well-known" : "https://accounts.google.com/.well-known/openid-configuration", + "client_id" : "your-client-id", + "client_secret" : "your-client-secret", + "scope" : [ + "openid", + "profile", + "email" + ], + "authenticationId" : "sub", + "propertyMap" : [ + { + "source" : "sub", + "target" : "id" + }, + { + "source" : "email", + "target" : "username" + } + ] +} +---- + ++ +A hand-written file must include `icon` and `scope`: the login and registration pages render the provider's button from `icon` and build the authorization request from `scope`, and they fail to render while an enabled provider lacks either. The Admin UI copies both from the catalog entry. + ++ +`conf/identityProviders.json` is the configuration of the identity provider service itself. Its `providers` list is the catalog of supported providers (Google, Facebook, and LinkedIn by default): the templates that the Admin UI offers under *Configure > Social ID Providers*, and that `POST /openidm/identityProviders?_action=availableProviders` returns. An entry in the catalog is not an active provider, and client credentials placed there are neither used nor encrypted. Keep the file in place: without it the identity provider service does not start, and no social provider can be configured. + [NOTE] 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..7107515368 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; @@ -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; } } 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..43fa6d6c0e 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 @@ -17,10 +17,14 @@ package org.forgerock.openidm.idp.impl; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.fail; import static org.forgerock.json.JsonValue.*; import static org.forgerock.json.resource.Requests.newReadRequest; import static org.forgerock.json.test.assertj.AssertJJsonValueAssert.assertThat; +import static org.mockito.Mockito.doThrow; 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 +100,59 @@ 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(); + } + + @Test + public void notifyListenersShouldNotifyEveryListenerWhenOneFails() throws Exception { + IdentityProviderListener failing = mock(IdentityProviderListener.class); + when(failing.getListenerName()).thenReturn("failing"); + doThrow(new IllegalArgumentException("unsupported type")).when(failing).identityProviderConfigChanged(); + IdentityProviderListener healthy = mock(IdentityProviderListener.class); + when(healthy.getListenerName()).thenReturn("healthy"); + + IdentityProviderService service = new IdentityProviderService(); + service.registerIdentityProviderListener(failing); + service.registerIdentityProviderListener(healthy); + + try { + service.notifyListeners(); + fail("Expected IdentityProviderServiceException"); + } catch (IdentityProviderServiceException e) { + assertThat(e.getCause()).isInstanceOf(IllegalArgumentException.class); + } + // whatever the iteration order, the failing listener does not stop the other one + verify(failing).identityProviderConfigChanged(); + verify(healthy).identityProviderConfigChanged(); + } } \ No newline at end of file diff --git a/openidm-selfservice/src/main/java/org/forgerock/openidm/selfservice/impl/SelfService.java b/openidm-selfservice/src/main/java/org/forgerock/openidm/selfservice/impl/SelfService.java index d7c0e256a4..0f073f3ebf 100644 --- a/openidm-selfservice/src/main/java/org/forgerock/openidm/selfservice/impl/SelfService.java +++ b/openidm-selfservice/src/main/java/org/forgerock/openidm/selfservice/impl/SelfService.java @@ -12,7 +12,7 @@ * information: "Portions copyright [year] [name of copyright owner]". * * Copyright 2015-2016 ForgeRock AS. - * Portions Copyrighted 2024 3A Systems LLC. + * Portions Copyrighted 2024-2026 3A Systems LLC. */ package org.forgerock.openidm.selfservice.impl; @@ -151,7 +151,7 @@ void bindIdentityProviderService(IdentityProviderService identityProviderService private ProgressStageProvider progressStageProvider; @Activate - void activate(ComponentContext context) throws Exception { + synchronized void activate(ComponentContext context) throws Exception { this.context = context; LOGGER.debug("Activating Service with configuration {}", context.getProperties()); try { @@ -308,7 +308,7 @@ public JsonValue remove(String s) { } @Deactivate - void deactivate(ComponentContext compContext) { + synchronized void deactivate(ComponentContext compContext) { LOGGER.debug("Deactivating Service {}", compContext.getProperties()); try { unregisterServiceRegistration(); @@ -340,8 +340,14 @@ public String getListenerName() { } @Override - public void identityProviderConfigChanged() + public synchronized void identityProviderConfigChanged() throws IdentityProviderServiceException { + // runs on DS bind threads too: serialize the unregister/register pair below, + // and ignore a change that arrives after deactivate + if (config == null) { + LOGGER.debug("No configuration for {}", PID); + return; + } LOGGER.debug("Configuring {} with changes from IdentityProviderConfig {}", PID, identityProviderService != null ? identityProviderService.getIdentityProviders() diff --git a/openidm-selfservice/src/test/java/org/forgerock/openidm/selfservice/impl/SelfServiceTest.java b/openidm-selfservice/src/test/java/org/forgerock/openidm/selfservice/impl/SelfServiceTest.java index 3ea7be2cf6..6735f0faac 100644 --- a/openidm-selfservice/src/test/java/org/forgerock/openidm/selfservice/impl/SelfServiceTest.java +++ b/openidm-selfservice/src/test/java/org/forgerock/openidm/selfservice/impl/SelfServiceTest.java @@ -12,6 +12,7 @@ * information: "Portions copyright [year] [name of copyright owner]". * * Copyright 2016 ForgeRock AS. + * Portions copyright 2026 3A Systems LLC */ package org.forgerock.openidm.selfservice.impl; @@ -19,6 +20,8 @@ import static org.forgerock.json.JsonValue.*; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import com.fasterxml.jackson.core.JsonParser; @@ -88,4 +91,16 @@ public void testAmendConfig() throws Exception { assertThat(selfServiceRegistration.isEqualTo(amendedSelfServiceRegistration)).isTrue(); } + + @Test + public void identityProviderConfigChangedShouldIgnoreChangeWithoutConfiguration() throws Exception { + final IdentityProviderService identityProviderService = mock(IdentityProviderService.class); + final SelfService selfService = new SelfService(); + selfService.bindIdentityProviderService(identityProviderService); + + // a provider change that arrives before activate or after deactivate has nothing to rebuild + selfService.identityProviderConfigChanged(); + + verify(identityProviderService, never()).registerIdentityProviderListener(selfService); + } } \ No newline at end of file diff --git a/openidm-zip/src/main/resources/conf/identityProviders.json b/openidm-zip/src/main/resources/conf/identityProviders.json new file mode 100644 index 0000000000..d961f130c6 --- /dev/null +++ b/openidm-zip/src/main/resources/conf/identityProviders.json @@ -0,0 +1,142 @@ +{ + "providers" : [ + { + "name" : "google", + "type" : "OPENID_CONNECT", + "icon" : "", + "authorization_endpoint" : "https://accounts.google.com/o/oauth2/v2/auth", + "token_endpoint" : "https://oauth2.googleapis.com/token", + "userinfo_endpoint" : "https://openidconnect.googleapis.com/v1/userinfo", + "well-known" : "https://accounts.google.com/.well-known/openid-configuration", + "client_id" : "", + "client_secret" : "", + "scope" : [ + "openid", + "profile", + "email" + ], + "authenticationId" : "sub", + "propertyMap" : [ + { + "source" : "sub", + "target" : "id" + }, + { + "source" : "name", + "target" : "displayName" + }, + { + "source" : "given_name", + "target" : "givenName" + }, + { + "source" : "family_name", + "target" : "familyName" + }, + { + "source" : "picture", + "target" : "photoUrl" + }, + { + "source" : "email", + "target" : "email" + }, + { + "source" : "email", + "target" : "username" + } + ] + }, + { + "name" : "facebook", + "type" : "OAUTH", + "icon" : "", + "authorization_endpoint" : "https://www.facebook.com/dialog/oauth", + "token_endpoint" : "https://graph.facebook.com/oauth/access_token", + "userinfo_endpoint" : "https://graph.facebook.com/me?fields=id,name,email,first_name,last_name", + "client_id" : "", + "client_secret" : "", + "scope" : [ + "public_profile", + "email" + ], + "authenticationId" : "id", + "propertyMap" : [ + { + "source" : "id", + "target" : "id" + }, + { + "source" : "name", + "target" : "displayName" + }, + { + "source" : "first_name", + "target" : "givenName" + }, + { + "source" : "last_name", + "target" : "familyName" + }, + { + "source" : "email", + "target" : "email" + }, + { + "source" : "email", + "target" : "username" + } + ] + }, + { + "name" : "linkedIn", + "type" : "OAUTH", + "icon" : "", + "authorization_endpoint" : "https://www.linkedin.com/oauth/v2/authorization", + "token_endpoint" : "https://www.linkedin.com/oauth/v2/accessToken", + "userinfo_endpoint" : "https://api.linkedin.com/v2/userinfo", + "client_id" : "", + "client_secret" : "", + "scope" : [ + "openid", + "profile", + "email" + ], + "authenticationId" : "sub", + "propertyMap" : [ + { + "source" : "sub", + "target" : "id" + }, + { + "source" : "name", + "target" : "displayName" + }, + { + "source" : "given_name", + "target" : "givenName" + }, + { + "source" : "family_name", + "target" : "familyName" + }, + { + "source" : "picture", + "target" : "photoUrl" + }, + { + "source" : "email", + "target" : "email" + }, + { + "source" : "email", + "target" : "username" + }, + { + "source" : "locale", + "target" : "locale" + } + ] + } + ] +} diff --git a/openidm-zip/src/test/java/org/forgerock/openidm/bin/defaults/script/IdentityProvidersJsonTest.java b/openidm-zip/src/test/java/org/forgerock/openidm/bin/defaults/script/IdentityProvidersJsonTest.java new file mode 100644 index 0000000000..6953f3f542 --- /dev/null +++ b/openidm-zip/src/test/java/org/forgerock/openidm/bin/defaults/script/IdentityProvidersJsonTest.java @@ -0,0 +1,58 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ +package org.forgerock.openidm.bin.defaults.script; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.fasterxml.jackson.databind.ObjectMapper; +import org.forgerock.json.JsonValue; +import org.forgerock.openidm.idp.config.ProviderConfig; +import org.testng.annotations.Test; + +import java.io.InputStream; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; + +/** + * Checks the catalog of supported identity providers shipped in conf/identityProviders.json. + * + * IdentityProviderService does not start without this file, and the Admin UI offers its entries as + * templates, keyed on the provider name, and copies their icon into the provider's own configuration. + */ +public class IdentityProvidersJsonTest { + @Test + public void testCatalogEntries() throws Exception { + // unlike ProviderConfigMapper, fail on unknown keys: the service would silently drop them + final ObjectMapper mapper = new ObjectMapper(); + final JsonValue catalog; + try (final InputStream configStream = getClass().getResourceAsStream("/conf/identityProviders.json")) { + assertThat(configStream).isNotNull(); + catalog = new JsonValue(mapper.readValue(configStream, Map.class)); + } + final List names = new ArrayList<>(); + for (JsonValue entry : catalog.get("providers")) { + final ProviderConfig config = mapper.convertValue(entry.getObject(), ProviderConfig.class); + names.add(config.getName()); + assertThat(config.getIcon()).isNotEmpty(); + assertThat(config.getScope()).isNotEmpty(); + assertThat(config.getAuthorizationEndpoint()).startsWith("https://"); + assertThat(config.getTokenEndpoint()).startsWith("https://"); + assertThat(config.getUserInfoEndpoint()).startsWith("https://"); + } + assertThat(names).containsExactly("google", "facebook", "linkedIn"); + } +}