Display proper error message when LDAP is unavailable
At the moment the error message 'Incorrect username or password.' is displayed when querying the LDAP server fails because it is unavailable. This message is misleading since the user cannot make the login succeed by correcting the username or password. With this change now a proper error message is displayed so that the user can understand that login is not possible due to the LDAP server not being available. Change-Id: I4afa503642537dbdb33c1031220281f923dddfc8 Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>
This commit is contained in:
		@@ -14,7 +14,40 @@
 | 
			
		||||
 | 
			
		||||
package com.google.gerrit.common.auth.userpass;
 | 
			
		||||
 | 
			
		||||
import com.google.gerrit.reviewdb.AuthType;
 | 
			
		||||
 | 
			
		||||
public class LoginResult {
 | 
			
		||||
  public boolean success;
 | 
			
		||||
  public boolean isNew;
 | 
			
		||||
 | 
			
		||||
  protected AuthType authType;
 | 
			
		||||
  protected Error error;
 | 
			
		||||
 | 
			
		||||
  protected LoginResult() {
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public LoginResult(final AuthType authType) {
 | 
			
		||||
    this.authType = authType;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public AuthType getAuthType() {
 | 
			
		||||
    return authType;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public void setError(final Error error) {
 | 
			
		||||
    this.error = error;
 | 
			
		||||
    success = error == null;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public Error getError() {
 | 
			
		||||
    return error;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public static enum Error {
 | 
			
		||||
    /** Username or password are invalid */
 | 
			
		||||
    INVALID_LOGIN,
 | 
			
		||||
 | 
			
		||||
    /** The authentication server is unavailable or the query to it timed out */
 | 
			
		||||
    AUTHENTICATION_UNAVAILABLE
 | 
			
		||||
  }
 | 
			
		||||
}
 | 
			
		||||
 
 | 
			
		||||
@@ -14,8 +14,10 @@
 | 
			
		||||
 | 
			
		||||
package com.google.gerrit.client.auth.userpass;
 | 
			
		||||
 | 
			
		||||
import com.google.gerrit.reviewdb.AuthType;
 | 
			
		||||
import com.google.gwt.i18n.client.Messages;
 | 
			
		||||
 | 
			
		||||
public interface UserPassMessages extends Messages {
 | 
			
		||||
  String signInAt(String hostname);
 | 
			
		||||
  String authenticationUnavailable(AuthType authType);
 | 
			
		||||
}
 | 
			
		||||
 
 | 
			
		||||
@@ -1 +1,2 @@
 | 
			
		||||
signInAt = Sign In to Gerrit Code Review at {0}
 | 
			
		||||
authenticationUnavailable = {0} authentication unavailable
 | 
			
		||||
 
 | 
			
		||||
@@ -209,7 +209,16 @@ public class UserPassSignInDialog extends SignInDialog {
 | 
			
		||||
          }
 | 
			
		||||
          Location.replace(Location.getPath() + "login" + to);
 | 
			
		||||
        } else {
 | 
			
		||||
          showError(Util.C.invalidLogin());
 | 
			
		||||
          final String message;
 | 
			
		||||
          switch (result.getError()) {
 | 
			
		||||
            case AUTHENTICATION_UNAVAILABLE:
 | 
			
		||||
              message = Util.M.authenticationUnavailable(result.getAuthType());
 | 
			
		||||
              break;
 | 
			
		||||
            case INVALID_LOGIN:
 | 
			
		||||
            default:
 | 
			
		||||
              message = Util.C.invalidLogin();
 | 
			
		||||
          }
 | 
			
		||||
          showError(message);
 | 
			
		||||
          enable(true);
 | 
			
		||||
          password.selectAll();
 | 
			
		||||
          Scheduler.get().scheduleDeferred(new ScheduledCommand() {
 | 
			
		||||
 
 | 
			
		||||
@@ -17,11 +17,14 @@ package com.google.gerrit.httpd.auth.ldap;
 | 
			
		||||
import com.google.gerrit.common.auth.userpass.LoginResult;
 | 
			
		||||
import com.google.gerrit.common.auth.userpass.UserPassAuthService;
 | 
			
		||||
import com.google.gerrit.httpd.WebSession;
 | 
			
		||||
import com.google.gerrit.reviewdb.AuthType;
 | 
			
		||||
import com.google.gerrit.server.account.AccountException;
 | 
			
		||||
import com.google.gerrit.server.account.AccountManager;
 | 
			
		||||
import com.google.gerrit.server.account.AccountUserNameException;
 | 
			
		||||
import com.google.gerrit.server.account.AuthRequest;
 | 
			
		||||
import com.google.gerrit.server.account.AuthResult;
 | 
			
		||||
import com.google.gerrit.server.auth.AuthenticationUnavailableException;
 | 
			
		||||
import com.google.gerrit.server.config.AuthConfig;
 | 
			
		||||
import com.google.gwt.user.client.rpc.AsyncCallback;
 | 
			
		||||
import com.google.inject.Inject;
 | 
			
		||||
import com.google.inject.Provider;
 | 
			
		||||
@@ -29,21 +32,23 @@ import com.google.inject.Provider;
 | 
			
		||||
class UserPassAuthServiceImpl implements UserPassAuthService {
 | 
			
		||||
  private final Provider<WebSession> webSession;
 | 
			
		||||
  private final AccountManager accountManager;
 | 
			
		||||
  private final AuthType authType;
 | 
			
		||||
 | 
			
		||||
  @Inject
 | 
			
		||||
  UserPassAuthServiceImpl(final Provider<WebSession> webSession,
 | 
			
		||||
      final AccountManager accountManager) {
 | 
			
		||||
      final AccountManager accountManager, final AuthConfig authConfig) {
 | 
			
		||||
    this.webSession = webSession;
 | 
			
		||||
    this.accountManager = accountManager;
 | 
			
		||||
    this.authType = authConfig.getAuthType();
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  @Override
 | 
			
		||||
  public void authenticate(String username, final String password,
 | 
			
		||||
      final AsyncCallback<LoginResult> callback) {
 | 
			
		||||
    LoginResult result = new LoginResult();
 | 
			
		||||
    LoginResult result = new LoginResult(authType);
 | 
			
		||||
    if (username == null || "".equals(username.trim()) //
 | 
			
		||||
        || password == null || "".equals(password)) {
 | 
			
		||||
      result.success = false;
 | 
			
		||||
      result.setError(LoginResult.Error.INVALID_LOGIN);
 | 
			
		||||
      callback.onSuccess(result);
 | 
			
		||||
      return;
 | 
			
		||||
    }
 | 
			
		||||
@@ -62,8 +67,12 @@ class UserPassAuthServiceImpl implements UserPassAuthService {
 | 
			
		||||
      // error screen with error message should be shown to the user
 | 
			
		||||
      callback.onFailure(e);
 | 
			
		||||
      return;
 | 
			
		||||
    } catch (AuthenticationUnavailableException e) {
 | 
			
		||||
      result.setError(LoginResult.Error.AUTHENTICATION_UNAVAILABLE);
 | 
			
		||||
      callback.onSuccess(result);
 | 
			
		||||
      return;
 | 
			
		||||
    } catch (AccountException e) {
 | 
			
		||||
      result.success = false;
 | 
			
		||||
      result.setError(LoginResult.Error.INVALID_LOGIN);
 | 
			
		||||
      callback.onSuccess(result);
 | 
			
		||||
      return;
 | 
			
		||||
    }
 | 
			
		||||
 
 | 
			
		||||
@@ -0,0 +1,26 @@
 | 
			
		||||
// Copyright (C) 2011 The Android Open Source Project
 | 
			
		||||
//
 | 
			
		||||
// Licensed under the Apache License, Version 2.0 (the "License");
 | 
			
		||||
// you may not use this file except in compliance with the License.
 | 
			
		||||
// You may obtain a copy of the License at
 | 
			
		||||
//
 | 
			
		||||
// http://www.apache.org/licenses/LICENSE-2.0
 | 
			
		||||
//
 | 
			
		||||
// Unless required by applicable law or agreed to in writing, software
 | 
			
		||||
// distributed under the License is distributed on an "AS IS" BASIS,
 | 
			
		||||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 | 
			
		||||
// See the License for the specific language governing permissions and
 | 
			
		||||
// limitations under the License.
 | 
			
		||||
 | 
			
		||||
package com.google.gerrit.server.auth;
 | 
			
		||||
 | 
			
		||||
import com.google.gerrit.server.account.AccountException;
 | 
			
		||||
 | 
			
		||||
/** A query to the authentication server failed */
 | 
			
		||||
public class AuthenticationUnavailableException extends AccountException {
 | 
			
		||||
  private static final long serialVersionUID = 1L;
 | 
			
		||||
 | 
			
		||||
  public AuthenticationUnavailableException(final String message, final Throwable why) {
 | 
			
		||||
    super(message, why);
 | 
			
		||||
  }
 | 
			
		||||
}
 | 
			
		||||
@@ -27,6 +27,7 @@ import com.google.gerrit.server.account.AccountState;
 | 
			
		||||
import com.google.gerrit.server.account.AuthRequest;
 | 
			
		||||
import com.google.gerrit.server.account.EmailExpander;
 | 
			
		||||
import com.google.gerrit.server.account.Realm;
 | 
			
		||||
import com.google.gerrit.server.auth.AuthenticationUnavailableException;
 | 
			
		||||
import com.google.gerrit.server.auth.ldap.Helper.LdapSchema;
 | 
			
		||||
import com.google.gerrit.server.cache.Cache;
 | 
			
		||||
import com.google.gerrit.server.cache.EntryCreator;
 | 
			
		||||
@@ -238,7 +239,7 @@ class LdapRealm implements Realm {
 | 
			
		||||
      }
 | 
			
		||||
    } catch (NamingException e) {
 | 
			
		||||
      log.error("Cannot query LDAP to autenticate user", e);
 | 
			
		||||
      throw new AccountException("Cannot query LDAP for account", e);
 | 
			
		||||
      throw new AuthenticationUnavailableException("Cannot query LDAP for account", e);
 | 
			
		||||
    }
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
 
 | 
			
		||||
		Reference in New Issue
	
	Block a user