Skip to content

Commit 74bb043

Browse files
yadvryadvr
committed
APIChecker: Rename refactor and add interface checkExistence
Signed-off-by: Rohit Yadav <bhaisaab@apache.org>
1 parent 18bdc58 commit 74bb043

4 files changed

Lines changed: 35 additions & 20 deletions

File tree

api/src/org/apache/cloudstack/acl/APIAccessChecker.java renamed to api/src/org/apache/cloudstack/acl/APIChecker.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,10 @@
1919
import org.apache.cloudstack.acl.RoleType;
2020
import com.cloud.utils.component.Adapter;
2121

22-
/**
23-
* APIAccessChecker checks the ownership and access control to API requests
24-
*/
25-
public interface APIAccessChecker extends Adapter {
26-
// Interface for checking access to an API for an user
27-
boolean canAccessAPI(RoleType roleType, String apiCommandName);
22+
// APIChecker checks the ownership and access control to API requests
23+
public interface APIChecker extends Adapter {
24+
// Interface for checking access for a role using apiname
25+
boolean checkAccess(RoleType roleType, String apiCommandName);
26+
// Interface for checking existence of an api by name
27+
boolean checkExistence(String apiCommandName);
2828
}

‎client/tomcatconf/components.xml.in‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ under the License.
5353
<dao name="Configuration configuration server" class="com.cloud.configuration.dao.ConfigurationDaoImpl">
5454
<param name="premium">true</param>
5555
</dao>
56-
<adapters key="org.apache.cloudstack.acl.APIAccessChecker">
56+
<adapters key="org.apache.cloudstack.acl.APIChecker">
5757
<adapter name="StaticRoleBasedAPIAccessChecker" class="org.apache.cloudstack.acl.StaticRoleBasedAPIAccessChecker"/>
5858
</adapters>
5959
<adapters key="com.cloud.agent.manager.allocator.HostAllocator">

‎plugins/acl/static-role-based/src/org/apache/cloudstack/acl/StaticRoleBasedAPIAccessChecker.java‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,12 @@
2929
import java.util.Map;
3030
import java.util.Set;
3131

32-
import static org.apache.cloudstack.acl.RoleType.*;
3332
import org.apache.log4j.Logger;
3433

3534
// This is the default API access checker that grab's the user's account
3635
// based on the account type, access is granted
37-
@Local(value=APIAccessChecker.class)
38-
public class StaticRoleBasedAPIAccessChecker extends AdapterBase implements APIAccessChecker {
36+
@Local(value=APIChecker.class)
37+
public class StaticRoleBasedAPIAccessChecker extends AdapterBase implements APIChecker {
3938

4039
protected static final Logger s_logger = Logger.getLogger(StaticRoleBasedAPIAccessChecker.class);
4140

@@ -50,10 +49,19 @@ protected StaticRoleBasedAPIAccessChecker() {
5049
}
5150

5251
@Override
53-
public boolean canAccessAPI(RoleType roleType, String commandName) {
52+
public boolean checkAccess(RoleType roleType, String commandName) {
5453
return s_roleBasedApisMap.get(roleType).contains(commandName);
5554
}
5655

56+
@Override
57+
public boolean checkExistence(String apiName) {
58+
for (RoleType roleType: RoleType.values()) {
59+
if (s_roleBasedApisMap.get(roleType).contains(apiName))
60+
return true;
61+
}
62+
return false;
63+
}
64+
5765
@Override
5866
public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
5967
super.configure(name, params);

‎server/src/com/cloud/api/ApiServer.java‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,7 @@
5151
import javax.servlet.http.HttpSession;
5252

5353
import com.cloud.utils.ReflectUtil;
54-
import org.apache.cloudstack.acl.APIAccessChecker;
55-
import org.apache.cloudstack.acl.ControlledEntity;
54+
import org.apache.cloudstack.acl.APIChecker;
5655
import org.apache.cloudstack.acl.RoleType;
5756
import org.apache.cloudstack.api.*;
5857
import org.apache.cloudstack.api.command.user.account.ListAccountsCmd;
@@ -146,8 +145,8 @@ public class ApiServer implements HttpRequestHandler {
146145
@Inject private DomainManager _domainMgr = null;
147146
@Inject private AsyncJobManager _asyncMgr = null;
148147

149-
@Inject(adapter = APIAccessChecker.class)
150-
protected Adapters<APIAccessChecker> _apiAccessCheckers;
148+
@Inject(adapter = APIChecker.class)
149+
protected Adapters<APIChecker> _apiAccessCheckers;
151150

152151
private Account _systemAccount = null;
153152
private User _systemUser = null;
@@ -558,7 +557,7 @@ public boolean verifyRequest(Map<String, Object[]> requestParameters, Long userI
558557
return true;
559558
} else {
560559
// check against every available command to see if the command exists or not
561-
if (!isCommandAvailable(null, commandName) && !commandName.equals("login") && !commandName.equals("logout")) {
560+
if (!doesCommandExist(commandName) && !commandName.equals("login") && !commandName.equals("logout")) {
562561
s_logger.debug("The given command:" + commandName + " does not exist or it is not available for user with id:" + userId);
563562
throw new ServerApiException(BaseCmd.UNSUPPORTED_ACTION_ERROR, "The given command does not exist or it is not available for user");
564563
}
@@ -790,17 +789,25 @@ public boolean verifyUser(Long userId) {
790789
return true;
791790
}
792791

793-
private boolean isCommandAvailable(User user, String commandName)
794-
throws PermissionDeniedException {
792+
private boolean doesCommandExist(String apiName) {
793+
for (APIChecker apiChecker : _apiAccessCheckers) {
794+
// If any checker has api info on the command, return true
795+
if (apiChecker.checkExistence(apiName))
796+
return true;
797+
}
798+
return false;
799+
}
800+
801+
private boolean isCommandAvailable(User user, String commandName) {
795802
if (user == null) {
796803
return false;
797804
}
798805

799806
Account account = _accountMgr.getAccount(user.getAccountId());
800807
RoleType roleType = _accountMgr.getRoleType(account);
801-
for (APIAccessChecker apiChecker : _apiAccessCheckers) {
808+
for (APIChecker apiChecker : _apiAccessCheckers) {
802809
// Fail the checking if any checker fails to verify
803-
if (!apiChecker.canAccessAPI(roleType, commandName))
810+
if (!apiChecker.checkAccess(roleType, commandName))
804811
return false;
805812
}
806813
return true;

0 commit comments

Comments
 (0)