incubator-cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Wido den Hollander <w...@widodh.nl>
Subject Re: [PATCH] CS-15018 Event USER.LOGIN should contain the client IP address
Date Mon, 04 Jun 2012 13:43:57 GMT
Hi,

On 06/04/2012 03:35 PM, saksham wrote:
>
> Signed-off-by: saksham<saksham.srivastava@citrix.com>
> ---
>   server/src/com/cloud/api/ApiServer.java            |    4 ++--
>   server/src/com/cloud/api/ApiServlet.java           |    2 +-
>   server/src/com/cloud/user/AccountManager.java      |    2 +-
>   server/src/com/cloud/user/AccountManagerImpl.java  |    4 ++--
>   .../com/cloud/user/MockAccountManagerImpl.java     |    2 +-
>   5 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/server/src/com/cloud/api/ApiServer.java b/server/src/com/cloud/api/ApiServer.java
> index 83133e4..8814bcb 100755
> --- a/server/src/com/cloud/api/ApiServer.java
> +++ b/server/src/com/cloud/api/ApiServer.java
> @@ -773,7 +773,7 @@ public class ApiServer implements HttpRequestHandler {
>           }
>       }
>
> -    public void loginUser(HttpSession session, String username, String password, Long
domainId, String domainPath, Map<String, Object[]>  requestParameters) throws CloudAuthenticationException
{
> +    public void loginUser(HttpSession session, String username, String password, Long
domainId, String domainPath, String ip_addr ,Map<String, Object[]>  requestParameters)
throws CloudAuthenticationException {

Shouldn't you use the mixed case naming here for the IP Address?

http://docs.cloudstack.org/CloudStack_Documentation/Design_Documents/Coding_Conventions

"Variable names must be in mixed case starting with lower case, E.g., 
virtualRouter"

>           // 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
>           if (domainId == null) {
> @@ -789,7 +789,7 @@ public class ApiServer implements HttpRequestHandler {
>               }
>           }
>
> -        UserAccount userAcct = _accountMgr.authenticateUser(username, password, domainId,
requestParameters);
> +        UserAccount userAcct = _accountMgr.authenticateUser(username, password, domainId,
ip_addr, requestParameters);

Same here

>           if (userAcct != null) {
>               String timezone = userAcct.getTimezone();
>               float offsetInHrs = 0f;
> diff --git a/server/src/com/cloud/api/ApiServlet.java b/server/src/com/cloud/api/ApiServlet.java
> index b7b7fff..a0da657 100755
> --- a/server/src/com/cloud/api/ApiServlet.java
> +++ b/server/src/com/cloud/api/ApiServlet.java
> @@ -203,7 +203,7 @@ public class ApiServlet extends HttpServlet {
>                       if (username != null) {
>                           String pwd = ((password == null) ? null : password[0]);
>                           try {
> -                            _apiServer.loginUser(session, username[0], pwd, domainId,
domain, params);
> +                            _apiServer.loginUser(session, username[0], pwd, domainId,
domain, req.getRemoteAddr(), params);
>                               auditTrailSb.insert(0,
>                                       "(userId=" + session.getAttribute("userid") + "
accountId=" + ((Account) session.getAttribute("accountobj")).getId() + " sessionId=" + session.getId()
+ ")");
>                               String loginResponse = getLoginSuccessResponse(session,
responseType);
> diff --git a/server/src/com/cloud/user/AccountManager.java b/server/src/com/cloud/user/AccountManager.java
> index a7f5a68..2f01550 100755
> --- a/server/src/com/cloud/user/AccountManager.java
> +++ b/server/src/com/cloud/user/AccountManager.java
> @@ -71,7 +71,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, Map<String,
Object[]>  requestParameters);
> +    UserAccount authenticateUser(String username, String password, Long domainId, String
ip_addr, Map<String, Object[]>  requestParameters);

And here

>
>       /**
>        * Locate a user by their apiKey
> diff --git a/server/src/com/cloud/user/AccountManagerImpl.java b/server/src/com/cloud/user/AccountManagerImpl.java
> index 35fbfe0..557b59c 100755
> --- a/server/src/com/cloud/user/AccountManagerImpl.java
> +++ b/server/src/com/cloud/user/AccountManagerImpl.java
> @@ -1618,7 +1618,7 @@ public class AccountManagerImpl implements AccountManager, AccountService,
Manag
>       }
>
>       @Override
> -    public UserAccount authenticateUser(String username, String password, Long domainId,
Map<String, Object[]>  requestParameters) {
> +    public UserAccount authenticateUser(String username, String password, Long domainId,
String ip_addr, Map<String, Object[]>  requestParameters) {

And here

>           UserAccount user = null;
>           if (password != null) {
>               user = getUserAccount(username, password, domainId, requestParameters);
> @@ -1720,7 +1720,7 @@ public class AccountManagerImpl implements AccountManager, AccountService,
Manag
>               if (s_logger.isDebugEnabled()) {
>                   s_logger.debug("User: " + username + " in domain " + domainId + " has
successfully logged in");
>               }
> -            EventUtils.saveEvent(user.getId(), user.getAccountId(), user.getDomainId(),
EventTypes.EVENT_USER_LOGIN, "user has logged in");
> +            EventUtils.saveEvent(user.getId(), user.getAccountId(), user.getDomainId(),
EventTypes.EVENT_USER_LOGIN, "user has logged in from IP Address "+ip_addr);

And here again. Also continuation of lines, better use: "user has logged 
in from IP Address " + ip_addr

That's easier to read.

>               return user;
>           } else {
>               if (s_logger.isDebugEnabled()) {
> diff --git a/server/test/com/cloud/user/MockAccountManagerImpl.java b/server/test/com/cloud/user/MockAccountManagerImpl.java
> index e6ab4fe..46c6fb7 100644
> --- a/server/test/com/cloud/user/MockAccountManagerImpl.java
> +++ b/server/test/com/cloud/user/MockAccountManagerImpl.java
> @@ -254,7 +254,7 @@ public class MockAccountManagerImpl implements Manager, AccountManager
{
>       }
>
>       @Override
> -    public UserAccount authenticateUser(String username, String password, Long domainId,
Map<String, Object[]>  requestParameters) {
> +    public UserAccount authenticateUser(String username, String password, Long domainId,
String ip_addr, Map<String, Object[]>  requestParameters) {

And ip_addr once more.

>           return null;
>       }
>

Wido

Mime
View raw message