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 9e9b231672)
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
This commit is contained in:
Rohit Yadav 2015-05-22 10:11:15 +01:00
parent 424b5bb8d0
commit 1c81b241e7
11 changed files with 39 additions and 18 deletions

View File

@ -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;
@ -37,7 +38,7 @@ import java.net.InetAddress;
public interface APIAuthenticator {
public String authenticate(String command, Map<String, Object[]> params,
HttpSession session, InetAddress remoteAddress, String responseType,
StringBuilder auditTrailSb, final HttpServletResponse resp) throws ServerApiException;
StringBuilder auditTrailSb, final HttpServletRequest req, final HttpServletResponse resp) throws ServerApiException;
public APIAuthenticationType getAPIType();

View File

@ -58,6 +58,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;
@ -105,7 +106,7 @@ public class GetServiceProviderMetaDataCmd extends BaseCmd implements APIAuthent
}
@Override
public String authenticate(String command, Map<String, Object[]> params, HttpSession session, InetAddress remoteAddress, String responseType, StringBuilder auditTrailSb, HttpServletResponse resp) throws ServerApiException {
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 {
SAMLMetaDataResponse response = new SAMLMetaDataResponse();
response.setResponseName(getCommandName());

View File

@ -62,6 +62,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;
@ -165,7 +166,7 @@ public class SAML2LoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthent
}
@Override
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 {
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 {
try {
if (!params.containsKey("SAMLResponse") && !params.containsKey("SAMLart")) {
String idpUrl = null;

View File

@ -43,6 +43,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;
@ -84,7 +85,7 @@ public class SAML2LogoutAPIAuthenticatorCmd extends BaseCmd implements APIAuthen
}
@Override
public String authenticate(String command, Map<String, Object[]> params, HttpSession session, InetAddress remoteAddress, String responseType, StringBuilder auditTrailSb, final HttpServletResponse resp) throws ServerApiException {
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 {
auditTrailSb.append("=== SAML SLO Logging out ===");
LogoutCmdResponse response = new LogoutCmdResponse();
response.setDescription("success");

View File

@ -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;
@ -59,6 +60,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, UnknownHostException {
GetServiceProviderMetaDataCmd cmd = new GetServiceProviderMetaDataCmd();
@ -79,7 +83,7 @@ public class GetServiceProviderMetaDataCmdTest {
Mockito.when(samlAuthManager.getIdpSingleLogOutUrl()).thenReturn(url);
Mockito.when(samlAuthManager.getSpSingleLogOutUrl()).thenReturn(url);
String result = cmd.authenticate("command", null, session, InetAddress.getByName("127.0.0.1"), HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), resp);
String result = cmd.authenticate("command", null, session, InetAddress.getByName("127.0.0.1"), HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), req, resp);
Assert.assertTrue(result.contains("md:EntityDescriptor"));
Mockito.verify(samlAuthManager, Mockito.atLeast(1)).getServiceProviderId();
@ -93,4 +97,4 @@ public class GetServiceProviderMetaDataCmdTest {
public void testGetAPIType() {
Assert.assertTrue(new GetServiceProviderMetaDataCmd().getAPIType() == APIAuthenticationType.LOGIN_API);
}
}
}

View File

@ -58,6 +58,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");
@ -172,14 +176,14 @@ public class SAML2LoginAPIAuthenticatorCmdTest {
Map<String, Object[]> params = new HashMap<String, Object[]>();
// SSO redirection test
cmd.authenticate("command", params, session, InetAddress.getByName("127.0.0.1"), HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), resp);
cmd.authenticate("command", params, session, InetAddress.getByName("127.0.0.1"), 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(SAMLUtils.SAML_RESPONSE, new String[]{"Some String"});
Mockito.stub(cmd.processSAMLResponse(Mockito.anyString())).toReturn(buildMockResponse());
try {
cmd.authenticate("command", params, session, InetAddress.getByName("127.0.0.1"), HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), resp);
cmd.authenticate("command", params, session, InetAddress.getByName("127.0.0.1"), HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), req, resp);
} catch (ServerApiException ignored) {
}
Mockito.verify(configDao, Mockito.atLeastOnce()).getValue(Mockito.anyString());
@ -192,4 +196,4 @@ public class SAML2LoginAPIAuthenticatorCmdTest {
public void testGetAPIType() {
Assert.assertTrue(new GetServiceProviderMetaDataCmd().getAPIType() == APIAuthenticationType.LOGIN_API);
}
}
}

View File

@ -32,6 +32,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;
@ -56,6 +57,9 @@ public class SAML2LogoutAPIAuthenticatorCmdTest {
@Mock
HttpServletResponse resp;
@Mock
HttpServletRequest req;
@Test
public void testAuthenticate() throws Exception {
SAML2LogoutAPIAuthenticatorCmd cmd = new SAML2LogoutAPIAuthenticatorCmd();
@ -82,7 +86,7 @@ public class SAML2LogoutAPIAuthenticatorCmdTest {
Mockito.when(session.getAttribute(Mockito.anyString())).thenReturn(null);
Mockito.when(configDao.getValue(Mockito.anyString())).thenReturn("someString");
cmd.authenticate("command", null, session, InetAddress.getByName("127.0.0.1"), HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), resp);
cmd.authenticate("command", null, session, InetAddress.getByName("127.0.0.1"), 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());
}
@ -91,4 +95,4 @@ public class SAML2LogoutAPIAuthenticatorCmdTest {
public void testGetAPIType() throws Exception {
Assert.assertTrue(new SAML2LogoutAPIAuthenticatorCmd().getAPIType() == APIAuthenticationType.LOGOUT_API);
}
}
}

View File

@ -197,7 +197,7 @@ public class ApiServlet extends HttpServlet {
}
try {
responseString = apiAuthenticator.authenticate(command, params, session, InetAddress.getByName(remoteAddress), responseType, auditTrailSb, resp);
responseString = apiAuthenticator.authenticate(command, params, session, InetAddress.getByName(remoteAddress), responseType, auditTrailSb, req, resp);
} catch (ServerApiException e) {
httpResponseCode = e.getErrorCode().getHttpCode();
responseString = e.getMessage();

View File

@ -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;
@ -104,8 +105,11 @@ public class DefaultLoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthe
}
@Override
public String authenticate(String command, Map<String, Object[]> params, HttpSession session, InetAddress remoteAddress, String responseType, StringBuilder auditTrailSb, final HttpServletResponse resp) throws ServerApiException {
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 {
// 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);

View File

@ -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;
@ -61,7 +62,7 @@ public class DefaultLogoutAPIAuthenticatorCmd extends BaseCmd implements APIAuth
}
@Override
public String authenticate(String command, Map<String, Object[]> params, HttpSession session, InetAddress remoteAddress, String responseType, StringBuilder auditTrailSb, final HttpServletResponse resp) throws ServerApiException {
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 {
auditTrailSb.append("=== Logging out ===");
LogoutCmdResponse response = new LogoutCmdResponse();
response.setDescription("success");

View File

@ -105,7 +105,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.same(InetAddress.getByName("127.0.0.1")), Mockito.anyString(), Mockito.isA(StringBuilder.class), Mockito.isA(HttpServletResponse.class))).thenReturn("{\"loginresponse\":{}");
Mockito.same(InetAddress.getByName("127.0.0.1")), 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);
@ -217,7 +217,7 @@ public class ApiServletTest {
Mockito.verify(authManager).getAPIAuthenticator("logout");
Mockito.verify(authenticator).authenticate(Mockito.anyString(), Mockito.anyMap(), Mockito.isA(HttpSession.class),
Mockito.eq(InetAddress.getByName("127.0.0.1")), Mockito.anyString(), Mockito.isA(StringBuilder.class), Mockito.isA(HttpServletResponse.class));
Mockito.eq(InetAddress.getByName("127.0.0.1")), Mockito.anyString(), Mockito.isA(StringBuilder.class), Mockito.isA(HttpServletRequest.class), Mockito.isA(HttpServletResponse.class));
Mockito.verify(session).invalidate();
}
@ -239,7 +239,7 @@ public class ApiServletTest {
Mockito.verify(authManager).getAPIAuthenticator("login");
Mockito.verify(authenticator).authenticate(Mockito.anyString(), Mockito.anyMap(), Mockito.isA(HttpSession.class),
Mockito.eq(InetAddress.getByName("127.0.0.1")), Mockito.anyString(), Mockito.isA(StringBuilder.class), Mockito.isA(HttpServletResponse.class));
Mockito.eq(InetAddress.getByName("127.0.0.1")), Mockito.anyString(), Mockito.isA(StringBuilder.class), Mockito.isA(HttpServletRequest.class), Mockito.isA(HttpServletResponse.class));
}
@Test