Skip to content

Commit c7eceda

Browse files
committed
fix and unit test
Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
1 parent c719b7f commit c7eceda

File tree

2 files changed

+118
-29
lines changed

2 files changed

+118
-29
lines changed

server/src/main/java/com/cloud/api/ApiServlet.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,7 @@ private boolean checkIfAuthenticatorIsOf2FA(String command) {
482482
return verify2FA;
483483
}
484484

485-
private boolean isStateChangingCommandNotUsingPOST(String command, String method, Map<String, Object[]> params) {
485+
protected boolean isStateChangingCommandNotUsingPOST(String command, String method, Map<String, Object[]> params) {
486486
if (BaseCmd.HTTPMethod.POST.toString().equalsIgnoreCase(method)) {
487487
return false;
488488
}
@@ -503,8 +503,9 @@ private boolean isStateChangingCommandNotUsingPOST(String command, String method
503503
GET_REQUEST_COMMANDS.matcher(command.toLowerCase()).matches()) {
504504
return false;
505505
}
506-
return !params.containsKey("name")
507-
|| !ApiServer.EnforcePostRequestsAndTimestamps.key().equalsIgnoreCase(params.get("name")[0].toString());
506+
return !command.equalsIgnoreCase("updateConfiguration") ||
507+
!params.containsKey("name") ||
508+
!ApiServer.EnforcePostRequestsAndTimestamps.key().equalsIgnoreCase(params.get("name")[0].toString());
508509
}
509510

510511
protected boolean skip2FAcheckForAPIs(String command) {

server/src/test/java/com/cloud/api/ApiServletTest.java

Lines changed: 114 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -16,22 +16,30 @@
1616
// under the License.
1717
package com.cloud.api;
1818

19-
import com.cloud.api.auth.ListUserTwoFactorAuthenticatorProvidersCmd;
20-
import com.cloud.api.auth.SetupUserTwoFactorAuthenticationCmd;
21-
import com.cloud.api.auth.ValidateUserTwoFactorAuthenticationCodeCmd;
22-
import com.cloud.server.ManagementServer;
23-
import com.cloud.user.Account;
24-
import com.cloud.user.AccountManagerImpl;
25-
import com.cloud.user.AccountService;
26-
import com.cloud.user.User;
27-
import com.cloud.user.UserAccount;
28-
import com.cloud.utils.HttpUtils;
29-
import com.cloud.vm.UserVmManager;
19+
import static org.mockito.ArgumentMatchers.nullable;
20+
21+
import java.io.IOException;
22+
import java.io.PrintWriter;
23+
import java.io.StringWriter;
24+
import java.io.UnsupportedEncodingException;
25+
import java.lang.reflect.Field;
26+
import java.net.InetAddress;
27+
import java.net.URLEncoder;
28+
import java.net.UnknownHostException;
29+
import java.util.HashMap;
30+
import java.util.Map;
31+
32+
import javax.servlet.http.HttpServletRequest;
33+
import javax.servlet.http.HttpServletResponse;
34+
import javax.servlet.http.HttpSession;
35+
36+
import org.apache.cloudstack.api.APICommand;
3037
import org.apache.cloudstack.api.ApiConstants;
3138
import org.apache.cloudstack.api.auth.APIAuthenticationManager;
3239
import org.apache.cloudstack.api.auth.APIAuthenticationType;
3340
import org.apache.cloudstack.api.auth.APIAuthenticator;
3441
import org.apache.cloudstack.api.command.admin.config.ListCfgsByCmd;
42+
import org.apache.cloudstack.api.command.admin.offering.IsAccountAllowedToCreateOfferingsWithTagsCmd;
3543
import org.apache.cloudstack.framework.config.ConfigKey;
3644
import org.junit.After;
3745
import org.junit.Assert;
@@ -42,21 +50,17 @@
4250
import org.mockito.Mockito;
4351
import org.mockito.junit.MockitoJUnitRunner;
4452

45-
import javax.servlet.http.HttpServletRequest;
46-
import javax.servlet.http.HttpServletResponse;
47-
import javax.servlet.http.HttpSession;
48-
import java.io.IOException;
49-
import java.io.PrintWriter;
50-
import java.io.StringWriter;
51-
import java.io.UnsupportedEncodingException;
52-
import java.lang.reflect.Field;
53-
import java.net.InetAddress;
54-
import java.net.URLEncoder;
55-
import java.net.UnknownHostException;
56-
import java.util.HashMap;
57-
import java.util.Map;
58-
59-
import static org.mockito.ArgumentMatchers.nullable;
53+
import com.cloud.api.auth.ListUserTwoFactorAuthenticatorProvidersCmd;
54+
import com.cloud.api.auth.SetupUserTwoFactorAuthenticationCmd;
55+
import com.cloud.api.auth.ValidateUserTwoFactorAuthenticationCodeCmd;
56+
import com.cloud.server.ManagementServer;
57+
import com.cloud.user.Account;
58+
import com.cloud.user.AccountManagerImpl;
59+
import com.cloud.user.AccountService;
60+
import com.cloud.user.User;
61+
import com.cloud.user.UserAccount;
62+
import com.cloud.utils.HttpUtils;
63+
import com.cloud.vm.UserVmManager;
6064

6165
@RunWith(MockitoJUnitRunner.class)
6266
public class ApiServletTest {
@@ -432,4 +436,88 @@ public void testVerify2FAWhenExpectedCommandIsNotCalled() throws UnknownHostExce
432436

433437
Assert.assertEquals(false, result);
434438
}
439+
440+
@Test
441+
public void isStateChangingCommandNotUsingPOSTReturnsFalseForPostMethod() {
442+
String command = "updateConfiguration";
443+
String method = "POST";
444+
Map<String, Object[]> params = new HashMap<>();
445+
446+
boolean result = servlet.isStateChangingCommandNotUsingPOST(command, method, params);
447+
448+
Assert.assertFalse(result);
449+
}
450+
451+
@Test
452+
public void isStateChangingCommandNotUsingPOSTReturnsTrueForNullCommandAndMethod() {
453+
String command = null;
454+
String method = null;
455+
Map<String, Object[]> params = new HashMap<>();
456+
457+
boolean result = servlet.isStateChangingCommandNotUsingPOST(command, method, params);
458+
459+
Assert.assertTrue(result);
460+
}
461+
462+
@Test
463+
public void isStateChangingCommandNotUsingPOSTReturnsFalseForGetHttpMethodAnnotation() {
464+
String command = "isAccountAllowedToCreateOfferingsWithTags";
465+
String method = "GET";
466+
Map<String, Object[]> params = new HashMap<>();
467+
Class<?> cmdClass = IsAccountAllowedToCreateOfferingsWithTagsCmd.class;
468+
APICommand apiCommand = cmdClass.getAnnotation(APICommand.class);
469+
Mockito.doReturn(cmdClass).when(apiServer).getCmdClass(command);
470+
Assert.assertNotNull(apiCommand);
471+
Assert.assertEquals("GET", apiCommand.httpMethod());
472+
boolean result = servlet.isStateChangingCommandNotUsingPOST(command, method, params);
473+
Assert.assertFalse(result);
474+
}
475+
476+
@Test
477+
public void isStateChangingCommandNotUsingPOSTReturnsFalseForMatchingGetRequestPattern() {
478+
String command = "listZones";
479+
String method = "GET";
480+
Map<String, Object[]> params = new HashMap<>();
481+
boolean result = servlet.isStateChangingCommandNotUsingPOST(command, method, params);
482+
Assert.assertFalse(result);
483+
}
484+
485+
@Test
486+
public void isStateChangingCommandNotUsingPOSTReturnsTrueForMissingNameParameter() {
487+
String command = "updateConfiguration";
488+
String method = "GET";
489+
Map<String, Object[]> params = new HashMap<>();
490+
boolean result = servlet.isStateChangingCommandNotUsingPOST(command, method, params);
491+
Assert.assertTrue(result);
492+
}
493+
494+
@Test
495+
public void isStateChangingCommandNotUsingPOSTReturnsFalseForUpdateConfigurationEnforcePostRequestsKey() {
496+
String command = "updateConfiguration";
497+
String method = "GET";
498+
Map<String, Object[]> params = new HashMap<>();
499+
params.put("name", new String[] { ApiServer.EnforcePostRequestsAndTimestamps.key() });
500+
boolean result = servlet.isStateChangingCommandNotUsingPOST(command, method, params);
501+
Assert.assertFalse(result);
502+
}
503+
504+
@Test
505+
public void isStateChangingCommandNotUsingPOSTReturnsFalseForWrongApiEnforcePostRequestsKey() {
506+
String command = "updateSomeApi";
507+
String method = "GET";
508+
Map<String, Object[]> params = new HashMap<>();
509+
params.put("name", new String[] { ApiServer.EnforcePostRequestsAndTimestamps.key() });
510+
boolean result = servlet.isStateChangingCommandNotUsingPOST(command, method, params);
511+
Assert.assertTrue(result);
512+
}
513+
514+
@Test
515+
public void isStateChangingCommandNotUsingPOSTReturnsFalseForUpdateConfigurationNonEnforcePostRequestsKey() {
516+
String command = "updateConfiguration";
517+
String method = "GET";
518+
Map<String, Object[]> params = new HashMap<>();
519+
params.put("name", new String[] { "key" });
520+
boolean result = servlet.isStateChangingCommandNotUsingPOST(command, method, params);
521+
Assert.assertTrue(result);
522+
}
435523
}

0 commit comments

Comments
 (0)