Skip to content

Commit 1c81b24

Browse files
yadvryadvr
committed
CLOUDSTACK-8505: Don't allow non-POST requests for default login API
We add a new contract to pass Http request to authentication plugin system. In the default login API, we disallow non-POST requests. Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com> (cherry picked from commit 9e9b231) Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com> Conflicts: api/src/org/apache/cloudstack/api/auth/APIAuthenticator.java plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/GetServiceProviderMetaDataCmd.java plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/SAML2LogoutAPIAuthenticatorCmd.java plugins/user-authenticators/saml2/test/org/apache/cloudstack/api/command/GetServiceProviderMetaDataCmdTest.java plugins/user-authenticators/saml2/test/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmdTest.java plugins/user-authenticators/saml2/test/org/apache/cloudstack/api/command/SAML2LogoutAPIAuthenticatorCmdTest.java server/src/com/cloud/api/ApiServlet.java server/src/com/cloud/api/auth/DefaultLoginAPIAuthenticatorCmd.java server/src/com/cloud/api/auth/DefaultLogoutAPIAuthenticatorCmd.java server/test/com/cloud/api/ApiServletTest.java
1 parent 424b5bb commit 1c81b24

11 files changed

Lines changed: 39 additions & 18 deletions

File tree

‎api/src/org/apache/cloudstack/api/auth/APIAuthenticator.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import org.apache.cloudstack.api.ServerApiException;
2020

21+
import javax.servlet.http.HttpServletRequest;
2122
import javax.servlet.http.HttpServletResponse;
2223
import javax.servlet.http.HttpSession;
2324
import java.util.List;
@@ -37,7 +38,7 @@
3738
public interface APIAuthenticator {
3839
public String authenticate(String command, Map<String, Object[]> params,
3940
HttpSession session, InetAddress remoteAddress, String responseType,
40-
StringBuilder auditTrailSb, final HttpServletResponse resp) throws ServerApiException;
41+
StringBuilder auditTrailSb, final HttpServletRequest req, final HttpServletResponse resp) throws ServerApiException;
4142

4243
public APIAuthenticationType getAPIType();
4344

‎plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/GetServiceProviderMetaDataCmd.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
import org.w3c.dom.Document;
5959

6060
import javax.inject.Inject;
61+
import javax.servlet.http.HttpServletRequest;
6162
import javax.servlet.http.HttpServletResponse;
6263
import javax.servlet.http.HttpSession;
6364
import javax.xml.parsers.DocumentBuilder;
@@ -105,7 +106,7 @@ public void execute() throws ServerApiException {
105106
}
106107

