incubator-cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chip Childers <chip.child...@sungard.com>
Subject Re: git commit: CLOUDSTACK-121: Fixed "Incorrect username/domainId login causes NullPointerException "
Date Fri, 05 Oct 2012 19:13:09 GMT
On Fri, Oct 5, 2012 at 2:49 PM, David Nalley <david@gnsa.us> wrote:
> On Fri, Oct 5, 2012 at 2:36 PM,  <alena1108@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.yadav@citrix.com>
>> Authored: Fri Oct 5 11:32:45 2012 -0700
>> Committer: Alena Prokharchyk <alena.prokharchyk@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.

Mime
View raw message