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.
