From c824e63f1ea07199aac563ef2bf45cbd8dcffad2 Mon Sep 17 00:00:00 2001 From: mharindu Date: Thu, 28 Apr 2016 16:43:13 +0530 Subject: [PATCH] Refactored and fixed findbug issues in OAuth extension --- .../mgt/oauth/extensions/OAuthExtUtils.java | 174 +++++++++++------- .../grant/ExtendedPasswordGrantHandler.java | 86 ++++----- 2 files changed, 141 insertions(+), 119 deletions(-) diff --git a/components/identity-extensions/org.wso2.carbon.device.mgt.oauth.extensions/src/main/java/org/wso2/carbon/device/mgt/oauth/extensions/OAuthExtUtils.java b/components/identity-extensions/org.wso2.carbon.device.mgt.oauth.extensions/src/main/java/org/wso2/carbon/device/mgt/oauth/extensions/OAuthExtUtils.java index c6a626db0d2..32ad46ccd14 100644 --- a/components/identity-extensions/org.wso2.carbon.device.mgt.oauth.extensions/src/main/java/org/wso2/carbon/device/mgt/oauth/extensions/OAuthExtUtils.java +++ b/components/identity-extensions/org.wso2.carbon.device.mgt.oauth.extensions/src/main/java/org/wso2/carbon/device/mgt/oauth/extensions/OAuthExtUtils.java @@ -46,7 +46,14 @@ public class OAuthExtUtils { private static final Log log = LogFactory.getLog(OAuthExtUtils.class); private static final String DEFAULT_SCOPE_NAME = "default"; private static final String UI_EXECUTE = "ui.execute"; + private static final String REST_API_SCOPE_CACHE = "REST_API_SCOPE_CACHE"; + /** + * This method is used to get the tenant id when given tenant domain. + * + * @param tenantDomain Tenant domain name. + * @return Returns the tenant id. + */ public static int getTenantId(String tenantDomain) { int tenantId = 0; if (tenantDomain != null) { @@ -62,9 +69,16 @@ public class OAuthExtUtils { return tenantId; } + /** + * This method is used to set scopes that are authorized to the OAuth token request message context. + * + * @param tokReqMsgCtx OAuth token request message context + * @return Returns true if success. + */ public static boolean setScopes(OAuthTokenReqMessageContext tokReqMsgCtx) { String[] requestedScopes = tokReqMsgCtx.getScope(); String[] defaultScope = new String[]{DEFAULT_SCOPE_NAME}; + //If no scopes were requested. if (requestedScopes == null || requestedScopes.length == 0) { tokReqMsgCtx.setScope(defaultScope); @@ -72,7 +86,6 @@ public class OAuthExtUtils { } String consumerKey = tokReqMsgCtx.getOauth2AccessTokenReqDTO().getClientId(); - String username = tokReqMsgCtx.getAuthorizedUser().getUserName(); List reqScopeList = Arrays.asList(requestedScopes); Map restAPIScopesOfCurrentTenant; @@ -80,23 +93,27 @@ public class OAuthExtUtils { Map appScopes; ApiMgtDAO apiMgtDAO = new ApiMgtDAO(); - //Get all the scopes and roles against the scopes defined for the APIs subscribed to the application. + + //Get all the scopes and permissions against the scopes defined for the APIs subscribed to the application. appScopes = apiMgtDAO.getScopeRolesOfApplication(consumerKey); + //Add API Manager rest API scopes set. This list should be loaded at server start up and keep //in memory and add it to each and every request coming. String tenantDomain = tokReqMsgCtx.getAuthorizedUser().getTenantDomain(); restAPIScopesOfCurrentTenant = (Map) Caching.getCacheManager(APIConstants.API_MANAGER_CACHE_MANAGER) - .getCache("REST_API_SCOPE_CACHE") + .getCache(REST_API_SCOPE_CACHE) .get(tenantDomain); if (restAPIScopesOfCurrentTenant != null) { appScopes.putAll(restAPIScopesOfCurrentTenant); } else { - restAPIScopesOfCurrentTenant = APIUtil.getRESTAPIScopesFromConfig(APIUtil.getTenantRESTAPIScopesConfig(tenantDomain)); + restAPIScopesOfCurrentTenant = APIUtil. + getRESTAPIScopesFromConfig(APIUtil.getTenantRESTAPIScopesConfig(tenantDomain)); + //call load tenant config for rest API. //then put cache appScopes.putAll(restAPIScopesOfCurrentTenant); Caching.getCacheManager(APIConstants.API_MANAGER_CACHE_MANAGER) - .getCache("REST_API_SCOPE_CACHE") + .getCache(REST_API_SCOPE_CACHE) .put(tenantDomain, restAPIScopesOfCurrentTenant); } //If no scopes can be found in the context of the application @@ -111,66 +128,9 @@ public class OAuthExtUtils { return true; } - int tenantId; - RealmService realmService = OAuthExtensionsDataHolder.getInstance().getRealmService(); - - try { - tenantId = realmService.getTenantManager().getTenantId(tenantDomain); - - // If tenant Id is not set in the tokenReqContext, deriving it from username. - if (tenantId == 0 || tenantId == -1) { - tenantId = IdentityTenantUtil.getTenantIdOfUser(username); - } - - } catch (UserStoreException e) { - //Log and return since we do not want to stop issuing the token in case of scope validation failures. - log.error("Error when getting the tenant's UserStoreManager or when getting roles of user ", e); - return false; - } - - UserRealm userRealm = OAuthExtensionsDataHolder.getInstance().getRealmService().getTenantUserRealm(tenantId); - List authorizedScopes = new ArrayList<>(); - boolean status; - //List userRoleList = new ArrayList<>(Arrays.asList(userRoles)); + // check for authorized scopes + List authorizedScopes = getAuthorizedScopes(tokReqMsgCtx, reqScopeList, appScopes); - //Iterate the requested scopes list. - for (String scope : reqScopeList) { - status = false; - //Get the set of roles associated with the requested scope. - String appPermissions = appScopes.get(scope); - //If the scope has been defined in the context of the App and if roles have been defined for the scope - if (appPermissions != null && appPermissions.length() != 0) { - List permissions = new ArrayList<>(Arrays.asList(appPermissions.replaceAll(" ", "").split(","))); - //Check if user has at least one of the roles associated with the scope - if (!permissions.isEmpty()) { - for (String permission : permissions) { - if (userRealm != null && userRealm.getAuthorizationManager() != null) { - String userStore = tokReqMsgCtx.getAuthorizedUser().getUserStoreDomain(); - - if (userStore != null) { - status = userRealm.getAuthorizationManager() - .isUserAuthorized(userStore + "/" + username, permission, UI_EXECUTE); - } else { - status = userRealm.getAuthorizationManager() - .isUserAuthorized(username, permission, UI_EXECUTE); - } - if (status) { - break; - } - } - } - if (status) { - authorizedScopes.add(scope); - } - } - } - //The requested scope is defined for the context of the App but no roles have been associated with the scope - //OR - //The scope string starts with 'device_'. - else if (appScopes.containsKey(scope) || isWhiteListedScope(scope)) { - authorizedScopes.add(scope); - } - } if (!authorizedScopes.isEmpty()) { String[] authScopesArr = authorizedScopes.toArray(new String[authorizedScopes.size()]); tokReqMsgCtx.setScope(authScopesArr); @@ -180,19 +140,17 @@ public class OAuthExtUtils { } catch (APIManagementException e) { log.error("Error while getting scopes of application " + e.getMessage()); return false; - } catch (UserStoreException e) { - e.printStackTrace(); } return true; } /** - * Determines if the scope is specified in the whitelist. + * Determines if the scope is specified in the white list. * * @param scope - The scope key to check * @return - 'true' if the scope is white listed. 'false' if not. */ - public static boolean isWhiteListedScope(String scope) { + private static boolean isWhiteListedScope(String scope) { // load white listed scopes List scopeSkipList = OAuthExtensionsDataHolder.getInstance().getWhitelistedScopes(); for (String scopeTobeSkipped : scopeSkipList) { @@ -204,7 +162,7 @@ public class OAuthExtUtils { } /** - * Get the set of default scopes. If a requested scope is matches with the patterns specified in the whitelist, + * Get the set of default scopes. If a requested scope is matches with the patterns specified in the white list, * then such scopes will be issued without further validation. If the scope list is empty, * token will be issued for default scope. * @@ -212,7 +170,7 @@ public class OAuthExtUtils { * @return - The subset of scopes that are allowed */ private static String[] getAllowedScopes(List requestedScopes) { - List authorizedScopes = new ArrayList(); + List authorizedScopes = new ArrayList<>(); //Iterate the requested scopes list. for (String scope : requestedScopes) { @@ -226,4 +184,80 @@ public class OAuthExtUtils { return authorizedScopes.toArray(new String[authorizedScopes.size()]); } + /** + * This method is used to get the authorized scopes out of requested scopes. It checks requested scopes with app + * scopes whether user has permissions to take actions for the requested scopes. + * + * @param tokReqMsgCtx OAuth token request message context. + * @param reqScopeList Requested scope list. + * @param appScopes App scopes. + * @return Returns a list of scopes. + */ + private static List getAuthorizedScopes(OAuthTokenReqMessageContext tokReqMsgCtx, List reqScopeList, + Map appScopes) { + + boolean status; + List authorizedScopes = new ArrayList<>(); + + int tenantId; + String username = tokReqMsgCtx.getAuthorizedUser().getUserName(); + String tenantDomain = tokReqMsgCtx.getAuthorizedUser().getTenantDomain(); + RealmService realmService = OAuthExtensionsDataHolder.getInstance().getRealmService(); + + try { + tenantId = realmService.getTenantManager().getTenantId(tenantDomain); + + // If tenant Id is not set in the tokenReqContext, deriving it from username. + if (tenantId == 0 || tenantId == -1) { + tenantId = IdentityTenantUtil.getTenantIdOfUser(username); + } + + UserRealm userRealm = OAuthExtensionsDataHolder.getInstance().getRealmService().getTenantUserRealm(tenantId); + + //Iterate the requested scopes list. + for (String scope : reqScopeList) { + status = false; + + //Get the set of roles associated with the requested scope. + String appPermissions = appScopes.get(scope); + + //If the scope has been defined in the context of the App and if permissions have been defined for the scope + if (appPermissions != null && appPermissions.length() != 0) { + List permissions = new ArrayList<>(Arrays.asList(appPermissions.replaceAll(" ", "").split(","))); + + //Check if user has at least one of the permission associated with the scope + if (!permissions.isEmpty()) { + for (String permission : permissions) { + if (userRealm != null && userRealm.getAuthorizationManager() != null) { + String userStore = tokReqMsgCtx.getAuthorizedUser().getUserStoreDomain(); + + if (userStore != null) { + status = userRealm.getAuthorizationManager() + .isUserAuthorized(userStore + "/" + username, permission, UI_EXECUTE); + } else { + status = userRealm.getAuthorizationManager() + .isUserAuthorized(username, permission, UI_EXECUTE); + } + if (status) { + break; + } + } + } + if (status) { + authorizedScopes.add(scope); + } + } + } + + //The scope string starts with 'device_'. + else if (appScopes.containsKey(scope) || isWhiteListedScope(scope)) { + authorizedScopes.add(scope); + } + } + } catch (UserStoreException e) { + log.error("Error occurred while initializing user store.", e); + } + return authorizedScopes; + } + } diff --git a/components/identity-extensions/org.wso2.carbon.device.mgt.oauth.extensions/src/main/java/org/wso2/carbon/device/mgt/oauth/extensions/handlers/grant/ExtendedPasswordGrantHandler.java b/components/identity-extensions/org.wso2.carbon.device.mgt.oauth.extensions/src/main/java/org/wso2/carbon/device/mgt/oauth/extensions/handlers/grant/ExtendedPasswordGrantHandler.java index 2223a915bc1..d39ea69f0c2 100644 --- a/components/identity-extensions/org.wso2.carbon.device.mgt.oauth.extensions/src/main/java/org/wso2/carbon/device/mgt/oauth/extensions/handlers/grant/ExtendedPasswordGrantHandler.java +++ b/components/identity-extensions/org.wso2.carbon.device.mgt.oauth.extensions/src/main/java/org/wso2/carbon/device/mgt/oauth/extensions/handlers/grant/ExtendedPasswordGrantHandler.java @@ -42,12 +42,7 @@ import org.wso2.carbon.user.core.service.RealmService; import org.wso2.carbon.user.core.util.UserCoreUtil; import javax.xml.namespace.QName; - -import java.util.ArrayList; -import java.util.HashMap; -import java.util.Iterator; -import java.util.List; -import java.util.Map; +import java.util.*; import java.util.concurrent.ConcurrentHashMap; @SuppressWarnings("unused") @@ -68,7 +63,7 @@ public class ExtendedPasswordGrantHandler extends PasswordGrantHandler { private static final String EMAIL_LOGIN = "EmailLogin"; private static final String PRIMARY_LOGIN = "primary"; - private Map> loginConfiguration = new ConcurrentHashMap<>(); + private Map> loginConfiguration = new ConcurrentHashMap<>(); private List requiredHeaderClaimUris = new ArrayList<>(); @@ -103,7 +98,7 @@ public class ExtendedPasswordGrantHandler extends PasswordGrantHandler { boolean isValidated = super.validateGrant(tokReqMsgCtx); - if(isValidated){ + if (isValidated) { int tenantId; tenantId = IdentityTenantUtil.getTenantIdOfUser(username); @@ -120,20 +115,17 @@ public class ExtendedPasswordGrantHandler extends PasswordGrantHandler { List respHeaders = new ArrayList<>(); if (oAuth2AccessTokenReqDTO.getResourceOwnerUsername() != null) { - try { - if (requiredHeaderClaimUris != null && !requiredHeaderClaimUris.isEmpty()) { // Get user's claim values from the default profile. String userStoreDomain = tokReqMsgCtx.getAuthorizedUser().getUserStoreDomain(); - String endUsernameWithDomain = UserCoreUtil.addDomainToName - (oAuth2AccessTokenReqDTO.getResourceOwnerUsername(), - userStoreDomain); + String endUsernameWithDomain = UserCoreUtil. + addDomainToName(oAuth2AccessTokenReqDTO.getResourceOwnerUsername(), userStoreDomain); - Claim[] mapClaimValues = getUserClaimValues(endUsernameWithDomain,userStoreManager); + Claim[] mapClaimValues = getUserClaimValues(endUsernameWithDomain, userStoreManager); - if(mapClaimValues != null && mapClaimValues.length > 0){ + if (mapClaimValues != null && mapClaimValues.length > 0) { ResponseHeader header; for (String claimUri : requiredHeaderClaimUris) { for (Claim claim : mapClaimValues) { @@ -146,26 +138,22 @@ public class ExtendedPasswordGrantHandler extends PasswordGrantHandler { } } } + } else if (log.isDebugEnabled()) { + log.debug("No claim values for user : " + endUsernameWithDomain); } - else if(log.isDebugEnabled()){ - log.debug("No claim values for user : "+endUsernameWithDomain); - } - } } catch (Exception e) { - throw new IdentityOAuth2Exception(e.getMessage(), e); + throw new IdentityOAuth2Exception("Error occurred while retrieving user claims", e); } } - - tokReqMsgCtx.addProperty("RESPONSE_HEADERS", respHeaders.toArray( - new ResponseHeader[respHeaders.size()])); + tokReqMsgCtx.addProperty("RESPONSE_HEADERS", respHeaders.toArray(new ResponseHeader[respHeaders.size()])); } return isValidated; } @Override - public boolean validateScope(OAuthTokenReqMessageContext tokReqMsgCtx){ + public boolean validateScope(OAuthTokenReqMessageContext tokReqMsgCtx) { return OAuthExtUtils.setScopes(tokReqMsgCtx); } @@ -183,7 +171,7 @@ public class ExtendedPasswordGrantHandler extends PasswordGrantHandler { * * @param userId - The username used to login. * @return true if secondary login name is used, - * false if primary login name has been used + * false if primary login name has been used */ private boolean isSecondaryLogin(String userId) { @@ -191,17 +179,14 @@ public class ExtendedPasswordGrantHandler extends PasswordGrantHandler { Map emailConf = loginConfiguration.get(EMAIL_LOGIN); if ("true".equalsIgnoreCase(emailConf.get(PRIMARY_LOGIN))) { return !isUserLoggedInEmail(userId); - } - else if ("false".equalsIgnoreCase(emailConf.get(PRIMARY_LOGIN))) { + } else if ("false".equalsIgnoreCase(emailConf.get(PRIMARY_LOGIN))) { return isUserLoggedInEmail(userId); } - } - else if (loginConfiguration.get(USERID_LOGIN) != null) { + } else if (loginConfiguration.get(USERID_LOGIN) != null) { Map userIdConf = loginConfiguration.get(USERID_LOGIN); if ("true".equalsIgnoreCase(userIdConf.get(PRIMARY_LOGIN))) { return isUserLoggedInEmail(userId); - } - else if ("false".equalsIgnoreCase(userIdConf.get(PRIMARY_LOGIN))) { + } else if ("false".equalsIgnoreCase(userIdConf.get(PRIMARY_LOGIN))) { return !isUserLoggedInEmail(userId); } } @@ -258,20 +243,22 @@ public class ExtendedPasswordGrantHandler extends PasswordGrantHandler { throws UserStoreException { Claim[] userClaims = userClaimsCache.getValueFromCache(authorizedUser); - if(userClaims != null){ + if (userClaims != null) { return userClaims; - }else{ - if(log.isDebugEnabled()){ + } else { + if (log.isDebugEnabled()) { log.debug("Cache miss for user claims. Username :" + authorizedUser); } userClaims = userStoreManager.getUserClaimValues( authorizedUser, null); - userClaimsCache.addToCache(authorizedUser,userClaims); + userClaimsCache.addToCache(authorizedUser, userClaims); return userClaims; } } - // Read the required claim configuration from identity.xml + /** + * Read the required claim configuration from identity.xml + */ private void parseRequiredHeaderClaimUris(OMElement requiredClaimUrisElem) { if (requiredClaimUrisElem == null) { return; @@ -291,21 +278,22 @@ public class ExtendedPasswordGrantHandler extends PasswordGrantHandler { /** * Read the primary/secondary login configuration * - * .... - * - * - * - * - * - * http://wso2.org/claims/emailaddress - * - * - * ..... - * + * .... + * + * + * + * + * + * http://wso2.org/claims/emailaddress + * + * + * ..... + * + * * @param oauthConfigElem - The '' xml configuration element in the api-manager.xml */ private void parseLoginConfig(OMElement oauthConfigElem) { - OMElement loginConfigElem = oauthConfigElem.getFirstChildWithName(getQNameWithIdentityNS(LOGIN_CONFIG)); + OMElement loginConfigElem = oauthConfigElem.getFirstChildWithName(getQNameWithIdentityNS(LOGIN_CONFIG)); if (loginConfigElem != null) { if (log.isDebugEnabled()) { log.debug("Login configuration is set "); @@ -313,7 +301,7 @@ public class ExtendedPasswordGrantHandler extends PasswordGrantHandler { // Primary/Secondary supported login mechanisms OMElement emailConfigElem = loginConfigElem.getFirstChildWithName(getQNameWithIdentityNS(EMAIL_LOGIN)); - OMElement userIdConfigElem = loginConfigElem.getFirstChildWithName(getQNameWithIdentityNS(USERID_LOGIN)); + OMElement userIdConfigElem = loginConfigElem.getFirstChildWithName(getQNameWithIdentityNS(USERID_LOGIN)); Map emailConf = new HashMap(2); emailConf.put(PRIMARY_LOGIN,