diff --git a/api/src/main/java/org/apache/cloudstack/api/APICommand.java b/api/src/main/java/org/apache/cloudstack/api/APICommand.java index c559be081165..b77649046ca9 100644 --- a/api/src/main/java/org/apache/cloudstack/api/APICommand.java +++ b/api/src/main/java/org/apache/cloudstack/api/APICommand.java @@ -50,4 +50,6 @@ RoleType[] authorized() default {}; Class[] entityType() default {}; + + String httpMethod() default ""; } diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java index 32de3db80f18..82a9a6817276 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java @@ -273,6 +273,7 @@ public class ApiConstants { public static final String HOST = "host"; public static final String HOST_CONTROL_STATE = "hostcontrolstate"; public static final String HOSTS_MAP = "hostsmap"; + public static final String HTTP_REQUEST_TYPE = "httprequesttype"; public static final String HYPERVISOR = "hypervisor"; public static final String INLINE = "inline"; public static final String INSTANCE = "instance"; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/IsAccountAllowedToCreateOfferingsWithTagsCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/IsAccountAllowedToCreateOfferingsWithTagsCmd.java index fcd6b03d3e59..4b1cd2ff7255 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/IsAccountAllowedToCreateOfferingsWithTagsCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/IsAccountAllowedToCreateOfferingsWithTagsCmd.java @@ -26,7 +26,8 @@ import org.apache.cloudstack.api.response.IsAccountAllowedToCreateOfferingsWithTagsResponse; @APICommand(name = "isAccountAllowedToCreateOfferingsWithTags", description = "Return true if the specified account is allowed to create offerings with tags.", - responseObject = IsAccountAllowedToCreateOfferingsWithTagsResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) + responseObject = IsAccountAllowedToCreateOfferingsWithTagsResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, + httpMethod = "GET") public class IsAccountAllowedToCreateOfferingsWithTagsCmd extends BaseCmd { @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = AccountResponse.class, description = "Account UUID", required = true) diff --git a/plugins/api/discovery/src/main/java/org/apache/cloudstack/api/response/ApiDiscoveryResponse.java b/plugins/api/discovery/src/main/java/org/apache/cloudstack/api/response/ApiDiscoveryResponse.java index 81a9df750cbe..41264b4af025 100644 --- a/plugins/api/discovery/src/main/java/org/apache/cloudstack/api/response/ApiDiscoveryResponse.java +++ b/plugins/api/discovery/src/main/java/org/apache/cloudstack/api/response/ApiDiscoveryResponse.java @@ -59,6 +59,10 @@ public class ApiDiscoveryResponse extends BaseResponse { @Param(description = "response field type") private String type; + @SerializedName(ApiConstants.HTTP_REQUEST_TYPE) + @Param(description = "Preferred HTTP request type for the API", since = "4.22.0") + private String httpRequestType; + public ApiDiscoveryResponse() { params = new HashSet(); apiResponse = new HashSet(); @@ -74,6 +78,7 @@ public ApiDiscoveryResponse(ApiDiscoveryResponse another) { this.params = new HashSet<>(another.getParams()); this.apiResponse = new HashSet<>(another.getApiResponse()); this.type = another.getType(); + this.httpRequestType = another.getHttpRequestType(); this.setObjectName(another.getObjectName()); } @@ -140,4 +145,12 @@ public Set getApiResponse() { public String getType() { return type; } + + public String getHttpRequestType() { + return httpRequestType; + } + + public void setHttpRequestType(String httpRequestType) { + this.httpRequestType = httpRequestType; + } } diff --git a/plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java b/plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java index 452b95cf2c05..d6d235162efb 100644 --- a/plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java +++ b/plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java @@ -50,6 +50,7 @@ import org.reflections.ReflectionUtils; import org.springframework.stereotype.Component; +import com.cloud.api.ApiServlet; import com.cloud.exception.PermissionDeniedException; import com.cloud.serializer.Param; import com.cloud.user.Account; @@ -189,7 +190,7 @@ private ApiResponseResponse getFieldResponseMap(Field responseField) { return responseResponse; } - private ApiDiscoveryResponse getCmdRequestMap(Class cmdClass, APICommand apiCmdAnnotation) { + protected ApiDiscoveryResponse getCmdRequestMap(Class cmdClass, APICommand apiCmdAnnotation) { String apiName = apiCmdAnnotation.name(); ApiDiscoveryResponse response = new ApiDiscoveryResponse(); response.setName(apiName); @@ -197,6 +198,12 @@ private ApiDiscoveryResponse getCmdRequestMap(Class cmdClass, APICommand apiC if (!apiCmdAnnotation.since().isEmpty()) { response.setSince(apiCmdAnnotation.since()); } + String httpRequestType = apiCmdAnnotation.httpMethod(); + if (StringUtils.isBlank(httpRequestType)) { + httpRequestType = ApiServlet.GET_REQUEST_COMMANDS.matcher(apiName.toLowerCase()).matches() ? + "GET" : "POST"; + } + response.setHttpRequestType(httpRequestType); Set fields = ReflectUtil.getAllFieldsForClass(cmdClass, new Class[] {BaseCmd.class, BaseAsyncCmd.class, BaseAsyncCreateCmd.class}); diff --git a/plugins/api/discovery/src/test/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImplTest.java b/plugins/api/discovery/src/test/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImplTest.java new file mode 100644 index 000000000000..e69b9523d449 --- /dev/null +++ b/plugins/api/discovery/src/test/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImplTest.java @@ -0,0 +1,123 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.discovery; + +import static org.mockito.ArgumentMatchers.any; + +import java.lang.reflect.Field; +import java.util.Set; + +import org.apache.cloudstack.api.APICommand; +import org.apache.cloudstack.api.BaseAsyncCmd; +import org.apache.cloudstack.api.BaseCmd; +import org.apache.cloudstack.api.Parameter; +import org.apache.cloudstack.api.command.admin.account.CreateAccountCmd; +import org.apache.cloudstack.api.command.admin.user.GetUserCmd; +import org.apache.cloudstack.api.command.user.discovery.ListApisCmd; +import org.apache.cloudstack.api.response.ApiDiscoveryResponse; +import org.apache.cloudstack.api.response.ApiParameterResponse; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.Mockito; +import org.mockito.Spy; +import org.mockito.junit.MockitoJUnitRunner; +import org.springframework.test.util.ReflectionTestUtils; + +import com.cloud.utils.ReflectUtil; + +@RunWith(MockitoJUnitRunner.class) +public class ApiDiscoveryServiceImplTest { + + @Mock + APICommand apiCommandMock; + + @Spy + @InjectMocks + ApiDiscoveryServiceImpl discoveryServiceSpy; + + @Before + public void setUp() { + Mockito.when(apiCommandMock.name()).thenReturn("listApis"); + Mockito.when(apiCommandMock.since()).thenReturn(""); + } + + @Test + public void getCmdRequestMapReturnsResponseWithCorrectApiNameAndDescription() { + Mockito.when(apiCommandMock.description()).thenReturn("Lists all APIs"); + ApiDiscoveryResponse response = discoveryServiceSpy.getCmdRequestMap(ListApisCmd.class, apiCommandMock); + Assert.assertEquals("listApis", response.getName()); + Assert.assertEquals("Lists all APIs", response.getDescription()); + } + + @Test + public void getCmdRequestMapSetsHttpRequestTypeToGetWhenApiNameMatchesGetPattern() { + Mockito.when(apiCommandMock.name()).thenReturn("getUser"); + Mockito.when(apiCommandMock.httpMethod()).thenReturn(""); + ApiDiscoveryResponse response = discoveryServiceSpy.getCmdRequestMap(GetUserCmd.class, apiCommandMock); + Assert.assertEquals("GET", response.getHttpRequestType()); + } + + @Test + public void getCmdRequestMapSetsHttpRequestTypeToPostWhenApiNameDoesNotMatchGetPattern() { + Mockito.when(apiCommandMock.name()).thenReturn("createAccount"); + Mockito.when(apiCommandMock.httpMethod()).thenReturn(""); + ApiDiscoveryResponse response = discoveryServiceSpy.getCmdRequestMap(CreateAccountCmd.class, apiCommandMock); + Assert.assertEquals("POST", response.getHttpRequestType()); + } + + @Test + public void getCmdRequestMapSetsAsyncToTrueForAsyncCommand() { + Mockito.when(apiCommandMock.name()).thenReturn("asyncApi"); + ApiDiscoveryResponse response = discoveryServiceSpy.getCmdRequestMap(BaseAsyncCmd.class, apiCommandMock); + Assert.assertTrue(response.getAsync()); + } + + @Test + public void getCmdRequestMapDoesNotAddParamsWithoutParameterAnnotation() { + ApiDiscoveryResponse response = discoveryServiceSpy.getCmdRequestMap(BaseCmd.class, apiCommandMock); + Assert.assertFalse(response.getParams().isEmpty()); + Assert.assertEquals(1, response.getParams().size()); + } + + @Test + public void getCmdRequestMapAddsParamsWithExposedAndIncludedInApiDocAnnotations() { + Field fieldMock = Mockito.mock(Field.class); + Parameter parameterMock = Mockito.mock(Parameter.class); + Mockito.when(parameterMock.expose()).thenReturn(true); + Mockito.when(parameterMock.includeInApiDoc()).thenReturn(true); + Mockito.when(parameterMock.name()).thenReturn("paramName"); + Mockito.when(parameterMock.since()).thenReturn(""); + Mockito.when(parameterMock.entityType()).thenReturn(new Class[]{Object.class}); + Mockito.when(parameterMock.description()).thenReturn("paramDescription"); + Mockito.when(parameterMock.type()).thenReturn(BaseCmd.CommandType.STRING); + Mockito.when(fieldMock.getAnnotation(Parameter.class)).thenReturn(parameterMock); + try (MockedStatic reflectUtilMockedStatic = Mockito.mockStatic(ReflectUtil.class)) { + reflectUtilMockedStatic.when(() -> ReflectUtil.getAllFieldsForClass(any(Class.class), any(Class[].class))) + .thenReturn(Set.of(fieldMock)); + ApiDiscoveryResponse response = discoveryServiceSpy.getCmdRequestMap(ListApisCmd.class, apiCommandMock); + Set params = response.getParams(); + Assert.assertEquals(1, params.size()); + ApiParameterResponse paramResponse = params.iterator().next(); + Assert.assertEquals("paramName", ReflectionTestUtils.getField(paramResponse, "name")); + } + } +} diff --git a/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaBalanceCmd.java b/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaBalanceCmd.java index 628eca56c2d1..13a43aa57afa 100644 --- a/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaBalanceCmd.java +++ b/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaBalanceCmd.java @@ -35,7 +35,8 @@ import org.apache.cloudstack.quota.vo.QuotaBalanceVO; import org.apache.cloudstack.api.response.QuotaStatementItemResponse; -@APICommand(name = "quotaBalance", responseObject = QuotaStatementItemResponse.class, description = "Create a quota balance statement", since = "4.7.0", requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) +@APICommand(name = "quotaBalance", responseObject = QuotaStatementItemResponse.class, description = "Create a quota balance statement", since = "4.7.0", requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, + httpMethod = "GET") public class QuotaBalanceCmd extends BaseCmd { diff --git a/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaEnabledCmd.java b/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaEnabledCmd.java index 4035a5205e6c..af1d146ea9dc 100644 --- a/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaEnabledCmd.java +++ b/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaEnabledCmd.java @@ -26,7 +26,8 @@ import javax.inject.Inject; -@APICommand(name = "quotaIsEnabled", responseObject = QuotaEnabledResponse.class, description = "Return true if the plugin is enabled", since = "4.7.0", requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) +@APICommand(name = "quotaIsEnabled", responseObject = QuotaEnabledResponse.class, description = "Return true if the plugin is enabled", since = "4.7.0", requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, + httpMethod = "GET") public class QuotaEnabledCmd extends BaseCmd { diff --git a/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaStatementCmd.java b/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaStatementCmd.java index 78fa0f7df847..f2b07488136a 100644 --- a/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaStatementCmd.java +++ b/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaStatementCmd.java @@ -35,7 +35,8 @@ import com.cloud.user.Account; -@APICommand(name = "quotaStatement", responseObject = QuotaStatementItemResponse.class, description = "Create a quota statement", since = "4.7.0", requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) +@APICommand(name = "quotaStatement", responseObject = QuotaStatementItemResponse.class, description = "Create a quota statement", since = "4.7.0", requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, + httpMethod = "GET") public class QuotaStatementCmd extends BaseCmd { diff --git a/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaSummaryCmd.java b/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaSummaryCmd.java index a1ef9b3746a5..bc6cca86d3e3 100644 --- a/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaSummaryCmd.java +++ b/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaSummaryCmd.java @@ -33,7 +33,8 @@ import javax.inject.Inject; -@APICommand(name = "quotaSummary", responseObject = QuotaSummaryResponse.class, description = "Lists balance and quota usage for all accounts", since = "4.7.0", requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) +@APICommand(name = "quotaSummary", responseObject = QuotaSummaryResponse.class, description = "Lists balance and quota usage for all accounts", since = "4.7.0", requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, + httpMethod = "GET") public class QuotaSummaryCmd extends BaseListCmd { @Parameter(name = ApiConstants.ACCOUNT, type = CommandType.STRING, required = false, description = "Optional, Account Id for which statement needs to be generated") diff --git a/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaTariffListCmd.java b/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaTariffListCmd.java index d054d5459313..e0bab07501b6 100644 --- a/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaTariffListCmd.java +++ b/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaTariffListCmd.java @@ -38,7 +38,8 @@ import java.util.Date; import java.util.List; -@APICommand(name = "quotaTariffList", responseObject = QuotaTariffResponse.class, description = "Lists all quota tariff plans", since = "4.7.0", requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) +@APICommand(name = "quotaTariffList", responseObject = QuotaTariffResponse.class, description = "Lists all quota tariff plans", since = "4.7.0", requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, + httpMethod = "GET") public class QuotaTariffListCmd extends BaseListCmd { @Inject diff --git a/plugins/integrations/cloudian/src/main/java/org/apache/cloudstack/cloudian/api/CloudianIsEnabledCmd.java b/plugins/integrations/cloudian/src/main/java/org/apache/cloudstack/cloudian/api/CloudianIsEnabledCmd.java index 56cb74e3cab7..3c334ba55c2e 100644 --- a/plugins/integrations/cloudian/src/main/java/org/apache/cloudstack/cloudian/api/CloudianIsEnabledCmd.java +++ b/plugins/integrations/cloudian/src/main/java/org/apache/cloudstack/cloudian/api/CloudianIsEnabledCmd.java @@ -31,7 +31,8 @@ responseObject = CloudianEnabledResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, since = "4.11.0", - authorized = {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User}) + authorized = {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User}, + httpMethod = "GET") public class CloudianIsEnabledCmd extends BaseCmd { @Inject diff --git a/plugins/maintenance/src/main/java/org/apache/cloudstack/api/command/ReadyForShutdownCmd.java b/plugins/maintenance/src/main/java/org/apache/cloudstack/api/command/ReadyForShutdownCmd.java index 782b23a04222..36ec4fff9c95 100644 --- a/plugins/maintenance/src/main/java/org/apache/cloudstack/api/command/ReadyForShutdownCmd.java +++ b/plugins/maintenance/src/main/java/org/apache/cloudstack/api/command/ReadyForShutdownCmd.java @@ -26,7 +26,8 @@ description = "Returns the status of CloudStack, whether a shutdown has been triggered and if ready to shutdown", since = "4.19.0", responseObject = ManagementServerMaintenanceResponse.class, - requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) + requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, + httpMethod = "GET") public class ReadyForShutdownCmd extends BaseMSMaintenanceActionCmd { public static final String APINAME = "readyForShutdown"; diff --git a/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/api/command/VerifyOAuthCodeAndGetUserCmd.java b/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/api/command/VerifyOAuthCodeAndGetUserCmd.java index bd49f87d6273..b3d2d335ba25 100644 --- a/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/api/command/VerifyOAuthCodeAndGetUserCmd.java +++ b/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/api/command/VerifyOAuthCodeAndGetUserCmd.java @@ -20,8 +20,10 @@ import java.util.List; import java.util.Map; -import com.cloud.api.response.ApiResponseSerializer; -import com.cloud.user.Account; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpSession; + import org.apache.cloudstack.acl.RoleType; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; @@ -37,13 +39,13 @@ import org.apache.cloudstack.oauth2.api.response.OauthProviderResponse; import org.apache.commons.lang.ArrayUtils; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import javax.servlet.http.HttpSession; +import com.cloud.api.response.ApiResponseSerializer; +import com.cloud.user.Account; @APICommand(name = "verifyOAuthCodeAndGetUser", description = "Verify the OAuth Code and fetch the corresponding user from provider", responseObject = OauthProviderResponse.class, entityType = {}, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, - authorized = {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User}, since = "4.19.0") + authorized = {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User}, since = "4.19.0", + httpMethod = "GET") public class VerifyOAuthCodeAndGetUserCmd extends BaseListCmd implements APIAuthenticator { ///////////////////////////////////////////////////// diff --git a/server/src/main/java/com/cloud/api/ApiServlet.java b/server/src/main/java/com/cloud/api/ApiServlet.java index 21d093758127..ae919baf398c 100644 --- a/server/src/main/java/com/cloud/api/ApiServlet.java +++ b/server/src/main/java/com/cloud/api/ApiServlet.java @@ -25,8 +25,8 @@ import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.regex.Pattern; import java.util.Set; +import java.util.regex.Pattern; import javax.inject.Inject; import javax.servlet.ServletConfig; @@ -52,10 +52,9 @@ import org.apache.cloudstack.managed.context.ManagedContext; import org.apache.cloudstack.utils.consoleproxy.ConsoleAccessUtils; import org.apache.commons.collections.MapUtils; - -import org.apache.logging.log4j.Logger; -import org.apache.logging.log4j.LogManager; import org.apache.commons.lang3.EnumUtils; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.Nullable; import org.springframework.stereotype.Component; import org.springframework.web.context.support.SpringBeanAutowiringSupport; @@ -70,12 +69,12 @@ import com.cloud.user.AccountService; import com.cloud.user.User; import com.cloud.user.UserAccount; - import com.cloud.utils.HttpUtils; -import com.cloud.utils.HttpUtils.ApiSessionKeySameSite; import com.cloud.utils.HttpUtils.ApiSessionKeyCheckOption; +import com.cloud.utils.HttpUtils.ApiSessionKeySameSite; import com.cloud.utils.StringUtils; import com.cloud.utils.db.EntityManager; +import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.net.NetUtils; @Component("apiServlet") @@ -84,9 +83,7 @@ public class ApiServlet extends HttpServlet { private static final Logger ACCESSLOGGER = LogManager.getLogger("apiserver." + ApiServlet.class.getName()); private static final String REPLACEMENT = "_"; private static final String LOGGER_REPLACEMENTS = "[\n\r\t]"; - private static final Pattern GET_REQUEST_COMMANDS = Pattern.compile("^(get|list|query|find)(\\w+)+$"); - private static final HashSet GET_REQUEST_COMMANDS_LIST = new HashSet<>(Set.of("isaccountallowedtocreateofferingswithtags", - "readyforshutdown", "cloudianisenabled", "quotabalance", "quotasummary", "quotatarifflist", "quotaisenabled", "quotastatement", "verifyoauthcodeandgetuser")); + public static final Pattern GET_REQUEST_COMMANDS = Pattern.compile("^(get|list|query|find)(\\w+)+$"); private static final HashSet POST_REQUESTS_TO_DISABLE_LOGGING = new HashSet<>(Set.of( "login", "oauthlogin", @@ -367,7 +364,7 @@ void processRequestInContext(final HttpServletRequest req, final HttpServletResp } } - if (apiServer.isPostRequestsAndTimestampsEnforced() && !isStateChangingCommandUsingPOST(command, req.getMethod(), params)) { + if (apiServer.isPostRequestsAndTimestampsEnforced() && isStateChangingCommandNotUsingPOST(command, req.getMethod(), params)) { String errorText = String.format("State changing command %s needs to be sent using POST request", command); if (command.equalsIgnoreCase("updateConfiguration") && params.containsKey("name")) { errorText = String.format("Changes for configuration %s needs to be sent using POST request", params.get("name")[0]); @@ -485,13 +482,30 @@ private boolean checkIfAuthenticatorIsOf2FA(String command) { return verify2FA; } - private boolean isStateChangingCommandUsingPOST(String command, String method, Map params) { - if (command == null || (!GET_REQUEST_COMMANDS.matcher(command.toLowerCase()).matches() && !GET_REQUEST_COMMANDS_LIST.contains(command.toLowerCase()) - && !command.equalsIgnoreCase("updateConfiguration") && !method.equals("POST"))) { + protected boolean isStateChangingCommandNotUsingPOST(String command, String method, Map params) { + if (BaseCmd.HTTPMethod.POST.toString().equalsIgnoreCase(method)) { + return false; + } + if (command == null || method == null) { + return true; + } + String commandHttpMethod = null; + try { + Class cmdClass = apiServer.getCmdClass(command); + if (cmdClass != null) { + APICommand at = cmdClass.getAnnotation(APICommand.class); + if (at != null && org.apache.commons.lang3.StringUtils.isNotBlank(at.httpMethod())) { + commandHttpMethod = at.httpMethod(); + } + } + } catch (CloudRuntimeException ignored) {} + if (BaseCmd.HTTPMethod.GET.toString().equalsIgnoreCase(commandHttpMethod) || + GET_REQUEST_COMMANDS.matcher(command.toLowerCase()).matches()) { return false; } - return !command.equalsIgnoreCase("updateConfiguration") || method.equals("POST") || (params.containsKey("name") - && params.get("name")[0].toString().equalsIgnoreCase(ApiServer.EnforcePostRequestsAndTimestamps.key())); + return !command.equalsIgnoreCase("updateConfiguration") || + !params.containsKey("name") || + !ApiServer.EnforcePostRequestsAndTimestamps.key().equalsIgnoreCase(params.get("name")[0].toString()); } protected boolean skip2FAcheckForAPIs(String command) { diff --git a/server/src/test/java/com/cloud/api/ApiServletTest.java b/server/src/test/java/com/cloud/api/ApiServletTest.java index 4d4f0a12098c..465944e97996 100644 --- a/server/src/test/java/com/cloud/api/ApiServletTest.java +++ b/server/src/test/java/com/cloud/api/ApiServletTest.java @@ -16,22 +16,30 @@ // under the License. package com.cloud.api; -import com.cloud.api.auth.ListUserTwoFactorAuthenticatorProvidersCmd; -import com.cloud.api.auth.SetupUserTwoFactorAuthenticationCmd; -import com.cloud.api.auth.ValidateUserTwoFactorAuthenticationCodeCmd; -import com.cloud.server.ManagementServer; -import com.cloud.user.Account; -import com.cloud.user.AccountManagerImpl; -import com.cloud.user.AccountService; -import com.cloud.user.User; -import com.cloud.user.UserAccount; -import com.cloud.utils.HttpUtils; -import com.cloud.vm.UserVmManager; +import static org.mockito.ArgumentMatchers.nullable; + +import java.io.IOException; +import java.io.PrintWriter; +import java.io.StringWriter; +import java.io.UnsupportedEncodingException; +import java.lang.reflect.Field; +import java.net.InetAddress; +import java.net.URLEncoder; +import java.net.UnknownHostException; +import java.util.HashMap; +import java.util.Map; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpSession; + +import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.auth.APIAuthenticationManager; import org.apache.cloudstack.api.auth.APIAuthenticationType; import org.apache.cloudstack.api.auth.APIAuthenticator; import org.apache.cloudstack.api.command.admin.config.ListCfgsByCmd; +import org.apache.cloudstack.api.command.admin.offering.IsAccountAllowedToCreateOfferingsWithTagsCmd; import org.apache.cloudstack.framework.config.ConfigKey; import org.junit.After; import org.junit.Assert; @@ -42,21 +50,17 @@ import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import javax.servlet.http.HttpSession; -import java.io.IOException; -import java.io.PrintWriter; -import java.io.StringWriter; -import java.io.UnsupportedEncodingException; -import java.lang.reflect.Field; -import java.net.InetAddress; -import java.net.URLEncoder; -import java.net.UnknownHostException; -import java.util.HashMap; -import java.util.Map; - -import static org.mockito.ArgumentMatchers.nullable; +import com.cloud.api.auth.ListUserTwoFactorAuthenticatorProvidersCmd; +import com.cloud.api.auth.SetupUserTwoFactorAuthenticationCmd; +import com.cloud.api.auth.ValidateUserTwoFactorAuthenticationCodeCmd; +import com.cloud.server.ManagementServer; +import com.cloud.user.Account; +import com.cloud.user.AccountManagerImpl; +import com.cloud.user.AccountService; +import com.cloud.user.User; +import com.cloud.user.UserAccount; +import com.cloud.utils.HttpUtils; +import com.cloud.vm.UserVmManager; @RunWith(MockitoJUnitRunner.class) public class ApiServletTest { @@ -432,4 +436,88 @@ public void testVerify2FAWhenExpectedCommandIsNotCalled() throws UnknownHostExce Assert.assertEquals(false, result); } + + @Test + public void isStateChangingCommandNotUsingPOSTReturnsFalseForPostMethod() { + String command = "updateConfiguration"; + String method = "POST"; + Map params = new HashMap<>(); + + boolean result = servlet.isStateChangingCommandNotUsingPOST(command, method, params); + + Assert.assertFalse(result); + } + + @Test + public void isStateChangingCommandNotUsingPOSTReturnsTrueForNullCommandAndMethod() { + String command = null; + String method = null; + Map params = new HashMap<>(); + + boolean result = servlet.isStateChangingCommandNotUsingPOST(command, method, params); + + Assert.assertTrue(result); + } + + @Test + public void isStateChangingCommandNotUsingPOSTReturnsFalseForGetHttpMethodAnnotation() { + String command = "isAccountAllowedToCreateOfferingsWithTags"; + String method = "GET"; + Map params = new HashMap<>(); + Class cmdClass = IsAccountAllowedToCreateOfferingsWithTagsCmd.class; + APICommand apiCommand = cmdClass.getAnnotation(APICommand.class); + Mockito.doReturn(cmdClass).when(apiServer).getCmdClass(command); + Assert.assertNotNull(apiCommand); + Assert.assertEquals("GET", apiCommand.httpMethod()); + boolean result = servlet.isStateChangingCommandNotUsingPOST(command, method, params); + Assert.assertFalse(result); + } + + @Test + public void isStateChangingCommandNotUsingPOSTReturnsFalseForMatchingGetRequestPattern() { + String command = "listZones"; + String method = "GET"; + Map params = new HashMap<>(); + boolean result = servlet.isStateChangingCommandNotUsingPOST(command, method, params); + Assert.assertFalse(result); + } + + @Test + public void isStateChangingCommandNotUsingPOSTReturnsTrueForMissingNameParameter() { + String command = "updateConfiguration"; + String method = "GET"; + Map params = new HashMap<>(); + boolean result = servlet.isStateChangingCommandNotUsingPOST(command, method, params); + Assert.assertTrue(result); + } + + @Test + public void isStateChangingCommandNotUsingPOSTReturnsFalseForUpdateConfigurationEnforcePostRequestsKey() { + String command = "updateConfiguration"; + String method = "GET"; + Map params = new HashMap<>(); + params.put("name", new String[] { ApiServer.EnforcePostRequestsAndTimestamps.key() }); + boolean result = servlet.isStateChangingCommandNotUsingPOST(command, method, params); + Assert.assertFalse(result); + } + + @Test + public void isStateChangingCommandNotUsingPOSTReturnsFalseForWrongApiEnforcePostRequestsKey() { + String command = "updateSomeApi"; + String method = "GET"; + Map params = new HashMap<>(); + params.put("name", new String[] { ApiServer.EnforcePostRequestsAndTimestamps.key() }); + boolean result = servlet.isStateChangingCommandNotUsingPOST(command, method, params); + Assert.assertTrue(result); + } + + @Test + public void isStateChangingCommandNotUsingPOSTReturnsFalseForUpdateConfigurationNonEnforcePostRequestsKey() { + String command = "updateConfiguration"; + String method = "GET"; + Map params = new HashMap<>(); + params.put("name", new String[] { "key" }); + boolean result = servlet.isStateChangingCommandNotUsingPOST(command, method, params); + Assert.assertTrue(result); + } }