From bae7081610f74d54065070ffff4cc05b0dd846d4 Mon Sep 17 00:00:00 2001 From: Thisara-Welmilla Date: Thu, 13 Feb 2025 15:41:29 +0530 Subject: [PATCH] Fix error at cloning authentication context. --- .../cache/AuthenticationContextLoader.java | 8 ++-- .../model/OptimizedApplicationConfig.java | 14 +++---- .../model/OptimizedAuthenticatorConfig.java | 9 ++-- .../context/AuthenticationContext.java | 42 ------------------- .../core/ApplicationAuthenticatorManager.java | 38 +++++++++++++++-- .../ApplicationAuthenticatorManagerTest.java | 24 ++++++++++- .../idp/mgt/IdentityProviderManager.java | 13 ++++++ 7 files changed, 81 insertions(+), 67 deletions(-) diff --git a/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/cache/AuthenticationContextLoader.java b/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/cache/AuthenticationContextLoader.java index 565cdff0a13e..cf20df9b1bf7 100644 --- a/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/cache/AuthenticationContextLoader.java +++ b/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/cache/AuthenticationContextLoader.java @@ -314,11 +314,11 @@ private IdentityProvider getIdPByIdPName(String idPName, String tenantDomain) private IdentityProvider getIdPByResourceID(String resourceId, String tenantDomain) throws SessionDataStorageOptimizationException { - IdentityProviderManager manager = - (IdentityProviderManager) FrameworkServiceDataHolder.getInstance().getIdentityProviderManager(); IdentityProvider idp; try { - idp = manager.getIdPByResourceId(resourceId, tenantDomain, false); + idp = ApplicationAuthenticatorManager.getInstance().getSerializableIdPByResourceId( + resourceId, tenantDomain); + if (idp == null) { throw new SessionDataStorageOptimizationClientException( String.format("Cannot find the Identity Provider by the resource ID: %s " + @@ -332,7 +332,7 @@ private IdentityProvider getIdPByResourceID(String resourceId, String tenantDoma throw new SessionDataStorageOptimizationServerException( String.format("IDP management server error. Failed to get the Identity Provider by " + "resource id: %s tenant domain: %s", resourceId, tenantDomain), e); - } catch (IdentityProviderManagementException e) { + } catch (IdentityProviderManagementException | FrameworkException e) { throw new SessionDataStorageOptimizationServerException( String.format("Failed to get the Identity Provider by resource id: %s tenant domain: %s", resourceId, tenantDomain), e); diff --git a/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/config/model/OptimizedApplicationConfig.java b/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/config/model/OptimizedApplicationConfig.java index 781d0167cf1a..eed283d611ed 100644 --- a/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/config/model/OptimizedApplicationConfig.java +++ b/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/config/model/OptimizedApplicationConfig.java @@ -25,7 +25,7 @@ import org.wso2.carbon.identity.application.authentication.framework.exception.session.storage.SessionDataStorageOptimizationClientException; import org.wso2.carbon.identity.application.authentication.framework.exception.session.storage.SessionDataStorageOptimizationException; import org.wso2.carbon.identity.application.authentication.framework.exception.session.storage.SessionDataStorageOptimizationServerException; -import org.wso2.carbon.identity.application.authentication.framework.internal.FrameworkServiceDataHolder; +import org.wso2.carbon.identity.application.authentication.framework.internal.core.ApplicationAuthenticatorManager; import org.wso2.carbon.identity.application.common.ApplicationAuthenticatorService; import org.wso2.carbon.identity.application.common.exception.AuthenticatorMgtException; import org.wso2.carbon.identity.application.common.model.AuthenticationStep; @@ -337,12 +337,10 @@ private IdentityProvider[] getFederatedIdPs(List federatedIdPResourceIds throws FrameworkException { IdentityProvider[] idPs = new IdentityProvider[federatedIdPResourceIds.size()]; - IdentityProviderManager manager = - (IdentityProviderManager) FrameworkServiceDataHolder.getInstance().getIdentityProviderManager(); for (int i = 0; i < federatedIdPResourceIds.size(); i++) { try { - IdentityProvider idp = manager.getIdPByResourceId(federatedIdPResourceIds.get(i), tenantDomain, - false); + IdentityProvider idp = ApplicationAuthenticatorManager.getInstance().getSerializableIdPByResourceId( + federatedIdPResourceIds.get(i), tenantDomain); if (idp == null) { throw new SessionDataStorageOptimizationClientException( String.format("Cannot find the IdP by the resource Id: %s Tenant Domain: %s", @@ -379,12 +377,10 @@ private IdentityProvider[] getIdPsFromOptimizedFederatedIdPs( throws FrameworkException { List idPList = new ArrayList<>(); - IdentityProviderManager manager = - (IdentityProviderManager) FrameworkServiceDataHolder.getInstance().getIdentityProviderManager(); for (OptimizedAuthStep.OptimizedFederatedIdP optimizedFederatedIdP : optimizedFederatedIdPs) { try { - IdentityProvider idPByResourceId = manager.getIdPByResourceId(optimizedFederatedIdP.getIdpResourceId(), - tenantDomain, false); + IdentityProvider idPByResourceId = ApplicationAuthenticatorManager.getInstance() + .getSerializableIdPByResourceId(optimizedFederatedIdP.getIdpResourceId(), tenantDomain); if (idPByResourceId == null) { throw new SessionDataStorageOptimizationClientException( String.format("Cannot find the IdP by the resource Id: %s Tenant Domain: %s", diff --git a/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/config/model/OptimizedAuthenticatorConfig.java b/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/config/model/OptimizedAuthenticatorConfig.java index 15b1132e4b87..1e29b984b209 100644 --- a/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/config/model/OptimizedAuthenticatorConfig.java +++ b/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/config/model/OptimizedAuthenticatorConfig.java @@ -26,13 +26,11 @@ import org.wso2.carbon.identity.application.authentication.framework.exception.session.storage.SessionDataStorageOptimizationClientException; import org.wso2.carbon.identity.application.authentication.framework.exception.session.storage.SessionDataStorageOptimizationException; import org.wso2.carbon.identity.application.authentication.framework.exception.session.storage.SessionDataStorageOptimizationServerException; -import org.wso2.carbon.identity.application.authentication.framework.internal.FrameworkServiceDataHolder; import org.wso2.carbon.identity.application.authentication.framework.internal.core.ApplicationAuthenticatorManager; import org.wso2.carbon.identity.application.common.model.IdentityProvider; import org.wso2.carbon.idp.mgt.IdentityProviderManagementClientException; import org.wso2.carbon.idp.mgt.IdentityProviderManagementException; import org.wso2.carbon.idp.mgt.IdentityProviderManagementServerException; -import org.wso2.carbon.idp.mgt.IdentityProviderManager; import java.io.Serializable; import java.util.ArrayList; @@ -111,11 +109,10 @@ private IdentityProvider getIdPByResourceID(String resourceId, String tenantDoma String.format("Null parameters passed while getting IDPs by the resource ID: %s " + "tenant domain: %s", resourceId, tenantDomain)); } - IdentityProviderManager manager = - (IdentityProviderManager) FrameworkServiceDataHolder.getInstance().getIdentityProviderManager(); IdentityProvider idp; try { - idp = manager.getIdPByResourceId(resourceId, tenantDomain, false); + idp = ApplicationAuthenticatorManager.getInstance().getSerializableIdPByResourceId( + resourceId, tenantDomain); if (idp == null) { throw new SessionDataStorageOptimizationClientException( String.format("Cannot find the Identity Provider by the resource ID: %s " + @@ -129,7 +126,7 @@ private IdentityProvider getIdPByResourceID(String resourceId, String tenantDoma throw new SessionDataStorageOptimizationServerException( String.format("IDP management server error occurred. Failed to get the Identity Provider by " + "resource id: %s tenant domain: %s", resourceId, tenantDomain), e); - } catch (IdentityProviderManagementException e) { + } catch (IdentityProviderManagementException | FrameworkException e) { throw new SessionDataStorageOptimizationException( String.format("IDP management error occurred. Failed to get the Identity Provider by " + "resource id: %s tenant domain: %s", resourceId, tenantDomain), e); diff --git a/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/context/AuthenticationContext.java b/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/context/AuthenticationContext.java index 59daec5c6def..58c28bdd5516 100644 --- a/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/context/AuthenticationContext.java +++ b/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/context/AuthenticationContext.java @@ -22,17 +22,12 @@ import org.apache.commons.lang.StringUtils; import org.apache.commons.lang3.SerializationUtils; import org.wso2.carbon.identity.application.authentication.framework.AuthenticatorStateInfo; -import org.wso2.carbon.identity.application.authentication.framework.config.model.AuthenticatorConfig; import org.wso2.carbon.identity.application.authentication.framework.config.model.ExternalIdPConfig; import org.wso2.carbon.identity.application.authentication.framework.config.model.SequenceConfig; -import org.wso2.carbon.identity.application.authentication.framework.config.model.StepConfig; import org.wso2.carbon.identity.application.authentication.framework.model.AuthenticatedIdPData; import org.wso2.carbon.identity.application.authentication.framework.model.AuthenticatedUser; import org.wso2.carbon.identity.application.authentication.framework.model.AuthenticationRequest; import org.wso2.carbon.identity.application.authentication.framework.util.FrameworkConstants; -import org.wso2.carbon.identity.application.common.model.FederatedAuthenticatorConfig; -import org.wso2.carbon.identity.application.common.model.IdentityProvider; -import org.wso2.carbon.identity.application.common.model.UserDefinedFederatedAuthenticatorConfig; import org.wso2.carbon.identity.base.IdentityRuntimeException; import org.wso2.carbon.identity.core.bean.context.MessageContext; import org.wso2.carbon.identity.core.util.IdentityTenantUtil; @@ -860,43 +855,6 @@ public void setExpiryTime(long expiryTimeNano) { */ public Object clone () { - removeNonSerializableObjects(); return SerializationUtils.clone(this); } - - private void removeNonSerializableObjects() { - - /* Remove non-serializable UserDefinedAuthenticatorEndpointConfig objects from the - UserDefinedFederatedAuthenticatorConfig in the context. The UserDefinedAuthenticatorEndpointConfig contains - the endpoint URI and the authentication type of the corresponding action. However, this information is not - used in the authentication flow. Instead, the action ID in the authenticator property is used to resolve the - corresponding action. */ - if (sequenceConfig == null || sequenceConfig.getStepMap() == null) { - return; - } - - for (StepConfig stepConfig : sequenceConfig.getStepMap().values()) { - if (stepConfig == null || stepConfig.getAuthenticatorList() == null) { - continue; - } - - for (AuthenticatorConfig authenticatorConfig : stepConfig.getAuthenticatorList()) { - if (stepConfig.getAuthenticatorList() == null) { - continue; - } - - for (IdentityProvider idp : authenticatorConfig.getIdps().values()) { - if (idp == null || idp.getFederatedAuthenticatorConfigs() == null) { - continue; - } - - for (FederatedAuthenticatorConfig authConfig : idp.getFederatedAuthenticatorConfigs()) { - if (authConfig instanceof UserDefinedFederatedAuthenticatorConfig) { - ((UserDefinedFederatedAuthenticatorConfig) authConfig).setEndpointConfig(null); - } - } - } - } - } - } } diff --git a/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/internal/core/ApplicationAuthenticatorManager.java b/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/internal/core/ApplicationAuthenticatorManager.java index e53d61ca485e..7ab668041848 100644 --- a/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/internal/core/ApplicationAuthenticatorManager.java +++ b/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/internal/core/ApplicationAuthenticatorManager.java @@ -18,12 +18,14 @@ package org.wso2.carbon.identity.application.authentication.framework.internal.core; +import com.google.gson.Gson; import org.wso2.carbon.identity.application.authentication.framework.ApplicationAuthenticator; import org.wso2.carbon.identity.application.authentication.framework.exception.FrameworkException; import org.wso2.carbon.identity.application.authentication.framework.internal.FrameworkServiceDataHolder; import org.wso2.carbon.identity.application.common.ApplicationAuthenticatorService; import org.wso2.carbon.identity.application.common.exception.AuthenticatorMgtException; import org.wso2.carbon.identity.application.common.model.FederatedAuthenticatorConfig; +import org.wso2.carbon.identity.application.common.model.IdentityProvider; import org.wso2.carbon.identity.application.common.model.UserDefinedFederatedAuthenticatorConfig; import org.wso2.carbon.identity.application.common.model.UserDefinedLocalAuthenticatorConfig; import org.wso2.carbon.identity.core.util.IdentityConfigParser; @@ -117,8 +119,8 @@ public List getAllAuthenticators(String tenantDomain) .getUserDefinedLocalAuthenticator(localConfig)); } - FederatedAuthenticatorConfig[] fedConfig = IdentityProviderManager.getInstance() - .getAllFederatedAuthenticators(tenantDomain); + FederatedAuthenticatorConfig[] fedConfig = FrameworkServiceDataHolder.getInstance() + .getIdentityProviderManager().getAllFederatedAuthenticators(tenantDomain); for (FederatedAuthenticatorConfig fedAuth : fedConfig) { if (fedAuth instanceof UserDefinedFederatedAuthenticatorConfig) { allAuthenticators.add(FrameworkServiceDataHolder.getInstance().getUserDefinedAuthenticatorService() @@ -164,8 +166,8 @@ public ApplicationAuthenticator getApplicationAuthenticatorByName(String authent } // Check whether the authenticator config is the user defined fed authenticator config, if so resolve it. - FederatedAuthenticatorConfig[] fedConfig = IdentityProviderManager.getInstance() - .getAllFederatedAuthenticators(tenantDomain); + FederatedAuthenticatorConfig[] fedConfig = FrameworkServiceDataHolder.getInstance() + .getIdentityProviderManager().getAllFederatedAuthenticators(tenantDomain); for (FederatedAuthenticatorConfig fedAuth : fedConfig) { if (fedAuth instanceof UserDefinedFederatedAuthenticatorConfig && fedAuth.getName().equals(authenticatorName)) { @@ -184,4 +186,32 @@ private boolean isAuthenticationActionEnabled() { return Boolean.parseBoolean((String) IdentityConfigParser.getInstance() .getConfiguration().get(AUTHENTICATION_ACTION_ENABLED_PROP)); } + + /** + * Return serializable identity provider for the given resource Id. + * + * @param resourceId Resource id of the identity provider. + * @param tenantDomain Tenant domain. + * @return IdentityProvider. + * @throws IdentityProviderManagementException If any error occurs while retrieving identity provider + * by resource id. + */ + public IdentityProvider getSerializableIdPByResourceId(String resourceId, String tenantDomain) + throws IdentityProviderManagementException { + + /* Remove non-serializable UserDefinedAuthenticatorEndpointConfig objects from the identityProviders in the + authentication context. + The UserDefinedAuthenticatorEndpointConfig contains the endpoint URI and authentication type for the + corresponding action. However, this information is not utilized in the authentication flow. Instead, + the action ID in the authenticator property is used to resolve the corresponding action. + Since the FederatedAuthenticatorConfig model is used in the IdentityProvider class, when creating a deep + clone of the Identity Provider, convert the UserDefinedFederatedAuthenticatorConfig object to + a FederatedAuthenticatorConfig instance. */ + IdentityProviderManager manager = + (IdentityProviderManager) FrameworkServiceDataHolder.getInstance().getIdentityProviderManager(); + Gson gson = new Gson(); + return gson.fromJson( + gson.toJson(manager.getIdPByResourceId(resourceId, tenantDomain, false)), + IdentityProvider.class); + } } diff --git a/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/test/java/org/wso2/carbon/identity/application/authentication/framework/internal/core/ApplicationAuthenticatorManagerTest.java b/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/test/java/org/wso2/carbon/identity/application/authentication/framework/internal/core/ApplicationAuthenticatorManagerTest.java index 77998f2f4a6c..53fadd6a83ca 100644 --- a/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/test/java/org/wso2/carbon/identity/application/authentication/framework/internal/core/ApplicationAuthenticatorManagerTest.java +++ b/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/test/java/org/wso2/carbon/identity/application/authentication/framework/internal/core/ApplicationAuthenticatorManagerTest.java @@ -35,6 +35,7 @@ import org.wso2.carbon.identity.application.authentication.framework.internal.FrameworkServiceDataHolder; import org.wso2.carbon.identity.application.common.ApplicationAuthenticatorService; import org.wso2.carbon.identity.application.common.model.FederatedAuthenticatorConfig; +import org.wso2.carbon.identity.application.common.model.IdentityProvider; import org.wso2.carbon.identity.application.common.model.UserDefinedFederatedAuthenticatorConfig; import org.wso2.carbon.identity.application.common.model.UserDefinedLocalAuthenticatorConfig; import org.wso2.carbon.identity.base.AuthenticatorPropertyConstants; @@ -47,6 +48,8 @@ import static org.junit.Assert.assertEquals; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mockStatic; import static org.mockito.Mockito.when; @@ -82,6 +85,8 @@ public class ApplicationAuthenticatorManagerTest extends AbstractFrameworkTest { private final MockedStatic mockedIdentityProviderManager = mockStatic(IdentityProviderManager.class); private final IdentityProviderManager identityProviderManager = mock(IdentityProviderManager.class); + private FederatedAuthenticatorConfig[] fedAuthConfig; + private IdentityProvider idp; @BeforeClass public void setUp() { @@ -106,9 +111,15 @@ public void setUpTest() throws Exception { List.of(new UserDefinedLocalAuthenticatorConfig( AuthenticatorPropertyConstants.AuthenticationType.IDENTIFICATION))); + fedAuthConfig = new FederatedAuthenticatorConfig[]{new UserDefinedFederatedAuthenticatorConfig()}; + idp = new IdentityProvider(); + idp.setIdentityProviderName("testIdp"); + idp.setFederatedAuthenticatorConfigs(fedAuthConfig); + + when(identityProviderManager.getAllFederatedAuthenticators(TENANT_DOMAIN)).thenReturn(fedAuthConfig); + when(identityProviderManager.getIdPByResourceId(anyString(), anyString(), anyBoolean())).thenReturn(idp); mockedIdentityProviderManager.when(IdentityProviderManager::getInstance).thenReturn(identityProviderManager); - when(identityProviderManager.getAllFederatedAuthenticators(TENANT_DOMAIN)).thenReturn(new - FederatedAuthenticatorConfig[]{new UserDefinedFederatedAuthenticatorConfig()}); + FrameworkServiceDataHolder.getInstance().setIdentityProviderManager(identityProviderManager); } @AfterClass @@ -174,6 +185,15 @@ public void testGetAuthenticatorByNameWithAuthActionTypeEnabledAndNullUserDefine assertEquals(1, result.size()); } + @Test + public void testGetSerializableIdPByResourceId() throws Exception { + + IdentityProvider serializableIdp = applicationAuthenticatorService.getSerializableIdPByResourceId( + "dummyResourceId", TENANT_DOMAIN); + Assert.assertFalse(serializableIdp.getFederatedAuthenticatorConfigs()[0] + instanceof UserDefinedFederatedAuthenticatorConfig); + } + private void setAuthenticatorActionEnableStatus(boolean isEnabled) { Map configMap = new HashMap<>(); diff --git a/components/idp-mgt/org.wso2.carbon.idp.mgt/src/main/java/org/wso2/carbon/idp/mgt/IdentityProviderManager.java b/components/idp-mgt/org.wso2.carbon.idp.mgt/src/main/java/org/wso2/carbon/idp/mgt/IdentityProviderManager.java index 5d012bdded21..13042231b650 100644 --- a/components/idp-mgt/org.wso2.carbon.idp.mgt/src/main/java/org/wso2/carbon/idp/mgt/IdentityProviderManager.java +++ b/components/idp-mgt/org.wso2.carbon.idp.mgt/src/main/java/org/wso2/carbon/idp/mgt/IdentityProviderManager.java @@ -881,6 +881,19 @@ public IdentityProvider getIdPById(String id, String tenantDomain, return identityProvider; } + /** + * Returns extended IDP with resource ID. + * Note: The UserDefinedFederatedAuthenticatorConfig object in the IdentityProvider is not serializable using + * org.apache.commons.lang3.SerializationUtils, which is used in the authentication framework to clone the + * authentication context. Hence, use getSerializableIdPByResourceId(String, String) in + * ApplicationAuthenticatorManager which provides an in IdentityProvider instance with the + * UserDefinedFederatedAuthenticatorConfig converted to a FederatedAuthenticatorConfig. + * @param resourceId Resource ID of the IDP. + * @param tenantDomain Tenant domain of the IDP. + * @param ignoreFileBasedIdps Whether to ignore file based idps or not. + * @return extended IDP. + * @throws IdentityProviderManagementException IdentityProviderManagementException + */ @Override public IdentityProvider getIdPByResourceId(String resourceId, String tenantDomain, boolean ignoreFileBasedIdps) throws IdentityProviderManagementException {