Skip to content

Commit 0f79223

Browse files
minchen07minchen07
committed
CLOUDSTACK-6613:IAM: authorizeSecurityGroupIngress fails when SG Name is
passed.
1 parent 51cb0f9 commit 0f79223

4 files changed

Lines changed: 35 additions & 26 deletions

File tree

‎api/src/org/apache/cloudstack/api/command/user/securitygroup/AuthorizeSecurityGroupEgressCmd.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,8 @@ public class AuthorizeSecurityGroupEgressCmd extends BaseAsyncCmd {
9595
@Parameter(name=ApiConstants.SECURITY_GROUP_ID, type=CommandType.UUID, description="The ID of the security group. Mutually exclusive with securityGroupName parameter", entityType=SecurityGroupResponse.class)
9696
private Long securityGroupId;
9797

98-
@ACL(accessType = AccessType.OperateEntry)
98+
// This @ACL will not work, since we don't have a way to convert this parameter to the entity like securityGroupId.
99+
//@ACL(accessType = AccessType.OperateEntry)
99100
@Parameter(name=ApiConstants.SECURITY_GROUP_NAME, type=CommandType.STRING, description="The name of the security group. Mutually exclusive with securityGroupName parameter")
100101
private String securityGroupName;
101102

‎api/src/org/apache/cloudstack/api/command/user/securitygroup/AuthorizeSecurityGroupIngressCmd.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,8 @@ public class AuthorizeSecurityGroupIngressCmd extends BaseAsyncCmd {
9595
@Parameter(name=ApiConstants.SECURITY_GROUP_ID, type=CommandType.UUID, description="The ID of the security group. Mutually exclusive with securityGroupName parameter", entityType=SecurityGroupResponse.class)
9696
private Long securityGroupId;
9797

98-
@ACL(accessType = AccessType.OperateEntry)
98+
// This @ACL will not work, since we don't have a way to convert this parameter to the entity like securityGroupId.
99+
//@ACL(accessType = AccessType.OperateEntry)
99100
@Parameter(name=ApiConstants.SECURITY_GROUP_NAME, type=CommandType.STRING, description="The name of the security group. Mutually exclusive with securityGroupName parameter")
100101
private String securityGroupName;
101102

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

Lines changed: 29 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -96,10 +96,14 @@
9696
import org.apache.cloudstack.api.ResponseObject;
9797
import org.apache.cloudstack.api.ResponseObject.ResponseView;
9898
import org.apache.cloudstack.api.ServerApiException;
99+
import org.apache.cloudstack.api.command.admin.account.ListAccountsCmdByAdmin;
99100
import org.apache.cloudstack.api.command.admin.host.ListHostsCmd;
100101
import org.apache.cloudstack.api.command.admin.router.ListRoutersCmd;
101102
import org.apache.cloudstack.api.command.admin.storage.ListStoragePoolsCmd;
102103
import org.apache.cloudstack.api.command.admin.user.ListUsersCmd;
104+
import org.apache.cloudstack.api.command.admin.vm.ListVMsCmdByAdmin;
105+
import org.apache.cloudstack.api.command.admin.volume.ListVolumesCmdByAdmin;
106+
import org.apache.cloudstack.api.command.admin.zone.ListZonesCmdByAdmin;
103107
import org.apache.cloudstack.api.command.user.account.ListAccountsCmd;
104108
import org.apache.cloudstack.api.command.user.account.ListProjectAccountsCmd;
105109
import org.apache.cloudstack.api.command.user.event.ListEventsCmd;
@@ -138,8 +142,8 @@
138142
import com.cloud.domain.DomainVO;
139143
import com.cloud.domain.dao.DomainDao;
140144
import com.cloud.event.ActionEventUtils;
141-
import com.cloud.event.EventTypes;
142145
import com.cloud.event.EventCategory;
146+
import com.cloud.event.EventTypes;
143147
import com.cloud.exception.AccountLimitException;
144148
import com.cloud.exception.CloudAuthenticationException;
145149
import com.cloud.exception.InsufficientCapacityException;
@@ -210,7 +214,7 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
210214
private static Map<String, List<Class<?>>> s_apiNameCmdClassMap = new HashMap<String, List<Class<?>>>();
211215

212216
private static ExecutorService s_executor = new ThreadPoolExecutor(10, 150, 60, TimeUnit.SECONDS, new LinkedBlockingQueue<Runnable>(), new NamedThreadFactory(
213-
"ApiServer"));
217+
"ApiServer"));
214218
@Inject
215219
MessageBus _messageBus;
216220

