From 573305f3d3ac55a79639dcb4cc55694ad7a914a5 Mon Sep 17 00:00:00 2001 From: Adam Strzelecki Date: Tue, 1 Dec 2015 14:49:49 +0100 Subject: [PATCH] LDAP: Optional user name attribute specification Consider following LDAP search query example: (&(objectClass=Person)(|(uid=%s)(mail=%s))) Right now on first login attempt Gogs will use the text supplied on login form as the newly created user name. In example query above the text matches against both e-mail or user name. So if user puts the e-mail then the new Gogs user name will be e-mail which may be undesired. Using optional user name attribute setting we can explicitly say we want Gogs user name to be certain LDAP attribute eg. `uid`, so even user will use e-mail to login 1st time, the new account will receive correct user name. --- conf/locale/locale_en-US.ini | 2 ++ models/login.go | 28 +++++++++++++++---- modules/auth/auth_form.go | 45 +++++++++++++++--------------- modules/auth/ldap/ldap.go | 50 ++++++++++++++++++---------------- routers/admin/auths.go | 31 +++++++++++---------- templates/admin/auth/edit.tmpl | 4 +++ templates/admin/auth/new.tmpl | 4 +++ 7 files changed, 97 insertions(+), 67 deletions(-) diff --git a/conf/locale/locale_en-US.ini b/conf/locale/locale_en-US.ini index e4272328b..763e3e5df 100644 --- a/conf/locale/locale_en-US.ini +++ b/conf/locale/locale_en-US.ini @@ -878,6 +878,8 @@ auths.bind_password = Bind Password auths.bind_password_helper = Warning: This password is stored in plain text. Do not use a high privileged account. auths.user_base = User Search Base auths.user_dn = User DN +auths.attribute_username = Username attribute +auths.attribute_username_placeholder = Leave empty to use sign-in form field value for user name. auths.attribute_name = First name attribute auths.attribute_surname = Surname attribute auths.attribute_mail = E-mail attribute diff --git a/models/login.go b/models/login.go index 6fde7457a..1ec5309db 100644 --- a/models/login.go +++ b/models/login.go @@ -225,16 +225,16 @@ func DeleteSource(source *LoginSource) error { // |_______ \/_______ /\____|__ /____| // \/ \/ \/ -// LoginUserLDAPSource queries if name/passwd can login against the LDAP directory pool, +// LoginUserLDAPSource queries if loginName/passwd can login against the LDAP directory pool, // and create a local user if success when enabled. // It returns the same LoginUserPlain semantic. -func LoginUserLDAPSource(u *User, name, passwd string, source *LoginSource, autoRegister bool) (*User, error) { +func LoginUserLDAPSource(u *User, loginName, passwd string, source *LoginSource, autoRegister bool) (*User, error) { cfg := source.Cfg.(*LDAPConfig) directBind := (source.Type == DLDAP) - fn, sn, mail, admin, logged := cfg.SearchEntry(name, passwd, directBind) + name, fn, sn, mail, admin, logged := cfg.SearchEntry(loginName, passwd, directBind) if !logged { // User not in LDAP, do nothing - return nil, ErrUserNotExist{0, name} + return nil, ErrUserNotExist{0, loginName} } if !autoRegister { @@ -242,6 +242,9 @@ func LoginUserLDAPSource(u *User, name, passwd string, source *LoginSource, auto } // Fallback. + if len(name) == 0 { + name = loginName + } if len(mail) == 0 { mail = fmt.Sprintf("%s@localhost", name) } @@ -249,10 +252,10 @@ func LoginUserLDAPSource(u *User, name, passwd string, source *LoginSource, auto u = &User{ LowerName: strings.ToLower(name), Name: name, - FullName: strings.TrimSpace(fn + " " + sn), + FullName: composeFullName(fn, sn, name), LoginType: source.Type, LoginSource: source.ID, - LoginName: name, + LoginName: loginName, Email: mail, IsAdmin: admin, IsActive: true, @@ -260,6 +263,19 @@ func LoginUserLDAPSource(u *User, name, passwd string, source *LoginSource, auto return u, CreateUser(u) } +func composeFullName(firstName, surename, 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 + default: + return firstName + " " + surename + } +} + // _________ __________________________ // / _____/ / \__ ___/\______ \ // \_____ \ / \ / \| | | ___/ diff --git a/modules/auth/auth_form.go b/modules/auth/auth_form.go index 6f356344d..1604792a6 100644 --- a/modules/auth/auth_form.go +++ b/modules/auth/auth_form.go @@ -10,28 +10,29 @@ import ( ) type AuthenticationForm struct { - ID int64 - Type int `binding:"Range(2,5)"` - Name string `binding:"Required;MaxSize(30)"` - Host string - Port int - BindDN string - BindPassword string - UserBase string - UserDN string `form:"user_dn"` - AttributeName string - AttributeSurname string - AttributeMail string - Filter string - AdminFilter string - IsActive bool - SMTPAuth string - SMTPHost string - SMTPPort int - AllowedDomains string - TLS bool - SkipVerify bool - PAMServiceName string `form:"pam_service_name"` + ID int64 + Type int `binding:"Range(2,5)"` + Name string `binding:"Required;MaxSize(30)"` + Host string + Port int + BindDN string + BindPassword string + UserBase string + UserDN string `form:"user_dn"` + AttributeUsername string + AttributeName string + AttributeSurname string + AttributeMail string + Filter string + AdminFilter string + IsActive bool + SMTPAuth string + SMTPHost string + SMTPPort int + AllowedDomains string + TLS bool + SkipVerify bool + PAMServiceName string `form:"pam_service_name"` } func (f *AuthenticationForm) Validate(ctx *macaron.Context, errs binding.Errors) binding.Errors { diff --git a/modules/auth/ldap/ldap.go b/modules/auth/ldap/ldap.go index 29a2a93b4..75795cf6d 100644 --- a/modules/auth/ldap/ldap.go +++ b/modules/auth/ldap/ldap.go @@ -18,21 +18,22 @@ import ( // Basic LDAP authentication service type Source struct { - Name string // canonical name (ie. corporate.ad) - Host string // LDAP host - Port int // port number - UseSSL bool // Use SSL - SkipVerify bool - BindDN string // DN to bind with - BindPassword string // Bind DN password - UserBase string // Base search path for users - UserDN string // Template for the DN of the user for simple auth - AttributeName string // First name attribute - AttributeSurname string // Surname attribute - AttributeMail string // E-mail attribute - Filter string // Query filter to validate entry - AdminFilter string // Query filter to check if user is admin - Enabled bool // if this source is disabled + Name string // canonical name (ie. corporate.ad) + Host string // LDAP host + Port int // port number + UseSSL bool // Use SSL + SkipVerify bool + BindDN string // DN to bind with + BindPassword string // Bind DN password + UserBase string // Base search path for users + UserDN string // Template for the DN of the user for simple auth + AttributeUsername string // Username attribute + AttributeName string // First name attribute + AttributeSurname string // Surname attribute + AttributeMail string // E-mail attribute + Filter string // Query filter to validate entry + AdminFilter string // Query filter to check if user is admin + Enabled bool // if this source is disabled } func (ls *Source) sanitizedUserQuery(username string) (string, bool) { @@ -109,7 +110,7 @@ func (ls *Source) FindUserDN(name string) (string, bool) { } // searchEntry : search an LDAP source if an entry (name, passwd) is valid and in the specific filter -func (ls *Source) SearchEntry(name, passwd string, directBind bool) (string, string, string, bool, bool) { +func (ls *Source) SearchEntry(name, passwd string, directBind bool) (string, string, string, string, bool, bool) { var userDN string if directBind { log.Trace("LDAP will bind directly via UserDN template: %s", ls.UserDN) @@ -117,7 +118,7 @@ func (ls *Source) SearchEntry(name, passwd string, directBind bool) (string, str var ok bool userDN, ok = ls.sanitizedUserDN(name) if !ok { - return "", "", "", false, false + return "", "", "", "", false, false } } else { log.Trace("LDAP will use BindDN.") @@ -125,7 +126,7 @@ func (ls *Source) SearchEntry(name, passwd string, directBind bool) (string, str var found bool userDN, found = ls.FindUserDN(name) if !found { - return "", "", "", false, false + return "", "", "", "", false, false } } @@ -133,7 +134,7 @@ func (ls *Source) SearchEntry(name, passwd string, directBind bool) (string, str if err != nil { log.Error(4, "LDAP Connect error (%s): %v", ls.Host, err) ls.Enabled = false - return "", "", "", false, false + return "", "", "", "", false, false } defer l.Close() @@ -141,13 +142,13 @@ func (ls *Source) SearchEntry(name, passwd string, directBind bool) (string, str err = l.Bind(userDN, passwd) if err != nil { log.Debug("LDAP auth. failed for %s, reason: %v", userDN, err) - return "", "", "", false, false + return "", "", "", "", false, false } log.Trace("Bound successfully with userDN: %s", userDN) userFilter, ok := ls.sanitizedUserQuery(name) if !ok { - return "", "", "", false, false + return "", "", "", "", false, false } search := ldap.NewSearchRequest( @@ -158,7 +159,7 @@ func (ls *Source) SearchEntry(name, passwd string, directBind bool) (string, str sr, err := l.Search(search) if err != nil { log.Error(4, "LDAP Search failed unexpectedly! (%v)", err) - return "", "", "", false, false + return "", "", "", "", false, false } else if len(sr.Entries) < 1 { if directBind { log.Error(4, "User filter inhibited user login.") @@ -166,9 +167,10 @@ func (ls *Source) SearchEntry(name, passwd string, directBind bool) (string, str log.Error(4, "LDAP Search failed unexpectedly! (0 entries)") } - return "", "", "", false, false + 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) @@ -190,7 +192,7 @@ func (ls *Source) SearchEntry(name, passwd string, directBind bool) (string, str } } - return name_attr, sn_attr, mail_attr, admin_attr, true + return username_attr, name_attr, sn_attr, mail_attr, admin_attr, true } func ldapDial(ls *Source) (*ldap.Conn, error) { diff --git a/routers/admin/auths.go b/routers/admin/auths.go index e264f7a8b..baa5efe3d 100644 --- a/routers/admin/auths.go +++ b/routers/admin/auths.go @@ -68,21 +68,22 @@ func NewAuthSource(ctx *middleware.Context) { func parseLDAPConfig(form auth.AuthenticationForm) *models.LDAPConfig { return &models.LDAPConfig{ Source: &ldap.Source{ - Name: form.Name, - Host: form.Host, - Port: form.Port, - UseSSL: form.TLS, - SkipVerify: form.SkipVerify, - BindDN: form.BindDN, - UserDN: form.UserDN, - BindPassword: form.BindPassword, - UserBase: form.UserBase, - AttributeName: form.AttributeName, - AttributeSurname: form.AttributeSurname, - AttributeMail: form.AttributeMail, - Filter: form.Filter, - AdminFilter: form.AdminFilter, - Enabled: true, + Name: form.Name, + Host: form.Host, + Port: form.Port, + UseSSL: form.TLS, + SkipVerify: form.SkipVerify, + BindDN: form.BindDN, + UserDN: form.UserDN, + BindPassword: form.BindPassword, + UserBase: form.UserBase, + AttributeUsername: form.AttributeUsername, + AttributeName: form.AttributeName, + AttributeSurname: form.AttributeSurname, + AttributeMail: form.AttributeMail, + Filter: form.Filter, + AdminFilter: form.AdminFilter, + Enabled: true, }, } } diff --git a/templates/admin/auth/edit.tmpl b/templates/admin/auth/edit.tmpl index 7423081bd..e1d1d7660 100644 --- a/templates/admin/auth/edit.tmpl +++ b/templates/admin/auth/edit.tmpl @@ -63,6 +63,10 @@ +
+ + +
diff --git a/templates/admin/auth/new.tmpl b/templates/admin/auth/new.tmpl index d342b2df1..448a4ab64 100644 --- a/templates/admin/auth/new.tmpl +++ b/templates/admin/auth/new.tmpl @@ -66,6 +66,10 @@
+
+ + +