models/login_source: code improvement

6 files changed, 116 insertions, 119 deletions

diff --git a/README.md b/README.md
index cab3c59d45..60aa87527b 100644
--- a/README.md
+++ b/README.md
@@ -3,7 +3,7 @@ Gogs - Go Git Service [![Build Status](https://travis-ci.org/gogits/gogs.svg?bra
 
 ![](https://github.com/gogits/gogs/blob/master/public/img/gogs-large-resize.png?raw=true)
 
-##### Current tip version: 0.9.96 (see [Releases](https://github.com/gogits/gogs/releases) for binary versions)
+##### Current tip version: 0.9.97 (see [Releases](https://github.com/gogits/gogs/releases) for binary versions)
 
 | Web | UI  | Preview  |
 |:-------------:|:-------:|:-------:|
diff --git a/gogs.go b/gogs.go
index be61885dc3..04689d4544 100644
--- a/gogs.go
+++ b/gogs.go
@@ -17,7 +17,7 @@ import (
 	"github.com/gogits/gogs/modules/setting"
 )
 
-const APP_VER = "0.9.96.0830"
+const APP_VER = "0.9.97.0830"
 
 func init() {
 	runtime.GOMAXPROCS(runtime.NumCPU())
diff --git a/models/error.go b/models/error.go
index 065857e001..182a944a6f 100644
--- a/models/error.go
+++ b/models/error.go
@@ -635,6 +635,19 @@ func (err ErrLoginSourceAlreadyExist) Error() string {
 	return fmt.Sprintf("login source already exists [name: %s]", err.Name)
 }
 
+type ErrLoginSourceInUse struct {
+	ID int64
+}
+
+func IsErrLoginSourceInUse(err error) bool {
+	_, ok := err.(ErrLoginSourceInUse)
+	return ok
+}
+
+func (err ErrLoginSourceInUse) Error() string {
+	return fmt.Sprintf("login source is still used by some users [id: %d]", err.ID)
+}
+
 // ___________
 // \__    ___/___ _____    _____
 //   |    |_/ __ \\__  \  /     \
diff --git a/models/login.go b/models/login_source.go
index bef1617dd6..82186fb713 100644
--- a/models/login.go
+++ b/models/login_source.go
@@ -24,13 +24,9 @@ import (
 	"github.com/gogits/gogs/modules/log"
 )
 
-var (
-	ErrAuthenticationUserUsed = errors.New("Authentication has been used by some users")
-)
-
 type LoginType int
 
-// Note: new type must be added at the end of list to maintain compatibility.
+// Note: new type must append to the end of list to maintain compatibility.
 const (
 	LOGIN_NOTYPE LoginType = iota
 	LOGIN_PLAIN            // 1
@@ -105,6 +101,7 @@ func (cfg *PAMConfig) ToDB() ([]byte, error) {
 	return json.Marshal(cfg)
 }
 
+// LoginSource represents an external way for authorizing users.
 type LoginSource struct {
 	ID        int64 `xorm:"pk autoincr"`
 	Type      LoginType
@@ -222,13 +219,6 @@ func (source *LoginSource) SMTP() *SMTPConfig {
 func (source *LoginSource) PAM() *PAMConfig {
 	return source.Cfg.(*PAMConfig)
 }
-
-// CountLoginSources returns number of login sources.
-func CountLoginSources() int64 {
-	count, _ := x.Count(new(LoginSource))
-	return count
-}
-
 func CreateLoginSource(source *LoginSource) error {
 	has, err := x.Get(&LoginSource{Name: source.Name})
 	if err != nil {
@@ -268,12 +258,18 @@ func DeleteSource(source *LoginSource) error {
 	if err != nil {
 		return err
 	} else if count > 0 {
-		return ErrAuthenticationUserUsed
+		return ErrLoginSourceInUse{source.ID}
 	}
 	_, err = x.Id(source.ID).Delete(new(LoginSource))
 	return err
 }
 
+// CountLoginSources returns number of login sources.
+func CountLoginSources() int64 {
+	count, _ := x.Count(new(LoginSource))
+	return count
+}
+
 // .____     ________      _____ __________
 // |    |    \______ \    /  _  \\______   \
 // |    |     |    |  \  /  /_\  \|     ___/
@@ -281,25 +277,35 @@ func DeleteSource(source *LoginSource) error {
 // |_______ \/_______  /\____|__  /____|
 //         \/        \/         \/
 
-// LoginUserLDAPSource queries if loginName/passwd can login against the LDAP directory pool,
+func composeFullName(firstname, surname, username string) string {
+	switch {
+	case len(firstname) == 0 && len(surname) == 0:
+		return username
+	case len(firstname) == 0:
+		return surname
+	case len(surname) == 0:
+		return firstname
+	default:
+		return firstname + " " + surname
+	}
+}
+
+// LoginViaLDAP queries if login/password is valid against the LDAP directory pool,
 // and create a local user if success when enabled.
-// It returns the same LoginUserPlain semantic.
-func LoginUserLDAPSource(u *User, loginName, passwd string, source *LoginSource, autoRegister bool) (*User, error) {
-	cfg := source.Cfg.(*LDAPConfig)
-	directBind := (source.Type == LOGIN_DLDAP)
-	username, fn, sn, mail, isAdmin, logged := cfg.SearchEntry(loginName, passwd, directBind)
-	if !logged {
+func LoginViaLDAP(user *User, login, passowrd string, source *LoginSource, autoRegister bool) (*User, error) {
+	username, fn, sn, mail, isAdmin, succeed := source.Cfg.(*LDAPConfig).SearchEntry(login, passowrd, source.Type == LOGIN_DLDAP)
+	if !succeed {
 		// User not in LDAP, do nothing
-		return nil, ErrUserNotExist{0, loginName}
+		return nil, ErrUserNotExist{0, login}
 	}
 
 	if !autoRegister {
-		return u, nil
+		return user, nil
 	}
 
 	// Fallback.
 	if len(username) == 0 {
-		username = loginName
+		username = login
 	}
 	// Validate username make sure it satisfies requirement.
 	if binding.AlphaDashDotPattern.MatchString(username) {
@@ -310,31 +316,18 @@ func LoginUserLDAPSource(u *User, loginName, passwd string, source *LoginSource,
 		mail = fmt.Sprintf("%s@localhost", username)
 	}
 
-	u = &User{
+	user = &User{
 		LowerName:   strings.ToLower(username),
 		Name:        username,
 		FullName:    composeFullName(fn, sn, username),
+		Email:       mail,
 		LoginType:   source.Type,
 		LoginSource: source.ID,
-		LoginName:   loginName,
-		Email:       mail,
-		IsAdmin:     isAdmin,
+		LoginName:   login,
 		IsActive:    true,
+		IsAdmin:     isAdmin,
 	}
-	return u, CreateUser(u)
-}
-
-func composeFullName(firstname, surname, username string) string {
-	switch {
-	case len(firstname) == 0 && len(surname) == 0:
-		return username
-	case len(firstname) == 0:
-		return surname
-	case len(surname) == 0:
-		return firstname
-	default:
-		return firstname + " " + surname
-	}
+	return user, CreateUser(user)
 }
 
 //   _________   __________________________
@@ -344,25 +337,21 @@ func composeFullName(firstname, surname, username string) string {
 // /_______  /\____|__  /____|    |____|
 //         \/         \/
 
-type loginAuth struct {
+type smtpLoginAuth struct {
 	username, password string
 }
 
-func LoginAuth(username, password string) smtp.Auth {
-	return &loginAuth{username, password}
+func (auth *smtpLoginAuth) Start(server *smtp.ServerInfo) (string, []byte, error) {
+	return "LOGIN", []byte(auth.username), nil
 }
 
-func (a *loginAuth) Start(server *smtp.ServerInfo) (string, []byte, error) {
-	return "LOGIN", []byte(a.username), nil
-}
-
-func (a *loginAuth) Next(fromServer []byte, more bool) ([]byte, error) {
+func (auth *smtpLoginAuth) Next(fromServer []byte, more bool) ([]byte, error) {
 	if more {
 		switch string(fromServer) {
 		case "Username:":
-			return []byte(a.username), nil
+			return []byte(auth.username), nil
 		case "Password:":
-			return []byte(a.password), nil
+			return []byte(auth.password), nil
 		}
 	}
 	return nil, nil
@@ -408,25 +397,24 @@ func SMTPAuth(a smtp.Auth, cfg *SMTPConfig) error {
 	return ErrUnsupportedLoginType
 }
 
-// Query if name/passwd can login against the LDAP directory pool
-// Create a local user if success
-// Return the same LoginUserPlain semantic
-func LoginUserSMTPSource(u *User, name, passwd string, sourceID int64, cfg *SMTPConfig, autoRegister bool) (*User, error) {
+// LoginViaSMTP queries if login/password is valid against the SMTP,
+// and create a local user if success when enabled.
+func LoginViaSMTP(user *User, login, password string, sourceID int64, cfg *SMTPConfig, autoRegister bool) (*User, error) {
 	// Verify allowed domains.
 	if len(cfg.AllowedDomains) > 0 {
-		idx := strings.Index(name, "@")
+		idx := strings.Index(login, "@")
 		if idx == -1 {
-			return nil, ErrUserNotExist{0, name}
-		} else if !com.IsSliceContainsStr(strings.Split(cfg.AllowedDomains, ","), name[idx+1:]) {
-			return nil, ErrUserNotExist{0, name}
+			return nil, ErrUserNotExist{0, login}
+		} else if !com.IsSliceContainsStr(strings.Split(cfg.AllowedDomains, ","), login[idx+1:]) {
+			return nil, ErrUserNotExist{0, login}
 		}
 	}
 
 	var auth smtp.Auth
 	if cfg.Auth == SMTP_PLAIN {
-		auth = smtp.PlainAuth("", name, passwd, cfg.Host)
+		auth = smtp.PlainAuth("", login, password, cfg.Host)
 	} else if cfg.Auth == SMTP_LOGIN {
-		auth = LoginAuth(name, passwd)
+		auth = &smtpLoginAuth{login, password}
 	} else {
 		return nil, errors.New("Unsupported SMTP auth type")
 	}
@@ -437,33 +425,32 @@ func LoginUserSMTPSource(u *User, name, passwd string, sourceID int64, cfg *SMTP
 		tperr, ok := err.(*textproto.Error)
 		if (ok && tperr.Code == 535) ||
 			strings.Contains(err.Error(), "Username and Password not accepted") {
-			return nil, ErrUserNotExist{0, name}
+			return nil, ErrUserNotExist{0, login}
 		}
 		return nil, err
 	}
 
 	if !autoRegister {
-		return u, nil
+		return user, nil
 	}
 
-	var loginName = name
-	idx := strings.Index(name, "@")
+	username := login
+	idx := strings.Index(login, "@")
 	if idx > -1 {
-		loginName = name[:idx]
+		username = login[:idx]
 	}
-	// fake a local user creation
-	u = &User{
-		LowerName:   strings.ToLower(loginName),
-		Name:        strings.ToLower(loginName),
+
+	user = &User{
+		LowerName:   strings.ToLower(username),
+		Name:        strings.ToLower(username),
+		Email:       login,
+		Passwd:      password,
 		LoginType:   LOGIN_SMTP,
 		LoginSource: sourceID,
-		LoginName:   name,
+		LoginName:   login,
 		IsActive:    true,
-		Passwd:      passwd,
-		Email:       name,
 	}
-	err := CreateUser(u)
-	return u, err
+	return user, CreateUser(user)
 }
 
 // __________  _____      _____
@@ -473,101 +460,99 @@ func LoginUserSMTPSource(u *User, name, passwd string, sourceID int64, cfg *SMTP
 //  |____|  \____|__  /\____|__  /
 //                  \/         \/
 
-// Query if name/passwd can login against PAM
-// Create a local user if success
-// Return the same LoginUserPlain semantic
-func LoginUserPAMSource(u *User, name, passwd string, sourceID int64, cfg *PAMConfig, autoRegister bool) (*User, error) {
-	if err := pam.PAMAuth(cfg.ServiceName, name, passwd); err != nil {
+// LoginViaPAM queries if login/password is valid against the PAM,
+// and create a local user if success when enabled.
+func LoginViaPAM(user *User, login, password string, sourceID int64, cfg *PAMConfig, autoRegister bool) (*User, error) {
+	if err := pam.PAMAuth(cfg.ServiceName, login, password); err != nil {
 		if strings.Contains(err.Error(), "Authentication failure") {
-			return nil, ErrUserNotExist{0, name}
+			return nil, ErrUserNotExist{0, login}
 		}
 		return nil, err
 	}
 
 	if !autoRegister {
-		return u, nil
+		return user, nil
 	}
 
-	// fake a local user creation
-	u = &User{
-		LowerName:   strings.ToLower(name),
-		Name:        name,
+	user = &User{
+		LowerName:   strings.ToLower(login),
+		Name:        login,
+		Email:       login,
+		Passwd:      password,
 		LoginType:   LOGIN_PAM,
 		LoginSource: sourceID,
-		LoginName:   name,
+		LoginName:   login,
 		IsActive:    true,
-		Passwd:      passwd,
-		Email:       name,
 	}
-	return u, CreateUser(u)
+	return user, CreateUser(user)
 }
 
-func ExternalUserLogin(u *User, name, passwd string, source *LoginSource, autoRegister bool) (*User, error) {
+func ExternalUserLogin(user *User, login, password string, source *LoginSource, autoRegister bool) (*User, error) {
 	if !source.IsActived {
 		return nil, ErrLoginSourceNotActived
 	}
 
 	switch source.Type {
 	case LOGIN_LDAP, LOGIN_DLDAP:
-		return LoginUserLDAPSource(u, name, passwd, source, autoRegister)
+		return LoginViaLDAP(user, login, password, source, autoRegister)
 	case LOGIN_SMTP:
-		return LoginUserSMTPSource(u, name, passwd, source.ID, source.Cfg.(*SMTPConfig), autoRegister)
+		return LoginViaSMTP(user, login, password, source.ID, source.Cfg.(*SMTPConfig), autoRegister)
 	case LOGIN_PAM:
-		return LoginUserPAMSource(u, name, passwd, source.ID, source.Cfg.(*PAMConfig), autoRegister)
+		return LoginViaPAM(user, login, password, source.ID, source.Cfg.(*PAMConfig), autoRegister)
 	}
 
 	return nil, ErrUnsupportedLoginType
 }
 
 // UserSignIn validates user name and password.
-func UserSignIn(uname, passwd string) (*User, error) {
-	var u *User
-	if strings.Contains(uname, "@") {
-		u = &User{Email: strings.ToLower(uname)}
+func UserSignIn(username, passowrd string) (*User, error) {
+	var user *User
+	if strings.Contains(username, "@") {
+		user = &User{Email: strings.ToLower(username)}
 	} else {
-		u = &User{LowerName: strings.ToLower(uname)}
+		user = &User{LowerName: strings.ToLower(username)}
 	}
 
-	userExists, err := x.Get(u)
+	hasUser, err := x.Get(user)
 	if err != nil {
 		return nil, err
 	}
 
-	if userExists {
-		switch u.LoginType {
+	if hasUser {
+		switch user.LoginType {
 		case LOGIN_NOTYPE, LOGIN_PLAIN:
-			if u.ValidatePassword(passwd) {
-				return u, nil
+			if user.ValidatePassword(passowrd) {
+				return user, nil
 			}
 
-			return nil, ErrUserNotExist{u.ID, u.Name}
+			return nil, ErrUserNotExist{user.ID, user.Name}
 
 		default:
 			var source LoginSource
-			hasSource, err := x.Id(u.LoginSource).Get(&source)
+			hasSource, err := x.Id(user.LoginSource).Get(&source)
 			if err != nil {
 				return nil, err
 			} else if !hasSource {
-				return nil, ErrLoginSourceNotExist{u.LoginSource}
+				return nil, ErrLoginSourceNotExist{user.LoginSource}
 			}
 
-			return ExternalUserLogin(u, u.LoginName, passwd, &source, false)
+			return ExternalUserLogin(user, user.LoginName, passowrd, &source, false)
 		}
 	}
 
-	var sources []LoginSource
+	sources := make([]*LoginSource, 3)
 	if err = x.UseBool().Find(&sources, &LoginSource{IsActived: true}); err != nil {
 		return nil, err
 	}
 
 	for _, source := range sources {
-		u, err := ExternalUserLogin(nil, uname, passwd, &source, true)
+		authUser, err := ExternalUserLogin(nil, username, passowrd, source, true)
 		if err == nil {
-			return u, nil
+			return authUser, nil
 		}
 
-		log.Warn("Failed to login '%s' via '%s': %v", uname, source.Name, err)
+		log.Warn("Failed to login '%s' via '%s': %v", username, source.Name, err)
 	}
 
-	return nil, ErrUserNotExist{u.ID, u.Name}
+	return nil, ErrUserNotExist{user.ID, user.Name}
 }
diff --git a/routers/admin/auths.go b/routers/admin/auths.go
index 8d3304eacf..df2512aa06 100644
--- a/routers/admin/auths.go
+++ b/routers/admin/auths.go
@@ -242,10 +242,9 @@ func DeleteAuthSource(ctx *context.Context) {
 	}
 
 	if err = models.DeleteSource(source); err != nil {
-		switch err {
-		case models.ErrAuthenticationUserUsed:
+		if models.IsErrLoginSourceInUse(err) {
 			ctx.Flash.Error(ctx.Tr("admin.auths.still_in_used"))
-		default:
+		} else {
 			ctx.Flash.Error(fmt.Sprintf("DeleteSource: %v", err))
 		}
 		ctx.JSON(200, map[string]interface{}{
diff --git a/templates/.VERSION b/templates/.VERSION
index 93b1bf2a43..60a098b413 100644
--- a/templates/.VERSION
+++ b/templates/.VERSION
@@ -1 +1 @@
-0.9.96.0830
\ No newline at end of file
+0.9.97.0830
\ No newline at end of file