Refine error returned by Authenticator

There has been inconsistency in terms of the error returned by
authenticator.
This commit introduces an error ErrAuth to explicitly flag an
authentication failure, which should be treated as user error such
as "invalid credentials", and other errors will be treated as system error.

src/ui/auth/authenticator.go

@@ -30,10 +30,26 @@ const frozenTime time.Duration = 1500 * time.Millisecond
 
 var lock = NewUserLock(frozenTime)
 
+//ErrAuth is the type of error to indicate a failed authentication due to user's error.
+type ErrAuth struct {
+	details string
+}
+
+//Error ...
+func (ea ErrAuth) Error() string {
+	return fmt.Sprintf("Failed to authenticate user, due to error '%s'", ea.details)
+}
+
+//NewErrAuth ...
+func NewErrAuth(msg string) ErrAuth {
+	return ErrAuth{details: msg}
+}
+
 // AuthenticateHelper provides interface for user management in different auth modes.
 type AuthenticateHelper interface {
 
-	// Authenticate ...
+	// Authenticate authenticate the user based on data in m.  Only when the error returned is an instance
+	// of ErrAuth, it will be considered a bad credentials, other errors will be treated as server side error.
 	Authenticate(m models.AuthModel) (*models.User, error)
 	// OnBoardUser will check if a user exists in user table, if not insert the user and
 	// put the id in the pointer of user model, if it does exist, fill in the user model based
@@ -104,13 +120,13 @@ func Login(m models.AuthModel) (*models.User, error) {
 		return nil, nil
 	}
 	user, err := authenticator.Authenticate(m)
-	if user == nil {
-		if err == nil {
+	if err != nil {
+		if _, ok = err.(ErrAuth); ok {
 			log.Debugf("Login failed, locking %s, and sleep for %v", m.Principal, frozenTime)
 			lock.Lock(m.Principal)
 			time.Sleep(frozenTime)
 		}
-		return user, err
+		return nil, err
 	}
 
 	err = authenticator.PostAuthenticate(user)

src/ui/auth/db/db.go

@@ -31,6 +31,9 @@ func (d *Auth) Authenticate(m models.AuthModel) (*models.User, error) {
 	if err != nil {
 		return nil, err
 	}
+	if u == nil {
+		return nil, auth.NewErrAuth("Invalid credentials")
+	}
 	return u, nil
 }
 

src/ui/auth/ldap/ldap.go

@@ -39,7 +39,7 @@ func (l *Auth) Authenticate(m models.AuthModel) (*models.User, error) {
 	p := m.Principal
 	if len(strings.TrimSpace(p)) == 0 {
 		log.Debugf("LDAP authentication failed for empty user id.")
-		return nil, nil
+		return nil, auth.NewErrAuth("Empty user id")
 	}
 
 	ldapSession, err := ldapUtils.LoadSystemLdapConfig()
@@ -50,7 +50,7 @@ func (l *Auth) Authenticate(m models.AuthModel) (*models.User, error) {
 
 	if err = ldapSession.Open(); err != nil {
 		log.Warningf("ldap connection fail: %v", err)
-		return nil, nil
+		return nil, err
 	}
 	defer ldapSession.Close()
 
@@ -58,15 +58,15 @@ func (l *Auth) Authenticate(m models.AuthModel) (*models.User, error) {
 
 	if err != nil {
 		log.Warningf("ldap search fail: %v", err)
-		return nil, nil
+		return nil, err
 	}
 
 	if len(ldapUsers) == 0 {
 		log.Warningf("Not found an entry.")
-		return nil, nil
+		return nil, auth.NewErrAuth("Not found an entry")
 	} else if len(ldapUsers) != 1 {
 		log.Warningf("Found more than one entry.")
-		return nil, nil
+		return nil, auth.NewErrAuth("Multiple entries found")
 	}
 
 	u := models.User{}
@@ -78,7 +78,7 @@ func (l *Auth) Authenticate(m models.AuthModel) (*models.User, error) {
 
 	if err = ldapSession.Bind(dn, m.Password); err != nil {
 		log.Warningf("Failed to bind user, username: %s, dn: %s, error: %v", u.Username, dn, err)
-		return nil, nil
+		return nil, auth.NewErrAuth(err.Error())
 	}
 
 	return &u, nil

src/ui/auth/ldap/ldap_test.go

@@ -108,10 +108,10 @@ func TestMain(m *testing.M) {
 
 func TestAuthenticate(t *testing.T) {
 	var person models.AuthModel
-	var auth *Auth
+	var authHelper *Auth
 	person.Principal = "test"
 	person.Password = "123456"
-	user, err := auth.Authenticate(person)
+	user, err := authHelper.Authenticate(person)
 	if err != nil {
 		t.Errorf("unexpected ldap authenticate fail: %v", err)
 	}
@@ -120,29 +120,23 @@ func TestAuthenticate(t *testing.T) {
 	}
 	person.Principal = "test"
 	person.Password = "1"
-	user, err = auth.Authenticate(person)
-	if err != nil {
-		t.Errorf("unexpected ldap error: %v", err)
-	}
-	if user != nil {
-		t.Errorf("Nil user expected for wrong password")
+	user, err = authHelper.Authenticate(person)
+
+	if _, ok := err.(auth.ErrAuth); !ok {
+		t.Errorf("Expected an ErrAuth on wrong password, but got: %v", err)
 	}
 	person.Principal = ""
 	person.Password = ""
-	user, err = auth.Authenticate(person)
-	if err != nil {
-		t.Errorf("unexpected ldap error: %v", err)
+	user, err = authHelper.Authenticate(person)
+	if _, ok := err.(auth.ErrAuth); !ok {
+		t.Errorf("Expected an ErrAuth on empty credentials, but got: %v", err)
 	}
-	if user != nil {
-		t.Errorf("Nil user for empty credentials")
-	}
-
 	//authenticate the second time
 	person2 := models.AuthModel{
 		Principal: "test",
 		Password:  "123456",
 	}
-	user2, err := auth.Authenticate(person2)
+	user2, err := authHelper.Authenticate(person2)
 
 	if err != nil {
 		t.Errorf("unexpected ldap error: %v", err)

src/ui/auth/uaa/uaa.go

@@ -42,7 +42,9 @@ func (u *Auth) Authenticate(m models.AuthModel) (*models.User, error) {
 		return nil, err
 	}
 	t, err := u.client.PasswordAuth(m.Principal, m.Password)
-	if t != nil && err == nil {
+	if err != nil {
+		return nil, auth.NewErrAuth(err.Error())
+	}
 	user := &models.User{
 		Username: m.Principal,
 	}
@@ -55,8 +57,6 @@ func (u *Auth) Authenticate(m models.AuthModel) (*models.User, error) {
 	}
 	return user, nil
 }
-	return nil, err
-}
 
 // OnBoardUser will check if a user exists in user table, if not insert the user and
 // put the id in the pointer of user model, if it does exist, return the user's profile.