From 83a6f82628904e6509ac42d04fbe6c94cbf089c6 Mon Sep 17 00:00:00 2001 From: Indeewai Wijesiri Date: Tue, 5 Mar 2024 10:01:48 +0530 Subject: [PATCH 1/8] Sharing federated tokens in oidc authentication --- .../oidc/OIDCAuthenticatorConstants.java | 7 + .../oidc/OpenIDConnectAuthenticator.java | 303 ++++++++++++++++++ .../oidc/OpenIDConnectAuthenticatorTest.java | 290 ++++++++++++++++- 3 files changed, 591 insertions(+), 9 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 5ad63870..5e709a77 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 @@ -31,6 +31,13 @@ public class OIDCAuthenticatorConstants { 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"; + public static final String REFRESH_TOKEN = "refresh_token"; + public static final String EXPIRES_IN = "expires_in"; + public static final String FEDERATED_TOKENS = "federated_tokens"; + public static final String SHARE_FEDERATED_TOKEN_CONFIG = "ShareFederatedToken"; + public static final String SHARE_FEDERATED_TOKEN_PARAM = "share_federated_token"; + public static final String FEDERATED_TOKEN_ALLOWED_SCOPE = "FederatedTokenAllowedScope"; + public static final String FEDERATED_TOKEN_SCOPE = "federated_token_scope"; 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 6923a659..df9e3511 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 @@ -23,6 +23,7 @@ import com.nimbusds.jwt.SignedJWT; import net.minidev.json.JSONArray; import org.apache.commons.codec.binary.Base64; +import org.apache.commons.collections.CollectionUtils; import org.apache.commons.collections.MapUtils; import org.apache.commons.lang.ArrayUtils; import org.apache.commons.lang.StringUtils; @@ -48,8 +49,10 @@ import org.wso2.carbon.identity.application.authentication.framework.exception.LogoutFailedException; import org.wso2.carbon.identity.application.authentication.framework.model.AdditionalData; 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.model.AuthenticatorData; import org.wso2.carbon.identity.application.authentication.framework.model.AuthenticatorMessage; +import org.wso2.carbon.identity.application.authentication.framework.model.FederatedToken; import org.wso2.carbon.identity.application.authentication.framework.util.FrameworkConstants; import org.wso2.carbon.identity.application.authentication.framework.util.FrameworkUtils; import org.wso2.carbon.identity.application.authenticator.oidc.internal.OpenIDConnectAuthenticatorDataHolder; @@ -94,6 +97,7 @@ import java.nio.charset.StandardCharsets; import java.text.ParseException; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; @@ -112,6 +116,7 @@ import static org.wso2.carbon.identity.application.authenticator.oidc.OIDCAuthenticatorConstants.ACCESS_TOKEN_PARAM; import static org.wso2.carbon.identity.application.authenticator.oidc.OIDCAuthenticatorConstants.AUTHENTICATOR_OIDC; import static org.wso2.carbon.identity.application.authenticator.oidc.OIDCAuthenticatorConstants.Claim.NONCE; +import static org.wso2.carbon.identity.application.authenticator.oidc.OIDCAuthenticatorConstants.SHARE_FEDERATED_TOKEN_CONFIG; import static org.wso2.carbon.identity.application.authenticator.oidc.OIDCAuthenticatorConstants.ID_TOKEN_PARAM; import static org.wso2.carbon.identity.application.authenticator.oidc.OIDCAuthenticatorConstants.LogConstants.ActionIDs.INITIATE_OUTBOUND_AUTH_REQUEST; import static org.wso2.carbon.identity.application.authenticator.oidc.OIDCAuthenticatorConstants.LogConstants.ActionIDs.PROCESS_AUTHENTICATION_RESPONSE; @@ -136,6 +141,10 @@ public class OpenIDConnectAuthenticator extends AbstractApplicationAuthenticator private static final String DYNAMIC_PARAMETER_LOOKUP_REGEX = "\\$\\{(\\w+)\\}"; private static final String IS_API_BASED = "IS_API_BASED"; private static final String REDIRECT_URL = "REDIRECT_URL"; + public static final String SPACE_REGEX = "\\s+"; + public static final String SPACE = " "; + public static final String SEMI_COLON_DELIMITER = ";"; + public static final String COMMA_DELIMITER = ","; private static Pattern pattern = Pattern.compile(DYNAMIC_PARAMETER_LOOKUP_REGEX); private static final String[] NON_USER_ATTRIBUTES = new String[]{"at_hash", "iss", "iat", "exp", "aud", "azp"}; private static final String AUTHENTICATOR_MESSAGE = "authenticatorMessage"; @@ -481,6 +490,9 @@ protected void initiateAuthenticationRequest(HttpServletRequest request, HttpSer String scopes = getScope(authenticatorProperties); + // Adding the scopes requested by the application side for token sharing. + scopes = addValidScopesForFederatedTokenSharing(context, authenticatorProperties, scopes); + String queryString = getQueryString(authenticatorProperties); if (StringUtils.isNotBlank(scopes)) { if (LoggerUtils.isDiagnosticLogsEnabled() && diagnosticLogBuilder != null) { @@ -580,6 +592,224 @@ protected void initiateAuthenticationRequest(HttpServletRequest request, HttpSer return; } + /** + * This method is used to append the application side requested scopes after validating. + * The application can request the optional scopes either via adaptive scripts or via the authorize + * request query parameters. + * i.e. Adaptive Script example: + * This will ignore any other definition (common, local) of the authenticatorParams. + * var onLoginRequest = function(context) { + * executeStep(1, { + * authenticatorParams: { + * federated: { + * "Google Calender": { + * federated_token_scope: "https://googleapis.calander.readonly https://google.calander.list" + * }}}}, {});} + * i.e Authorize request query param example: + * /authorize?response_type=id_token&client_id={ClientId}&redirect_uri={https://app/callback} + * &scope=email profile openid + * &federated_token_scope=Google Calender;read write,Microsoft Authenticator;https://googleapis.calender + * + * @param context The authentication context. + * @param authenticatorProperties The authenticator properties. + * @param scopes The scopes defined in the authenticator properties. + * @return The IDP defined scope and the validated scopes requested by the application. + */ + private String addValidScopesForFederatedTokenSharing(AuthenticationContext context, + Map authenticatorProperties, String scopes) { + /* + The scopes for the federated tokens are evaluated only of the authenticator configuration ShareFederatedToken + is enabled and the application has requested the federated token. + */ + if (!Boolean.parseBoolean(authenticatorProperties.get(SHARE_FEDERATED_TOKEN_CONFIG)) || + !requestedToShareFederatedToken(context)) { + return scopes; + } + + // Get the application requested scopes for the federated tokens. + String requestedScopesForTokenSharing = getRequestedScopesForTokenSharing(context); + + // Validating the application requested scopes by the authenticator allowed scopes for federated token sharing. + String validScopesForTokenSharing = validateScopeForTokenSharing( + authenticatorProperties.get(OIDCAuthenticatorConstants.FEDERATED_TOKEN_ALLOWED_SCOPE), + requestedScopesForTokenSharing); + + if (StringUtils.isNotBlank(validScopesForTokenSharing)) { + if (LOG.isDebugEnabled()) { + LOG.debug("Valid scopes matched for federated token sharing: " + validScopesForTokenSharing); + } + scopes = (scopes + SPACE + validScopesForTokenSharing).trim(); + } + return scopes; + } + + /** + * This method returns the scopes requested by the application for the federated tokens. + * + * @param context The authentication context. + * @return The scopes requested by the application for token sharing. + */ + private String getRequestedScopesForTokenSharing(AuthenticationContext context) { + // The first priority is given to the parameters setup at the adaptive script. Then the query parameters. + String requestedScopesViaAdaptiveScript = + getAdaptiveScriptValues(context, OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE); + // Checks if there exists scopes requested via adaptive script. + if (StringUtils.isNotBlank(requestedScopesViaAdaptiveScript)) { + if (LOG.isDebugEnabled()) { + LOG.debug("Adaptive script parameter found for " + OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE); + } + return requestedScopesViaAdaptiveScript; + } else { + if (LOG.isDebugEnabled()) { + LOG.debug("No adaptive script parameter found for " + + OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE + " hence checking the query parameters."); + } + // Checks the scopes requested via authorize request query parameters. + return getRequestedScopedViaQueryParams(context); + } + } + + /** + * This method returns the adaptive script federated authenticator param value for a given parameter name. + * + * @param context The authentication context with federated authenticator params. + * @param authenticatorParamName The federated authenticator parameter name. + * @return The adaptive script federated authenticator param value for the given parameter name. + */ + private String getAdaptiveScriptValues(AuthenticationContext context, String authenticatorParamName) { + + Map runtimeParams = this.getRuntimeParams(context); + String requestedScopesViaAdaptiveScript = null; + if (runtimeParams != null) { + requestedScopesViaAdaptiveScript = runtimeParams.get(authenticatorParamName); + } + return requestedScopesViaAdaptiveScript; + } + + /** + * The optional scope string cannot have scattered segments for the same authenticator. + * Only the very first segment is considered. + * i.e. A valid string: + * Google Calender has read write scopes, Microsoft Authenticator has https://googleapis.calender scope + * A valid string: + * federated_token_scope=Google Calander;read write,Microsoft Authenticator;https://googleapis.calender + * A valid string: + * federated_token_scope=Google Calender;read https://googleapis.calender.read + * + * @param context The authentication context with authentication request having the query parameters. + * @return The scopes requested by the application via the query parameters for federated token sharing. + */ + private String getRequestedScopedViaQueryParams(AuthenticationContext context) { + + if (context.getExternalIdP() == null) { + if (LOG.isDebugEnabled()) { + LOG.debug("No external IDP found in the authentication context. " + + "Cannot retrieve the query parameters."); + } + return null; + } + + String authenticatorName = context.getExternalIdP().getName(); + if (StringUtils.isBlank(authenticatorName)) { + if (LOG.isDebugEnabled()) { + LOG.debug("No external IDP name found in the authentication context. " + + "Cannot retrieve the query parameters."); + } + return null; + } + + String scopeString = getQueryParameter(context, OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE); + // The optional scopes should come with the authenticator name only. + if (StringUtils.isBlank(scopeString) || !scopeString.contains(SEMI_COLON_DELIMITER)) { + if (LOG.isDebugEnabled()) { + LOG.debug("Query parameter " + OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE + " is missing " + + SEMI_COLON_DELIMITER + " delimiter or the value is empty."); + } + return null; + } + + String[] scopeSegments = StringUtils.split(scopeString, COMMA_DELIMITER); + StringBuilder filteredScopes = new StringBuilder(); + + for (String scopesFollowedByAuthenticator : scopeSegments) { + String[] scopes = StringUtils.split(scopesFollowedByAuthenticator, SEMI_COLON_DELIMITER); + if (ArrayUtils.isNotEmpty(scopes) && scopes.length == 2 && authenticatorName.equals(scopes[0])) { + filteredScopes.append(scopes[1]).append(SPACE); + if (LOG.isDebugEnabled()) { + LOG.debug( + "Scopes found for the authenticator " + authenticatorName + " in the query parameter " + + OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE + " : " + scopes[1]); + } + } + } + + String requestedScopes = filteredScopes.toString(); + if (LOG.isDebugEnabled() && StringUtils.isBlank(requestedScopes)) { + LOG.debug("No valid values found for the authenticator " + authenticatorName + + " in the query parameter " + OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE); + } + + return filteredScopes.toString(); + } + + /** + * This method evaluates whether application has requested to share the token. The first priority is given to the + * authenticator parameters set at the adaptive script. Then the query parameters. + * + * @param context The authentication context. + * @return Whether the application has requested to share the token. + */ + private boolean requestedToShareFederatedToken(AuthenticationContext context) { + + // The first priority is given to the parameters setup at the adaptive script. Then the query parameters. + String shareFederatedToken = + getAdaptiveScriptValues(context, OIDCAuthenticatorConstants.SHARE_FEDERATED_TOKEN_PARAM); + + if (LOG.isDebugEnabled() && StringUtils.isNotBlank(shareFederatedToken)) { + LOG.debug("Adaptive script parameter " + OIDCAuthenticatorConstants.SHARE_FEDERATED_TOKEN_PARAM + + " found for the federated token sharing. "); + } + // Checks if the token sharing is requested via adaptive script. + if (StringUtils.isBlank(shareFederatedToken)) { + // Checks if the token sharing is requested via authorize request query parameters. + if (LOG.isDebugEnabled()) { + LOG.debug("No adaptive script parameter " + OIDCAuthenticatorConstants.SHARE_FEDERATED_TOKEN_PARAM + + " found for the federated token sharing. Checking the query parameters."); + } + shareFederatedToken = getQueryParameter(context, OIDCAuthenticatorConstants.SHARE_FEDERATED_TOKEN_PARAM); + } + return Boolean.parseBoolean(shareFederatedToken); + } + + /** + * This method is used to retrieve the query parameters from the authentication request. + * + * @param context The authentication context with authentication request. + * @param queryParamName The required query parameter name. + * @return The query parameter value. + */ + private String getQueryParameter(AuthenticationContext context, String queryParamName) { + + AuthenticationRequest authenticationRequest = context.getAuthenticationRequest(); + if (authenticationRequest == null || StringUtils.isBlank(queryParamName)) { + if (LOG.isDebugEnabled()) { + LOG.debug("Invalid authentication request or invalid query parameter name : " + queryParamName); + } + return null; + } + String[] queryParamValues = authenticationRequest.getRequestQueryParam(queryParamName); + if (ArrayUtils.isNotEmpty(queryParamValues)) { + if (LOG.isDebugEnabled()) { + LOG.debug("Query parameter found for," + queryParamName); + } + return queryParamValues[0]; + } + if (LOG.isDebugEnabled()) { + LOG.debug("No value found for the query parameter : " + queryParamName); + } + return null; + } + private static void setAuthenticatorMessageToContext(ErrorMessages errorMessage, AuthenticationContext context) { @@ -630,6 +860,10 @@ protected void processAuthenticationResponse(HttpServletRequest request, HttpSer OAuthClientResponse oAuthResponse = requestAccessToken(request, context); // TODO : return access token and id token to framework mapAccessToken(request, context, oAuthResponse); + + // Adding the federated tokens to the context for token sharing. + addFederatedTokensToContext(context, oAuthResponse); + String idToken = mapIdToken(context, request, oAuthResponse); Map authenticatorProperties = context.getAuthenticatorProperties(); @@ -779,6 +1013,75 @@ protected void mapAccessToken(HttpServletRequest request, AuthenticationContext context.setProperty(OIDCAuthenticatorConstants.ACCESS_TOKEN, accessToken); } + /** + * Add the federated tokens to the authentication context. This is used to share the tokens with the application. + * + * @param context The authentication context for the request on which the federated tokens are kept. + * @param oAuthResponse The OAuth client response. + */ + private void addFederatedTokensToContext(AuthenticationContext context, OAuthClientResponse oAuthResponse) { + /* + Federated tokens are added only of the authenticator configuration ShareFederatedToken is enabled and the + application has requested the federated token. + */ + if (context.getAuthenticatorProperties() == null || !Boolean.parseBoolean( + context.getAuthenticatorProperties().get(OIDCAuthenticatorConstants.SHARE_FEDERATED_TOKEN_CONFIG)) || + !requestedToShareFederatedToken(context)) { + return; + } + + // If there is an existing list of federated tokens obtained in a previous step, utilizing the same list. + List federatedTokens; + Object federatedTokensObj = context.getProperty(OIDCAuthenticatorConstants.FEDERATED_TOKENS); + if (federatedTokensObj instanceof List) { + federatedTokens = (List) federatedTokensObj; + } else { + federatedTokens = new ArrayList<>(); + } + + String identityProviderName = context.getCurrentAuthenticator(); + if (context.getExternalIdP() != null) { + identityProviderName = context.getExternalIdP().getIdPName(); + } + + FederatedToken federatedToken = new FederatedToken(identityProviderName, + oAuthResponse.getParam(OIDCAuthenticatorConstants.ACCESS_TOKEN)); + federatedToken.setRefreshToken(oAuthResponse.getParam(OIDCAuthenticatorConstants.REFRESH_TOKEN)); + federatedToken.setTokenValidityPeriod(oAuthResponse.getParam(OIDCAuthenticatorConstants.EXPIRES_IN)); + federatedToken.setScope(oAuthResponse.getParam(OIDCAuthenticatorConstants.SCOPE)); + federatedTokens.add(federatedToken); + + context.setProperty(OIDCAuthenticatorConstants.FEDERATED_TOKENS, federatedTokens); + } + + /** + * This returns the intersection of the allowed scopes defined at the IDP configuration and the requested scopes + * from the application side for federated toke sharing. + * + * @param allowedScope The administrator defined scopes in the IDP configuration for federated toke sharing. + * @param requestedScope The application side requested scopes for federated toke sharing. + * @return The intersection of the allowed and the requested scopes for federated toke sharing. + */ + private String validateScopeForTokenSharing(String allowedScope, String requestedScope) { + + if (StringUtils.isNotBlank(allowedScope) && StringUtils.isNotBlank(requestedScope)) { + Set allowedScopesSet = new HashSet<>(Arrays.asList(allowedScope.split(SPACE_REGEX))); + Set requestedScopesSet = new HashSet<>(Arrays.asList(requestedScope.split(SPACE_REGEX))); + + Set subset = new HashSet<>(requestedScopesSet); + subset.retainAll(allowedScopesSet); + + if (CollectionUtils.isNotEmpty(subset)) { + return StringUtils.join(subset, SPACE); + } else { + if (LOG.isDebugEnabled()) { + LOG.debug("No valid scopes matched for federated token sharing."); + } + } + } + return null; + } + /** * Generates OAuth client and returns the oAuthResponse according to the flow supported by the authenticator. * Overridden in Google Authenticator for Google one tap. 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 c8e33ec5..caf63915 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 @@ -49,6 +49,7 @@ import org.wso2.carbon.identity.application.authentication.framework.exception.FrameworkException; 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.model.FederatedToken; import org.wso2.carbon.identity.application.authentication.framework.util.FrameworkConstants; import org.wso2.carbon.identity.application.authentication.framework.util.FrameworkUtils; import org.wso2.carbon.identity.application.authenticator.oidc.internal.OpenIDConnectAuthenticatorDataHolder; @@ -61,6 +62,7 @@ import org.wso2.carbon.identity.claim.metadata.mgt.ClaimMetadataManagementService; import org.wso2.carbon.identity.core.ServiceURL; import org.wso2.carbon.identity.core.ServiceURLBuilder; +import org.wso2.carbon.identity.core.URLBuilderException; import org.wso2.carbon.identity.core.util.IdentityCoreConstants; import org.wso2.carbon.identity.core.util.IdentityUtil; import org.wso2.carbon.identity.oauth2.IdentityOAuth2Exception; @@ -81,6 +83,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; + import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -100,12 +103,13 @@ 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.AUTHENTICATOR_FRIENDLY_NAME; 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.AUTHENTICATOR_OIDC; import static org.wso2.carbon.identity.application.authenticator.oidc.OIDCAuthenticatorConstants.Claim.NONCE; +import static org.wso2.carbon.identity.application.authenticator.oidc.OIDCAuthenticatorConstants.SHARE_FEDERATED_TOKEN_CONFIG; +import static org.wso2.carbon.identity.application.authenticator.oidc.OIDCAuthenticatorConstants.OIDC_FEDERATION_NONCE; +import static org.wso2.carbon.identity.application.authenticator.oidc.OIDCAuthenticatorConstants.SHARE_FEDERATED_TOKEN_PARAM; /*** * Unit test class for OpenIDConnectAuthenticator class. @@ -118,6 +122,9 @@ "org.wso2.carbon.identity.application.authentication.framework.exception.AuthenticationFailedException"}) public class OpenIDConnectAuthenticatorTest extends PowerMockTestCase { + public static final String OIDC_PARAM_MAP_STRING = "oidc:param.map"; + public static final String HTTPS_LOCALHOST_9443 = "https://localhost:9443"; + public static final String COMMA_SEPARATOR = ","; @Mock private HttpServletRequest mockServletRequest; @@ -187,6 +194,9 @@ public class OpenIDConnectAuthenticatorTest extends PowerMockTestCase { private static Map authenticatorParamProperties; private static String clientId = "u5FIfG5xzLvBGiamoAYzzcqpBqga"; private static String accessToken = "4952b467-86b2-31df-b63c-0bf25cec4f86s"; + private static String refreshToken = "6357238-86b2-31df-b63c-0bf25cec4f86s"; + private static String expiresIn = "3600"; + private String scope = "openid email profile"; private static String idToken = "eyJ4NXQiOiJOVEF4Wm1NeE5ETXlaRGczTVRVMVpHTTBNekV6T0RKaFpXSTRORE5" + "sWkRVMU9HRmtOakZpTVEiLCJraWQiOiJOVEF4Wm1NeE5ETXlaRGczTVRVMVpHTTBNekV6T0RKaFpXSTRORE5sWkRVMU9" + "HRmtOakZpTVEiLCJhbGciOiJSUzI1NiJ9.eyJzdWIiOiJhZG1pbiIsImF1ZCI6WyJ1NUZJZkc1eHpMdkJHaWFtb0FZenpjc" + @@ -238,7 +248,7 @@ public void init() { public Object[][] getSeperator() { return new String[][]{ - {","}, + {COMMA_SEPARATOR}, {",,,"} }; } @@ -607,7 +617,7 @@ public void testPassProcessAuthenticationWithParamValue() throws Exception { authenticatorProperties.put("callbackUrl", "http://localhost:8080/playground2/oauth2client"); Map paramMap = new HashMap<>(); paramMap.put("redirect_uri", "http:/localhost:9443/oauth2/redirect"); - when(mockAuthenticationContext.getProperty("oidc:param.map")).thenReturn(paramMap); + when(mockAuthenticationContext.getProperty(OIDC_PARAM_MAP_STRING)).thenReturn(paramMap); setParametersForOAuthClientResponse(mockOAuthClientResponse, accessToken, idToken); when(openIDConnectAuthenticatorDataHolder.getClaimMetadataManagementService()).thenReturn (claimMetadataManagementService); @@ -959,10 +969,10 @@ private void setupTest() throws Exception { when(mockUserRealm.getUserStoreManager()).thenReturn(mockUserStoreManager); when(mockUserStoreManager.getRealmConfiguration()).thenReturn(mockRealmConfiguration); when(mockRealmConfiguration.getUserStoreProperty(IdentityCoreConstants.MULTI_ATTRIBUTE_SEPARATOR)) - .thenReturn(","); + .thenReturn(COMMA_SEPARATOR); mockStatic(IdentityUtil.class); when(IdentityUtil.getServerURL("", false, false)) - .thenReturn("https://localhost:9443"); + .thenReturn(HTTPS_LOCALHOST_9443); mockStatic(ServiceURLBuilder.class); when(ServiceURLBuilder.create()).thenReturn(serviceURLBuilder); @@ -985,7 +995,7 @@ private void mockAuthenticationRequestContext(AuthenticationContext mockAuthenti when(mockAuthenticationContext.getAuthenticatorProperties()).thenReturn(authenticatorProperties); paramValueMap = new HashMap<>(); - when(mockAuthenticationContext.getProperty("oidc:param.map")).thenReturn(paramValueMap); + when(mockAuthenticationContext.getProperty(OIDC_PARAM_MAP_STRING)).thenReturn(paramValueMap); when(mockAuthenticationContext.getContextIdentifier()).thenReturn(""); when(mockAuthenticationContext.getExternalIdP()).thenReturn(getDummyExternalIdPConfig()); when(mockAuthenticationContext.getAuthenticationRequest()).thenReturn(mockAuthenticationRequest); @@ -1081,4 +1091,266 @@ private ExternalIdPConfig getDummyExternalIdPConfig() { identityProvider.setIdentityProviderName("DummyIDPName"); return new ExternalIdPConfig(identityProvider); } + + private ExternalIdPConfig getExternalIdPConfig(String idpName) { + + IdentityProvider identityProvider = new IdentityProvider(); + identityProvider.setIdentityProviderName(idpName); + return new ExternalIdPConfig(identityProvider); + } + + /** + * This method generates test criteria on the query parameter change on sharing the federated token for + * different combination of the IDP configuration value. The adaptive script parameter values are same + * to the query parameter values. + * + * @return Object[][] The test criteria. + */ + @DataProvider(name = "shareFederatedTokenParams") + public Object[][] shareFederatedTokenParams() { + + return new Object[][]{ + {"IDP config is enabled, application requested the federated token, " + + "adaptive script requested the federated token", "true", "true", "true"}, + {"IDP config is disabled, application requested the federated token", "false", "true", "true"}, + {"IDP config is enabled, application did not request the federated token", "true", "false", "false"}, + {"IDP config is disabled, application did not request the federated token", "false", "false", + "false"}, + {"IDP config is disabled, application did not send the parameter", "true", null, null}, + {"IDP config is disabled, application did not send the parameter", "false", null, null}, + {"No IDP config found, application requested the federated token", null, "true", "true"}, + {"No IDP config found, application did not request the federated token", null, "false", "false"}, + {"No IDP config found, application did not send the parameter", null, null, null} + }; + } + + /** + * This method generates test criteria on the adaptive script parameter change on sharing the federated token. + * + * @return Object[][] The test criteria. + */ + @DataProvider(name = "shareFederatedTokenParamsForAdaptiveScriptConfigs") + public Object[][] shareFederatedTokenParamsForAdaptiveScriptConfigs() { + + return new Object[][]{ + {"IDP config is enabled, authorize call requested the federated token," + + " adaptive script requested the federated token", "true", "true", "true"}, + {"IDP config is enabled, authorize call requested the federated token," + + " adaptive script did not request the federated token", "true", "true", "false"}, + {"IDP config is enabled, authorize call requested the federated token," + + " no adaptive script config found", "true", "true", null}, + {"IDP config is disabled, authorize call requested the federated token", "false", "true", "true"}, + {"IDP config is enabled, authorize call did not request the federated token," + + " adaptive script did not request the federated token", "true", "false", "false"}, + {"IDP config is enabled, authorize call did not request the federated token," + + " adaptive script requested the federated token", "true", "false", "true"}, + {"IDP config is enabled, authorize call did not request the federated token," + + " no adaptive script config found", "true", "false", null}, + {"IDP config is disabled, authorize call did not request the federated token", + "false", "false", "false"}, + {"IDP config is disabled, authorize call did not send the parameter," + + " no adaptive script config found", "true", null, null}, + {"IDP config is disabled, authorize call did not send the parameter," + + " adaptive script requested the federated token", "true", null, "true"}, + {"IDP config is disabled, authorize call did not send the parameter," + + " adaptive script did not request the federated token", "true", null, "false"}, + {"IDP config is disabled, authorize call did not send the parameter", "false", null, null}, + {"No IDP config found, authorize call requested the federated token", null, "true", "true"}, + {"No IDP config found, authorize call did not request the federated token", null, "false", "false"}, + {"No IDP config found, authorize call did not send the parameter", null, null, null} + }; + } + + @Test(dataProvider = "shareFederatedTokenParamsForAdaptiveScriptConfigs") + public void testShareFederatedTokenParamsForAdaptiveScriptConfigs(String errorMessage, + String enableShareTokenIDPConfig, + String shareTokenQP, + String shareTokeAdaptiveScriptParam) + throws Exception { + + testShareFederatedTokenForIDPConfigAndQueryParameter(errorMessage, enableShareTokenIDPConfig, + shareTokenQP, shareTokeAdaptiveScriptParam); + } + + @Test(dataProvider = "shareFederatedTokenParams") + public void testShareFederatedTokenForIDPConfigAndQueryParameter(String errorMessage, + String enableShareTokenIDPConfig, + String shareTokenQP, + String shareTokeAdaptiveScriptParam) + throws Exception { + + String authenticator = "Google Calender"; + mockStaticClasses(); + mockServiceVariables(); + + AuthenticationRequest authenticationRequest = new AuthenticationRequest(); + mapQueryParamsToAuthenticationRequest(authenticationRequest, SHARE_FEDERATED_TOKEN_PARAM, shareTokenQP); + AuthenticationContext authenticationContext = + getAuthenticationContext(authenticator, authenticationRequest, enableShareTokenIDPConfig); + addAdaptiveScriptParams(authenticationContext, SHARE_FEDERATED_TOKEN_PARAM, shareTokeAdaptiveScriptParam); + mockIDPAuthentication(); + + openIDConnectAuthenticator.processAuthenticationResponse(mockServletRequest, mockServletResponse, + authenticationContext); + + if (Boolean.parseBoolean(enableShareTokenIDPConfig) && ((StringUtils.isBlank(shareTokeAdaptiveScriptParam) && + Boolean.parseBoolean(shareTokenQP)) || Boolean.parseBoolean(shareTokeAdaptiveScriptParam))) { + assertNotNull(authenticationContext.getProperty(OIDCAuthenticatorConstants.FEDERATED_TOKENS), + errorMessage); + + if (authenticationContext.getProperty(OIDCAuthenticatorConstants.FEDERATED_TOKENS) != null && + authenticationContext.getProperty(OIDCAuthenticatorConstants.FEDERATED_TOKENS) instanceof List) { + List federatedToken = (List) authenticationContext.getProperty( + OIDCAuthenticatorConstants.FEDERATED_TOKENS); + assertEquals(federatedToken.get(0).getAccessToken(), accessToken, "No access token found"); + assertEquals(federatedToken.get(0).getRefreshToken(), refreshToken, "No refresh token found"); + assertEquals(federatedToken.get(0).getTokenValidityPeriod(), expiresIn, "No expiry time found"); + assertEquals(federatedToken.get(0).getScope(), scope, "No scope found"); + } + } else { + assertNull(authenticationContext.getProperty(OIDCAuthenticatorConstants.FEDERATED_TOKENS), + errorMessage); + } + } + + /** + * This method generates an authentication instance for the federated token sharing. + * + * @param authenticator The federated authenticator name. + * @param authenticationRequest The authentication request. + * @param enableShareTokenIDPConfig The IDP config of the enabling the federated token sharing. + * @return AuthenticationContext The generated authentication context for the federated token sharing. + */ + private AuthenticationContext getAuthenticationContext(String authenticator, + AuthenticationRequest authenticationRequest, + String enableShareTokenIDPConfig) { + + AuthenticationContext authenticationContext = new AuthenticationContext(); + authenticationContext.setAuthenticationRequest(authenticationRequest); + authenticationContext.setExternalIdP(getExternalIdPConfig(authenticator)); + authenticationContext.setProperty(OIDC_PARAM_MAP_STRING, paramValueMap); + authenticationContext.setContextIdentifier(""); + + if (StringUtils.isNotBlank(enableShareTokenIDPConfig)) { + authenticatorProperties.put(SHARE_FEDERATED_TOKEN_CONFIG, enableShareTokenIDPConfig); + } + authenticationContext.setAuthenticatorProperties(authenticatorProperties); + + return authenticationContext; + } + + /** + * This method maps the authorize request query parameters for testing. + * + * @param authenticationRequest The authentication request to map the query parameters. + * @param queryParameterName The query parameter name. + * @param queryParameterValue The query parameter value. + */ + private void mapQueryParamsToAuthenticationRequest(AuthenticationRequest authenticationRequest, + String queryParameterName, String queryParameterValue) { + if (StringUtils.isNotBlank(queryParameterValue)) { + String[] queryParam = new String[]{queryParameterValue}; + authenticationRequest.addRequestQueryParam(queryParameterName, queryParam); + } + } + + /** + * This method mocks the adaptive parameters to the authentication context. + * + * @param authenticationContext The authentication context to be tested. + * @param shareTokeAdaptiveScriptParamName The adaptive scrip parameter name. + * @param shareTokeAdaptiveScriptParam The adaptive script parameter value. + */ + private void addAdaptiveScriptParams(AuthenticationContext authenticationContext, + String shareTokeAdaptiveScriptParamName, String shareTokeAdaptiveScriptParam) { + + Map adaptiveScriptParam = new HashMap<>(); + Map> runtimeParams = new HashMap<>(); + authenticationContext.addParameter("RUNTIME_PARAMS", runtimeParams); + runtimeParams.put(OIDCAuthenticatorConstants.AUTHENTICATOR_NAME, adaptiveScriptParam); + + if (StringUtils.isNotBlank(shareTokeAdaptiveScriptParam)) { + adaptiveScriptParam.put(shareTokeAdaptiveScriptParamName, shareTokeAdaptiveScriptParam); + } + } + + /** + * This method mocks the external IDP authentication flow and the value mappings. + * + * @throws Exception Throws the exceptions on error. + */ + private void mockIDPAuthentication() throws Exception { + + IdentityProviderProperty[] identityProviderProperties = getIdentityProviderProperties(); + when(externalIdPConfig.getIdentityProvider()).thenReturn(identityProvider); + when(identityProvider.getIdpProperties()).thenReturn(identityProviderProperties); + whenNew(OAuthClient.class).withAnyArguments().thenReturn(mockOAuthClient); + when(mockOAuthClient.accessToken(Matchers.anyObject())).thenReturn(mockOAuthJSONAccessTokenResponse); + when(mockOAuthJSONAccessTokenResponse.getParam(OIDCAuthenticatorConstants.ACCESS_TOKEN)).thenReturn( + accessToken); + when(mockOAuthJSONAccessTokenResponse.getParam(OIDCAuthenticatorConstants.ID_TOKEN)).thenReturn(idToken); + when(mockOAuthJSONAccessTokenResponse.getParam(OIDCAuthenticatorConstants.REFRESH_TOKEN)).thenReturn( + refreshToken); + when(mockOAuthJSONAccessTokenResponse.getParam(OIDCAuthenticatorConstants.EXPIRES_IN)).thenReturn(expiresIn); + when(mockOAuthJSONAccessTokenResponse.getParam(OIDCAuthenticatorConstants.SCOPE)).thenReturn(scope); + } + + /** + * This method mocks the service variable for the authorization call. + * + * @throws OAuthProblemException Throws on the oAuth flow exception. + * @throws UserStoreException Throws on the user store exception. + * @throws URLBuilderException Throws on the url builder exception. + */ + private void mockServiceVariables() throws OAuthProblemException, UserStoreException, URLBuilderException { + + when(OAuthAuthzResponse.oauthCodeAuthzResponse(mockServletRequest)).thenReturn(mockOAuthzResponse); + when(mockServletRequest.getParameter("domain")).thenReturn(superTenantDomain); + when(mockOAuthzResponse.getCode()).thenReturn("200"); + setParametersForOAuthClientResponse(mockOAuthClientResponse, accessToken, idToken); + when(OpenIDConnectAuthenticatorDataHolder.getInstance()).thenReturn(openIDConnectAuthenticatorDataHolder); + when(openIDConnectAuthenticatorDataHolder.getRealmService()).thenReturn(mockRealmService); + when(mockRealmService.getTenantManager()).thenReturn(mockTenantManger); + when(mockTenantManger.getTenantId(anyString())).thenReturn(TENANT_ID); + when(mockRealmService.getTenantUserRealm(anyInt())).thenReturn(mockUserRealm); + when(mockUserRealm.getUserStoreManager()).thenReturn(mockUserStoreManager); + when(mockUserStoreManager.getRealmConfiguration()).thenReturn(mockRealmConfiguration); + when(mockRealmConfiguration.getUserStoreProperty(IdentityCoreConstants.MULTI_ATTRIBUTE_SEPARATOR)) + .thenReturn(COMMA_SEPARATOR); + when(IdentityUtil.getServerURL("", false, false)) + .thenReturn(HTTPS_LOCALHOST_9443); + when(ServiceURLBuilder.create()).thenReturn(serviceURLBuilder); + when(serviceURLBuilder.addPath(anyString())).thenReturn(serviceURLBuilder); + when(serviceURLBuilder.addParameter(anyString(), anyString())).thenReturn(serviceURLBuilder); + when(serviceURLBuilder.build()).thenReturn(serviceURL); + when(LoggerUtils.isDiagnosticLogsEnabled()).thenReturn(true); + } + + /** + * This method do all the static level mocks. + */ + private void mockStaticClasses() { + + mockStatic(OAuthAuthzResponse.class); + mockStatic(OpenIDConnectAuthenticatorDataHolder.class); + mockStatic(IdentityUtil.class); + mockStatic(ServiceURLBuilder.class); + mockStatic(LoggerUtils.class); + } + + /** + * This method creates the external identity provider properties. + * + * @return IdentityProviderProperty[] The array of the idp properties. + */ + private IdentityProviderProperty[] getIdentityProviderProperties() { + + IdentityProviderProperty property = new IdentityProviderProperty(); + property.setName(IdPManagementConstants.IS_TRUSTED_TOKEN_ISSUER); + property.setValue("false"); + IdentityProviderProperty[] identityProviderProperties = new IdentityProviderProperty[1]; + identityProviderProperties[0] = property; + return identityProviderProperties; + } + } From 473c2f0d7c5775dee2cb9ced70449de004c3184f Mon Sep 17 00:00:00 2001 From: Indeewai Wijesiri Date: Wed, 6 Mar 2024 21:23:10 +0530 Subject: [PATCH 2/8] Addressing review comments on the logger info --- .../oidc/OpenIDConnectAuthenticator.java | 65 ++++++++++++------- .../util/OpenIDConnectAuthenticatorUtil.java | 39 +++++++++++ 2 files changed, 79 insertions(+), 25 deletions(-) create mode 100644 components/org.wso2.carbon.identity.application.authenticator.oidc/src/main/java/org/wso2/carbon/identity/application/authenticator/oidc/util/OpenIDConnectAuthenticatorUtil.java 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 df9e3511..fdeca06f 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 @@ -59,6 +59,7 @@ import org.wso2.carbon.identity.application.authenticator.oidc.model.OIDCStateInfo; import org.wso2.carbon.identity.application.authenticator.oidc.util.OIDCErrorConstants.ErrorMessages; import org.wso2.carbon.identity.application.authenticator.oidc.util.OIDCTokenValidationUtil; +import org.wso2.carbon.identity.application.authenticator.oidc.util.OpenIDConnectAuthenticatorUtil; import org.wso2.carbon.identity.application.common.model.ClaimMapping; import org.wso2.carbon.identity.application.common.model.FederatedAuthenticatorConfig; import org.wso2.carbon.identity.application.common.model.IdentityProvider; @@ -636,7 +637,8 @@ private String addValidScopesForFederatedTokenSharing(AuthenticationContext cont if (StringUtils.isNotBlank(validScopesForTokenSharing)) { if (LOG.isDebugEnabled()) { - LOG.debug("Valid scopes matched for federated token sharing: " + validScopesForTokenSharing); + LOG.debug("Valid scopes matched for federated token sharing: " + validScopesForTokenSharing + + ", IDP: " + OpenIDConnectAuthenticatorUtil.getFederatedAuthenticatorName(context)); } scopes = (scopes + SPACE + validScopesForTokenSharing).trim(); } @@ -650,19 +652,23 @@ private String addValidScopesForFederatedTokenSharing(AuthenticationContext cont * @return The scopes requested by the application for token sharing. */ private String getRequestedScopesForTokenSharing(AuthenticationContext context) { + + String authenticatorName = OpenIDConnectAuthenticatorUtil.getFederatedAuthenticatorName(context); // The first priority is given to the parameters setup at the adaptive script. Then the query parameters. String requestedScopesViaAdaptiveScript = getAdaptiveScriptValues(context, OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE); // Checks if there exists scopes requested via adaptive script. if (StringUtils.isNotBlank(requestedScopesViaAdaptiveScript)) { if (LOG.isDebugEnabled()) { - LOG.debug("Adaptive script parameter found for " + OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE); + LOG.debug("Adaptive script parameter found for " + OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE + + " in federated token sharing, IDP: " + authenticatorName); } return requestedScopesViaAdaptiveScript; } else { if (LOG.isDebugEnabled()) { - LOG.debug("No adaptive script parameter found for " - + OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE + " hence checking the query parameters."); + LOG.debug("No adaptive script parameter found for " + + OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE + " in federated token sharing, IDP: " + + authenticatorName + " hence checking the query parameters."); } // Checks the scopes requested via authorize request query parameters. return getRequestedScopedViaQueryParams(context); @@ -679,11 +685,11 @@ private String getRequestedScopesForTokenSharing(AuthenticationContext context) private String getAdaptiveScriptValues(AuthenticationContext context, String authenticatorParamName) { Map runtimeParams = this.getRuntimeParams(context); - String requestedScopesViaAdaptiveScript = null; + String requestedParamViaAdaptiveScript = null; if (runtimeParams != null) { - requestedScopesViaAdaptiveScript = runtimeParams.get(authenticatorParamName); + requestedParamViaAdaptiveScript = runtimeParams.get(authenticatorParamName); } - return requestedScopesViaAdaptiveScript; + return requestedParamViaAdaptiveScript; } /** @@ -703,7 +709,7 @@ private String getRequestedScopedViaQueryParams(AuthenticationContext context) { if (context.getExternalIdP() == null) { if (LOG.isDebugEnabled()) { - LOG.debug("No external IDP found in the authentication context. " + + LOG.debug("No external IDP found in the authentication context for federated token sharing. " + "Cannot retrieve the query parameters."); } return null; @@ -712,7 +718,7 @@ private String getRequestedScopedViaQueryParams(AuthenticationContext context) { String authenticatorName = context.getExternalIdP().getName(); if (StringUtils.isBlank(authenticatorName)) { if (LOG.isDebugEnabled()) { - LOG.debug("No external IDP name found in the authentication context. " + + LOG.debug("No external IDP name found in the authentication context for federated token sharing. " + "Cannot retrieve the query parameters."); } return null; @@ -723,7 +729,8 @@ private String getRequestedScopedViaQueryParams(AuthenticationContext context) { if (StringUtils.isBlank(scopeString) || !scopeString.contains(SEMI_COLON_DELIMITER)) { if (LOG.isDebugEnabled()) { LOG.debug("Query parameter " + OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE + " is missing " + - SEMI_COLON_DELIMITER + " delimiter or the value is empty."); + SEMI_COLON_DELIMITER + " delimiter or the value is empty in federated token sharing, IDP: " + + authenticatorName); } return null; } @@ -736,17 +743,18 @@ private String getRequestedScopedViaQueryParams(AuthenticationContext context) { if (ArrayUtils.isNotEmpty(scopes) && scopes.length == 2 && authenticatorName.equals(scopes[0])) { filteredScopes.append(scopes[1]).append(SPACE); if (LOG.isDebugEnabled()) { - LOG.debug( - "Scopes found for the authenticator " + authenticatorName + " in the query parameter " + - OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE + " : " + scopes[1]); + LOG.debug("Scopes found for the authenticator " + authenticatorName + " in the query parameter " + + OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE + " : " + scopes[1] + + "in federated token sharing, IDP: " + authenticatorName); } } } String requestedScopes = filteredScopes.toString(); if (LOG.isDebugEnabled() && StringUtils.isBlank(requestedScopes)) { - LOG.debug("No valid values found for the authenticator " + authenticatorName + - " in the query parameter " + OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE); + LOG.debug("No valid values found for the authenticator " + authenticatorName + " in the query parameter " + + OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE + " for federated token sharing, IDP: " + + authenticatorName); } return filteredScopes.toString(); @@ -761,20 +769,22 @@ private String getRequestedScopedViaQueryParams(AuthenticationContext context) { */ private boolean requestedToShareFederatedToken(AuthenticationContext context) { + String authenticatorName = OpenIDConnectAuthenticatorUtil.getFederatedAuthenticatorName(context); // The first priority is given to the parameters setup at the adaptive script. Then the query parameters. String shareFederatedToken = getAdaptiveScriptValues(context, OIDCAuthenticatorConstants.SHARE_FEDERATED_TOKEN_PARAM); if (LOG.isDebugEnabled() && StringUtils.isNotBlank(shareFederatedToken)) { LOG.debug("Adaptive script parameter " + OIDCAuthenticatorConstants.SHARE_FEDERATED_TOKEN_PARAM + - " found for the federated token sharing. "); + " found for federated token sharing, IDP: " + authenticatorName); } // Checks if the token sharing is requested via adaptive script. if (StringUtils.isBlank(shareFederatedToken)) { // Checks if the token sharing is requested via authorize request query parameters. if (LOG.isDebugEnabled()) { LOG.debug("No adaptive script parameter " + OIDCAuthenticatorConstants.SHARE_FEDERATED_TOKEN_PARAM + - " found for the federated token sharing. Checking the query parameters."); + " found for federated token sharing, IDP: " + authenticatorName + + " Checking the query parameters."); } shareFederatedToken = getQueryParameter(context, OIDCAuthenticatorConstants.SHARE_FEDERATED_TOKEN_PARAM); } @@ -790,22 +800,26 @@ private boolean requestedToShareFederatedToken(AuthenticationContext context) { */ private String getQueryParameter(AuthenticationContext context, String queryParamName) { + String authenticatorName = OpenIDConnectAuthenticatorUtil.getFederatedAuthenticatorName(context); AuthenticationRequest authenticationRequest = context.getAuthenticationRequest(); if (authenticationRequest == null || StringUtils.isBlank(queryParamName)) { if (LOG.isDebugEnabled()) { - LOG.debug("Invalid authentication request or invalid query parameter name : " + queryParamName); + LOG.debug("Invalid authentication request or invalid query parameter name : " + queryParamName + + " for federated token sharing, IDP: " + authenticatorName); } return null; } String[] queryParamValues = authenticationRequest.getRequestQueryParam(queryParamName); if (ArrayUtils.isNotEmpty(queryParamValues)) { if (LOG.isDebugEnabled()) { - LOG.debug("Query parameter found for," + queryParamName); + LOG.debug("Query parameter found for," + queryParamName + " in federated token sharing, IDP: " + + authenticatorName); } return queryParamValues[0]; } if (LOG.isDebugEnabled()) { - LOG.debug("No value found for the query parameter : " + queryParamName); + LOG.debug("No value found for the query parameter : " + queryParamName + + " in federated token sharing, IDP: " + authenticatorName); } return null; } @@ -1020,6 +1034,7 @@ protected void mapAccessToken(HttpServletRequest request, AuthenticationContext * @param oAuthResponse The OAuth client response. */ private void addFederatedTokensToContext(AuthenticationContext context, OAuthClientResponse oAuthResponse) { + /* Federated tokens are added only of the authenticator configuration ShareFederatedToken is enabled and the application has requested the federated token. @@ -1039,10 +1054,7 @@ private void addFederatedTokensToContext(AuthenticationContext context, OAuthCli federatedTokens = new ArrayList<>(); } - String identityProviderName = context.getCurrentAuthenticator(); - if (context.getExternalIdP() != null) { - identityProviderName = context.getExternalIdP().getIdPName(); - } + String identityProviderName = OpenIDConnectAuthenticatorUtil.getFederatedAuthenticatorName(context); FederatedToken federatedToken = new FederatedToken(identityProviderName, oAuthResponse.getParam(OIDCAuthenticatorConstants.ACCESS_TOKEN)); @@ -1052,6 +1064,9 @@ private void addFederatedTokensToContext(AuthenticationContext context, OAuthCli federatedTokens.add(federatedToken); context.setProperty(OIDCAuthenticatorConstants.FEDERATED_TOKENS, federatedTokens); + if (LOG.isDebugEnabled()) { + LOG.debug("Federated tokens added to the authentication context, IDP: " + identityProviderName); + } } /** @@ -1075,7 +1090,7 @@ private String validateScopeForTokenSharing(String allowedScope, String requeste return StringUtils.join(subset, SPACE); } else { if (LOG.isDebugEnabled()) { - LOG.debug("No valid scopes matched for federated token sharing."); + LOG.debug("No valid scopes found for federated token sharing."); } } } diff --git a/components/org.wso2.carbon.identity.application.authenticator.oidc/src/main/java/org/wso2/carbon/identity/application/authenticator/oidc/util/OpenIDConnectAuthenticatorUtil.java b/components/org.wso2.carbon.identity.application.authenticator.oidc/src/main/java/org/wso2/carbon/identity/application/authenticator/oidc/util/OpenIDConnectAuthenticatorUtil.java new file mode 100644 index 00000000..9a81eee8 --- /dev/null +++ b/components/org.wso2.carbon.identity.application.authenticator.oidc/src/main/java/org/wso2/carbon/identity/application/authenticator/oidc/util/OpenIDConnectAuthenticatorUtil.java @@ -0,0 +1,39 @@ +/* + * + * Copyright (c) 2024, WSO2 LLC. (http://www.wso2.com). All Rights Reserved. + * + * This software is the property of WSO2 LLC. and its suppliers, if any. + * Dissemination of any information or reproduction of any material contained + * herein in any form is strictly forbidden, unless permitted by WSO2 expressly. + * You may not alter or remove any copyright or other notice from copies of this content. + * + */ + +package org.wso2.carbon.identity.application.authenticator.oidc.util; + +import org.apache.commons.lang.StringUtils; +import org.wso2.carbon.identity.application.authentication.framework.context.AuthenticationContext; + +/** + * + */ +public class OpenIDConnectAuthenticatorUtil { + + /** + * This method returns the current federated authenticator name. If there is no external IdP, then the current + * authenticator name is returned. + * + * @param context Authentication context. + * @return Federated authenticator name. + */ + public static String getFederatedAuthenticatorName(AuthenticationContext context) { + + if (context == null) { + return StringUtils.EMPTY; + } + if (context.getExternalIdP() == null) { + return context.getCurrentAuthenticator(); + } + return context.getExternalIdP().getIdPName(); + } +} From 1fa1fba4c6641e23ef137cad4282ea9cf384b8f4 Mon Sep 17 00:00:00 2001 From: Indeewai Wijesiri Date: Wed, 6 Mar 2024 21:31:55 +0530 Subject: [PATCH 3/8] Addressing the review comments --- .../oidc/OpenIDConnectAuthenticator.java | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 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 fdeca06f..b8eb3637 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 @@ -637,7 +637,7 @@ private String addValidScopesForFederatedTokenSharing(AuthenticationContext cont if (StringUtils.isNotBlank(validScopesForTokenSharing)) { if (LOG.isDebugEnabled()) { - LOG.debug("Valid scopes matched for federated token sharing: " + validScopesForTokenSharing + + LOG.debug("Valid scopes found for federated token sharing: " + validScopesForTokenSharing + ", IDP: " + OpenIDConnectAuthenticatorUtil.getFederatedAuthenticatorName(context)); } scopes = (scopes + SPACE + validScopesForTokenSharing).trim(); @@ -1071,11 +1071,11 @@ private void addFederatedTokensToContext(AuthenticationContext context, OAuthCli /** * This returns the intersection of the allowed scopes defined at the IDP configuration and the requested scopes - * from the application side for federated toke sharing. + * from the application side for federated token sharing. * - * @param allowedScope The administrator defined scopes in the IDP configuration for federated toke sharing. - * @param requestedScope The application side requested scopes for federated toke sharing. - * @return The intersection of the allowed and the requested scopes for federated toke sharing. + * @param allowedScope The administrator defined scopes in the IDP configuration for federated token sharing. + * @param requestedScope The application side requested scopes for federated token sharing. + * @return The intersection of the allowed and the requested scopes for federated token sharing. */ private String validateScopeForTokenSharing(String allowedScope, String requestedScope) { @@ -1088,10 +1088,9 @@ private String validateScopeForTokenSharing(String allowedScope, String requeste if (CollectionUtils.isNotEmpty(subset)) { return StringUtils.join(subset, SPACE); - } else { - if (LOG.isDebugEnabled()) { - LOG.debug("No valid scopes found for federated token sharing."); - } + } + if (LOG.isDebugEnabled()) { + LOG.debug("No valid scopes found for federated token sharing."); } } return null; From 448389ab2384a393db6ba46a9f52c80361477960 Mon Sep 17 00:00:00 2001 From: Indeewai Wijesiri Date: Fri, 8 Mar 2024 06:53:24 +0530 Subject: [PATCH 4/8] Addressing review comments on refactoring --- .../oidc/OIDCAuthenticatorConstants.java | 1 - .../oidc/OpenIDConnectAuthenticator.java | 217 +++++++++++------- .../util/OpenIDConnectAuthenticatorUtil.java | 39 ---- .../oidc/OpenIDConnectAuthenticatorTest.java | 22 +- 4 files changed, 147 insertions(+), 132 deletions(-) delete mode 100644 components/org.wso2.carbon.identity.application.authenticator.oidc/src/main/java/org/wso2/carbon/identity/application/authenticator/oidc/util/OpenIDConnectAuthenticatorUtil.java 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 5e709a77..ebc84f65 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 @@ -33,7 +33,6 @@ public class OIDCAuthenticatorConstants { public static final String CLIENT_ID_PARAM = "clientId"; public static final String REFRESH_TOKEN = "refresh_token"; public static final String EXPIRES_IN = "expires_in"; - public static final String FEDERATED_TOKENS = "federated_tokens"; public static final String SHARE_FEDERATED_TOKEN_CONFIG = "ShareFederatedToken"; public static final String SHARE_FEDERATED_TOKEN_PARAM = "share_federated_token"; public static final String FEDERATED_TOKEN_ALLOWED_SCOPE = "FederatedTokenAllowedScope"; 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 b8eb3637..dda7a3d1 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 @@ -59,7 +59,6 @@ import org.wso2.carbon.identity.application.authenticator.oidc.model.OIDCStateInfo; import org.wso2.carbon.identity.application.authenticator.oidc.util.OIDCErrorConstants.ErrorMessages; import org.wso2.carbon.identity.application.authenticator.oidc.util.OIDCTokenValidationUtil; -import org.wso2.carbon.identity.application.authenticator.oidc.util.OpenIDConnectAuthenticatorUtil; import org.wso2.carbon.identity.application.common.model.ClaimMapping; import org.wso2.carbon.identity.application.common.model.FederatedAuthenticatorConfig; import org.wso2.carbon.identity.application.common.model.IdentityProvider; @@ -142,14 +141,29 @@ public class OpenIDConnectAuthenticator extends AbstractApplicationAuthenticator private static final String DYNAMIC_PARAMETER_LOOKUP_REGEX = "\\$\\{(\\w+)\\}"; private static final String IS_API_BASED = "IS_API_BASED"; private static final String REDIRECT_URL = "REDIRECT_URL"; - public static final String SPACE_REGEX = "\\s+"; - public static final String SPACE = " "; - public static final String SEMI_COLON_DELIMITER = ";"; - public static final String COMMA_DELIMITER = ","; + private static final String SPACE_REGEX = "\\s+"; + private static final String SPACE = " "; + private static final String SEMI_COLON_DELIMITER = ";"; + private static final String COMMA_DELIMITER = ","; private static Pattern pattern = Pattern.compile(DYNAMIC_PARAMETER_LOOKUP_REGEX); private static final String[] NON_USER_ATTRIBUTES = new String[]{"at_hash", "iss", "iat", "exp", "aud", "azp"}; private static final String AUTHENTICATOR_MESSAGE = "authenticatorMessage"; + /** + * This method returns the current federated authenticator name. If there is no external IdP, then the current + * authenticator name is returned. + * + * @param context Authentication context. + * @return Federated authenticator name. + */ + private String getFederatedAuthenticatorName(AuthenticationContext context) { + + if (context == null || context.getExternalIdP() == null) { + return StringUtils.EMPTY; + } + return context.getExternalIdP().getIdPName(); + } + @Override public AuthenticatorFlowStatus process(HttpServletRequest request, HttpServletResponse response, AuthenticationContext context) @@ -491,8 +505,15 @@ protected void initiateAuthenticationRequest(HttpServletRequest request, HttpSer String scopes = getScope(authenticatorProperties); - // Adding the scopes requested by the application side for token sharing. - scopes = addValidScopesForFederatedTokenSharing(context, authenticatorProperties, scopes); + /* + The scopes for the federated tokens are evaluated only if the authenticator + configuration ShareFederatedToken is enabled and the application has requested the federated token. + */ + if (Boolean.parseBoolean(authenticatorProperties.get(SHARE_FEDERATED_TOKEN_CONFIG)) && + requestedToShareFederatedToken(context)) { + // Adding the scopes requested by the application side for token sharing. + scopes = addValidScopesForFederatedTokenSharing(context, authenticatorProperties, scopes); + } String queryString = getQueryString(authenticatorProperties); if (StringUtils.isNotBlank(scopes)) { @@ -618,33 +639,59 @@ protected void initiateAuthenticationRequest(HttpServletRequest request, HttpSer */ private String addValidScopesForFederatedTokenSharing(AuthenticationContext context, Map authenticatorProperties, String scopes) { - /* - The scopes for the federated tokens are evaluated only of the authenticator configuration ShareFederatedToken - is enabled and the application has requested the federated token. - */ - if (!Boolean.parseBoolean(authenticatorProperties.get(SHARE_FEDERATED_TOKEN_CONFIG)) || - !requestedToShareFederatedToken(context)) { - return scopes; - } // Get the application requested scopes for the federated tokens. String requestedScopesForTokenSharing = getRequestedScopesForTokenSharing(context); // Validating the application requested scopes by the authenticator allowed scopes for federated token sharing. - String validScopesForTokenSharing = validateScopeForTokenSharing( + Set validScopesForTokenSharing = validateScopeForTokenSharing( authenticatorProperties.get(OIDCAuthenticatorConstants.FEDERATED_TOKEN_ALLOWED_SCOPE), requestedScopesForTokenSharing); - if (StringUtils.isNotBlank(validScopesForTokenSharing)) { + if (CollectionUtils.isEmpty(validScopesForTokenSharing)) { if (LOG.isDebugEnabled()) { - LOG.debug("Valid scopes found for federated token sharing: " + validScopesForTokenSharing + - ", IDP: " + OpenIDConnectAuthenticatorUtil.getFederatedAuthenticatorName(context)); + LOG.debug("No matching scopes found for federated token sharing."); } - scopes = (scopes + SPACE + validScopesForTokenSharing).trim(); + return scopes; + } + + if (LOG.isDebugEnabled()) { + LOG.debug("Valid scopes found for federated token sharing: " + validScopesForTokenSharing + + ", IDP: " + getFederatedAuthenticatorName(context)); + } + /* + Remove the duplicate scopes among the validated scopes for federated token sharing and the existing scopes + of the authenticators. + */ + scopes = removeDuplicateScopes(scopes, validScopesForTokenSharing); + if (LOG.isDebugEnabled()) { + LOG.debug("The scopes for the IDP: " + getFederatedAuthenticatorName(context) + " : " + scopes + + " after considering federated token sharing."); } return scopes; } + /** + * This method is used to remove the duplicate scopes. + * + * @param scopes The scopes defined in the authenticator. i.e. "openid email profile" + * @param validScopesForTokenSharing The validated scopes requested by the application and the allowed scopes + * for the token sharing. + * @return The scopes after removing the duplicate scopes. + */ + private String removeDuplicateScopes(String scopes, Set validScopesForTokenSharing) { + + if (StringUtils.isBlank(scopes)) { + scopes = StringUtils.join(validScopesForTokenSharing, SPACE); + } + + Set scopeSet = new HashSet<>(Arrays.asList(scopes.split(SPACE_REGEX))); + scopeSet.addAll(validScopesForTokenSharing); + + scopes = StringUtils.join(scopeSet, SPACE); + return scopes; + } + /** * This method returns the scopes requested by the application for the federated tokens. * @@ -653,8 +700,8 @@ private String addValidScopesForFederatedTokenSharing(AuthenticationContext cont */ private String getRequestedScopesForTokenSharing(AuthenticationContext context) { - String authenticatorName = OpenIDConnectAuthenticatorUtil.getFederatedAuthenticatorName(context); - // The first priority is given to the parameters setup at the adaptive script. Then the query parameters. + String authenticatorName = getFederatedAuthenticatorName(context); + // The first priority is given to the parameters passed from the adaptive script. Then the query parameters. String requestedScopesViaAdaptiveScript = getAdaptiveScriptValues(context, OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE); // Checks if there exists scopes requested via adaptive script. @@ -665,13 +712,15 @@ private String getRequestedScopesForTokenSharing(AuthenticationContext context) } return requestedScopesViaAdaptiveScript; } else { - if (LOG.isDebugEnabled()) { - LOG.debug("No adaptive script parameter found for " + - OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE + " in federated token sharing, IDP: " + - authenticatorName + " hence checking the query parameters."); + String requestedScopesViaQueryParams = getRequestedScopesViaQueryParams(context); + if (LOG.isDebugEnabled() && StringUtils.isNotBlank(requestedScopesViaQueryParams)) { + LOG.debug("No adaptive script parameter: " + OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE + + " found. Query parameter: " + OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE + + " value: " + requestedScopesViaQueryParams + " found for federated token sharing, IDP: " + + authenticatorName); } // Checks the scopes requested via authorize request query parameters. - return getRequestedScopedViaQueryParams(context); + return requestedScopesViaQueryParams; } } @@ -685,11 +734,10 @@ private String getRequestedScopesForTokenSharing(AuthenticationContext context) private String getAdaptiveScriptValues(AuthenticationContext context, String authenticatorParamName) { Map runtimeParams = this.getRuntimeParams(context); - String requestedParamViaAdaptiveScript = null; if (runtimeParams != null) { - requestedParamViaAdaptiveScript = runtimeParams.get(authenticatorParamName); + return runtimeParams.get(authenticatorParamName); } - return requestedParamViaAdaptiveScript; + return StringUtils.EMPTY; } /** @@ -705,31 +753,37 @@ private String getAdaptiveScriptValues(AuthenticationContext context, String aut * @param context The authentication context with authentication request having the query parameters. * @return The scopes requested by the application via the query parameters for federated token sharing. */ - private String getRequestedScopedViaQueryParams(AuthenticationContext context) { + private String getRequestedScopesViaQueryParams(AuthenticationContext context) { - if (context.getExternalIdP() == null) { + String authenticatorName = getFederatedAuthenticatorName(context); + if (StringUtils.isBlank(authenticatorName)) { if (LOG.isDebugEnabled()) { - LOG.debug("No external IDP found in the authentication context for federated token sharing. " + + LOG.debug("No external IDP name found in the authentication context for federated token sharing. " + "Cannot retrieve the query parameters."); } return null; } - String authenticatorName = context.getExternalIdP().getName(); - if (StringUtils.isBlank(authenticatorName)) { + String scopeString = getQueryParameter(context, OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE); + if (StringUtils.isBlank(scopeString)) { if (LOG.isDebugEnabled()) { - LOG.debug("No external IDP name found in the authentication context for federated token sharing. " + - "Cannot retrieve the query parameters."); + LOG.debug("No query parameter " + OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE + + " found in federated token sharing, IDP: " + authenticatorName); } return null; } - - String scopeString = getQueryParameter(context, OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE); - // The optional scopes should come with the authenticator name only. - if (StringUtils.isBlank(scopeString) || !scopeString.contains(SEMI_COLON_DELIMITER)) { + /* + The requested scopes for particular authenticator should come with the authenticator name separated + by a semicolon. + i.e. A valid string: + When Google Calender has read write scopes and Microsoft Authenticator has https://googleapis.calender scope + A valid requested scopes string: + federated_token_scope=Google Calander;read write,Microsoft Authenticator;https://googleapis.calender + */ + if (!scopeString.contains(SEMI_COLON_DELIMITER)) { if (LOG.isDebugEnabled()) { LOG.debug("Query parameter " + OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE + " is missing " + - SEMI_COLON_DELIMITER + " delimiter or the value is empty in federated token sharing, IDP: " + + SEMI_COLON_DELIMITER + " delimiter in federated token sharing, IDP: " + authenticatorName); } return null; @@ -740,13 +794,8 @@ private String getRequestedScopedViaQueryParams(AuthenticationContext context) { for (String scopesFollowedByAuthenticator : scopeSegments) { String[] scopes = StringUtils.split(scopesFollowedByAuthenticator, SEMI_COLON_DELIMITER); - if (ArrayUtils.isNotEmpty(scopes) && scopes.length == 2 && authenticatorName.equals(scopes[0])) { + if (ArrayUtils.getLength(scopes) == 2 && authenticatorName.equals(scopes[0])) { filteredScopes.append(scopes[1]).append(SPACE); - if (LOG.isDebugEnabled()) { - LOG.debug("Scopes found for the authenticator " + authenticatorName + " in the query parameter " + - OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE + " : " + scopes[1] + - "in federated token sharing, IDP: " + authenticatorName); - } } } @@ -769,7 +818,7 @@ private String getRequestedScopedViaQueryParams(AuthenticationContext context) { */ private boolean requestedToShareFederatedToken(AuthenticationContext context) { - String authenticatorName = OpenIDConnectAuthenticatorUtil.getFederatedAuthenticatorName(context); + String authenticatorName = getFederatedAuthenticatorName(context); // The first priority is given to the parameters setup at the adaptive script. Then the query parameters. String shareFederatedToken = getAdaptiveScriptValues(context, OIDCAuthenticatorConstants.SHARE_FEDERATED_TOKEN_PARAM); @@ -781,12 +830,13 @@ private boolean requestedToShareFederatedToken(AuthenticationContext context) { // Checks if the token sharing is requested via adaptive script. if (StringUtils.isBlank(shareFederatedToken)) { // Checks if the token sharing is requested via authorize request query parameters. + shareFederatedToken = getQueryParameter(context, OIDCAuthenticatorConstants.SHARE_FEDERATED_TOKEN_PARAM); if (LOG.isDebugEnabled()) { - LOG.debug("No adaptive script parameter " + OIDCAuthenticatorConstants.SHARE_FEDERATED_TOKEN_PARAM + - " found for federated token sharing, IDP: " + authenticatorName + - " Checking the query parameters."); + LOG.debug("No adaptive script parameter: " + OIDCAuthenticatorConstants.SHARE_FEDERATED_TOKEN_PARAM + + " found. Query parameter: " + OIDCAuthenticatorConstants.SHARE_FEDERATED_TOKEN_PARAM + + " value: " + shareFederatedToken + " found for federated token sharing, IDP: " + + authenticatorName); } - shareFederatedToken = getQueryParameter(context, OIDCAuthenticatorConstants.SHARE_FEDERATED_TOKEN_PARAM); } return Boolean.parseBoolean(shareFederatedToken); } @@ -800,7 +850,7 @@ private boolean requestedToShareFederatedToken(AuthenticationContext context) { */ private String getQueryParameter(AuthenticationContext context, String queryParamName) { - String authenticatorName = OpenIDConnectAuthenticatorUtil.getFederatedAuthenticatorName(context); + String authenticatorName = getFederatedAuthenticatorName(context); AuthenticationRequest authenticationRequest = context.getAuthenticationRequest(); if (authenticationRequest == null || StringUtils.isBlank(queryParamName)) { if (LOG.isDebugEnabled()) { @@ -875,8 +925,17 @@ protected void processAuthenticationResponse(HttpServletRequest request, HttpSer // TODO : return access token and id token to framework mapAccessToken(request, context, oAuthResponse); - // Adding the federated tokens to the context for token sharing. - addFederatedTokensToContext(context, oAuthResponse); + /* + Federated tokens are added only of the authenticator configuration ShareFederatedToken is enabled and the + application has requested the federated token. + */ + if (context.getAuthenticatorProperties() != null && Boolean.parseBoolean( + context.getAuthenticatorProperties().get(OIDCAuthenticatorConstants.SHARE_FEDERATED_TOKEN_CONFIG)) && + requestedToShareFederatedToken(context)) { + // Adding the federated tokens to the context for token sharing. + addFederatedTokensToContext(context, oAuthResponse); + } + String idToken = mapIdToken(context, request, oAuthResponse); @@ -1035,26 +1094,16 @@ protected void mapAccessToken(HttpServletRequest request, AuthenticationContext */ private void addFederatedTokensToContext(AuthenticationContext context, OAuthClientResponse oAuthResponse) { - /* - Federated tokens are added only of the authenticator configuration ShareFederatedToken is enabled and the - application has requested the federated token. - */ - if (context.getAuthenticatorProperties() == null || !Boolean.parseBoolean( - context.getAuthenticatorProperties().get(OIDCAuthenticatorConstants.SHARE_FEDERATED_TOKEN_CONFIG)) || - !requestedToShareFederatedToken(context)) { - return; - } - // If there is an existing list of federated tokens obtained in a previous step, utilizing the same list. List federatedTokens; - Object federatedTokensObj = context.getProperty(OIDCAuthenticatorConstants.FEDERATED_TOKENS); + Object federatedTokensObj = context.getProperty(FrameworkConstants.FEDERATED_TOKENS); if (federatedTokensObj instanceof List) { federatedTokens = (List) federatedTokensObj; } else { federatedTokens = new ArrayList<>(); } - String identityProviderName = OpenIDConnectAuthenticatorUtil.getFederatedAuthenticatorName(context); + String identityProviderName = getFederatedAuthenticatorName(context); FederatedToken federatedToken = new FederatedToken(identityProviderName, oAuthResponse.getParam(OIDCAuthenticatorConstants.ACCESS_TOKEN)); @@ -1063,7 +1112,7 @@ private void addFederatedTokensToContext(AuthenticationContext context, OAuthCli federatedToken.setScope(oAuthResponse.getParam(OIDCAuthenticatorConstants.SCOPE)); federatedTokens.add(federatedToken); - context.setProperty(OIDCAuthenticatorConstants.FEDERATED_TOKENS, federatedTokens); + context.setProperty(FrameworkConstants.FEDERATED_TOKENS, federatedTokens); if (LOG.isDebugEnabled()) { LOG.debug("Federated tokens added to the authentication context, IDP: " + identityProviderName); } @@ -1075,25 +1124,29 @@ private void addFederatedTokensToContext(AuthenticationContext context, OAuthCli * * @param allowedScope The administrator defined scopes in the IDP configuration for federated token sharing. * @param requestedScope The application side requested scopes for federated token sharing. - * @return The intersection of the allowed and the requested scopes for federated token sharing. + * @return The intersection of the allowed and the requested scopes for federated token sharing as a set of list. */ - private String validateScopeForTokenSharing(String allowedScope, String requestedScope) { - - if (StringUtils.isNotBlank(allowedScope) && StringUtils.isNotBlank(requestedScope)) { - Set allowedScopesSet = new HashSet<>(Arrays.asList(allowedScope.split(SPACE_REGEX))); - Set requestedScopesSet = new HashSet<>(Arrays.asList(requestedScope.split(SPACE_REGEX))); + private Set validateScopeForTokenSharing(String allowedScope, String requestedScope) { - Set subset = new HashSet<>(requestedScopesSet); - subset.retainAll(allowedScopesSet); - - if (CollectionUtils.isNotEmpty(subset)) { - return StringUtils.join(subset, SPACE); + if (StringUtils.isBlank(allowedScope)) { + if (LOG.isDebugEnabled()) { + LOG.debug("No scopes are allowed for federated token sharing."); } + return null; + } + if (StringUtils.isBlank(requestedScope)) { if (LOG.isDebugEnabled()) { - LOG.debug("No valid scopes found for federated token sharing."); + LOG.debug("No scopes are requested for federated token sharing."); } + return null; } - return null; + Set allowedScopesSet = new HashSet<>(Arrays.asList(allowedScope.split(SPACE_REGEX))); + Set requestedScopesSet = new HashSet<>(Arrays.asList(requestedScope.split(SPACE_REGEX))); + + Set subset = new HashSet<>(requestedScopesSet); + subset.retainAll(allowedScopesSet); + + return subset; } /** diff --git a/components/org.wso2.carbon.identity.application.authenticator.oidc/src/main/java/org/wso2/carbon/identity/application/authenticator/oidc/util/OpenIDConnectAuthenticatorUtil.java b/components/org.wso2.carbon.identity.application.authenticator.oidc/src/main/java/org/wso2/carbon/identity/application/authenticator/oidc/util/OpenIDConnectAuthenticatorUtil.java deleted file mode 100644 index 9a81eee8..00000000 --- a/components/org.wso2.carbon.identity.application.authenticator.oidc/src/main/java/org/wso2/carbon/identity/application/authenticator/oidc/util/OpenIDConnectAuthenticatorUtil.java +++ /dev/null @@ -1,39 +0,0 @@ -/* - * - * Copyright (c) 2024, WSO2 LLC. (http://www.wso2.com). All Rights Reserved. - * - * This software is the property of WSO2 LLC. and its suppliers, if any. - * Dissemination of any information or reproduction of any material contained - * herein in any form is strictly forbidden, unless permitted by WSO2 expressly. - * You may not alter or remove any copyright or other notice from copies of this content. - * - */ - -package org.wso2.carbon.identity.application.authenticator.oidc.util; - -import org.apache.commons.lang.StringUtils; -import org.wso2.carbon.identity.application.authentication.framework.context.AuthenticationContext; - -/** - * - */ -public class OpenIDConnectAuthenticatorUtil { - - /** - * This method returns the current federated authenticator name. If there is no external IdP, then the current - * authenticator name is returned. - * - * @param context Authentication context. - * @return Federated authenticator name. - */ - public static String getFederatedAuthenticatorName(AuthenticationContext context) { - - if (context == null) { - return StringUtils.EMPTY; - } - if (context.getExternalIdP() == null) { - return context.getCurrentAuthenticator(); - } - return context.getExternalIdP().getIdPName(); - } -} 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 caf63915..171de488 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 @@ -1164,18 +1164,18 @@ public Object[][] shareFederatedTokenParamsForAdaptiveScriptConfigs() { @Test(dataProvider = "shareFederatedTokenParamsForAdaptiveScriptConfigs") public void testShareFederatedTokenParamsForAdaptiveScriptConfigs(String errorMessage, String enableShareTokenIDPConfig, - String shareTokenQP, + String shareTokenQueryParameter, String shareTokeAdaptiveScriptParam) throws Exception { testShareFederatedTokenForIDPConfigAndQueryParameter(errorMessage, enableShareTokenIDPConfig, - shareTokenQP, shareTokeAdaptiveScriptParam); + shareTokenQueryParameter, shareTokeAdaptiveScriptParam); } @Test(dataProvider = "shareFederatedTokenParams") public void testShareFederatedTokenForIDPConfigAndQueryParameter(String errorMessage, String enableShareTokenIDPConfig, - String shareTokenQP, + String shareTokenQueryParameter, String shareTokeAdaptiveScriptParam) throws Exception { @@ -1184,7 +1184,8 @@ public void testShareFederatedTokenForIDPConfigAndQueryParameter(String errorMes mockServiceVariables(); AuthenticationRequest authenticationRequest = new AuthenticationRequest(); - mapQueryParamsToAuthenticationRequest(authenticationRequest, SHARE_FEDERATED_TOKEN_PARAM, shareTokenQP); + mapQueryParamsToAuthenticationRequest(authenticationRequest, SHARE_FEDERATED_TOKEN_PARAM, + shareTokenQueryParameter); AuthenticationContext authenticationContext = getAuthenticationContext(authenticator, authenticationRequest, enableShareTokenIDPConfig); addAdaptiveScriptParams(authenticationContext, SHARE_FEDERATED_TOKEN_PARAM, shareTokeAdaptiveScriptParam); @@ -1194,21 +1195,22 @@ public void testShareFederatedTokenForIDPConfigAndQueryParameter(String errorMes authenticationContext); if (Boolean.parseBoolean(enableShareTokenIDPConfig) && ((StringUtils.isBlank(shareTokeAdaptiveScriptParam) && - Boolean.parseBoolean(shareTokenQP)) || Boolean.parseBoolean(shareTokeAdaptiveScriptParam))) { - assertNotNull(authenticationContext.getProperty(OIDCAuthenticatorConstants.FEDERATED_TOKENS), + Boolean.parseBoolean(shareTokenQueryParameter)) || + Boolean.parseBoolean(shareTokeAdaptiveScriptParam))) { + assertNotNull(authenticationContext.getProperty(FrameworkConstants.FEDERATED_TOKENS), errorMessage); - if (authenticationContext.getProperty(OIDCAuthenticatorConstants.FEDERATED_TOKENS) != null && - authenticationContext.getProperty(OIDCAuthenticatorConstants.FEDERATED_TOKENS) instanceof List) { + if (authenticationContext.getProperty(FrameworkConstants.FEDERATED_TOKENS) != null && + authenticationContext.getProperty(FrameworkConstants.FEDERATED_TOKENS) instanceof List) { List federatedToken = (List) authenticationContext.getProperty( - OIDCAuthenticatorConstants.FEDERATED_TOKENS); + FrameworkConstants.FEDERATED_TOKENS); assertEquals(federatedToken.get(0).getAccessToken(), accessToken, "No access token found"); assertEquals(federatedToken.get(0).getRefreshToken(), refreshToken, "No refresh token found"); assertEquals(federatedToken.get(0).getTokenValidityPeriod(), expiresIn, "No expiry time found"); assertEquals(federatedToken.get(0).getScope(), scope, "No scope found"); } } else { - assertNull(authenticationContext.getProperty(OIDCAuthenticatorConstants.FEDERATED_TOKENS), + assertNull(authenticationContext.getProperty(FrameworkConstants.FEDERATED_TOKENS), errorMessage); } } From d2b40f9255b4a5ef3b8f9d44bd58ccdba9f50f70 Mon Sep 17 00:00:00 2001 From: Indeewai Wijesiri Date: Fri, 8 Mar 2024 11:56:39 +0530 Subject: [PATCH 5/8] Addressing the review comments --- .../oidc/OpenIDConnectAuthenticator.java | 83 +++++++++---------- 1 file changed, 41 insertions(+), 42 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 dda7a3d1..ff20f6c4 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 @@ -149,21 +149,6 @@ public class OpenIDConnectAuthenticator extends AbstractApplicationAuthenticator private static final String[] NON_USER_ATTRIBUTES = new String[]{"at_hash", "iss", "iat", "exp", "aud", "azp"}; private static final String AUTHENTICATOR_MESSAGE = "authenticatorMessage"; - /** - * This method returns the current federated authenticator name. If there is no external IdP, then the current - * authenticator name is returned. - * - * @param context Authentication context. - * @return Federated authenticator name. - */ - private String getFederatedAuthenticatorName(AuthenticationContext context) { - - if (context == null || context.getExternalIdP() == null) { - return StringUtils.EMPTY; - } - return context.getExternalIdP().getIdPName(); - } - @Override public AuthenticatorFlowStatus process(HttpServletRequest request, HttpServletResponse response, AuthenticationContext context) @@ -656,8 +641,8 @@ private String addValidScopesForFederatedTokenSharing(AuthenticationContext cont } if (LOG.isDebugEnabled()) { - LOG.debug("Valid scopes found for federated token sharing: " + validScopesForTokenSharing + - ", IDP: " + getFederatedAuthenticatorName(context)); + LOG.debug("Valid scopes found for the IDP" + getFederatedAuthenticatorName(context) + + " in federated token sharing: " + validScopesForTokenSharing); } /* Remove the duplicate scopes among the validated scopes for federated token sharing and the existing scopes @@ -700,7 +685,6 @@ private String removeDuplicateScopes(String scopes, Set validScopesForTo */ private String getRequestedScopesForTokenSharing(AuthenticationContext context) { - String authenticatorName = getFederatedAuthenticatorName(context); // The first priority is given to the parameters passed from the adaptive script. Then the query parameters. String requestedScopesViaAdaptiveScript = getAdaptiveScriptValues(context, OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE); @@ -708,7 +692,7 @@ private String getRequestedScopesForTokenSharing(AuthenticationContext context) if (StringUtils.isNotBlank(requestedScopesViaAdaptiveScript)) { if (LOG.isDebugEnabled()) { LOG.debug("Adaptive script parameter found for " + OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE - + " in federated token sharing, IDP: " + authenticatorName); + + " in federated token sharing, IDP: " + getFederatedAuthenticatorName(context)); } return requestedScopesViaAdaptiveScript; } else { @@ -717,9 +701,8 @@ private String getRequestedScopesForTokenSharing(AuthenticationContext context) LOG.debug("No adaptive script parameter: " + OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE + " found. Query parameter: " + OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE + " value: " + requestedScopesViaQueryParams + " found for federated token sharing, IDP: " - + authenticatorName); + + getFederatedAuthenticatorName(context)); } - // Checks the scopes requested via authorize request query parameters. return requestedScopesViaQueryParams; } } @@ -727,15 +710,15 @@ private String getRequestedScopesForTokenSharing(AuthenticationContext context) /** * This method returns the adaptive script federated authenticator param value for a given parameter name. * - * @param context The authentication context with federated authenticator params. - * @param authenticatorParamName The federated authenticator parameter name. + * @param context The authentication context with federated authenticator params. + * @param param The federated authenticator parameter name. * @return The adaptive script federated authenticator param value for the given parameter name. */ - private String getAdaptiveScriptValues(AuthenticationContext context, String authenticatorParamName) { + private String getAdaptiveScriptValues(AuthenticationContext context, String param) { Map runtimeParams = this.getRuntimeParams(context); if (runtimeParams != null) { - return runtimeParams.get(authenticatorParamName); + return runtimeParams.get(param); } return StringUtils.EMPTY; } @@ -782,9 +765,9 @@ private String getRequestedScopesViaQueryParams(AuthenticationContext context) { */ if (!scopeString.contains(SEMI_COLON_DELIMITER)) { if (LOG.isDebugEnabled()) { - LOG.debug("Query parameter " + OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE + " is missing " + - SEMI_COLON_DELIMITER + " delimiter in federated token sharing, IDP: " + - authenticatorName); + LOG.debug("Query parameter name: " + OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE + " value: " + + scopeString + " is missing " + SEMI_COLON_DELIMITER + + " delimiter in federated token sharing, IDP: " + authenticatorName); } return null; } @@ -794,19 +777,19 @@ private String getRequestedScopesViaQueryParams(AuthenticationContext context) { for (String scopesFollowedByAuthenticator : scopeSegments) { String[] scopes = StringUtils.split(scopesFollowedByAuthenticator, SEMI_COLON_DELIMITER); - if (ArrayUtils.getLength(scopes) == 2 && authenticatorName.equals(scopes[0])) { - filteredScopes.append(scopes[1]).append(SPACE); + if (ArrayUtils.getLength(scopes) == 2 && + StringUtils.equals(authenticatorName, StringUtils.trim(scopes[0]))) { + filteredScopes.append(StringUtils.trim(scopes[1])).append(SPACE); } } String requestedScopes = filteredScopes.toString(); if (LOG.isDebugEnabled() && StringUtils.isBlank(requestedScopes)) { - LOG.debug("No valid values found for the authenticator " + authenticatorName + " in the query parameter " + - OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE + " for federated token sharing, IDP: " + - authenticatorName); + LOG.debug("No valid values found for the IDP: " + authenticatorName + " in the query parameter " + + OIDCAuthenticatorConstants.FEDERATED_TOKEN_SCOPE + " for federated token sharing"); } - return filteredScopes.toString(); + return requestedScopes; } /** @@ -818,16 +801,15 @@ private String getRequestedScopesViaQueryParams(AuthenticationContext context) { */ private boolean requestedToShareFederatedToken(AuthenticationContext context) { - String authenticatorName = getFederatedAuthenticatorName(context); // The first priority is given to the parameters setup at the adaptive script. Then the query parameters. String shareFederatedToken = getAdaptiveScriptValues(context, OIDCAuthenticatorConstants.SHARE_FEDERATED_TOKEN_PARAM); if (LOG.isDebugEnabled() && StringUtils.isNotBlank(shareFederatedToken)) { LOG.debug("Adaptive script parameter " + OIDCAuthenticatorConstants.SHARE_FEDERATED_TOKEN_PARAM + - " found for federated token sharing, IDP: " + authenticatorName); + " found for federated token sharing, IDP: " + getFederatedAuthenticatorName(context)); } - // Checks if the token sharing is requested via adaptive script. + if (StringUtils.isBlank(shareFederatedToken)) { // Checks if the token sharing is requested via authorize request query parameters. shareFederatedToken = getQueryParameter(context, OIDCAuthenticatorConstants.SHARE_FEDERATED_TOKEN_PARAM); @@ -835,7 +817,7 @@ private boolean requestedToShareFederatedToken(AuthenticationContext context) { LOG.debug("No adaptive script parameter: " + OIDCAuthenticatorConstants.SHARE_FEDERATED_TOKEN_PARAM + " found. Query parameter: " + OIDCAuthenticatorConstants.SHARE_FEDERATED_TOKEN_PARAM + " value: " + shareFederatedToken + " found for federated token sharing, IDP: " - + authenticatorName); + + getFederatedAuthenticatorName(context)); } } return Boolean.parseBoolean(shareFederatedToken); @@ -850,12 +832,11 @@ private boolean requestedToShareFederatedToken(AuthenticationContext context) { */ private String getQueryParameter(AuthenticationContext context, String queryParamName) { - String authenticatorName = getFederatedAuthenticatorName(context); AuthenticationRequest authenticationRequest = context.getAuthenticationRequest(); if (authenticationRequest == null || StringUtils.isBlank(queryParamName)) { if (LOG.isDebugEnabled()) { LOG.debug("Invalid authentication request or invalid query parameter name : " + queryParamName + - " for federated token sharing, IDP: " + authenticatorName); + " for federated token sharing, IDP: " + getFederatedAuthenticatorName(context)); } return null; } @@ -863,13 +844,13 @@ private String getQueryParameter(AuthenticationContext context, String queryPara if (ArrayUtils.isNotEmpty(queryParamValues)) { if (LOG.isDebugEnabled()) { LOG.debug("Query parameter found for," + queryParamName + " in federated token sharing, IDP: " + - authenticatorName); + getFederatedAuthenticatorName(context)); } return queryParamValues[0]; } if (LOG.isDebugEnabled()) { LOG.debug("No value found for the query parameter : " + queryParamName + - " in federated token sharing, IDP: " + authenticatorName); + " in federated token sharing, IDP: " + getFederatedAuthenticatorName(context)); } return null; } @@ -2105,4 +2086,22 @@ private boolean isNativeSDKBasedFederationCall(HttpServletRequest request) { return request.getParameter(ACCESS_TOKEN_PARAM) != null && request.getParameter(ID_TOKEN_PARAM) != null; } + + /** + * This method returns the current federated authenticator name. If there is no external IdP, then the current + * authenticator name is returned. + * + * @param context Authentication context. + * @return Federated authenticator name. + */ + private String getFederatedAuthenticatorName(AuthenticationContext context) { + + if (context == null || context.getExternalIdP() == null || context.getExternalIdP().getIdPName() == null) { + if (LOG.isDebugEnabled()) { + LOG.debug("Cannot resolve the authenticator name from the authentication context"); + } + return StringUtils.EMPTY; + } + return context.getExternalIdP().getIdPName(); + } } From 55d730c53c65381c2320598caa84bb9f7256b08e Mon Sep 17 00:00:00 2001 From: Indeewai Wijesiri Date: Fri, 8 Mar 2024 12:05:15 +0530 Subject: [PATCH 6/8] Addressing the review comments --- .../authenticator/oidc/OpenIDConnectAuthenticator.java | 5 +++-- 1 file changed, 3 insertions(+), 2 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 ff20f6c4..59f33df8 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 @@ -601,8 +601,9 @@ protected void initiateAuthenticationRequest(HttpServletRequest request, HttpSer /** * This method is used to append the application side requested scopes after validating. - * The application can request the optional scopes either via adaptive scripts or via the authorize - * request query parameters. + * The application can request the scopes for federated token sharing either via adaptive scripts + * or via the authorize request query parameters. The adaptive script has the first priority + * while the request query parameters will be evaluated later. * i.e. Adaptive Script example: * This will ignore any other definition (common, local) of the authenticatorParams. * var onLoginRequest = function(context) { From df6a9e1d41e3263359fdf606457051fe65194a63 Mon Sep 17 00:00:00 2001 From: Indeewai Wijesiri Date: Fri, 8 Mar 2024 14:03:21 +0530 Subject: [PATCH 7/8] Addressing the review comments --- .../oidc/OpenIDConnectAuthenticator.java | 4 ++-- .../oidc/OpenIDConnectAuthenticatorTest.java | 12 +++++------- pom.xml | 2 +- 3 files changed, 8 insertions(+), 10 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 59f33df8..912676e4 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 @@ -844,7 +844,7 @@ private String getQueryParameter(AuthenticationContext context, String queryPara String[] queryParamValues = authenticationRequest.getRequestQueryParam(queryParamName); if (ArrayUtils.isNotEmpty(queryParamValues)) { if (LOG.isDebugEnabled()) { - LOG.debug("Query parameter found for," + queryParamName + " in federated token sharing, IDP: " + + LOG.debug("Query parameter found for, " + queryParamName + " in federated token sharing, IDP: " + getFederatedAuthenticatorName(context)); } return queryParamValues[0]; @@ -908,7 +908,7 @@ protected void processAuthenticationResponse(HttpServletRequest request, HttpSer mapAccessToken(request, context, oAuthResponse); /* - Federated tokens are added only of the authenticator configuration ShareFederatedToken is enabled and the + Federated tokens are added only if the authenticator configuration ShareFederatedToken is enabled and the application has requested the federated token. */ if (context.getAuthenticatorProperties() != null && Boolean.parseBoolean( 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 171de488..38d06d20 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 @@ -122,9 +122,9 @@ "org.wso2.carbon.identity.application.authentication.framework.exception.AuthenticationFailedException"}) public class OpenIDConnectAuthenticatorTest extends PowerMockTestCase { - public static final String OIDC_PARAM_MAP_STRING = "oidc:param.map"; - public static final String HTTPS_LOCALHOST_9443 = "https://localhost:9443"; - public static final String COMMA_SEPARATOR = ","; + private static final String OIDC_PARAM_MAP_STRING = "oidc:param.map"; + private static final String HTTPS_LOCALHOST_9443 = "https://localhost:9443"; + private static final String COMMA_SEPARATOR = ","; @Mock private HttpServletRequest mockServletRequest; @@ -1197,8 +1197,7 @@ public void testShareFederatedTokenForIDPConfigAndQueryParameter(String errorMes if (Boolean.parseBoolean(enableShareTokenIDPConfig) && ((StringUtils.isBlank(shareTokeAdaptiveScriptParam) && Boolean.parseBoolean(shareTokenQueryParameter)) || Boolean.parseBoolean(shareTokeAdaptiveScriptParam))) { - assertNotNull(authenticationContext.getProperty(FrameworkConstants.FEDERATED_TOKENS), - errorMessage); + assertNotNull(authenticationContext.getProperty(FrameworkConstants.FEDERATED_TOKENS), errorMessage); if (authenticationContext.getProperty(FrameworkConstants.FEDERATED_TOKENS) != null && authenticationContext.getProperty(FrameworkConstants.FEDERATED_TOKENS) instanceof List) { @@ -1210,8 +1209,7 @@ public void testShareFederatedTokenForIDPConfigAndQueryParameter(String errorMes assertEquals(federatedToken.get(0).getScope(), scope, "No scope found"); } } else { - assertNull(authenticationContext.getProperty(FrameworkConstants.FEDERATED_TOKENS), - errorMessage); + assertNull(authenticationContext.getProperty(FrameworkConstants.FEDERATED_TOKENS), errorMessage); } } diff --git a/pom.xml b/pom.xml index 10b885de..d898ba78 100644 --- a/pom.xml +++ b/pom.xml @@ -304,7 +304,7 @@ ${project.version} - 5.25.560 + 7.0.93 1.0.0.wso2v3 2.4.7 3.0.0.wso2v4 From 6b2d0371e35073725d304a2221deb5a7c8f069f3 Mon Sep 17 00:00:00 2001 From: Indeewai Wijesiri Date: Fri, 8 Mar 2024 15:59:22 +0530 Subject: [PATCH 8/8] Addressing the review comment --- .../authenticator/oidc/OpenIDConnectAuthenticator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 912676e4..f266d1a4 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 @@ -2099,7 +2099,7 @@ private String getFederatedAuthenticatorName(AuthenticationContext context) { if (context == null || context.getExternalIdP() == null || context.getExternalIdP().getIdPName() == null) { if (LOG.isDebugEnabled()) { - LOG.debug("Cannot resolve the authenticator name from the authentication context"); + LOG.debug("Cannot resolve the authenticator name from the authentication context."); } return StringUtils.EMPTY; }