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.ya...@shapeblue.com> Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/9e9b2316 Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/9e9b2316 Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/9e9b2316 Branch: refs/heads/master Commit: 9e9b231672e934292f9940d1363039a553fc7ad9 Parents: 212a05a Author: Rohit Yadav <rohit.ya...@shapeblue.com> Authored: Fri May 22 10:11:15 2015 +0100 Committer: Rohit Yadav <rohit.ya...@shapeblue.com> Committed: Fri May 22 10:11:15 2015 +0100 ---------------------------------------------------------------------- api/src/org/apache/cloudstack/api/auth/APIAuthenticator.java | 3 ++- .../api/command/GetServiceProviderMetaDataCmd.java | 3 ++- .../api/command/SAML2LoginAPIAuthenticatorCmd.java | 3 ++- .../api/command/SAML2LogoutAPIAuthenticatorCmd.java | 3 ++- .../api/command/GetServiceProviderMetaDataCmdTest.java | 6 +++++- .../api/command/SAML2LoginAPIAuthenticatorCmdTest.java | 8 ++++++-- .../api/command/SAML2LogoutAPIAuthenticatorCmdTest.java | 6 +++++- server/src/com/cloud/api/ApiServlet.java | 2 +- .../com/cloud/api/auth/DefaultLoginAPIAuthenticatorCmd.java | 8 ++++++-- .../com/cloud/api/auth/DefaultLogoutAPIAuthenticatorCmd.java | 3 ++- server/test/com/cloud/api/ApiServletTest.java | 6 +++--- 11 files changed, 36 insertions(+), 15 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/9e9b2316/api/src/org/apache/cloudstack/api/auth/APIAuthenticator.java ---------------------------------------------------------------------- diff --git a/api/src/org/apache/cloudstack/api/auth/APIAuthenticator.java b/api/src/org/apache/cloudstack/api/auth/APIAuthenticator.java index 67fa1d8..4139740 100644 --- a/api/src/org/apache/cloudstack/api/auth/APIAuthenticator.java +++ b/api/src/org/apache/cloudstack/api/auth/APIAuthenticator.java @@ -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(); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/9e9b2316/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/GetServiceProviderMetaDataCmd.java ---------------------------------------------------------------------- diff --git a/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/GetServiceProviderMetaDataCmd.java b/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/GetServiceProviderMetaDataCmd.java index da79a94..e730836 100644 --- a/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/GetServiceProviderMetaDataCmd.java +++ b/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/GetServiceProviderMetaDataCmd.java @@ -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; @@ -104,7 +105,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()); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/9e9b2316/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java ---------------------------------------------------------------------- diff --git a/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java b/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java index f40a4ee..a10afb6 100644 --- a/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java +++ b/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java @@ -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; @@ -164,7 +165,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("SAMLResponse") && !params.containsKey("SAMLart")) { String idpUrl = null; http://git-wip-us.apache.org/repos/asf/cloudstack/blob/9e9b2316/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/SAML2LogoutAPIAuthenticatorCmd.java ---------------------------------------------------------------------- diff --git a/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/SAML2LogoutAPIAuthenticatorCmd.java b/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/SAML2LogoutAPIAuthenticatorCmd.java index 7b1c367..992e431 100644 --- a/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/SAML2LogoutAPIAuthenticatorCmd.java +++ b/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/SAML2LogoutAPIAuthenticatorCmd.java @@ -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; @@ -83,7 +84,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"); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/9e9b2316/plugins/user-authenticators/saml2/test/org/apache/cloudstack/api/command/GetServiceProviderMetaDataCmdTest.java ---------------------------------------------------------------------- diff --git a/plugins/user-authenticators/saml2/test/org/apache/cloudstack/api/command/GetServiceProviderMetaDataCmdTest.java b/plugins/user-authenticators/saml2/test/org/apache/cloudstack/api/command/GetServiceProviderMetaDataCmdTest.java index 3826390..cb16f0c 100644 --- a/plugins/user-authenticators/saml2/test/org/apache/cloudstack/api/command/GetServiceProviderMetaDataCmdTest.java +++ b/plugins/user-authenticators/saml2/test/org/apache/cloudstack/api/command/GetServiceProviderMetaDataCmdTest.java @@ -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; @@ -57,6 +58,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(); @@ -77,7 +81,7 @@ public class GetServiceProviderMetaDataCmdTest { Mockito.when(samlAuthManager.getIdpSingleLogOutUrl()).thenReturn(url); Mockito.when(samlAuthManager.getSpSingleLogOutUrl()).thenReturn(url); - 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")); Mockito.verify(samlAuthManager, Mockito.atLeast(1)).getServiceProviderId(); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/9e9b2316/plugins/user-authenticators/saml2/test/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmdTest.java ---------------------------------------------------------------------- diff --git a/plugins/user-authenticators/saml2/test/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmdTest.java b/plugins/user-authenticators/saml2/test/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmdTest.java index b12d432..30ecc93 100644 --- a/plugins/user-authenticators/saml2/test/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmdTest.java +++ b/plugins/user-authenticators/saml2/test/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmdTest.java @@ -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; @@ -95,6 +96,9 @@ public class SAML2LoginAPIAuthenticatorCmdTest { @Mock HttpServletResponse resp; + @Mock + HttpServletRequest req; + private Response buildMockResponse() throws Exception { Response samlMessage = new ResponseBuilder().buildObject(); samlMessage.setID("foo"); @@ -171,14 +175,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(SAMLUtils.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(configDao, Mockito.atLeastOnce()).getValue(Mockito.anyString()); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/9e9b2316/plugins/user-authenticators/saml2/test/org/apache/cloudstack/api/command/SAML2LogoutAPIAuthenticatorCmdTest.java ---------------------------------------------------------------------- diff --git a/plugins/user-authenticators/saml2/test/org/apache/cloudstack/api/command/SAML2LogoutAPIAuthenticatorCmdTest.java b/plugins/user-authenticators/saml2/test/org/apache/cloudstack/api/command/SAML2LogoutAPIAuthenticatorCmdTest.java index a6005b7..e9834c9 100644 --- a/plugins/user-authenticators/saml2/test/org/apache/cloudstack/api/command/SAML2LogoutAPIAuthenticatorCmdTest.java +++ b/plugins/user-authenticators/saml2/test/org/apache/cloudstack/api/command/SAML2LogoutAPIAuthenticatorCmdTest.java @@ -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; @@ -55,6 +56,9 @@ public class SAML2LogoutAPIAuthenticatorCmdTest { @Mock HttpServletResponse resp; + @Mock + HttpServletRequest req; + @Test public void testAuthenticate() throws Exception { SAML2LogoutAPIAuthenticatorCmd cmd = new SAML2LogoutAPIAuthenticatorCmd(); @@ -81,7 +85,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, "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()); } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/9e9b2316/server/src/com/cloud/api/ApiServlet.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/api/ApiServlet.java b/server/src/com/cloud/api/ApiServlet.java index 7dada94..8d34dfe 100644 --- a/server/src/com/cloud/api/ApiServlet.java +++ b/server/src/com/cloud/api/ApiServlet.java @@ -190,7 +190,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); } catch (ServerApiException e) { httpResponseCode = e.getErrorCode().getHttpCode(); responseString = e.getMessage(); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/9e9b2316/server/src/com/cloud/api/auth/DefaultLoginAPIAuthenticatorCmd.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/api/auth/DefaultLoginAPIAuthenticatorCmd.java b/server/src/com/cloud/api/auth/DefaultLoginAPIAuthenticatorCmd.java index fa23abd..ae633a3 100644 --- a/server/src/com/cloud/api/auth/DefaultLoginAPIAuthenticatorCmd.java +++ b/server/src/com/cloud/api/auth/DefaultLoginAPIAuthenticatorCmd.java @@ -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); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/9e9b2316/server/src/com/cloud/api/auth/DefaultLogoutAPIAuthenticatorCmd.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/api/auth/DefaultLogoutAPIAuthenticatorCmd.java b/server/src/com/cloud/api/auth/DefaultLogoutAPIAuthenticatorCmd.java index ee7936a..5d25ae8 100644 --- a/server/src/com/cloud/api/auth/DefaultLogoutAPIAuthenticatorCmd.java +++ b/server/src/com/cloud/api/auth/DefaultLogoutAPIAuthenticatorCmd.java @@ -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"); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/9e9b2316/server/test/com/cloud/api/ApiServletTest.java ---------------------------------------------------------------------- diff --git a/server/test/com/cloud/api/ApiServletTest.java b/server/test/com/cloud/api/ApiServletTest.java index 1a9c13d..492ab0e 100644 --- a/server/test/com/cloud/api/ApiServletTest.java +++ b/server/test/com/cloud/api/ApiServletTest.java @@ -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)); } }
