From 152e715999bfec9859423034aec3df90442961f1 Mon Sep 17 00:00:00 2001 From: Unknwon Date: Wed, 31 Aug 2016 01:22:41 -0700 Subject: [PATCH] models/login_source: code improvement --- README.md | 2 +- gogs.go | 2 +- models/error.go | 13 ++ models/{login.go => login_source.go} | 211 +++++++++++++-------------- routers/admin/auths.go | 5 +- templates/.VERSION | 2 +- 6 files changed, 116 insertions(+), 119 deletions(-) rename models/{login.go => login_source.go} (73%) diff --git a/README.md b/README.md index cab3c59d4..60aa87527 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 be61885dc..04689d454 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 065857e00..182a944a6 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 similarity index 73% rename from models/login.go rename to models/login_source.go index bef1617dd..82186fb71 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 8d3304eac..df2512aa0 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 93b1bf2a4..60a098b41 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