On Fri, Oct 5, 2012 at 3:13 PM, Chip Childers <chip.child...@sungard.com> wrote: > 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.
Doh, yes- you are right.
