incubator-cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Nalley <da...@gnsa.us>
Subject Re: git commit: CLOUDSTACK-121: Fixed "Incorrect username/domainId login causes NullPointerException "
Date Fri, 05 Oct 2012 18:49:27 GMT
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.

Mime
View raw message