107108
@Override
108-
public String authenticate(String command, Map<String, Object[]> params, HttpSession session, InetAddress remoteAddress, String responseType, StringBuilder auditTrailSb, HttpServletResponse resp) throws ServerApiException {
109+
public String authenticate(String command, Map<String, Object[]> params, HttpSession session, InetAddress remoteAddress, String responseType, StringBuilder auditTrailSb, final HttpServletRequest req, final HttpServletResponse resp) throws ServerApiException {
109110
SAMLMetaDataResponse response = new SAMLMetaDataResponse();
110111
response.setResponseName(getCommandName());
111112

‎plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@
6262

6363
import javax.inject.Inject;
6464
import javax.servlet.http.Cookie;
65+
import javax.servlet.http.HttpServletRequest;
6566
import javax.servlet.http.HttpServletResponse;
6667
import javax.servlet.http.HttpSession;
6768
import javax.xml.parsers.ParserConfigurationException;
@@ -165,7 +166,7 @@ public Response processSAMLResponse(String responseMessage) {
165166
}
166167

167168
@Override
168-
public String authenticate(final String command, final Map<String, Object[]> params, final HttpSession session, final InetAddress remoteAddress, final String responseType, final StringBuilder auditTrailSb, final HttpServletResponse resp) throws ServerApiException {
169+
public String authenticate(final String command, final Map<String, Object[]> params, final HttpSession session, final InetAddress remoteAddress, final String responseType, final StringBuilder auditTrailSb, final HttpServletRequest req, final HttpServletResponse resp) throws ServerApiException {
169170
try {
170171
if (!params.containsKey("SAMLResponse") && !params.containsKey("SAMLart")) {
171172
String idpUrl = null;

‎plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/SAML2LogoutAPIAuthenticatorCmd.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
import org.xml.sax.SAXException;
4444

4545
import javax.inject.Inject;
46+
import javax.servlet.http.HttpServletRequest;
4647
import javax.servlet.http.HttpServletResponse;
4748
import javax.servlet.http.HttpSession;
4849
import javax.xml.parsers.ParserConfigurationException;
@@ -84,7 +85,7 @@ public void execute() throws ServerApiException {
8485
}
8586

8687
@Override
87-
public String authenticate(String command, Map<String, Object[]> params, HttpSession session, InetAddress remoteAddress, String responseType, StringBuilder auditTrailSb, final HttpServletResponse resp) throws ServerApiException {
88+
public String authenticate(String command, Map<String, Object[]> params, HttpSession session, InetAddress remoteAddress, String responseType, StringBuilder auditTrailSb, final HttpServletRequest req, final HttpServletResponse resp) throws ServerApiException {
8889
auditTrailSb.append("=== SAML SLO Logging out ===");
8990
LogoutCmdResponse response = new LogoutCmdResponse();
9091
response.setDescription("success");

‎plugins/user-authenticators/saml2/test/org/apache/cloudstack/api/command/GetServiceProviderMetaDataCmdTest.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import org.mockito.Mockito;
3232
import org.mockito.runners.MockitoJUnitRunner;
3333

34+
import javax.servlet.http.HttpServletRequest;
3435
import javax.servlet.http.HttpServletResponse;
3536
import javax.servlet.http.HttpSession;
3637
import java.lang.reflect.Field;
@@ -59,6 +60,9 @@ public class GetServiceProviderMetaDataCmdTest {
5960
@Mock
6061
HttpServletResponse resp;
6162

63+
@Mock
64+
HttpServletRequest req;
65+
6266
@Test
6367
public void testAuthenticate() throws NoSuchFieldException, SecurityException, IllegalArgumentException, IllegalAccessException, CertificateParsingException, CertificateEncodingException, NoSuchAlgorithmException, InvalidKeyException, NoSuchProviderException, SignatureException, UnknownHostException {
6468
GetServiceProviderMetaDataCmd cmd = new GetServiceProviderMetaDataCmd();
@@ -79,7 +83,7 @@ public void testAuthenticate() throws NoSuchFieldException, SecurityException, I
7983
Mockito.when(samlAuthManager.getIdpSingleLogOutUrl()).thenReturn(url);
8084
Mockito.when(samlAuthManager.getSpSingleLogOutUrl()).thenReturn(url);
8185

82-
String result = cmd.authenticate("command", null, session, InetAddress.getByName("127.0.0.1"), HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), resp);
86+
String result = cmd.authenticate("command", null, session, InetAddress.getByName("127.0.0.1"), HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), req, resp);
8387
Assert.assertTrue(result.contains("md:EntityDescriptor"));
8488

8589
Mockito.verify(samlAuthManager, Mockito.atLeast(1)).getServiceProviderId();
@@ -93,4 +97,4 @@ public void testAuthenticate() throws NoSuchFieldException, SecurityException, I
9397
public void testGetAPIType() {
9498
Assert.assertTrue(new GetServiceProviderMetaDataCmd().getAPIType() == APIAuthenticationType.LOGIN_API);
9599
}
96-
}
100+
}

‎plugins/user-authenticators/saml2/test/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmdTest.java‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
import org.opensaml.saml2.core.impl.StatusCodeBuilder;
5959
import org.opensaml.saml2.core.impl.SubjectBuilder;
6060

61+
import javax.servlet.http.HttpServletRequest;
6162
import javax.servlet.http.HttpServletResponse;
6263
import javax.servlet.http.HttpSession;
6364
import java.lang.reflect.Field;
@@ -96,6 +97,9 @@ public class SAML2LoginAPIAuthenticatorCmdTest {
9697
@Mock
9798
HttpServletResponse resp;
9899

100+
@Mock
101+
HttpServletRequest req;
102+
99103
private Response buildMockResponse() throws Exception {
100104
Response samlMessage = new ResponseBuilder().buildObject();
101105
samlMessage.setID("foo");
@@ -172,14 +176,14 @@ public void testAuthenticate() throws Exception {
172176
Map<String, Object[]> params = new HashMap<String, Object[]>();
173177

174178
// SSO redirection test
175-
cmd.authenticate("command", params, session, InetAddress.getByName("127.0.0.1"), HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), resp);
179+
cmd.authenticate("command", params, session, InetAddress.getByName("127.0.0.1"), HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), req, resp);
176180
Mockito.verify(resp, Mockito.times(1)).sendRedirect(Mockito.anyString());
177181

178182
// SSO SAMLResponse verification test, this should throw ServerApiException for auth failure
179183
params.put(SAMLUtils.SAML_RESPONSE, new String[]{"Some String"});
180184
Mockito.stub(cmd.processSAMLResponse(Mockito.anyString())).toReturn(buildMockResponse());
181185
try {
182-
cmd.authenticate("command", params, session, InetAddress.getByName("127.0.0.1"), HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), resp);
186+
cmd.authenticate("command", params, session, InetAddress.getByName("127.0.0.1"), HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), req, resp);
183187
} catch (ServerApiException ignored) {
184188
}
185189
Mockito.verify(configDao, Mockito.atLeastOnce()).getValue(Mockito.anyString());
@@ -192,4 +196,4 @@ public void testAuthenticate() throws Exception {
192196
public void testGetAPIType() {
193197
Assert.assertTrue(new GetServiceProviderMetaDataCmd().getAPIType() == APIAuthenticationType.LOGIN_API);
194198
}
195-
}
199+
}

‎plugins/user-authenticators/saml2/test/org/apache/cloudstack/api/command/SAML2LogoutAPIAuthenticatorCmdTest.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.mockito.Mockito;
3333
import org.mockito.runners.MockitoJUnitRunner;
3434

35+
import javax.servlet.http.HttpServletRequest;
3536
import javax.servlet.http.HttpServletResponse;
3637
import javax.servlet.http.HttpSession;
3738
import java.lang.reflect.Field;
@@ -56,6 +57,9 @@ public class SAML2LogoutAPIAuthenticatorCmdTest {
5657
@Mock
5758
HttpServletResponse resp;
5859

60+
@Mock
61+
HttpServletRequest req;
62+
5963
@Test
6064
public void testAuthenticate() throws Exception {
6165
SAML2LogoutAPIAuthenticatorCmd cmd = new SAML2LogoutAPIAuthenticatorCmd();
@@ -82,7 +86,7 @@ public void testAuthenticate() throws Exception {
8286
Mockito.when(session.getAttribute(Mockito.anyString())).thenReturn(null);
8387
Mockito.when(configDao.getValue(Mockito.anyString())).thenReturn("someString");
8488

85-
cmd.authenticate("command", null, session, InetAddress.getByName("127.0.0.1"), HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), resp);
89+
cmd.authenticate("command", null, session, InetAddress.getByName("127.0.0.1"), HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), req, resp);
8690
Mockito.verify(resp, Mockito.times(1)).sendRedirect(Mockito.anyString());
8791
Mockito.verify(session, Mockito.atLeastOnce()).getAttribute(Mockito.anyString());
8892
}
@@ -91,4 +95,4 @@ public void testAuthenticate() throws Exception {
9195
public void testGetAPIType() throws Exception {
9296
Assert.assertTrue(new SAML2LogoutAPIAuthenticatorCmd().getAPIType() == APIAuthenticationType.LOGOUT_API);
9397
}
94-
}
98+
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ void processRequestInContext(final HttpServletRequest req, final HttpServletResp
197197
}
198198

199199
try {
200-
responseString = apiAuthenticator.authenticate(command, params, session, InetAddress.getByName(remoteAddress), responseType, auditTrailSb, resp);
200+
responseString = apiAuthenticator.authenticate(command, params, session, InetAddress.getByName(remoteAddress), responseType, auditTrailSb, req, resp);
201201
} catch (ServerApiException e) {
202202
httpResponseCode = e.getErrorCode().getHttpCode();
203203
responseString = e.getMessage();

‎server/src/com/cloud/api/auth/DefaultLoginAPIAuthenticatorCmd.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import org.apache.log4j.Logger;
3434

3535
import javax.inject.Inject;
36+
import javax.servlet.http.HttpServletRequest;
3637
import javax.servlet.http.HttpServletResponse;
3738
import javax.servlet.http.HttpSession;
3839
import java.util.List;
@@ -104,8 +105,11 @@ public void execute() throws ServerApiException {
104105
}
105106

106107
@Override
107-
public String authenticate(String command, Map<String, Object[]> params, HttpSession session, InetAddress remoteAddress, String responseType, StringBuilder auditTrailSb, final HttpServletResponse resp) throws ServerApiException {
108-
108+
public String authenticate(String command, Map<String, Object[]> params, HttpSession session, InetAddress remoteAddress, String responseType, StringBuilder auditTrailSb, final HttpServletRequest req, final HttpServletResponse resp) throws ServerApiException {
109+
// Disallow non POST requests
110+
if (HTTPMethod.valueOf(req.getMethod()) != HTTPMethod.POST) {
111+
throw new ServerApiException(ApiErrorCode.METHOD_NOT_ALLOWED, "Please use HTTP POST to authenticate using this API");
112+
}
109113
// FIXME: ported from ApiServlet, refactor and cleanup
110114
final String[] username = (String[])params.get(ApiConstants.USERNAME);
111115
final String[] password = (String[])params.get(ApiConstants.PASSWORD);

‎server/src/com/cloud/api/auth/DefaultLogoutAPIAuthenticatorCmd.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.apache.cloudstack.api.response.LogoutCmdResponse;
2929
import org.apache.log4j.Logger;
3030

31+
import javax.servlet.http.HttpServletRequest;
3132
import javax.servlet.http.HttpServletResponse;
3233
import javax.servlet.http.HttpSession;
3334
import java.util.List;
@@ -61,7 +62,7 @@ public void execute() throws ServerApiException {
6162
}
6263

6364
@Override
64-
public String authenticate(String command, Map<String, Object[]> params, HttpSession session, InetAddress remoteAddress, String responseType, StringBuilder auditTrailSb, final HttpServletResponse resp) throws ServerApiException {
65+
public String authenticate(String command, Map<String, Object[]> params, HttpSession session, InetAddress remoteAddress, String responseType, StringBuilder auditTrailSb, final HttpServletRequest req, final HttpServletResponse resp) throws ServerApiException {
6566
auditTrailSb.append("=== Logging out ===");
6667
LogoutCmdResponse response = new LogoutCmdResponse();
6768
response.setDescription("success");

0 commit comments

Comments
 (0)