Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix hybrid flow for configuring multiple hybrid flow response types #2677

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix hybrid flow for configuring multiple hybrid flow response types #2677

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions ...y.oauth/src/main/java/org/wso2/carbon/identity/oauth/config/OAuthServerConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,9 @@ public class OAuthServerConfiguration {
private List<String> supportedTokenEndpointSigningAlgorithms = new ArrayList<>();
private Boolean roleBasedScopeIssuerEnabledConfig = false;
private String scopeMetadataExtensionImpl = null;
private static final List<String> HYBRID_RESPONSE_TYPES = Arrays.asList("code token",
"code id_token", "code id_token token");
private List<String> configuredHybridResponseTypes = new ArrayList<>();

private final List<String> restrictedQueryParameters = new ArrayList<>();

Expand Down Expand Up @@ -973,6 +976,11 @@ public boolean useRetainOldAccessTokens() {
return Boolean.TRUE.toString().equalsIgnoreCase(retainOldAccessTokens);
}

public List<String> getConfiguredHybridResponseTypes() {

return configuredHybridResponseTypes;
}

public boolean isTokenCleanupEnabled() {

return Boolean.TRUE.toString().equalsIgnoreCase(tokenCleanupFeatureEnable);
Expand Down Expand Up @@ -2966,6 +2974,11 @@ private void parseSupportedResponseTypesConfig(OMElement oauthConfigElem) {
}
if (responseTypeName != null && !"".equals(responseTypeName) &&
responseTypeHandlerImplClass != null && !"".equals(responseTypeHandlerImplClass)) {

// check for the configured hybrid response type
if (HYBRID_RESPONSE_TYPES.contains(responseTypeName)) {
configuredHybridResponseTypes.add(responseTypeName);
}
supportedResponseTypeClassNames.put(responseTypeName, responseTypeHandlerImplClass);
OMElement responseTypeValidatorClassNameElement = supportedResponseTypeElement
.getFirstChildWithName(
Expand Down
39 changes: 33 additions & 6 deletions ...2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth/dao/OAuthAppDAO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1992,14 +1992,41 @@ private void setSpOIDCProperties(Map<String, List<String>> spOIDCProperties, OAu
oauthApp.setSubjectTokenExpiryTime(Integer.parseInt(subjectTokenExpiryTime));
}

boolean hybridFlowEnabled = Boolean.parseBoolean(getFirstPropertyValue(spOIDCProperties,
HYBRID_FLOW_ENABLED));
oauthApp.setHybridFlowEnabled(hybridFlowEnabled);
String hybridFlowEnabledProperty = getFirstPropertyValue(spOIDCProperties, HYBRID_FLOW_ENABLED);

String hybridFlowResponseType = getFirstPropertyValue(spOIDCProperties,
OAuthConstants.OIDCConfigProperties.HYBRID_FLOW_RESPONSE_TYPE);
// Check if the application has the `hybridFlowEnabled` property configured
if (hybridFlowEnabledProperty == null) {
// No hybridFlowEnabled property; use server's default behavior for hybrid flow.
if (LOG.isDebugEnabled()) {
LOG.debug(String.format("The application with consumer key %s does not have the 'hybridFlowEnabled' " +
"property configured. Using server default behavior to enable hybrid flow with all " +
"configured response types.", oauthApp.getOauthConsumerKey()));
}

oauthApp.setHybridFlowResponseType(hybridFlowResponseType);
List<String> configuredHybridResponseTypes = OAuthServerConfiguration.getInstance()
.getConfiguredHybridResponseTypes();

if (configuredHybridResponseTypes.isEmpty()) {
// No configured hybrid response types; hybrid flow is disabled
oauthApp.setHybridFlowEnabled(false);
oauthApp.setHybridFlowResponseType(null);
} else {
// Enable hybrid flow with all configured response types
oauthApp.setHybridFlowEnabled(true);
String hybridFlowResponseType = String.join(", ", configuredHybridResponseTypes);
oauthApp.setHybridFlowResponseType(hybridFlowResponseType);
}
} else {
// Hybrid flow property is defined; parse and configure
boolean hybridFlowEnabled = Boolean.parseBoolean(hybridFlowEnabledProperty);
oauthApp.setHybridFlowEnabled(hybridFlowEnabled);

String hybridFlowResponseType = getFirstPropertyValue(spOIDCProperties,
OAuthConstants.OIDCConfigProperties.HYBRID_FLOW_RESPONSE_TYPE);

// Configure the hybrid flow response type (null if not explicitly set)
oauthApp.setHybridFlowResponseType(hybridFlowResponseType);
}
}

private String getFirstPropertyValue(Map<String, List<String>> propertyMap, String key) {
Expand Down
32 changes: 18 additions & 14 deletions ...rg/wso2/carbon/identity/oauth2/authz/validators/AbstractResponseTypeRequestValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,7 @@

import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

import javax.servlet.http.HttpServletRequest;

Expand Down Expand Up @@ -208,6 +206,7 @@ private void validateHybridFlowRequest(HttpServletRequest request, OAuthAppDO ap
String responseType = request.getParameter(OAuthConstants.OAuth20Params.RESPONSE_TYPE);
boolean hybridFlowEnabled = appDO.isHybridFlowEnabled();

// Check if the response type is a hybrid response type.
if (OAuth2Util.isHybridResponseType(responseType)) {
if (!hybridFlowEnabled) {
if (log.isDebugEnabled()) {
Expand All @@ -217,11 +216,14 @@ private void validateHybridFlowRequest(HttpServletRequest request, OAuthAppDO ap
throw new InvalidOAuthClientException("Hybrid flow is not enabled for the application.");
}

String configuredHybridFlowResponseType = appDO.getHybridFlowResponseType();
if (!isRequestedResponseTypeConfigured(responseType, configuredHybridFlowResponseType)) {
// Retrieve the list of allowed hybrid response types
List<String> hybridResponseTypeList = getHybridResponseType(appDO);

// Validate the requested response type
if (!hybridResponseTypeList.contains(responseType)) {
if (log.isDebugEnabled()) {
log.debug("Requested response type " + responseType + " is not configured for the hybrid flow " +
"for the application with client ID: " + appDO.getOauthConsumerKey());
log.debug(String.format("Requested response type '%s' is not configured for the hybrid " +
"flow for the application with client ID: %s", responseType, appDO.getOauthConsumerKey()));
}

throw new InvalidOAuthClientException("Requested response type " + responseType +
Expand All @@ -230,16 +232,18 @@ private void validateHybridFlowRequest(HttpServletRequest request, OAuthAppDO ap
}
}

private boolean isRequestedResponseTypeConfigured(String responseType, String configuredHybridFlowResponseType) {
private List<String> getHybridResponseType(OAuthAppDO appDO) throws InvalidOAuthClientException {

Set<String> configuredResponseTypes = new HashSet<>(Arrays.asList(configuredHybridFlowResponseType.split(" ")));
String[] requestedResponseTypes = responseType.split(" ");
for (String requestedType : requestedResponseTypes) {
if (!configuredResponseTypes.contains(requestedType)) {
return false;
}
String configuredHybridFlowResponseType = appDO.getHybridFlowResponseType();

// Validate if the configured response type string is null or empty
if (configuredHybridFlowResponseType == null || configuredHybridFlowResponseType.trim().isEmpty()) {
throw new InvalidOAuthClientException(String.format("No hybrid flow response types are configured " +
"for the application with client ID: %s.", appDO.getOauthConsumerKey()));
}
return true;

// Split the configured hybrid response types into a list
return Arrays.asList(configuredHybridFlowResponseType.split(","));
}

private OAuth2ClientValidationResponseDTO validateCallBack(String clientId, String callbackURI, OAuthAppDO appDO) {
Expand Down
110 changes: 110 additions & 0 deletions ...rbon.identity.oauth/src/test/java/org/wso2/carbon/identity/oauth/dao/OAuthAppDAOTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
import java.sql.SQLException;
import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.UUID;

Expand Down Expand Up @@ -1164,6 +1165,115 @@ public void testGetAppInformationByAppNameWithAppResidentOrgIdAndWithException()
}
}

@DataProvider(name = "testGetAppInformationForHybridFlowData")
public Object[][] testGetAppInformationForHybridFlowData() {

return new Object[][]{
{true, "code token", true, "code token"},
{true, "code token, code id_token", true, "code token, code id_token"},
{false, "code id_token", false, "code id_token"},
};
}

@Test(dataProvider = "testGetAppInformationForHybridFlowData")
public void testGetAppInformationForHybridFlowTest
(boolean hybridFlowEnabled, String hybridFlowResponseType,
boolean hybridFlowEnabledExpected, String hybridFlowResponseTypeExpected) throws Exception {

try (MockedStatic<OAuthServerConfiguration> oAuthServerConfiguration = mockStatic(
OAuthServerConfiguration.class);
MockedStatic<IdentityTenantUtil> identityTenantUtil = mockStatic(IdentityTenantUtil.class);
MockedStatic<IdentityUtil> identityUtil = mockStatic(IdentityUtil.class);
MockedStatic<IdentityDatabaseUtil> identityDatabaseUtil = mockStatic(IdentityDatabaseUtil.class);
MockedStatic<OAuthComponentServiceHolder> oAuthComponentServiceHolder =
mockStatic(OAuthComponentServiceHolder.class)) {
setupMocksForTest(oAuthServerConfiguration, identityTenantUtil, identityUtil);
mockUserstore(oAuthComponentServiceHolder);
try (Connection connection = getConnection(DB_NAME)) {
mockIdentityUtilDataBaseConnection(connection, identityDatabaseUtil);
OAuthAppDO defaultOAuthAppDO = getDefaultOAuthAppDO();

// Add Impersonation OIDC properties.
defaultOAuthAppDO.setHybridFlowEnabled(hybridFlowEnabled);
defaultOAuthAppDO.setHybridFlowResponseType(hybridFlowResponseType);
addOAuthApplication(defaultOAuthAppDO, TENANT_ID);

OAuthAppDAO appDAO = new OAuthAppDAO();
OAuthAppDO oAuthAppDO = appDAO.getAppInformation(CONSUMER_KEY);
assertNotNull(oAuthAppDO);
assertEquals(oAuthAppDO.isHybridFlowEnabled(), hybridFlowEnabledExpected);
assertEquals(oAuthAppDO.getHybridFlowResponseType(), hybridFlowResponseTypeExpected);
appDAO.removeConsumerApplication(CONSUMER_KEY);
}
} finally {
resetPrivilegedCarbonContext();
}
}

@DataProvider(name = "testGetMigratedAppInformationForHybridFlowData")
public Object[][] testGetMigratedAppInformationForHybridFlowData() {

return new Object[][]{
{Arrays.asList("code token", "code id_token", "code id_token token"), true,
"code token, code id_token, code id_token token"},
{Arrays.asList("code id_token"), true, "code id_token"},
{Arrays.asList(), false, null},
};
}

@Test(dataProvider = "testGetMigratedAppInformationForHybridFlowData")
public void testGetMigratedAppInformationForHybridFlowTest(List<String> configuredHybridFlowResponseType,
boolean hybridFlowEnabledExpected,
String hybridFlowResponseTypeExpected) throws Exception {

try (MockedStatic<OAuthServerConfiguration> oAuthServerConfiguration = mockStatic(
OAuthServerConfiguration.class);
MockedStatic<IdentityTenantUtil> identityTenantUtil = mockStatic(IdentityTenantUtil.class);
MockedStatic<IdentityUtil> identityUtil = mockStatic(IdentityUtil.class);
MockedStatic<IdentityDatabaseUtil> identityDatabaseUtil = mockStatic(IdentityDatabaseUtil.class);
MockedStatic<OAuthComponentServiceHolder> oAuthComponentServiceHolder =
mockStatic(OAuthComponentServiceHolder.class)) {
mockUserstore(oAuthComponentServiceHolder);
final String deleteHybridFlowProperty =
createDeleteQuery(CONSUMER_KEY, "hybridFlowEnabled");
final String deleteHybridFlowResponseTypeProperty =
createDeleteQuery(CONSUMER_KEY, "hybridFlowResponseType");
setupMocksForTest(oAuthServerConfiguration, identityTenantUtil, identityUtil);
when(mockedServerConfig.getConfiguredHybridResponseTypes()).thenReturn(configuredHybridFlowResponseType);
try (Connection connection = getConnection(DB_NAME)) {
mockIdentityUtilDataBaseConnection(connection, identityDatabaseUtil);
OAuthAppDO defaultOAuthAppDO = getDefaultOAuthAppDO();

addOAuthApplication(defaultOAuthAppDO, TENANT_ID);

executeDeleteQuery(connection, deleteHybridFlowProperty);
executeDeleteQuery(connection, deleteHybridFlowResponseTypeProperty);

OAuthAppDAO appDAO = new OAuthAppDAO();
OAuthAppDO oAuthAppDO = appDAO.getAppInformation(CONSUMER_KEY);
assertNotNull(oAuthAppDO);
assertEquals(oAuthAppDO.isHybridFlowEnabled(), hybridFlowEnabledExpected);
assertEquals(oAuthAppDO.getHybridFlowResponseType(), hybridFlowResponseTypeExpected);
appDAO.removeConsumerApplication(CONSUMER_KEY);
}
} finally {
resetPrivilegedCarbonContext();
}
}

private String createDeleteQuery(String consumerKey, String propertyKey) {

return "DELETE FROM IDN_OIDC_PROPERTY WHERE CONSUMER_KEY='"
+ consumerKey + "' AND PROPERTY_KEY ='" + propertyKey + "'";
}

private void executeDeleteQuery(Connection connection, String query) throws SQLException {

try (PreparedStatement stmt = connection.prepareStatement(query)) {
stmt.execute();
}
}

private OAuthAppDO getDefaultOAuthAppDO() {
AuthenticatedUser authenticatedUser = new AuthenticatedUser();
authenticatedUser.setUserName(USER_NAME);
Expand Down
Loading
Loading