@@ -442,7 +446,7 @@ public void checkCharacterInkParams(final Map params) {
442446
final Matcher matcher = pattern.matcher(value[0]);
443447
if (matcher.find()) {
444448
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Received value " + value[0] + " for parameter " + key +
445-
" is invalid, contains illegal ASCII non-printable characters");
449+
" is invalid, contains illegal ASCII non-printable characters");
446450
}
447451
}
448452
stringMap.put(key, value[0]);
@@ -506,7 +510,7 @@ public String handleRequest(final Map params, final String responseType, final S
506510
StringUtils.cleanString(response));
507511
}
508512
else
509-
buildAuditTrail(auditTrailSb, command[0], response);
513+
buildAuditTrail(auditTrailSb, command[0], response);
510514
} else {
511515
if (!command[0].equalsIgnoreCase("login") && !command[0].equalsIgnoreCase("logout")) {
512516
final String errorString = "Unknown API command: " + command[0];
@@ -612,7 +616,7 @@ private String queueCommand(final BaseCmd cmdObj, final Map<String, String> para
612616
objectUuid = createCmd.getEntityUuid();
613617
params.put("id", objectId.toString());
614618
Class entityClass = EventTypes.getEntityClassForEvent(createCmd.getEventType());
615-
if(entityClass != null)
619+
if (entityClass != null)
616620
ctx.putContextParameter(entityClass.getName(), objectId);
617621
} else {
618622
// Extract the uuid before params are processed and id reflects internal db id
@@ -628,7 +632,7 @@ private String queueCommand(final BaseCmd cmdObj, final Map<String, String> para
628632
if (caller != null) {
629633
params.put("ctxAccountId", String.valueOf(caller.getId()));
630634
}
631-
if(objectUuid != null){
635+
if (objectUuid != null) {
632636
params.put("uuid", objectUuid);
633637
}
634638

@@ -637,14 +641,14 @@ private String queueCommand(final BaseCmd cmdObj, final Map<String, String> para
637641

638642
// Add the resource id in the call context, also add some other first class object ids (for now vm) if available.
639643
// TODO - this should be done for all the uuids passed in the cmd - so should be moved where uuid to id conversion happens.
640-
if(EventTypes.getEntityForEvent(asyncCmd.getEventType()) != null){
644+
if (EventTypes.getEntityForEvent(asyncCmd.getEventType()) != null) {
641645
ctx.putContextParameter(EventTypes.getEntityForEvent(asyncCmd.getEventType()), objectUuid);
642646
}
643647

644648
// save the scheduled event
645649
final Long eventId =
646-
ActionEventUtils.onScheduledActionEvent((callerUserId == null) ? User.UID_SYSTEM : callerUserId, asyncCmd.getEntityOwnerId(), asyncCmd.getEventType(),
647-
asyncCmd.getEventDescription(), asyncCmd.isDisplay(), startEventId);
650+
ActionEventUtils.onScheduledActionEvent((callerUserId == null) ? User.UID_SYSTEM : callerUserId, asyncCmd.getEntityOwnerId(), asyncCmd.getEventType(),
651+
asyncCmd.getEventDescription(), asyncCmd.isDisplay(), startEventId);
648652
if (startEventId == 0) {
649653
// There was no create event before, set current event id as start eventId
650654
startEventId = eventId;
@@ -681,13 +685,15 @@ private String queueCommand(final BaseCmd cmdObj, final Map<String, String> para
681685
// if the command is of the listXXXCommand, we will need to also return the
682686
// the job id and status if possible
683687
// For those listXXXCommand which we have already created DB views, this step is not needed since async job is joined in their db views.
684-
if (cmdObj instanceof BaseListCmd && !(cmdObj instanceof ListVMsCmd) && !(cmdObj instanceof ListRoutersCmd) && !(cmdObj instanceof ListSecurityGroupsCmd) &&
685-
!(cmdObj instanceof ListTagsCmd) && !(cmdObj instanceof ListEventsCmd) && !(cmdObj instanceof ListVMGroupsCmd) && !(cmdObj instanceof ListProjectsCmd) &&
686-
!(cmdObj instanceof ListProjectAccountsCmd) && !(cmdObj instanceof ListProjectInvitationsCmd) && !(cmdObj instanceof ListHostsCmd) &&
687-
!(cmdObj instanceof ListVolumesCmd) && !(cmdObj instanceof ListUsersCmd) && !(cmdObj instanceof ListAccountsCmd) &&
688-
!(cmdObj instanceof ListStoragePoolsCmd) && !(cmdObj instanceof ListDiskOfferingsCmd) && !(cmdObj instanceof ListServiceOfferingsCmd) &&
689-
!(cmdObj instanceof ListZonesCmd)) {
690-
buildAsyncListResponse((BaseListCmd) cmdObj, caller);
688+
if (cmdObj instanceof BaseListCmd && !(cmdObj instanceof ListVMsCmd) && !(cmdObj instanceof ListVMsCmdByAdmin) && !(cmdObj instanceof ListRoutersCmd)
689+
&& !(cmdObj instanceof ListSecurityGroupsCmd) &&
690+
!(cmdObj instanceof ListTagsCmd) && !(cmdObj instanceof ListEventsCmd) && !(cmdObj instanceof ListVMGroupsCmd) && !(cmdObj instanceof ListProjectsCmd) &&
691+
!(cmdObj instanceof ListProjectAccountsCmd) && !(cmdObj instanceof ListProjectInvitationsCmd) && !(cmdObj instanceof ListHostsCmd) &&
692+
!(cmdObj instanceof ListVolumesCmd) && !(cmdObj instanceof ListVolumesCmdByAdmin) && !(cmdObj instanceof ListUsersCmd) && !(cmdObj instanceof ListAccountsCmd)
693+
&& !(cmdObj instanceof ListAccountsCmdByAdmin) &&
694+
!(cmdObj instanceof ListStoragePoolsCmd) && !(cmdObj instanceof ListDiskOfferingsCmd) && !(cmdObj instanceof ListServiceOfferingsCmd) &&
695+
!(cmdObj instanceof ListZonesCmd) && !(cmdObj instanceof ListZonesCmdByAdmin)) {
696+
buildAsyncListResponse((BaseListCmd)cmdObj, caller);
691697
}
692698

693699
SerializationContext.current().setUuidTranslation(true);
@@ -861,7 +867,7 @@ public boolean verifyRequest(final Map<String, Object[]> requestParameters, fina
861867

862868
if (user.getState() != Account.State.enabled || !account.getState().equals(Account.State.enabled)) {
863869
s_logger.info("disabled or locked user accessing the api, userid = " + user.getId() + "; name = " + user.getUsername() + "; state: " + user.getState() +
864-
"; accountState: " + account.getState());
870+
"; accountState: " + account.getState());
865871
return false;
866872
}
867873

@@ -917,7 +923,7 @@ public Long fetchDomainId(final String domainUUID) {
917923

918924
@Override
919925
public void loginUser(final HttpSession session, final String username, final String password, Long domainId, final String domainPath, final String loginIpAddress,
920-
final Map<String, Object[]> requestParameters) throws CloudAuthenticationException {
926+
final Map<String, Object[]> requestParameters) throws CloudAuthenticationException {
921927
// We will always use domainId first. If that does not exist, we will use domain name. If THAT doesn't exist
922928
// we will default to ROOT
923929
if (domainId == null) {
@@ -1006,7 +1012,7 @@ public boolean verifyUser(final Long userId) {
10061012
}
10071013

10081014
if ((user == null) || (user.getRemoved() != null) || !user.getState().equals(Account.State.enabled) || (account == null) ||
1009-
!account.getState().equals(Account.State.enabled)) {
1015+
!account.getState().equals(Account.State.enabled)) {
10101016
s_logger.warn("Deleted/Disabled/Locked user with id=" + userId + " attempting to access public API");
10111017
return false;
10121018
}
@@ -1102,10 +1108,10 @@ public ListenerThread(final ApiServer requestHandler, final int port) {
11021108

11031109
_params = new BasicHttpParams();
11041110
_params.setIntParameter(CoreConnectionPNames.SO_TIMEOUT, 30000)
1105-
.setIntParameter(CoreConnectionPNames.SOCKET_BUFFER_SIZE, 8 * 1024)
1106-
.setBooleanParameter(CoreConnectionPNames.STALE_CONNECTION_CHECK, false)
1107-
.setBooleanParameter(CoreConnectionPNames.TCP_NODELAY, true)
1108-
.setParameter(CoreProtocolPNames.ORIGIN_SERVER, "HttpComponents/1.1");
1111+
.setIntParameter(CoreConnectionPNames.SOCKET_BUFFER_SIZE, 8 * 1024)
1112+
.setBooleanParameter(CoreConnectionPNames.STALE_CONNECTION_CHECK, false)
1113+
.setBooleanParameter(CoreConnectionPNames.TCP_NODELAY, true)
1114+
.setParameter(CoreProtocolPNames.ORIGIN_SERVER, "HttpComponents/1.1");
11091115

11101116
// Set up the HTTP protocol processor
11111117
final BasicHttpProcessor httpproc = new BasicHttpProcessor();

‎server/src/com/cloud/api/dispatch/ParamProcessWorker.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,8 @@ public void processParameters(final BaseCmd cmd, final Map params) {
155155
// for maps, specify access to be checkd on key or value.
156156
// Find the controlled entity DBid by uuid
157157

158-
if (parameterAnnotation.entityType() != null) {
158+
if (parameterAnnotation.entityType() != null && parameterAnnotation.entityType().length > 0
159+
&& parameterAnnotation.entityType()[0].getAnnotation(EntityReference.class) != null) {
159160
final Class<?>[] entityList = parameterAnnotation.entityType()[0].getAnnotation(EntityReference.class).value();
160161

161162
// Check if the parameter type is a single

0 commit comments

Comments
 (0)