mirror of https://github.com/apache/cloudstack.git
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.
(cherry picked from commit 9e9b231672)
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
Conflicts:
plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java
server/src/com/cloud/api/ApiServlet.java
(cherry picked from commit 8b9b4832f483797c8ab123bf27262634430efcb9)
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
This commit is contained in:
parent
c909df56e9
commit
5571b76cda
|
|
@ -18,6 +18,7 @@ package org.apache.cloudstack.api.auth;
|
|||
|
||||
import org.apache.cloudstack.api.ServerApiException;
|
||||
|
||||
import javax.servlet.http.HttpServletRequest;
|
||||
import javax.servlet.http.HttpServletResponse;
|
||||
import javax.servlet.http.HttpSession;
|
||||
import java.util.List;
|
||||
|
|
@ -36,7 +37,7 @@ import java.util.Map;
|
|||
public interface APIAuthenticator {
|
||||
public String authenticate(String command, Map<String, Object[]> params,
|
||||
HttpSession session, String remoteAddress, String responseType,
|
||||
StringBuilder auditTrailSb, final HttpServletResponse resp) throws ServerApiException;
|
||||
StringBuilder auditTrailSb, final HttpServletRequest req, final HttpServletResponse resp) throws ServerApiException;
|
||||
|
||||
public APIAuthenticationType getAPIType();
|
||||
|
||||
|
|
|
|||
|
|
@ -72,6 +72,7 @@ import org.opensaml.xml.security.x509.X509KeyInfoGeneratorFactory;
|
|||
import org.w3c.dom.Document;
|
||||
|
||||
import javax.inject.Inject;
|
||||
import javax.servlet.http.HttpServletRequest;
|
||||
import javax.servlet.http.HttpServletResponse;
|
||||
import javax.servlet.http.HttpSession;
|
||||
import javax.xml.parsers.DocumentBuilder;
|
||||
|
|
@ -119,7 +120,7 @@ public class GetServiceProviderMetaDataCmd extends BaseCmd implements APIAuthent
|
|||
}
|
||||
|
||||
@Override
|
||||
public String authenticate(String command, Map<String, Object[]> params, HttpSession session, String remoteAddress, String responseType, StringBuilder auditTrailSb, HttpServletResponse resp) throws ServerApiException {
|
||||
public String authenticate(String command, Map<String, Object[]> params, HttpSession session, String remoteAddress, String responseType, StringBuilder auditTrailSb, final HttpServletRequest req, final HttpServletResponse resp) throws ServerApiException {
|
||||
SAMLMetaDataResponse response = new SAMLMetaDataResponse();
|
||||
response.setResponseName(getCommandName());
|
||||
|
||||
|
|
|
|||
|
|
@ -65,6 +65,7 @@ import org.xml.sax.SAXException;
|
|||
|
||||
import javax.inject.Inject;
|
||||
import javax.servlet.http.Cookie;
|
||||
import javax.servlet.http.HttpServletRequest;
|
||||
import javax.servlet.http.HttpServletResponse;
|
||||
import javax.servlet.http.HttpSession;
|
||||
import javax.xml.parsers.ParserConfigurationException;
|
||||
|
|
@ -137,7 +138,7 @@ public class SAML2LoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthent
|
|||
}
|
||||
|
||||
@Override
|
||||
public String authenticate(final String command, final Map<String, Object[]> params, final HttpSession session, final String remoteAddress, final String responseType, final StringBuilder auditTrailSb, final HttpServletResponse resp) throws ServerApiException {
|
||||
public String authenticate(final String command, final Map<String, Object[]> params, final HttpSession session, final String remoteAddress, final String responseType, final StringBuilder auditTrailSb, final HttpServletRequest req, final HttpServletResponse resp) throws ServerApiException {
|
||||
try {
|
||||
if (!params.containsKey(SAMLPluginConstants.SAML_RESPONSE) && !params.containsKey("SAMLart")) {
|
||||
String idpId = null;
|
||||
|
|
|
|||
|
|
@ -42,6 +42,7 @@ import org.opensaml.xml.io.UnmarshallingException;
|
|||
import org.xml.sax.SAXException;
|
||||
|
||||
import javax.inject.Inject;
|
||||
import javax.servlet.http.HttpServletRequest;
|
||||
import javax.servlet.http.HttpServletResponse;
|
||||
import javax.servlet.http.HttpSession;
|
||||
import javax.xml.parsers.ParserConfigurationException;
|
||||
|
|
@ -81,7 +82,7 @@ public class SAML2LogoutAPIAuthenticatorCmd extends BaseCmd implements APIAuthen
|
|||
}
|
||||
|
||||
@Override
|
||||
public String authenticate(String command, Map<String, Object[]> params, HttpSession session, String remoteAddress, String responseType, StringBuilder auditTrailSb, final HttpServletResponse resp) throws ServerApiException {
|
||||
public String authenticate(String command, Map<String, Object[]> params, HttpSession session, String remoteAddress, String responseType, StringBuilder auditTrailSb, final HttpServletRequest req, final HttpServletResponse resp) throws ServerApiException {
|
||||
auditTrailSb.append("=== SAML SLO Logging out ===");
|
||||
LogoutCmdResponse response = new LogoutCmdResponse();
|
||||
response.setDescription("success");
|
||||
|
|
|
|||
|
|
@ -33,6 +33,7 @@ import org.mockito.Mock;
|
|||
import org.mockito.Mockito;
|
||||
import org.mockito.runners.MockitoJUnitRunner;
|
||||
|
||||
import javax.servlet.http.HttpServletRequest;
|
||||
import javax.servlet.http.HttpServletResponse;
|
||||
import javax.servlet.http.HttpSession;
|
||||
import java.lang.reflect.Field;
|
||||
|
|
@ -60,6 +61,9 @@ public class GetServiceProviderMetaDataCmdTest {
|
|||
@Mock
|
||||
HttpServletResponse resp;
|
||||
|
||||
@Mock
|
||||
HttpServletRequest req;
|
||||
|
||||
@Test
|
||||
public void testAuthenticate() throws NoSuchFieldException, SecurityException, IllegalArgumentException, IllegalAccessException, CertificateParsingException, CertificateEncodingException, NoSuchAlgorithmException, InvalidKeyException, NoSuchProviderException, SignatureException {
|
||||
GetServiceProviderMetaDataCmd cmd = new GetServiceProviderMetaDataCmd();
|
||||
|
|
@ -87,7 +91,7 @@ public class GetServiceProviderMetaDataCmdTest {
|
|||
|
||||
Mockito.when(samlAuthManager.getSPMetadata()).thenReturn(providerMetadata);
|
||||
|
||||
String result = cmd.authenticate("command", null, session, "random", HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), resp);
|
||||
String result = cmd.authenticate("command", null, session, "random", HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), req, resp);
|
||||
Assert.assertTrue(result.contains("md:EntityDescriptor"));
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -61,6 +61,7 @@ import org.opensaml.saml2.core.impl.StatusBuilder;
|
|||
import org.opensaml.saml2.core.impl.StatusCodeBuilder;
|
||||
import org.opensaml.saml2.core.impl.SubjectBuilder;
|
||||
|
||||
import javax.servlet.http.HttpServletRequest;
|
||||
import javax.servlet.http.HttpServletResponse;
|
||||
import javax.servlet.http.HttpSession;
|
||||
import java.lang.reflect.Field;
|
||||
|
|
@ -96,6 +97,9 @@ public class SAML2LoginAPIAuthenticatorCmdTest {
|
|||
@Mock
|
||||
HttpServletResponse resp;
|
||||
|
||||
@Mock
|
||||
HttpServletRequest req;
|
||||
|
||||
private Response buildMockResponse() throws Exception {
|
||||
Response samlMessage = new ResponseBuilder().buildObject();
|
||||
samlMessage.setID("foo");
|
||||
|
|
@ -176,14 +180,14 @@ public class SAML2LoginAPIAuthenticatorCmdTest {
|
|||
Map<String, Object[]> params = new HashMap<String, Object[]>();
|
||||
|
||||
// SSO redirection test
|
||||
cmd.authenticate("command", params, session, "random", HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), resp);
|
||||
cmd.authenticate("command", params, session, "random", HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), req, resp);
|
||||
Mockito.verify(resp, Mockito.times(1)).sendRedirect(Mockito.anyString());
|
||||
|
||||
// SSO SAMLResponse verification test, this should throw ServerApiException for auth failure
|
||||
params.put(SAMLPluginConstants.SAML_RESPONSE, new String[]{"Some String"});
|
||||
Mockito.stub(cmd.processSAMLResponse(Mockito.anyString())).toReturn(buildMockResponse());
|
||||
try {
|
||||
cmd.authenticate("command", params, session, "random", HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), resp);
|
||||
cmd.authenticate("command", params, session, "random", HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), req, resp);
|
||||
} catch (ServerApiException ignored) {
|
||||
}
|
||||
Mockito.verify(userAccountDao, Mockito.times(0)).getUserAccount(Mockito.anyString(), Mockito.anyLong());
|
||||
|
|
|
|||
|
|
@ -31,6 +31,7 @@ import org.mockito.Mock;
|
|||
import org.mockito.Mockito;
|
||||
import org.mockito.runners.MockitoJUnitRunner;
|
||||
|
||||
import javax.servlet.http.HttpServletRequest;
|
||||
import javax.servlet.http.HttpServletResponse;
|
||||
import javax.servlet.http.HttpSession;
|
||||
import java.lang.reflect.Field;
|
||||
|
|
@ -51,6 +52,9 @@ public class SAML2LogoutAPIAuthenticatorCmdTest {
|
|||
@Mock
|
||||
HttpServletResponse resp;
|
||||
|
||||
@Mock
|
||||
HttpServletRequest req;
|
||||
|
||||
@Test
|
||||
public void testAuthenticate() throws Exception {
|
||||
SAML2LogoutAPIAuthenticatorCmd cmd = new SAML2LogoutAPIAuthenticatorCmd();
|
||||
|
|
@ -68,7 +72,7 @@ public class SAML2LogoutAPIAuthenticatorCmdTest {
|
|||
X509Certificate cert = SAMLUtils.generateRandomX509Certificate(SAMLUtils.generateRandomKeyPair());
|
||||
Mockito.when(session.getAttribute(Mockito.anyString())).thenReturn(null);
|
||||
|
||||
cmd.authenticate("command", null, session, "random", HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), resp);
|
||||
cmd.authenticate("command", null, session, "random", HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), req, resp);
|
||||
Mockito.verify(resp, Mockito.times(1)).sendRedirect(Mockito.anyString());
|
||||
Mockito.verify(session, Mockito.atLeastOnce()).getAttribute(Mockito.anyString());
|
||||
}
|
||||
|
|
|
|||
|
|
@ -189,7 +189,7 @@ public class ApiServlet extends HttpServlet {
|
|||
}
|
||||
|
||||
try {
|
||||
responseString = apiAuthenticator.authenticate(command, params, session, remoteAddress, responseType, auditTrailSb, resp);
|
||||
responseString = apiAuthenticator.authenticate(command, params, session, remoteAddress, responseType, auditTrailSb, req, resp);
|
||||
if (session != null && session.getAttribute(ApiConstants.SESSIONKEY) != null) {
|
||||
resp.addHeader("SET-COOKIE", String.format("%s=%s;HttpOnly", ApiConstants.SESSIONKEY, session.getAttribute(ApiConstants.SESSIONKEY)));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -33,6 +33,7 @@ import org.apache.cloudstack.api.response.LoginCmdResponse;
|
|||
import org.apache.log4j.Logger;
|
||||
|
||||
import javax.inject.Inject;
|
||||
import javax.servlet.http.HttpServletRequest;
|
||||
import javax.servlet.http.HttpServletResponse;
|
||||
import javax.servlet.http.HttpSession;
|
||||
import java.util.List;
|
||||
|
|
@ -103,8 +104,11 @@ public class DefaultLoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthe
|
|||
}
|
||||
|
||||
@Override
|
||||
public String authenticate(String command, Map<String, Object[]> params, HttpSession session, String remoteAddress, String responseType, StringBuilder auditTrailSb, final HttpServletResponse resp) throws ServerApiException {
|
||||
|
||||
public String authenticate(String command, Map<String, Object[]> params, HttpSession session, String remoteAddress, String responseType, StringBuilder auditTrailSb, final HttpServletRequest req, final HttpServletResponse resp) throws ServerApiException {
|
||||
// Disallow non POST requests
|
||||
if (HTTPMethod.valueOf(req.getMethod()) != HTTPMethod.POST) {
|
||||
throw new ServerApiException(ApiErrorCode.METHOD_NOT_ALLOWED, "Please use HTTP POST to authenticate using this API");
|
||||
}
|
||||
// FIXME: ported from ApiServlet, refactor and cleanup
|
||||
final String[] username = (String[])params.get(ApiConstants.USERNAME);
|
||||
final String[] password = (String[])params.get(ApiConstants.PASSWORD);
|
||||
|
|
|
|||
|
|
@ -28,6 +28,7 @@ import org.apache.cloudstack.api.auth.PluggableAPIAuthenticator;
|
|||
import org.apache.cloudstack.api.response.LogoutCmdResponse;
|
||||
import org.apache.log4j.Logger;
|
||||
|
||||
import javax.servlet.http.HttpServletRequest;
|
||||
import javax.servlet.http.HttpServletResponse;
|
||||
import javax.servlet.http.HttpSession;
|
||||
import java.util.List;
|
||||
|
|
@ -60,7 +61,7 @@ public class DefaultLogoutAPIAuthenticatorCmd extends BaseCmd implements APIAuth
|
|||
}
|
||||
|
||||
@Override
|
||||
public String authenticate(String command, Map<String, Object[]> params, HttpSession session, String remoteAddress, String responseType, StringBuilder auditTrailSb, final HttpServletResponse resp) throws ServerApiException {
|
||||
public String authenticate(String command, Map<String, Object[]> params, HttpSession session, String remoteAddress, String responseType, StringBuilder auditTrailSb, final HttpServletRequest req, final HttpServletResponse resp) throws ServerApiException {
|
||||
auditTrailSb.append("=== Logging out ===");
|
||||
LogoutCmdResponse response = new LogoutCmdResponse();
|
||||
response.setDescription("success");
|
||||
|
|
|
|||
|
|
@ -99,7 +99,7 @@ public class ApiServletTest {
|
|||
|
||||
Mockito.when(authManager.getAPIAuthenticator(Mockito.anyString())).thenReturn(authenticator);
|
||||
Mockito.when(authenticator.authenticate(Mockito.anyString(), Mockito.anyMap(), Mockito.isA(HttpSession.class),
|
||||
Mockito.anyString(), Mockito.anyString(), Mockito.isA(StringBuilder.class), Mockito.isA(HttpServletResponse.class))).thenReturn("{\"loginresponse\":{}");
|
||||
Mockito.anyString(), Mockito.anyString(), Mockito.isA(StringBuilder.class), Mockito.isA(HttpServletRequest.class), Mockito.isA(HttpServletResponse.class))).thenReturn("{\"loginresponse\":{}");
|
||||
|
||||
Field authManagerField = ApiServlet.class.getDeclaredField("_authManager");
|
||||
authManagerField.setAccessible(true);
|
||||
|
|
@ -210,7 +210,7 @@ public class ApiServletTest {
|
|||
|
||||
Mockito.verify(authManager).getAPIAuthenticator("logout");
|
||||
Mockito.verify(authenticator).authenticate(Mockito.anyString(), Mockito.anyMap(), Mockito.isA(HttpSession.class),
|
||||
Mockito.anyString(), Mockito.anyString(), Mockito.isA(StringBuilder.class), Mockito.isA(HttpServletResponse.class));
|
||||
Mockito.anyString(), Mockito.anyString(), Mockito.isA(StringBuilder.class), Mockito.isA(HttpServletRequest.class), Mockito.isA(HttpServletResponse.class));
|
||||
Mockito.verify(session).invalidate();
|
||||
}
|
||||
|
||||
|
|
@ -232,6 +232,6 @@ public class ApiServletTest {
|
|||
|
||||
Mockito.verify(authManager).getAPIAuthenticator("login");
|
||||
Mockito.verify(authenticator).authenticate(Mockito.anyString(), Mockito.anyMap(), Mockito.isA(HttpSession.class),
|
||||
Mockito.anyString(), Mockito.anyString(), Mockito.isA(StringBuilder.class), Mockito.isA(HttpServletResponse.class));
|
||||
Mockito.anyString(), Mockito.anyString(), Mockito.isA(StringBuilder.class), Mockito.isA(HttpServletRequest.class), Mockito.isA(HttpServletResponse.class));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue