On Fri, Oct 5, 2012 at 2:49 PM, David Nalley <da...@gnsa.us> wrote: > On Fri, Oct 5, 2012 at 2:36 PM, <alena1...@apache.org> wrote: >> Updated Branches: >> refs/heads/4.0 8ba6c8b17 -> f7ebb76f5 >> >> >> CLOUDSTACK-121: Fixed "Incorrect username/domainId login causes >> NullPointerException " >> >> >> Project: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/repo >> Commit: >> http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/commit/f7ebb76f >> Tree: >> http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/tree/f7ebb76f >> Diff: >> http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/diff/f7ebb76f >> >> Branch: refs/heads/4.0 >> Commit: f7ebb76f57a0c88c8b379aacb1a4e3fd653a325f >> Parents: 8ba6c8b >> Author: Rohit Yadav <rohit.ya...@citrix.com> >> Authored: Fri Oct 5 11:32:45 2012 -0700 >> Committer: Alena Prokharchyk <alena.prokharc...@citrix.com> >> Committed: Fri Oct 5 11:33:21 2012 -0700 >> >> ---------------------------------------------------------------------- >> api/src/com/cloud/user/UserAccount.java | 2 ++ >> server/src/com/cloud/user/AccountManagerImpl.java | 15 ++++++++------- >> 2 files changed, 10 insertions(+), 7 deletions(-) >> ---------------------------------------------------------------------- >> >> >> http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/f7ebb76f/api/src/com/cloud/user/UserAccount.java >> ---------------------------------------------------------------------- >> diff --git a/api/src/com/cloud/user/UserAccount.java >> b/api/src/com/cloud/user/UserAccount.java >> index 734e16b..2a6bd4f 100644 >> --- a/api/src/com/cloud/user/UserAccount.java >> +++ b/api/src/com/cloud/user/UserAccount.java >> @@ -56,4 +56,6 @@ public interface UserAccount { >> String getRegistrationToken(); >> >> boolean isRegistered(); >> + >> + int getLoginAttempts(); >> } >> >> http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/f7ebb76f/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 fa9fafb..4d3f45c 100755 >> --- a/server/src/com/cloud/user/AccountManagerImpl.java >> +++ b/server/src/com/cloud/user/AccountManagerImpl.java >> @@ -1862,24 +1862,25 @@ public class AccountManagerImpl implements >> AccountManager, AccountService, Manag >> } >> >> UserAccount userAccount = >> _userAccountDao.getUserAccount(username, domainId); >> - UserAccountVO user = >> _userAccountDao.findById(userAccount.getId()); >> - if (user != null) { >> - if ((user.getState().toString()).equals("enabled")) { >> - if (!isInternalAccount(user.getType())) { >> + if (userAccount != null) { >> + if >> (userAccount.getState().equalsIgnoreCase(Account.State.enabled.toString())) { >> + if (!isInternalAccount(userAccount.getType())) { >> //Internal accounts are not disabled >> - int attemptsMade = user.getLoginAttempts() + 1; >> + int attemptsMade = userAccount.getLoginAttempts() + >> 1; >> if (attemptsMade < _allowedLoginAttempts) { >> updateLoginAttempts(userAccount.getId(), >> attemptsMade, false); >> s_logger.warn("Login attempt failed. You have " >> + ( _allowedLoginAttempts - attemptsMade ) + " attempt(s) remaining"); >> } else { >> updateLoginAttempts(userAccount.getId(), >> _allowedLoginAttempts, true); >> - s_logger.warn("User " + user.getUsername() + " >> has been disabled due to multiple failed login attempts." + >> + s_logger.warn("User " + >> userAccount.getUsername() + " has been disabled due to multiple failed login >> attempts." + >> " Please contact admin."); >> } >> } >> } else { >> - s_logger.info("User " + user.getUsername() + " is >> disabled/locked"); >> + s_logger.info("User " + userAccount.getUsername() + " >> is disabled/locked"); >> } >> + } else { >> + s_logger.warn("Authentication failure: No user with name " >> + username + " for domainId " + domainId); >> } >> return null; >> } >> > > This is a bit of information leakage? I mean we are essentially > telling someone what usernames don't exist, which by process of > elimination means that folks can determine what user accounts are > valid. >
Aren't these just log statements? That means they are within the system, and therefore aren't exposing anything new to an operator.
