From fe9eccd23ed3c47cb7dd0ba25ec5a488599ec8fc Mon Sep 17 00:00:00 2001 From: chashikajw Date: Fri, 19 Jan 2024 15:45:15 +0530 Subject: [PATCH] Add system property to avoid allowlisting apim rest api scopes --- .../identity/oauth/common/OAuthConstants.java | 2 + .../internal/OAuth2ServiceComponent.java | 3 ++ .../OAuth2ServiceComponentHolder.java | 9 +++++ .../scope/RoleBasedScopeIssuer.java | 38 +++++++++++++------ 4 files changed, 40 insertions(+), 12 deletions(-) diff --git a/components/org.wso2.carbon.identity.oauth.common/src/main/java/org/wso2/carbon/identity/oauth/common/OAuthConstants.java b/components/org.wso2.carbon.identity.oauth.common/src/main/java/org/wso2/carbon/identity/oauth/common/OAuthConstants.java index e1a7c24b374..79c58dbd8fc 100644 --- a/components/org.wso2.carbon.identity.oauth.common/src/main/java/org/wso2/carbon/identity/oauth/common/OAuthConstants.java +++ b/components/org.wso2.carbon.identity.oauth.common/src/main/java/org/wso2/carbon/identity/oauth/common/OAuthConstants.java @@ -219,6 +219,8 @@ public static SubjectType fromValue(String text) { public static final String RESTRICT_UNASSIGNED_SCOPES = "restrict.unassigned.scopes"; + public static final String RESTRICT_APIM_REST_API_SCOPES = "restrict.apim.restapi.scopes"; + public static final String TENANT_NAME_FROM_CONTEXT = "TenantNameFromContext"; //Oauth2 sp expire time configuration. diff --git a/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/internal/OAuth2ServiceComponent.java b/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/internal/OAuth2ServiceComponent.java index d0c8f448322..73afdc75594 100644 --- a/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/internal/OAuth2ServiceComponent.java +++ b/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/internal/OAuth2ServiceComponent.java @@ -438,6 +438,9 @@ protected void activate(ComponentContext context) { boolean restrictUnassignedScopes = Boolean.parseBoolean(System.getProperty( OAuthConstants.RESTRICT_UNASSIGNED_SCOPES)); OAuth2ServiceComponentHolder.setRestrictUnassignedScopes(restrictUnassignedScopes); + boolean restrictApimRestApiScopes = Boolean.parseBoolean(System.getProperty( + OAuthConstants.RESTRICT_APIM_REST_API_SCOPES)); + OAuth2ServiceComponentHolder.setRestrictApimRestApiScopes(restrictApimRestApiScopes); if (OAuthServerConfiguration.getInstance().isUseLegacyScopesAsAliasForNewScopesEnabled()) { initializeLegacyScopeToNewScopeMappings(); } diff --git a/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/internal/OAuth2ServiceComponentHolder.java b/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/internal/OAuth2ServiceComponentHolder.java index d83bd57b014..2d8b2fefe67 100644 --- a/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/internal/OAuth2ServiceComponentHolder.java +++ b/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/internal/OAuth2ServiceComponentHolder.java @@ -101,6 +101,7 @@ public class OAuth2ServiceComponentHolder { private static List jwtRenewWithoutRevokeAllowedGrantTypes = new ArrayList<>(); private static ConsentServerConfigsManagementService consentServerConfigsManagementService; private static boolean restrictUnassignedScopes; + private static boolean restrictApimRestApiScopes; private static ConfigurationContextService configurationContextService; private List jwtAccessTokenClaimProviders = new ArrayList<>(); private final List oAuthAuthorizationRequestBuilders = new ArrayList<>(); @@ -551,6 +552,14 @@ public static void setRestrictUnassignedScopes(boolean restrictUnassignedScopes) OAuth2ServiceComponentHolder.restrictUnassignedScopes = restrictUnassignedScopes; } + public static boolean isRestrictApimRestApiScopes() { + return restrictApimRestApiScopes; + } + + public static void setRestrictApimRestApiScopes(boolean restrictApimRestApiScopes) { + OAuth2ServiceComponentHolder.restrictApimRestApiScopes = restrictApimRestApiScopes; + } + public static ConfigurationContextService getConfigurationContextService() { return configurationContextService; diff --git a/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/validators/scope/RoleBasedScopeIssuer.java b/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/validators/scope/RoleBasedScopeIssuer.java index 99c7e3731e5..49401689f3a 100644 --- a/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/validators/scope/RoleBasedScopeIssuer.java +++ b/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/validators/scope/RoleBasedScopeIssuer.java @@ -283,14 +283,21 @@ public List getScopes(OAuthAuthzReqMessageContext oAuthAuthzReqMessageCo List authorizedScopes; List requestedScopes = null; List scopes = new ArrayList<>(); + boolean isRestrictApimRestApiScopes = OAuth2ServiceComponentHolder.isRestrictApimRestApiScopes(); + boolean isRestrictUnassignedScopes = OAuth2ServiceComponentHolder.isRestrictUnassignedScopes(); if (oAuthAuthzReqMessageContext.getApprovedScope() != null) { requestedScopes = new ArrayList<>(Arrays.asList(oAuthAuthzReqMessageContext.getApprovedScope())); - for (String scope : requestedScopes) { - // If requestedScopes contains Product REST APIs (Publisher/DevPortal/Admin) scopes, just let them pass - // to the final scope list returned from RoleBasedScopeIssuer. This is because RoleBasedScopeIssuer is - // not responsible for validating Product REST API scopes. Those will be handled by SystemScopeIssuer. - if (checkForProductRestAPIScopes(scope)) { - scopes.add(scope); + // If both system properties are true, it does not allowlist the APIM REST API scopes. This is to solve + // the security concern introduced by allow listing product REST APIs scopes. + if (!(isRestrictUnassignedScopes && isRestrictApimRestApiScopes)) { + for (String scope : requestedScopes) { + // If requestedScopes contains Product REST APIs (Publisher/DevPortal/Admin) scopes, just let + // them pass to the final scope list returned from RoleBasedScopeIssuer. This is because + // RoleBasedScopeIssuer is not responsible for validating Product REST API scopes. Those will be + // handled by SystemScopeIssuer. + if (checkForProductRestAPIScopes(scope)) { + scopes.add(scope); + } } } requestedScopes.removeAll(scopes); @@ -352,12 +359,19 @@ public List getScopes(OAuthTokenReqMessageContext tokReqMsgCtx) { List authorizedScopes; List scopes = new ArrayList<>(); List requestedScopes = new ArrayList<>(Arrays.asList(tokReqMsgCtx.getScope())); - for (String scope : requestedScopes) { - // If requestedScopes contains Product REST APIs (Publisher/DevPortal/Admin) scopes, just let them pass to - // the final scope list returned from RoleBasedScopeIssuer. This is because RoleBasedScopeIssuer is not - // responsible for validating Product REST API scopes. Those will be handled by the SystemScopeIssuer. - if (checkForProductRestAPIScopes(scope)) { - scopes.add(scope); + boolean isRestrictApimRestApiScopes = OAuth2ServiceComponentHolder.isRestrictApimRestApiScopes(); + boolean isRestrictUnassignedScopes = OAuth2ServiceComponentHolder.isRestrictUnassignedScopes(); + // If both system properties are true, it does not allowlist the APIM REST API scopes. This is to solve + // the security concern introduced by allow listing product REST APIs scopes. + if (!(isRestrictUnassignedScopes && isRestrictApimRestApiScopes)) { + for (String scope : requestedScopes) { + // If requestedScopes contains Product REST APIs (Publisher/DevPortal/Admin) scopes, just let + // them pass to the final scope list returned from RoleBasedScopeIssuer. This is because + // RoleBasedScopeIssuer is not responsible for validating Product REST API scopes. Those will be + // handled by the SystemScopeIssuer. + if (checkForProductRestAPIScopes(scope)) { + scopes.add(scope); + } } } requestedScopes.removeAll(scopes);