From 1a846969d2f6535d44b7999606cef6c5fecb1e1a Mon Sep 17 00:00:00 2001 From: dhaura Date: Wed, 25 Oct 2023 13:56:09 +0530 Subject: [PATCH 1/2] Improve getAuthInitiationData() method to support native SDK based federation. --- .../oidc/OIDCAuthenticatorConstants.java | 1 + .../oidc/OpenIDConnectAuthenticator.java | 36 ++++++++-- .../oidc/OpenIDConnectAuthenticatorTest.java | 65 +++++++++++++++---- 3 files changed, 84 insertions(+), 18 deletions(-) diff --git a/components/org.wso2.carbon.identity.application.authenticator.oidc/src/main/java/org/wso2/carbon/identity/application/authenticator/oidc/OIDCAuthenticatorConstants.java b/components/org.wso2.carbon.identity.application.authenticator.oidc/src/main/java/org/wso2/carbon/identity/application/authenticator/oidc/OIDCAuthenticatorConstants.java index d32476d3..8973a37e 100644 --- a/components/org.wso2.carbon.identity.application.authenticator.oidc/src/main/java/org/wso2/carbon/identity/application/authenticator/oidc/OIDCAuthenticatorConstants.java +++ b/components/org.wso2.carbon.identity.application.authenticator.oidc/src/main/java/org/wso2/carbon/identity/application/authenticator/oidc/OIDCAuthenticatorConstants.java @@ -30,6 +30,7 @@ public class OIDCAuthenticatorConstants { public static final String CODE_PARAM = "code.param"; public static final String ID_TOKEN_PARAM = "idToken"; public static final String SESSION_DATA_KEY_PARAM = "sessionDataKey"; + public static final String CLIENT_ID_PARAM = "clientId"; private OIDCAuthenticatorConstants() { diff --git a/components/org.wso2.carbon.identity.application.authenticator.oidc/src/main/java/org/wso2/carbon/identity/application/authenticator/oidc/OpenIDConnectAuthenticator.java b/components/org.wso2.carbon.identity.application.authenticator.oidc/src/main/java/org/wso2/carbon/identity/application/authenticator/oidc/OpenIDConnectAuthenticator.java index be667c62..32297711 100644 --- a/components/org.wso2.carbon.identity.application.authenticator.oidc/src/main/java/org/wso2/carbon/identity/application/authenticator/oidc/OpenIDConnectAuthenticator.java +++ b/components/org.wso2.carbon.identity.application.authenticator.oidc/src/main/java/org/wso2/carbon/identity/application/authenticator/oidc/OpenIDConnectAuthenticator.java @@ -1290,21 +1290,43 @@ public Optional getAuthInitiationData(AuthenticationContext c authenticatorData.setIdp(idpName); List requiredParameterList = new ArrayList<>(); - requiredParameterList.add(OIDCAuthenticatorConstants.OAUTH2_GRANT_TYPE_CODE); - requiredParameterList.add(OIDCAuthenticatorConstants.OAUTH2_PARAM_STATE); + if (isTrustedTokenIssuer(context)) { + requiredParameterList.add(ACCESS_TOKEN_PARAM); + requiredParameterList.add(ID_TOKEN_PARAM); + authenticatorData.setPromptType(FrameworkConstants.AuthenticatorPromptType.INTERNAL_PROMPT); + authenticatorData.setAdditionalData(getAdditionalData(context, true)); + } else { + requiredParameterList.add(OIDCAuthenticatorConstants.OAUTH2_GRANT_TYPE_CODE); + requiredParameterList.add(OIDCAuthenticatorConstants.OAUTH2_PARAM_STATE); + authenticatorData.setPromptType(FrameworkConstants.AuthenticatorPromptType.REDIRECTION_PROMPT); + authenticatorData.setAdditionalData(getAdditionalData(context, false)); + } authenticatorData.setRequiredParams(requiredParameterList); - authenticatorData.setPromptType(FrameworkConstants.AuthenticatorPromptType.REDIRECTION_PROMPT); - authenticatorData.setAdditionalData(getAdditionalData(context)); return Optional.of(authenticatorData); } - private static AdditionalData getAdditionalData(AuthenticationContext context) { + private static AdditionalData getAdditionalData( + AuthenticationContext context, boolean isNativeSDKBasedFederationCall) { AdditionalData additionalData = new AdditionalData(); - additionalData.setRedirectUrl((String) context.getProperty(OIDCAuthenticatorConstants.AUTHENTICATOR_NAME + - REDIRECT_URL_SUFFIX)); + + if (isNativeSDKBasedFederationCall) { + Map additionalAuthenticationParams = new HashMap<>(); + + String nonce = (String) context.getProperty(OIDC_FEDERATION_NONCE); + if (StringUtils.isNotBlank(nonce)) { + additionalAuthenticationParams.put(NONCE, nonce); + } + additionalAuthenticationParams.put(OIDCAuthenticatorConstants.CLIENT_ID_PARAM, + context.getAuthenticatorProperties().get(OIDCAuthenticatorConstants.CLIENT_ID)); + + additionalData.setAdditionalAuthenticationParams(additionalAuthenticationParams); + } else { + additionalData.setRedirectUrl((String) context.getProperty(OIDCAuthenticatorConstants.AUTHENTICATOR_NAME + + REDIRECT_URL_SUFFIX)); + } return additionalData; } diff --git a/components/org.wso2.carbon.identity.application.authenticator.oidc/src/test/java/org/wso2/carbon/identity/application/authenticator/oidc/OpenIDConnectAuthenticatorTest.java b/components/org.wso2.carbon.identity.application.authenticator.oidc/src/test/java/org/wso2/carbon/identity/application/authenticator/oidc/OpenIDConnectAuthenticatorTest.java index fa120d54..3c827629 100644 --- a/components/org.wso2.carbon.identity.application.authenticator.oidc/src/test/java/org/wso2/carbon/identity/application/authenticator/oidc/OpenIDConnectAuthenticatorTest.java +++ b/components/org.wso2.carbon.identity.application.authenticator.oidc/src/test/java/org/wso2/carbon/identity/application/authenticator/oidc/OpenIDConnectAuthenticatorTest.java @@ -43,7 +43,6 @@ import org.wso2.carbon.identity.application.authentication.framework.context.AuthenticationContext; import org.wso2.carbon.identity.application.authentication.framework.exception.AuthenticationFailedException; import org.wso2.carbon.identity.application.authentication.framework.exception.FrameworkException; -import org.wso2.carbon.identity.application.authentication.framework.model.AdditionalData; import org.wso2.carbon.identity.application.authentication.framework.model.AuthenticationRequest; import org.wso2.carbon.identity.application.authentication.framework.model.AuthenticatorData; import org.wso2.carbon.identity.application.authentication.framework.util.FrameworkConstants; @@ -92,12 +91,8 @@ import static org.testng.Assert.assertNotNull; import static org.testng.Assert.assertNull; import static org.testng.Assert.assertTrue; -import static org.wso2.carbon.identity.application.authenticator.oidc.OIDCAuthenticatorConstants.OIDC_FEDERATION_NONCE; -import static org.wso2.carbon.identity.application.authenticator.oidc.OIDCAuthenticatorConstants.AUTHENTICATOR_OIDC; -import static org.wso2.carbon.identity.application.authenticator.oidc.OIDCAuthenticatorConstants.REDIRECTION_PROMPT; -import static org.wso2.carbon.identity.application.authenticator.oidc.OIDCAuthenticatorConstants.AUTHENTICATOR_NAME; -import static org.wso2.carbon.identity.application.authenticator.oidc.OIDCAuthenticatorConstants. - AUTHENTICATOR_FRIENDLY_NAME; +import static org.wso2.carbon.identity.application.authenticator.oidc.OIDCAuthenticatorConstants.*; +import static org.wso2.carbon.identity.application.authenticator.oidc.OIDCAuthenticatorConstants.Claim.NONCE; /*** * Unit test class for OpenIDConnectAuthenticator class. @@ -172,6 +167,7 @@ public class OpenIDConnectAuthenticatorTest extends PowerMockTestCase { private static Map authenticatorProperties; private static Map authenticatorParamProperties; + private static String clientId = "DpPQt4KnvcehtUuQ0Jf6i0gl0E0a"; private static String accessToken = "4952b467-86b2-31df-b63c-0bf25cec4f86s"; private static String idToken = "eyJ4NXQiOiJOVEF4Wm1NeE5ETXlaRGczTVRVMVpHTTBNekV6T0RKaFpXSTRORE5" + "sWkRVMU9HRmtOakZpTVEiLCJraWQiOiJOVEF4Wm1NeE5ETXlaRGczTVRVMVpHTTBNekV6T0RKaFpXSTRORE5sWkRVMU9" + @@ -186,6 +182,10 @@ public class OpenIDConnectAuthenticatorTest extends PowerMockTestCase { private static String sessionDataKey = "7b1c8131-c6bd-4682-892e-1a948a9e57e8"; private static String nonce = "0ed8f1b3-e83f-46c0-8d52-f0d2e7925f98"; private static String invalidNonce = "7ed8f1b3-e83f-46c0-8d52-f0d2e7925f98"; + private static String redirectUrl = "https://accounts.google.com/o/oauth2/v2/auth?scope=openid&" + + "response_type=code&redirect_uri=https%3A%2F%2Flocalhost%3A9443%2Fcommonauth&" + + "state=958e9049-8cd2-4580-8745-6679ac8d33f6%2COIDC&nonce=0ed8f1b3-e83f-46c0-8d52-f0d2e7925f98&" + + "client_id=sample.client-id"; private static OAuthClientResponse token; private Map paramValueMap; private int TENANT_ID = 1234; @@ -823,13 +823,11 @@ public void testGetAuthInitiationData() { when(mockAuthenticationContext.getExternalIdP()).thenReturn(externalIdPConfig); when(externalIdPConfig.getIdPName()).thenReturn("LOCAL"); when(mockAuthenticationContext.getAuthenticationRequest()).thenReturn(mockAuthenticationRequest); + when(mockAuthenticationContext.getProperty( + OIDCAuthenticatorConstants.AUTHENTICATOR_NAME + REDIRECT_URL_SUFFIX)).thenReturn(redirectUrl); Optional authenticatorData = openIDConnectAuthenticator.getAuthInitiationData (mockAuthenticationContext); - List requiredParameterList = new ArrayList<>(); - requiredParameterList.add(OIDCAuthenticatorConstants.OAUTH2_GRANT_TYPE_CODE); - requiredParameterList.add(OIDCAuthenticatorConstants.OAUTH2_PARAM_STATE); - Assert.assertTrue(authenticatorData.isPresent()); AuthenticatorData authenticatorDataObj = authenticatorData.get(); @@ -840,7 +838,52 @@ public void testGetAuthInitiationData() { 2); Assert.assertEquals(authenticatorDataObj.getPromptType(), FrameworkConstants.AuthenticatorPromptType.REDIRECTION_PROMPT); + Assert.assertTrue(authenticatorDataObj.getRequiredParams() + .contains(OIDCAuthenticatorConstants.OAUTH2_GRANT_TYPE_CODE)); + Assert.assertTrue(authenticatorDataObj.getRequiredParams() + .contains(OIDCAuthenticatorConstants.OAUTH2_PARAM_STATE)); + Assert.assertEquals(authenticatorDataObj.getAdditionalData().getRedirectUrl(), redirectUrl); + } + + @Test + public void testGetAuthInitiationDataForNativeSDKBasedFederation() { + IdentityProviderProperty property = new IdentityProviderProperty(); + property.setName(IdPManagementConstants.IS_TRUSTED_TOKEN_ISSUER); + property.setValue("true"); + IdentityProviderProperty[] identityProviderProperties = new IdentityProviderProperty[1]; + identityProviderProperties[0] = property; + + when(mockAuthenticationContext.getExternalIdP()).thenReturn(externalIdPConfig); + when(externalIdPConfig.getIdPName()).thenReturn("LOCAL"); + when(externalIdPConfig.getIdentityProvider()).thenReturn(identityProvider); + when(identityProvider.getIdpProperties()).thenReturn(identityProviderProperties); + when(mockAuthenticationContext.getAuthenticationRequest()).thenReturn(mockAuthenticationRequest); + when(mockAuthenticationContext.getProperty(OIDC_FEDERATION_NONCE)).thenReturn(nonce); + when(mockAuthenticationContext.getAuthenticatorProperties()).thenReturn(authenticatorProperties); + authenticatorProperties.put(CLIENT_ID, clientId); + + Optional authenticatorData = openIDConnectAuthenticator.getAuthInitiationData + (mockAuthenticationContext); + + Assert.assertTrue(authenticatorData.isPresent()); + AuthenticatorData authenticatorDataObj = authenticatorData.get(); + + Assert.assertEquals(authenticatorDataObj.getName(), AUTHENTICATOR_NAME); + Assert.assertEquals(authenticatorDataObj.getI18nKey(), AUTHENTICATOR_OIDC); + Assert.assertEquals(authenticatorDataObj.getDisplayName(), AUTHENTICATOR_FRIENDLY_NAME); + Assert.assertEquals(authenticatorDataObj.getRequiredParams().size(), + 2); + Assert.assertEquals(authenticatorDataObj.getPromptType(), + FrameworkConstants.AuthenticatorPromptType.INTERNAL_PROMPT); + Assert.assertTrue(authenticatorDataObj.getRequiredParams() + .contains(OIDCAuthenticatorConstants.ACCESS_TOKEN_PARAM)); + Assert.assertTrue(authenticatorDataObj.getRequiredParams() + .contains(OIDCAuthenticatorConstants.ID_TOKEN_PARAM)); + Assert.assertEquals(authenticatorDataObj.getAdditionalData() + .getAdditionalAuthenticationParams().get(NONCE), nonce); + Assert.assertEquals(authenticatorDataObj.getAdditionalData() + .getAdditionalAuthenticationParams().get(CLIENT_ID_PARAM), clientId); } private ExternalIdPConfig getDummyExternalIdPConfig() { From 118af13880b079e9aedad79b9809c3d2eaced78e Mon Sep 17 00:00:00 2001 From: dhaura Date: Wed, 25 Oct 2023 14:03:45 +0530 Subject: [PATCH 2/2] Improve code formatting --- .../oidc/OpenIDConnectAuthenticator.java | 2 -- .../oidc/OpenIDConnectAuthenticatorTest.java | 15 +++++++++------ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/components/org.wso2.carbon.identity.application.authenticator.oidc/src/main/java/org/wso2/carbon/identity/application/authenticator/oidc/OpenIDConnectAuthenticator.java b/components/org.wso2.carbon.identity.application.authenticator.oidc/src/main/java/org/wso2/carbon/identity/application/authenticator/oidc/OpenIDConnectAuthenticator.java index 32297711..d4a834d4 100644 --- a/components/org.wso2.carbon.identity.application.authenticator.oidc/src/main/java/org/wso2/carbon/identity/application/authenticator/oidc/OpenIDConnectAuthenticator.java +++ b/components/org.wso2.carbon.identity.application.authenticator.oidc/src/main/java/org/wso2/carbon/identity/application/authenticator/oidc/OpenIDConnectAuthenticator.java @@ -1298,7 +1298,6 @@ public Optional getAuthInitiationData(AuthenticationContext c } else { requiredParameterList.add(OIDCAuthenticatorConstants.OAUTH2_GRANT_TYPE_CODE); requiredParameterList.add(OIDCAuthenticatorConstants.OAUTH2_PARAM_STATE); - authenticatorData.setPromptType(FrameworkConstants.AuthenticatorPromptType.REDIRECTION_PROMPT); authenticatorData.setAdditionalData(getAdditionalData(context, false)); } @@ -1321,7 +1320,6 @@ private static AdditionalData getAdditionalData( } additionalAuthenticationParams.put(OIDCAuthenticatorConstants.CLIENT_ID_PARAM, context.getAuthenticatorProperties().get(OIDCAuthenticatorConstants.CLIENT_ID)); - additionalData.setAdditionalAuthenticationParams(additionalAuthenticationParams); } else { additionalData.setRedirectUrl((String) context.getProperty(OIDCAuthenticatorConstants.AUTHENTICATOR_NAME + diff --git a/components/org.wso2.carbon.identity.application.authenticator.oidc/src/test/java/org/wso2/carbon/identity/application/authenticator/oidc/OpenIDConnectAuthenticatorTest.java b/components/org.wso2.carbon.identity.application.authenticator.oidc/src/test/java/org/wso2/carbon/identity/application/authenticator/oidc/OpenIDConnectAuthenticatorTest.java index 3c827629..7c529c66 100644 --- a/components/org.wso2.carbon.identity.application.authenticator.oidc/src/test/java/org/wso2/carbon/identity/application/authenticator/oidc/OpenIDConnectAuthenticatorTest.java +++ b/components/org.wso2.carbon.identity.application.authenticator.oidc/src/test/java/org/wso2/carbon/identity/application/authenticator/oidc/OpenIDConnectAuthenticatorTest.java @@ -71,8 +71,6 @@ import java.net.HttpURLConnection; import java.net.URL; import java.util.HashMap; -import java.util.List; -import java.util.ArrayList; import java.util.Map; import java.util.Optional; import javax.servlet.http.HttpServletRequest; @@ -91,7 +89,11 @@ import static org.testng.Assert.assertNotNull; import static org.testng.Assert.assertNull; import static org.testng.Assert.assertTrue; -import static org.wso2.carbon.identity.application.authenticator.oidc.OIDCAuthenticatorConstants.*; +import static org.wso2.carbon.identity.application.authenticator.oidc.OIDCAuthenticatorConstants.OIDC_FEDERATION_NONCE; +import static org.wso2.carbon.identity.application.authenticator.oidc.OIDCAuthenticatorConstants.AUTHENTICATOR_OIDC; +import static org.wso2.carbon.identity.application.authenticator.oidc.OIDCAuthenticatorConstants.AUTHENTICATOR_NAME; +import static org.wso2.carbon.identity.application.authenticator.oidc.OIDCAuthenticatorConstants. + AUTHENTICATOR_FRIENDLY_NAME; import static org.wso2.carbon.identity.application.authenticator.oidc.OIDCAuthenticatorConstants.Claim.NONCE; /*** @@ -824,7 +826,8 @@ public void testGetAuthInitiationData() { when(externalIdPConfig.getIdPName()).thenReturn("LOCAL"); when(mockAuthenticationContext.getAuthenticationRequest()).thenReturn(mockAuthenticationRequest); when(mockAuthenticationContext.getProperty( - OIDCAuthenticatorConstants.AUTHENTICATOR_NAME + REDIRECT_URL_SUFFIX)).thenReturn(redirectUrl); + OIDCAuthenticatorConstants.AUTHENTICATOR_NAME + OIDCAuthenticatorConstants.REDIRECT_URL_SUFFIX)) + .thenReturn(redirectUrl); Optional authenticatorData = openIDConnectAuthenticator.getAuthInitiationData (mockAuthenticationContext); @@ -861,7 +864,7 @@ public void testGetAuthInitiationDataForNativeSDKBasedFederation() { when(mockAuthenticationContext.getAuthenticationRequest()).thenReturn(mockAuthenticationRequest); when(mockAuthenticationContext.getProperty(OIDC_FEDERATION_NONCE)).thenReturn(nonce); when(mockAuthenticationContext.getAuthenticatorProperties()).thenReturn(authenticatorProperties); - authenticatorProperties.put(CLIENT_ID, clientId); + authenticatorProperties.put(OIDCAuthenticatorConstants.CLIENT_ID, clientId); Optional authenticatorData = openIDConnectAuthenticator.getAuthInitiationData (mockAuthenticationContext); @@ -883,7 +886,7 @@ public void testGetAuthInitiationDataForNativeSDKBasedFederation() { Assert.assertEquals(authenticatorDataObj.getAdditionalData() .getAdditionalAuthenticationParams().get(NONCE), nonce); Assert.assertEquals(authenticatorDataObj.getAdditionalData() - .getAdditionalAuthenticationParams().get(CLIENT_ID_PARAM), clientId); + .getAdditionalAuthenticationParams().get(OIDCAuthenticatorConstants.CLIENT_ID_PARAM), clientId); } private ExternalIdPConfig getDummyExternalIdPConfig() {