Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Account autocreation from LDAP after reverse proxy authentication #12960

Closed
wants to merge 9 commits into from
16 changes: 8 additions & 8 deletions models/login_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -456,8 +456,8 @@ func composeFullName(firstname, surname, username string) string {

// LoginViaLDAP queries if login/password is valid against the LDAP directory pool,
// and create a local user if success when enabled.
func LoginViaLDAP(user *User, login, password string, source *LoginSource) (*User, error) {
sr := source.Cfg.(*LDAPConfig).SearchEntry(login, password, source.Type == LoginDLDAP)
func LoginViaLDAP(user *User, login, password string, alreadyAuthenticated bool, source *LoginSource) (*User, error) {
sr := source.Cfg.(*LDAPConfig).SearchEntry(login, password, source.Type == LoginDLDAP, alreadyAuthenticated)
if sr == nil {
// User not in LDAP, do nothing
return nil, ErrUserNotExist{0, login, 0}
Expand Down Expand Up @@ -701,15 +701,15 @@ func LoginViaPAM(user *User, login, password string, sourceID int64, cfg *PAMCon
}

// ExternalUserLogin attempts a login using external source types.
func ExternalUserLogin(user *User, login, password string, source *LoginSource) (*User, error) {
func ExternalUserLogin(user *User, login, password string, alreadyAuthenticated bool, source *LoginSource) (*User, error) {
if !source.IsActived {
return nil, ErrLoginSourceNotActived
}

var err error
switch source.Type {
case LoginLDAP, LoginDLDAP:
user, err = LoginViaLDAP(user, login, password, source)
user, err = LoginViaLDAP(user, login, password, alreadyAuthenticated, source)
case LoginSMTP:
user, err = LoginViaSMTP(user, login, password, source.ID, source.Cfg.(*SMTPConfig))
case LoginPAM:
Expand All @@ -731,8 +731,8 @@ func ExternalUserLogin(user *User, login, password string, source *LoginSource)
return user, nil
}

// UserSignIn validates user name and password.
func UserSignIn(username, password string) (*User, error) {
// UserSignIn validates user name and password. Password verification in LDAP skipped if already authenticated.
func UserSignIn(username, password string, alreadyAuthenticated bool) (*User, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess one thing I don't really like about this is that the user has already signed in - we're just abusing this function to lookup the user in the external source - and only in one place (at least so far). Maybe we should be doing a ExternalUserLookup or something like that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Further if the user is in the db then this alreadyAuthenticated path fails.

Copy link
Contributor Author

@pboguslawski pboguslawski Oct 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Further if the user is in the db then this alreadyAuthenticated path fails.

This mod works ok for us in scenario with LDAP user backend + reverse proxy auth using HTTP header. No problems with initial account creation nor logging in/updating existing accounts from LDAP. If you see any bug please describe how to reproduce.

Maybe we should be doing a ExternalUserLookup or something like that.

This mod was created for our needs with minimal changes in upstream code. Feel free to enhance it.

var user *User
if strings.Contains(username, "@") {
user = &User{Email: strings.ToLower(strings.TrimSpace(username))}
Expand Down Expand Up @@ -793,7 +793,7 @@ func UserSignIn(username, password string) (*User, error) {
return nil, ErrLoginSourceNotExist{user.LoginSource}
}

return ExternalUserLogin(user, user.LoginName, password, &source)
return ExternalUserLogin(user, user.LoginName, password, alreadyAuthenticated, &source)
}
}

Expand All @@ -807,7 +807,7 @@ func UserSignIn(username, password string) (*User, error) {
// don't try to authenticate against OAuth2 and SSPI sources here
continue
}
authUser, err := ExternalUserLogin(nil, username, password, source)
authUser, err := ExternalUserLogin(nil, username, password, alreadyAuthenticated, source)
if err == nil {
return authUser, nil
}
Expand Down
18 changes: 14 additions & 4 deletions modules/auth/ldap/ldap.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,12 +230,22 @@ func checkRestricted(l *ldap.Conn, ls *Source, userDN 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) *SearchResult {
func (ls *Source) SearchEntry(name, passwd string, directBind bool, alreadyAuthenticated bool) *SearchResult {
// See https://tools.ietf.org/search/rfc4513#section-5.1.2
if len(passwd) == 0 {
if len(passwd) == 0 && !alreadyAuthenticated {
log.Debug("Auth. failed for %s, password cannot be empty", name)
return nil
}
if directBind && alreadyAuthenticated {
log.Debug("Cannot bind using user %s credentials - user already authenticated. BindDN must be used.", name)
return nil
}

if !ls.AttributesInBind && alreadyAuthenticated {
log.Debug("Cannot get attributes using user %s credentials - user already authenticated; --attributes-in-bind must be used.", name)
return nil
}

Comment on lines +235 to +248
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if len(passwd) == 0 && !alreadyAuthenticated {
log.Debug("Auth. failed for %s, password cannot be empty", name)
return nil
}
if directBind && alreadyAuthenticated {
log.Debug("Cannot bind using user %s credentials - user already authenticated. BindDN must be used.", name)
return nil
}
if !ls.AttributesInBind && alreadyAuthenticated {
log.Debug("Cannot get attributes using user %s credentials - user already authenticated; --attributes-in-bind must be used.", name)
return nil
}
if alreadyAuthenticated {
if directBind {
log.Debug("Cannot bind pre-authenticated user %s. BindDN must be used.", name)
return nil
}
if !ls.AttributesInBind {
log.Debug("Cannot get attributes for pre-authenticated user %s without --attributes-in-bind.", name)
return nil
}
} else if len(passwd) == 0 {
log.Debug("Auth. failed for %s, password cannot be empty", name)
return nil
}

l, err := dial(ls)
if err != nil {
log.Error("LDAP Connect error, %s:%v", ls.Host, err)
Expand Down Expand Up @@ -393,8 +403,8 @@ func (ls *Source) SearchEntry(name, passwd string, directBind bool) *SearchResul
isRestricted = checkRestricted(l, ls, userDN)
}

if !directBind && ls.AttributesInBind {
// binds user (checking password) after looking-up attributes in BindDN context
if !directBind && ls.AttributesInBind && !alreadyAuthenticated {
// binds user (checking password) after looking-up attributes in BindDN context if not already authenticated
err = bindUser(l, userDN, passwd)
if err != nil {
return nil
Expand Down
2 changes: 1 addition & 1 deletion modules/auth/sso/basic.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func (b *Basic) VerifyAuthData(ctx *macaron.Context, sess session.Store) *models
}

if u == nil {
u, err = models.UserSignIn(uname, passwd)
u, err = models.UserSignIn(uname, passwd, false)
if err != nil {
if !models.IsErrUserNotExist(err) {
log.Error("UserSignIn: %v", err)
Expand Down
61 changes: 54 additions & 7 deletions modules/auth/sso/reverseproxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package sso

import (
"fmt"
"strings"

"code.gitea.io/gitea/models"
Expand Down Expand Up @@ -60,20 +61,66 @@ func (r *ReverseProxy) IsEnabled() bool {
// the revese proxy.
// If a username is available in the "setting.ReverseProxyAuthUser" header an existing
// user object is returned (populated with username or email found in header).
// Returns nil if header is empty.
// Returns nil if header is empty or internal API is being called.
func (r *ReverseProxy) VerifyAuthData(ctx *macaron.Context, sess session.Store) *models.User {

// Internal API should not use this auth method.
if isInternalPath(ctx) {
return nil
}

// Just return user if session is estabilshed already.
user := SessionUser(sess)
if user != nil {
return user
}

// If no session established, get username from header.
username := r.getUserName(ctx)
if len(username) == 0 {
return nil
}

user, err := models.GetUserByName(username)
if err != nil {
if models.IsErrUserNotExist(err) && r.isAutoRegisterAllowed() {
return r.newUser(ctx)
var err error

if r.isAutoRegisterAllowed() {
// Use auto registration from reverse proxy if ENABLE_REVERSE_PROXY_AUTO_REGISTRATION enabled.
if user, err = models.GetUserByName(username); err != nil {
if models.IsErrUserNotExist(err) && r.isAutoRegisterAllowed() {
if user = r.newUser(ctx); user == nil {
return nil
}
} else {
log.Error("GetUserByName: %v", err)
return nil
}
}
log.Error("GetUserByName: %v", err)
return nil
} else {
// Use auto registration from other backends if ENABLE_REVERSE_PROXY_AUTO_REGISTRATION not enabled.
if user, err = models.UserSignIn(username, "", true); err != nil {
if !models.IsErrUserNotExist(err) {
log.Error("UserSignIn: %v", err)
}
return nil
}
}

// Make sure requests to API paths and PWA resources do not create a new session.
if !isAPIPath(ctx) && !isAttachmentDownload(ctx) {

// Update last user login timestamp.
user.SetLastLogin()
if err = models.UpdateUserCols(user, false, "last_login_unix"); err != nil {
log.Error(fmt.Sprintf("VerifyAuthData: error updating user last login time [user: %d]", user.ID))
}

// Initialize new session. Will set lang and CSRF cookies.
handleSignIn(ctx, sess, user)

// Unfortunatelly we cannot do redirect here (would break git HTTP requests) to
// reload page with user locale so first page after login may be displayed in
// wrong language. Language handling in SSO mode should be reconsidered
// in future gitea versions.
}

return user
Expand Down
2 changes: 1 addition & 1 deletion routers/org/setting.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ func SettingsDelete(ctx *context.Context) {

org := ctx.Org.Organization
if ctx.Req.Method == "POST" {
if _, err := models.UserSignIn(ctx.User.Name, ctx.Query("password")); err != nil {
if _, err := models.UserSignIn(ctx.User.Name, ctx.Query("password"), false); err != nil {
if models.IsErrUserNotExist(err) {
ctx.RenderWithErr(ctx.Tr("form.enterred_invalid_password"), tplSettingsDelete, nil)
} else {
Expand Down
2 changes: 1 addition & 1 deletion routers/repo/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ func HTTP(ctx *context.Context) {

if authUser == nil {
// Check username and password
authUser, err = models.UserSignIn(authUsername, authPasswd)
authUser, err = models.UserSignIn(authUsername, authPasswd, false)
if err != nil {
if models.IsErrUserProhibitLogin(err) {
ctx.HandleText(http.StatusForbidden, "User is not permitted to login")
Expand Down
4 changes: 2 additions & 2 deletions routers/user/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ func SignInPost(ctx *context.Context, form auth.SignInForm) {
return
}

u, err := models.UserSignIn(form.UserName, form.Password)
u, err := models.UserSignIn(form.UserName, form.Password, false)
if err != nil {
if models.IsErrUserNotExist(err) {
ctx.RenderWithErr(ctx.Tr("form.username_password_incorrect"), tplSignIn, &form)
Expand Down Expand Up @@ -805,7 +805,7 @@ func LinkAccountPostSignIn(ctx *context.Context, signInForm auth.SignInForm) {
return
}

u, err := models.UserSignIn(signInForm.UserName, signInForm.Password)
u, err := models.UserSignIn(signInForm.UserName, signInForm.Password, false)
if err != nil {
if models.IsErrUserNotExist(err) {
ctx.Data["user_exists"] = true
Expand Down
2 changes: 1 addition & 1 deletion routers/user/auth_openid.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ func ConnectOpenIDPost(ctx *context.Context, form auth.ConnectOpenIDForm) {
ctx.Data["EnableOpenIDSignUp"] = setting.Service.EnableOpenIDSignUp
ctx.Data["OpenID"] = oid

u, err := models.UserSignIn(form.UserName, form.Password)
u, err := models.UserSignIn(form.UserName, form.Password, false)
if err != nil {
if models.IsErrUserNotExist(err) {
ctx.RenderWithErr(ctx.Tr("form.username_password_incorrect"), tplConnectOID, &form)
Expand Down
2 changes: 1 addition & 1 deletion routers/user/setting/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ func DeleteAccount(ctx *context.Context) {
ctx.Data["Title"] = ctx.Tr("settings")
ctx.Data["PageIsSettingsAccount"] = true

if _, err := models.UserSignIn(ctx.User.Name, ctx.Query("password")); err != nil {
if _, err := models.UserSignIn(ctx.User.Name, ctx.Query("password"), false); err != nil {
if models.IsErrUserNotExist(err) {
loadAccountData(ctx)

Expand Down