mirror of
				https://github.com/apache/cloudstack.git
				synced 2025-11-04 00:02:37 +01:00 
			
		
		
		
	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>
This commit is contained in:
		
							parent
							
								
									212a05a345
								
							
						
					
					
						commit
						9e9b231672
					
				@ -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();
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
@ -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());
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
@ -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;
 | 
			
		||||
 | 
			
		||||
@ -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");
 | 
			
		||||
 | 
			
		||||
@ -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();
 | 
			
		||||
 | 
			
		||||
@ -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());
 | 
			
		||||
 | 
			
		||||
@ -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());
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
@ -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();
 | 
			
		||||
 | 
			
		||||
@ -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…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user