From a752f0905551a0d8fd6edb893f748cb9490a28dc Mon Sep 17 00:00:00 2001 From: Unknwon Date: Tue, 12 Jul 2016 07:07:57 +0800 Subject: [PATCH] #2709 validate username attribute fetched from LDAP --- cmd/web.go | 2 +- glide.lock | 4 ++-- models/login.go | 38 ++++++++++++++++++++++---------------- modules/auth/ldap/ldap.go | 14 +++++++------- 4 files changed, 32 insertions(+), 26 deletions(-) diff --git a/cmd/web.go b/cmd/web.go index 24e99f6a35..8465f1e4ff 100644 --- a/cmd/web.go +++ b/cmd/web.go @@ -79,7 +79,7 @@ func checkVersion() { // Check dependency version. checkers := []VerChecker{ {"github.com/go-xorm/xorm", func() string { return xorm.Version }, "0.5.5.0711"}, - {"github.com/go-macaron/binding", binding.Version, "0.2.1"}, + {"github.com/go-macaron/binding", binding.Version, "0.3.2"}, {"github.com/go-macaron/cache", cache.Version, "0.1.2"}, {"github.com/go-macaron/csrf", csrf.Version, "0.1.0"}, {"github.com/go-macaron/i18n", i18n.Version, "0.3.0"}, diff --git a/glide.lock b/glide.lock index 15d4ca3471..25087f54b2 100644 --- a/glide.lock +++ b/glide.lock @@ -8,7 +8,7 @@ imports: - name: github.com/codegangsta/cli version: 1efa31f08b9333f1bd4882d61f9d668a70cd902e - name: github.com/go-macaron/binding - version: bd00823a7e9aa00cb3b1738fde244573ba7cce2c + version: 9440f336b443056c90d7d448a0a55ad8c7599880 - name: github.com/go-macaron/cache version: 56173531277692bc2925924d51fda1cd0a6b8178 subpackages: @@ -43,7 +43,7 @@ imports: - name: github.com/gogits/git-module version: db93fa550116997bbe0b62baa653b8e6f4135258 - name: github.com/gogits/go-gogs-client - version: 872cf281aac97429da02b6cdea942c9388eb65fb + version: ee68cd9eefff11615f336e9965762f6736eeecc8 - name: github.com/issue9/identicon version: d36b54562f4cf70c83653e13dc95c220c79ef521 - name: github.com/jaytaylor/html2text diff --git a/models/login.go b/models/login.go index 22edc25b55..84f17d18e5 100644 --- a/models/login.go +++ b/models/login.go @@ -15,6 +15,7 @@ import ( "time" "github.com/Unknwon/com" + "github.com/go-macaron/binding" "github.com/go-xorm/core" "github.com/go-xorm/xorm" @@ -280,7 +281,7 @@ func DeleteSource(source *LoginSource) error { func LoginUserLDAPSource(u *User, loginName, passwd string, source *LoginSource, autoRegister bool) (*User, error) { cfg := source.Cfg.(*LDAPConfig) directBind := (source.Type == LOGIN_DLDAP) - name, fn, sn, mail, admin, logged := cfg.SearchEntry(loginName, passwd, directBind) + username, fn, sn, mail, isAdmin, logged := cfg.SearchEntry(loginName, passwd, directBind) if !logged { // User not in LDAP, do nothing return nil, ErrUserNotExist{0, loginName} @@ -291,37 +292,42 @@ func LoginUserLDAPSource(u *User, loginName, passwd string, source *LoginSource, } // Fallback. - if len(name) == 0 { - name = loginName + if len(username) == 0 { + username = loginName } + // Validate username make sure it satisfies requirement. + if !binding.AlphaDashDotPattern.MatchString(username) { + return nil, fmt.Errorf("Invalid pattern for attribute 'username' [%s]: must be valid alpha or numeric or dash(-_) or dot characters", username) + } + if len(mail) == 0 { - mail = fmt.Sprintf("%s@localhost", name) + mail = fmt.Sprintf("%s@localhost", username) } u = &User{ - LowerName: strings.ToLower(name), - Name: name, - FullName: composeFullName(fn, sn, name), + LowerName: strings.ToLower(username), + Name: username, + FullName: composeFullName(fn, sn, username), LoginType: source.Type, LoginSource: source.ID, LoginName: loginName, Email: mail, - IsAdmin: admin, + IsAdmin: isAdmin, IsActive: true, } return u, CreateUser(u) } -func composeFullName(firstName, surename, userName string) string { +func composeFullName(firstname, surname, username string) string { switch { - case len(firstName) == 0 && len(surename) == 0: - return userName - case len(firstName) == 0: - return surename - case len(surename) == 0: - return firstName + case len(firstname) == 0 && len(surname) == 0: + return username + case len(firstname) == 0: + return surname + case len(surname) == 0: + return firstname default: - return firstName + " " + surename + return firstname + " " + surname } } diff --git a/modules/auth/ldap/ldap.go b/modules/auth/ldap/ldap.go index 598929d9e5..55364bfcce 100644 --- a/modules/auth/ldap/ldap.go +++ b/modules/auth/ldap/ldap.go @@ -210,12 +210,12 @@ func (ls *Source) SearchEntry(name, passwd string, directBind bool) (string, str return "", "", "", "", false, false } - username_attr := sr.Entries[0].GetAttributeValue(ls.AttributeUsername) - name_attr := sr.Entries[0].GetAttributeValue(ls.AttributeName) - sn_attr := sr.Entries[0].GetAttributeValue(ls.AttributeSurname) - mail_attr := sr.Entries[0].GetAttributeValue(ls.AttributeMail) + username := sr.Entries[0].GetAttributeValue(ls.AttributeUsername) + firstname := sr.Entries[0].GetAttributeValue(ls.AttributeName) + surname := sr.Entries[0].GetAttributeValue(ls.AttributeSurname) + mail := sr.Entries[0].GetAttributeValue(ls.AttributeMail) - admin_attr := false + isAdmin := false if len(ls.AdminFilter) > 0 { log.Trace("Checking admin with filter %s and base %s", ls.AdminFilter, userDN) search = ldap.NewSearchRequest( @@ -229,7 +229,7 @@ func (ls *Source) SearchEntry(name, passwd string, directBind bool) (string, str } else if len(sr.Entries) < 1 { log.Error(4, "LDAP Admin Search failed") } else { - admin_attr = true + isAdmin = true } } @@ -241,5 +241,5 @@ func (ls *Source) SearchEntry(name, passwd string, directBind bool) (string, str } } - return username_attr, name_attr, sn_attr, mail_attr, admin_attr, true + return username, firstname, surname, mail, isAdmin, true }