Updated Branches: refs/heads/master 0dca44efe -> ad063ed61
StaticRoleBasedAPIAccessChecker: Throw exception on failed check Plugin should not be responsible for existence of checking an API, this was wrong. Throw exception boldly when checkAccess fails. Signed-off-by: Rohit Yadav <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/commit/ad063ed6 Tree: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/tree/ad063ed6 Diff: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/diff/ad063ed6 Branch: refs/heads/master Commit: ad063ed61055ca26b23594b4c47e30a3c22974d7 Parents: 0dca44e Author: Rohit Yadav <[email protected]> Authored: Fri Jan 11 19:23:32 2013 -0800 Committer: Rohit Yadav <[email protected]> Committed: Fri Jan 11 19:24:11 2013 -0800 ---------------------------------------------------------------------- api/src/org/apache/cloudstack/acl/APIChecker.java | 5 +-- .../acl/StaticRoleBasedAPIAccessChecker.java | 17 ++++++-------- server/src/com/cloud/api/ApiServer.java | 15 ++---------- 3 files changed, 12 insertions(+), 25 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/ad063ed6/api/src/org/apache/cloudstack/acl/APIChecker.java ---------------------------------------------------------------------- diff --git a/api/src/org/apache/cloudstack/acl/APIChecker.java b/api/src/org/apache/cloudstack/acl/APIChecker.java index 61dd7de..b14dfe1 100644 --- a/api/src/org/apache/cloudstack/acl/APIChecker.java +++ b/api/src/org/apache/cloudstack/acl/APIChecker.java @@ -16,13 +16,12 @@ // under the License. package org.apache.cloudstack.acl; +import com.cloud.exception.PermissionDeniedException; import org.apache.cloudstack.acl.RoleType; import com.cloud.utils.component.Adapter; // APIChecker checks the ownership and access control to API requests public interface APIChecker extends Adapter { // Interface for checking access for a role using apiname - boolean checkAccess(RoleType roleType, String apiCommandName); - // Interface for checking existence of an api by name - boolean checkExistence(String apiCommandName); + boolean checkAccess(RoleType roleType, String apiCommandName) throws PermissionDeniedException; } http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/ad063ed6/plugins/acl/static-role-based/src/org/apache/cloudstack/acl/StaticRoleBasedAPIAccessChecker.java ---------------------------------------------------------------------- diff --git a/plugins/acl/static-role-based/src/org/apache/cloudstack/acl/StaticRoleBasedAPIAccessChecker.java b/plugins/acl/static-role-based/src/org/apache/cloudstack/acl/StaticRoleBasedAPIAccessChecker.java index 380b671..affd69e 100644 --- a/plugins/acl/static-role-based/src/org/apache/cloudstack/acl/StaticRoleBasedAPIAccessChecker.java +++ b/plugins/acl/static-role-based/src/org/apache/cloudstack/acl/StaticRoleBasedAPIAccessChecker.java @@ -16,6 +16,7 @@ // under the License. package org.apache.cloudstack.acl; +import com.cloud.exception.PermissionDeniedException; import com.cloud.server.ManagementServer; import com.cloud.utils.component.AdapterBase; import com.cloud.utils.component.ComponentLocator; @@ -48,17 +49,13 @@ public class StaticRoleBasedAPIAccessChecker extends AdapterBase implements APIC } @Override - public boolean checkAccess(RoleType roleType, String commandName) { - return s_roleBasedApisMap.get(roleType).contains(commandName); - } - - @Override - public boolean checkExistence(String apiName) { - for (RoleType roleType: RoleType.values()) { - if (s_roleBasedApisMap.get(roleType).contains(apiName)) - return true; + public boolean checkAccess(RoleType roleType, String commandName) + throws PermissionDeniedException { + boolean isAllowed = s_roleBasedApisMap.get(roleType).contains(commandName); + if (!isAllowed) { + throw new PermissionDeniedException("The API does not exist or is blacklisted. Role type=" + roleType.toString() + " is not allowed to request the api: " + commandName); } - return false; + return isAllowed; } @Override http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/ad063ed6/server/src/com/cloud/api/ApiServer.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/api/ApiServer.java b/server/src/com/cloud/api/ApiServer.java index 52f2aef..03462e4 100755 --- a/server/src/com/cloud/api/ApiServer.java +++ b/server/src/com/cloud/api/ApiServer.java @@ -556,7 +556,7 @@ public class ApiServer implements HttpRequestHandler { return true; } else { // check against every available command to see if the command exists or not - if (!doesCommandExist(commandName) && !commandName.equals("login") && !commandName.equals("logout")) { + if (!_apiNameCmdClassMap.containsKey(commandName) && !commandName.equals("login") && !commandName.equals("logout")) { s_logger.debug("The given command:" + commandName + " does not exist or it is not available for user with id:" + userId); throw new ServerApiException(BaseCmd.UNSUPPORTED_ACTION_ERROR, "The given command does not exist or it is not available for user"); } @@ -780,18 +780,9 @@ public class ApiServer implements HttpRequestHandler { return true; } - private boolean doesCommandExist(String apiName) { - for (APIChecker apiChecker : _apiAccessCheckers) { - // If any checker has api info on the command, return true - if (apiChecker.checkExistence(apiName)) - return true; - } - return false; - } - - private boolean isCommandAvailable(User user, String commandName) { + private boolean isCommandAvailable(User user, String commandName) throws PermissionDeniedException { if (user == null) { - return false; + throw new PermissionDeniedException("User is null for role based API access check for command" + commandName); } Account account = _accountMgr.getAccount(user.getAccountId());
