Code wise, looks good to me. As soon as I get time, I'll merge it on master (perhaps cherry-pick on 4.5 since it solve for few possible bugs and improves code) after this passes some functional tests with default and saml2 auth login.
Cheers. On Fri, Nov 21, 2014 at 4:47 PM, <wid...@apache.org> wrote: > Repository: cloudstack > Updated Branches: > refs/heads/inetaddress [created] 4bd49df3f > > > Use InetAddress for passing Remote Address instead of String > > > Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo > Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/4bd49df3 > Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/4bd49df3 > Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/4bd49df3 > > Branch: refs/heads/inetaddress > Commit: 4bd49df3f537d1781db1d15775bfa21b323c813e > Parents: 83656a6 > Author: Wido den Hollander <w...@widodh.nl> > Authored: Fri Nov 21 12:10:35 2014 +0100 > Committer: Wido den Hollander <w...@widodh.nl> > Committed: Fri Nov 21 12:10:35 2014 +0100 > > ---------------------------------------------------------------------- > .../org/apache/cloudstack/api/ApiServerService.java | 3 ++- > .../apache/cloudstack/api/auth/APIAuthenticator.java | 3 ++- > .../contrail/management/MockAccountManager.java | 3 ++- > .../api/command/GetServiceProviderMetaDataCmd.java | 3 ++- > .../api/command/SAML2LoginAPIAuthenticatorCmd.java | 3 ++- > .../api/command/SAML2LogoutAPIAuthenticatorCmd.java | 3 ++- > .../command/GetServiceProviderMetaDataCmdTest.java | 6 ++++-- > .../command/SAML2LoginAPIAuthenticatorCmdTest.java | 5 +++-- > .../command/SAML2LogoutAPIAuthenticatorCmdTest.java | 3 ++- > server/src/com/cloud/api/ApiServer.java | 2 +- > server/src/com/cloud/api/ApiServlet.java | 4 +++- > .../api/auth/DefaultLoginAPIAuthenticatorCmd.java | 3 ++- > .../api/auth/DefaultLogoutAPIAuthenticatorCmd.java | 3 ++- > server/src/com/cloud/user/AccountManager.java | 3 ++- > server/src/com/cloud/user/AccountManagerImpl.java | 12 +++++------- > server/test/com/cloud/api/ApiServletTest.java | 14 ++++++++------ > .../test/com/cloud/user/MockAccountManagerImpl.java | 3 ++- > 17 files changed, 46 insertions(+), 30 deletions(-) > ---------------------------------------------------------------------- > > > > http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4bd49df3/api/src/org/apache/cloudstack/api/ApiServerService.java > ---------------------------------------------------------------------- > diff --git a/api/src/org/apache/cloudstack/api/ApiServerService.java > b/api/src/org/apache/cloudstack/api/ApiServerService.java > index 69215c5..6147408 100644 > --- a/api/src/org/apache/cloudstack/api/ApiServerService.java > +++ b/api/src/org/apache/cloudstack/api/ApiServerService.java > @@ -19,13 +19,14 @@ package org.apache.cloudstack.api; > import com.cloud.exception.CloudAuthenticationException; > import javax.servlet.http.HttpSession; > import java.util.Map; > +import java.net.InetAddress; > > public interface ApiServerService { > public boolean verifyRequest(Map<String, Object[]> requestParameters, > Long userId) throws ServerApiException; > > public Long fetchDomainId(String domainUUID); > > - public ResponseObject loginUser(HttpSession session, String username, > String password, Long domainId, String domainPath, String loginIpAddress, > + public ResponseObject loginUser(HttpSession session, String username, > String password, Long domainId, String domainPath, InetAddress > loginIpAddress, > Map<String, Object[]> > requestParameters) throws CloudAuthenticationException; > > public void logoutUser(long userId); > > > http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4bd49df3/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..0087561 100644 > --- a/api/src/org/apache/cloudstack/api/auth/APIAuthenticator.java > +++ b/api/src/org/apache/cloudstack/api/auth/APIAuthenticator.java > @@ -22,6 +22,7 @@ import javax.servlet.http.HttpServletResponse; > import javax.servlet.http.HttpSession; > import java.util.List; > import java.util.Map; > +import java.net.InetAddress; > > /* > * APIAuthenticator is an interface that defines method that > @@ -35,7 +36,7 @@ import java.util.Map; > * */ > public interface APIAuthenticator { > public String authenticate(String command, Map<String, Object[]> > params, > - HttpSession session, String remoteAddress, > String responseType, > + HttpSession session, InetAddress > remoteAddress, String responseType, > StringBuilder auditTrailSb, final > HttpServletResponse resp) throws ServerApiException; > > public APIAuthenticationType getAPIType(); > > > http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4bd49df3/plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/management/MockAccountManager.java > ---------------------------------------------------------------------- > diff --git > a/plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/management/MockAccountManager.java > b/plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/management/MockAccountManager.java > index 2f97a0f..ddc6df1 100644 > --- > a/plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/management/MockAccountManager.java > +++ > b/plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/management/MockAccountManager.java > @@ -19,6 +19,7 @@ package > org.apache.cloudstack.network.contrail.management; > > import java.util.List; > import java.util.Map; > +import java.net.InetAddress; > > import javax.inject.Inject; > import javax.naming.ConfigurationException; > @@ -231,7 +232,7 @@ public class MockAccountManager extends ManagerBase > implements AccountManager { > } > > @Override > - public UserAccount authenticateUser(String arg0, String arg1, Long > arg2, String arg3, Map<String, Object[]> arg4) { > + public UserAccount authenticateUser(String arg0, String arg1, Long > arg2, InetAddress arg3, Map<String, Object[]> arg4) { > // TODO Auto-generated method stub > return null; > } > > > http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4bd49df3/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 194d94f..87c9431 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 > @@ -72,6 +72,7 @@ import java.io.IOException; > import java.io.StringWriter; > import java.util.List; > import java.util.Map; > +import java.net.InetAddress; > > @APICommand(name = "getSPMetadata", description = "Returns SAML2 > CloudStack Service Provider MetaData", responseObject = > SAMLMetaDataResponse.class, entityType = {}) > public class GetServiceProviderMetaDataCmd extends BaseCmd implements > APIAuthenticator { > @@ -103,7 +104,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, InetAddress remoteAddress, String > responseType, StringBuilder auditTrailSb, HttpServletResponse resp) throws > ServerApiException { > SAMLMetaDataResponse response = new SAMLMetaDataResponse(); > response.setResponseName(getCommandName()); > > > > http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4bd49df3/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 c838ece..913c1ae 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 > @@ -67,6 +67,7 @@ import javax.xml.parsers.ParserConfigurationException; > import javax.xml.stream.FactoryConfigurationError; > import java.io.IOException; > import java.net.URLEncoder; > +import java.net.InetAddress; > import java.security.InvalidKeyException; > import java.security.NoSuchAlgorithmException; > import java.security.PrivateKey; > @@ -160,7 +161,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 InetAddress > remoteAddress, final String responseType, final StringBuilder auditTrailSb, > 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/4bd49df3/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..3608fed 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 > @@ -50,6 +50,7 @@ import javax.xml.stream.FactoryConfigurationError; > import java.io.IOException; > import java.util.List; > import java.util.Map; > +import java.net.InetAddress; > > @APICommand(name = "samlSlo", description = "SAML Global Log Out API", > responseObject = LogoutCmdResponse.class, entityType = {}) > public class SAML2LogoutAPIAuthenticatorCmd extends BaseCmd implements > APIAuthenticator { > @@ -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, InetAddress remoteAddress, String > responseType, StringBuilder auditTrailSb, 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/4bd49df3/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..c727d84 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 > @@ -41,6 +41,8 @@ import java.security.SignatureException; > import java.security.cert.CertificateEncodingException; > import java.security.cert.CertificateParsingException; > import java.security.cert.X509Certificate; > +import java.net.InetAddress; > +import java.net.UnknownHostException; > > @RunWith(MockitoJUnitRunner.class) > public class GetServiceProviderMetaDataCmdTest { > @@ -58,7 +60,7 @@ public class GetServiceProviderMetaDataCmdTest { > HttpServletResponse resp; > > @Test > - public void testAuthenticate() throws NoSuchFieldException, > SecurityException, IllegalArgumentException, IllegalAccessException, > CertificateParsingException, CertificateEncodingException, > NoSuchAlgorithmException, InvalidKeyException, NoSuchProviderException, > SignatureException { > + public void testAuthenticate() throws NoSuchFieldException, > SecurityException, IllegalArgumentException, IllegalAccessException, > CertificateParsingException, CertificateEncodingException, > NoSuchAlgorithmException, InvalidKeyException, NoSuchProviderException, > SignatureException, UnknownHostException { > GetServiceProviderMetaDataCmd cmd = new > GetServiceProviderMetaDataCmd(); > > Field apiServerField = > GetServiceProviderMetaDataCmd.class.getDeclaredField("_apiServer"); > @@ -77,7 +79,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, > InetAddress.getByName("127.0.0.1"), HttpUtils.RESPONSE_TYPE_JSON, new > StringBuilder(), resp); > Assert.assertTrue(result.contains("md:EntityDescriptor")); > > Mockito.verify(samlAuthManager, > Mockito.atLeast(1)).getServiceProviderId(); > > > http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4bd49df3/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 514edb5..f14b02c 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 > @@ -65,6 +65,7 @@ import java.lang.reflect.Field; > import java.security.cert.X509Certificate; > import java.util.HashMap; > import java.util.Map; > +import java.net.InetAddress; > > @RunWith(MockitoJUnitRunner.class) > public class SAML2LoginAPIAuthenticatorCmdTest { > @@ -171,14 +172,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, > InetAddress.getByName("127.0.0.1"), HttpUtils.RESPONSE_TYPE_JSON, new > StringBuilder(), 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, > InetAddress.getByName("127.0.0.1"), HttpUtils.RESPONSE_TYPE_JSON, new > StringBuilder(), resp); > } catch (ServerApiException ignored) { > } > Mockito.verify(configDao, > Mockito.atLeastOnce()).getValue(Mockito.anyString()); > > > http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4bd49df3/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..9ef00b4 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 > @@ -36,6 +36,7 @@ import javax.servlet.http.HttpServletResponse; > import javax.servlet.http.HttpSession; > import java.lang.reflect.Field; > import java.security.cert.X509Certificate; > +import java.net.InetAddress; > > @RunWith(MockitoJUnitRunner.class) > public class SAML2LogoutAPIAuthenticatorCmdTest { > @@ -81,7 +82,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, > InetAddress.getByName("127.0.0.1"), HttpUtils.RESPONSE_TYPE_JSON, new > StringBuilder(), 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/4bd49df3/server/src/com/cloud/api/ApiServer.java > ---------------------------------------------------------------------- > diff --git a/server/src/com/cloud/api/ApiServer.java > b/server/src/com/cloud/api/ApiServer.java > index b335a66..435efa0 100755 > --- a/server/src/com/cloud/api/ApiServer.java > +++ b/server/src/com/cloud/api/ApiServer.java > @@ -982,7 +982,7 @@ public class ApiServer extends ManagerBase implements > HttpRequestHandler, ApiSer > } > > @Override > - public ResponseObject loginUser(final HttpSession session, final > String username, final String password, Long domainId, final String > domainPath, final String loginIpAddress, > + public ResponseObject loginUser(final HttpSession session, final > String username, final String password, Long domainId, final String > domainPath, final InetAddress loginIpAddress, > final Map<String, Object[]> requestParameters) throws > CloudAuthenticationException { > // We will always use domainId first. If that does not exist, we > will use domain name. If THAT doesn't exist > // we will default to ROOT > > > http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4bd49df3/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 3d2e843..675ebed 100644 > --- a/server/src/com/cloud/api/ApiServlet.java > +++ b/server/src/com/cloud/api/ApiServlet.java > @@ -18,6 +18,7 @@ package com.cloud.api; > > import java.io.UnsupportedEncodingException; > import java.net.URLDecoder; > +import java.net.InetAddress; > import java.util.HashMap; > import java.util.Map; > > @@ -182,7 +183,8 @@ public class ApiServlet extends HttpServlet { > } > > try { > - responseString = > apiAuthenticator.authenticate(command, params, session, remoteAddress, > responseType, auditTrailSb, resp); > + InetAddress remoteAddr = > InetAddress.getByName(remoteAddress); > + responseString = > apiAuthenticator.authenticate(command, params, session, remoteAddr, > responseType, auditTrailSb, resp); > } catch (ServerApiException e) { > httpResponseCode = e.getErrorCode().getHttpCode(); > responseString = e.getMessage(); > > > http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4bd49df3/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..39c95ea 100644 > --- a/server/src/com/cloud/api/auth/DefaultLoginAPIAuthenticatorCmd.java > +++ b/server/src/com/cloud/api/auth/DefaultLoginAPIAuthenticatorCmd.java > @@ -37,6 +37,7 @@ import javax.servlet.http.HttpServletResponse; > import javax.servlet.http.HttpSession; > import java.util.List; > import java.util.Map; > +import java.net.InetAddress; > > @APICommand(name = "login", description = "Logs a user into the > CloudStack. A successful login attempt will generate a JSESSIONID cookie > value that can be passed in subsequent Query command calls until the > \"logout\" command has been issued or the session has expired.", > requestHasSensitiveInfo = true, responseObject = LoginCmdResponse.class, > entityType = {}) > public class DefaultLoginAPIAuthenticatorCmd extends BaseCmd implements > APIAuthenticator { > @@ -103,7 +104,7 @@ 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, InetAddress remoteAddress, String > responseType, StringBuilder auditTrailSb, final HttpServletResponse resp) > throws ServerApiException { > > // FIXME: ported from ApiServlet, refactor and cleanup > final String[] username = > (String[])params.get(ApiConstants.USERNAME); > > > http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4bd49df3/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..748c8e5 100644 > --- a/server/src/com/cloud/api/auth/DefaultLogoutAPIAuthenticatorCmd.java > +++ b/server/src/com/cloud/api/auth/DefaultLogoutAPIAuthenticatorCmd.java > @@ -32,6 +32,7 @@ import javax.servlet.http.HttpServletResponse; > import javax.servlet.http.HttpSession; > import java.util.List; > import java.util.Map; > +import java.net.InetAddress; > > @APICommand(name = "logout", description = "Logs out the user", > responseObject = LogoutCmdResponse.class, entityType = {}) > public class DefaultLogoutAPIAuthenticatorCmd extends BaseCmd implements > APIAuthenticator { > @@ -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, InetAddress remoteAddress, String > responseType, StringBuilder auditTrailSb, 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/4bd49df3/server/src/com/cloud/user/AccountManager.java > ---------------------------------------------------------------------- > diff --git a/server/src/com/cloud/user/AccountManager.java > b/server/src/com/cloud/user/AccountManager.java > index 194c5d2..7b9e093 100755 > --- a/server/src/com/cloud/user/AccountManager.java > +++ b/server/src/com/cloud/user/AccountManager.java > @@ -18,6 +18,7 @@ package com.cloud.user; > > import java.util.List; > import java.util.Map; > +import java.net.InetAddress; > > import org.apache.cloudstack.acl.ControlledEntity; > import org.apache.cloudstack.api.command.admin.account.UpdateAccountCmd; > @@ -71,7 +72,7 @@ public interface AccountManager extends AccountService { > * made, and the signature itself in the single sign-on > case > * @return a user object, null if the user failed to authenticate > */ > - UserAccount authenticateUser(String username, String password, Long > domainId, String loginIpAddress, Map<String, Object[]> requestParameters); > + UserAccount authenticateUser(String username, String password, Long > domainId, InetAddress loginIpAddress, Map<String, Object[]> > requestParameters); > > /** > * Locate a user by their apiKey > > > http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4bd49df3/server/src/com/cloud/user/AccountManagerImpl.java > ---------------------------------------------------------------------- > diff --git a/server/src/com/cloud/user/AccountManagerImpl.java > b/server/src/com/cloud/user/AccountManagerImpl.java > index 3c00f91..f816ee4 100755 > --- a/server/src/com/cloud/user/AccountManagerImpl.java > +++ b/server/src/com/cloud/user/AccountManagerImpl.java > @@ -17,6 +17,7 @@ > package com.cloud.user; > > import java.net.URLEncoder; > +import java.net.InetAddress; > import java.security.NoSuchAlgorithmException; > import java.util.ArrayList; > import java.util.Collections; > @@ -1972,7 +1973,7 @@ public class AccountManagerImpl extends ManagerBase > implements AccountManager, M > } > > @Override > - public UserAccount authenticateUser(String username, String password, > Long domainId, String loginIpAddress, Map<String, Object[]> > requestParameters) { > + public UserAccount authenticateUser(String username, String password, > Long domainId, InetAddress loginIpAddress, Map<String, Object[]> > requestParameters) { > UserAccount user = null; > if (password != null) { > user = getUserAccount(username, password, domainId, > requestParameters); > @@ -2080,13 +2081,10 @@ public class AccountManagerImpl extends > ManagerBase implements AccountManager, M > if (s_logger.isDebugEnabled()) { > s_logger.debug("User: " + username + " in domain " + > domainId + " has successfully logged in"); > } > - if (NetUtils.isValidIp(loginIpAddress)) { > - ActionEventUtils.onActionEvent(user.getId(), > user.getAccountId(), user.getDomainId(), EventTypes.EVENT_USER_LOGIN, "user > has logged in from IP Address " + > + > + ActionEventUtils.onActionEvent(user.getId(), > user.getAccountId(), user.getDomainId(), EventTypes.EVENT_USER_LOGIN, "user > has logged in from IP Address " + > loginIpAddress); > - } else { > - ActionEventUtils.onActionEvent(user.getId(), > user.getAccountId(), user.getDomainId(), EventTypes.EVENT_USER_LOGIN, > - "user has logged in. The IP Address cannot be > determined"); > - } > + > return user; > } else { > if (s_logger.isDebugEnabled()) { > > > http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4bd49df3/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..902217d 100644 > --- a/server/test/com/cloud/api/ApiServletTest.java > +++ b/server/test/com/cloud/api/ApiServletTest.java > @@ -42,6 +42,8 @@ import java.io.StringWriter; > import java.io.UnsupportedEncodingException; > import java.lang.reflect.Field; > import java.net.URLEncoder; > +import java.net.InetAddress; > +import java.net.UnknownHostException; > import java.util.HashMap; > > @RunWith(MockitoJUnitRunner.class) > @@ -83,7 +85,7 @@ public class ApiServletTest { > > @Before > public void setup() throws SecurityException, NoSuchFieldException, > - IllegalArgumentException, IllegalAccessException, IOException > { > + IllegalArgumentException, IllegalAccessException, > IOException, UnknownHostException { > servlet = new ApiServlet(); > responseWriter = new StringWriter(); > Mockito.when(response.getWriter()).thenReturn( > @@ -99,7 +101,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\":{}"); > + InetAddress.getByName("127.0.0.1"), Mockito.anyString(), > Mockito.isA(StringBuilder.class), > Mockito.isA(HttpServletResponse.class))).thenReturn("{\"loginresponse\":{}"); > > Field authManagerField = > ApiServlet.class.getDeclaredField("_authManager"); > authManagerField.setAccessible(true); > @@ -193,7 +195,7 @@ public class ApiServletTest { > } > > @Test > - public void processRequestInContextLogout() { > + public void processRequestInContextLogout() throws > UnknownHostException { > Mockito.when(request.getMethod()).thenReturn("GET"); > Mockito.when(request.getSession(Mockito.anyBoolean())).thenReturn( > session); > @@ -210,13 +212,13 @@ 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)); > + InetAddress.getByName("127.0.0.1"), Mockito.anyString(), > Mockito.isA(StringBuilder.class), Mockito.isA(HttpServletResponse.class)); > Mockito.verify(session).invalidate(); > } > > @SuppressWarnings("unchecked") > @Test > - public void processRequestInContextLogin() { > + public void processRequestInContextLogin() throws > UnknownHostException { > Mockito.when(request.getMethod()).thenReturn("GET"); > Mockito.when(request.getSession(Mockito.anyBoolean())).thenReturn( > session); > @@ -232,6 +234,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)); > + InetAddress.getByName("127.0.0.1"), Mockito.anyString(), > Mockito.isA(StringBuilder.class), Mockito.isA(HttpServletResponse.class)); > } > } > > > http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4bd49df3/server/test/com/cloud/user/MockAccountManagerImpl.java > ---------------------------------------------------------------------- > diff --git a/server/test/com/cloud/user/MockAccountManagerImpl.java > b/server/test/com/cloud/user/MockAccountManagerImpl.java > index 8620031..6dcd46c 100644 > --- a/server/test/com/cloud/user/MockAccountManagerImpl.java > +++ b/server/test/com/cloud/user/MockAccountManagerImpl.java > @@ -18,6 +18,7 @@ package com.cloud.user; > > import java.util.List; > import java.util.Map; > +import java.net.InetAddress; > > import javax.ejb.Local; > import javax.naming.ConfigurationException; > @@ -250,7 +251,7 @@ public class MockAccountManagerImpl extends > ManagerBase implements Manager, Acco > } > > @Override > - public UserAccount authenticateUser(String username, String password, > Long domainId, String loginIpAddress, Map<String, Object[]> > requestParameters) { > + public UserAccount authenticateUser(String username, String password, > Long domainId, InetAddress loginIpAddress, Map<String, Object[]> > requestParameters) { > return null; > } > > >
