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

Jump to bottom

Fix hybrid flow migration issue #2575

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix hybrid flow migration issue #2575

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
aed7f7f
fix hybrid flow migration
Thumimku Sep 22, 2024
00e0b11
support hybrid flow server config partially
Thumimku Sep 24, 2024
c1267b9
remove app after testing
Thumimku Sep 24, 2024
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
1 change: 1 addition & 0 deletions ...tity.oauth.common/src/main/java/org/wso2/carbon/identity/oauth/common/OAuthConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,7 @@ public static class OIDCConfigProperties {
public static final String TOKEN_TYPE = "tokenType";
public static final String HYBRID_FLOW_ENABLED = "hybridFlowEnabled";
public static final String HYBRID_FLOW_RESPONSE_TYPE = "hybridFlowResponseType";
public static final String HYBRID_FLOW_CODE_ID_TOKEN_TOKEN = "code id_token token";
public static final String BYPASS_CLIENT_CREDENTIALS = "bypassClientCredentials";
public static final String RENEW_REFRESH_TOKEN = "renewRefreshToken";
public static final String TOKEN_BINDING_TYPE = "tokenBindingType";
Expand Down
25 changes: 19 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 @@ -79,6 +79,7 @@
import static org.wso2.carbon.identity.oauth.common.OAuthConstants.OIDCConfigProperties.BACK_CHANNEL_LOGOUT_URL;
import static org.wso2.carbon.identity.oauth.common.OAuthConstants.OIDCConfigProperties.BYPASS_CLIENT_CREDENTIALS;
import static org.wso2.carbon.identity.oauth.common.OAuthConstants.OIDCConfigProperties.FRONT_CHANNEL_LOGOUT_URL;
import static org.wso2.carbon.identity.oauth.common.OAuthConstants.OIDCConfigProperties.HYBRID_FLOW_CODE_ID_TOKEN_TOKEN;
import static org.wso2.carbon.identity.oauth.common.OAuthConstants.OIDCConfigProperties.HYBRID_FLOW_ENABLED;
import static org.wso2.carbon.identity.oauth.common.OAuthConstants.OIDCConfigProperties.HYBRID_FLOW_RESPONSE_TYPE;
import static org.wso2.carbon.identity.oauth.common.OAuthConstants.OIDCConfigProperties.ID_TOKEN_ENCRYPTED;
Expand Down Expand Up @@ -2007,14 +2008,26 @@
Boolean.parseBoolean(isAccessTokenClaimsSeparationEnabled));
}

boolean hybridFlowEnabled = Boolean.parseBoolean(getFirstPropertyValue(spOIDCProperties,
HYBRID_FLOW_ENABLED));
oauthApp.setHybridFlowEnabled(hybridFlowEnabled);
String hybridFlowEnabledProperty = getFirstPropertyValue(spOIDCProperties, HYBRID_FLOW_ENABLED);
if (hybridFlowEnabledProperty == null) {
// This application doesn't have hybridFlowEnabled property, hence providing the previous behaviour.
// which is enabling the hybrid flow with all allowed response types.
if (LOG.isDebugEnabled()) {
LOG.debug(String.format("This application with consumer key %s doesn't have hybridFlowEnabled " +

Check warning on line 2016 in components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth/dao/OAuthAppDAO.java

View check run for this annotation

Codecov / codecov/patch

components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth/dao/OAuthAppDAO.java#L2016 

Added line #L2016 was not covered by tests
"property, hence providing the previous behaviour, which is enabling the hybrid flow with " +
"all allowed response types.", oauthApp.getOauthConsumerKey()));

Check warning on line 2018 in components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth/dao/OAuthAppDAO.java

View check run for this annotation

Codecov / codecov/patch

components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth/dao/OAuthAppDAO.java#L2018 

Added line #L2018 was not covered by tests
}
oauthApp.setHybridFlowEnabled(true);
oauthApp.setHybridFlowResponseType(HYBRID_FLOW_CODE_ID_TOKEN_TOKEN);
} else {
boolean hybridFlowEnabled = Boolean.parseBoolean(hybridFlowEnabledProperty);
oauthApp.setHybridFlowEnabled(hybridFlowEnabled);

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

oauthApp.setHybridFlowResponseType(hybridFlowResponseType);
oauthApp.setHybridFlowResponseType(hybridFlowResponseType);
}
}

private String getFirstPropertyValue(Map<String, List<String>> propertyMap, String key) {
Expand Down
93 changes: 93 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 @@ -797,6 +797,99 @@ public void testGetAppInformationWithOIDCProperties(Boolean isRenewRefreshEnable
}
}

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

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a new line

@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)) {
Comment on lines +820 to +823
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix the alignment

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);
}
} finally {
resetPrivilegedCarbonContext();
}
}

@Test
public void testGetMigratedAppInformationForHybridFlowTest() 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");

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the additional line.


setupMocksForTest(oAuthServerConfiguration, identityTenantUtil, identityUtil);
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(), true);
assertEquals(oAuthAppDO.getHybridFlowResponseType(), "code id_token token");
}
} 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();
}
}

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

Expand Down
Loading