From f5a3c0dd6c9d4c5611d115e0fe2c0b25724b8b49 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 22 Feb 2022 21:33:06 +0800 Subject: [PATCH] Fix ldap loginname (#18789) (#18804) * Use email_address table to check user's email when login with email adress * Update services/auth/signin.go * Fix test * Fix test * Fix logging in with ldap username != loginname * Fix if user does not exist yet * Make more clear this is loginName * Fix formatting Co-authored-by: Lunny Xiao Co-authored-by: zeripath Co-authored-by: Johan Van de Wauw Co-authored-by: zeripath --- integrations/signin_test.go | 2 -- models/user/user_test.go | 14 ++++++++++++++ services/auth/signin.go | 9 +++++---- services/auth/source/ldap/source_authenticate.go | 8 ++++++-- 4 files changed, 25 insertions(+), 8 deletions(-) diff --git a/integrations/signin_test.go b/integrations/signin_test.go index 3ea8866150c3..a6e4b7d4d2f1 100644 --- a/integrations/signin_test.go +++ b/integrations/signin_test.go @@ -51,8 +51,6 @@ func TestSignin(t *testing.T) { {username: "wrongUsername", password: "password", message: i18n.Tr("en", "form.username_password_incorrect")}, {username: "user15", password: "wrongPassword", message: i18n.Tr("en", "form.username_password_incorrect")}, {username: "user1@example.com", password: "wrongPassword", message: i18n.Tr("en", "form.username_password_incorrect")}, - // test for duplicate email - {username: "user2@example.com", password: "password", message: i18n.Tr("en", "form.email_been_used")}, } for _, s := range samples { diff --git a/models/user/user_test.go b/models/user/user_test.go index 70591c8c1213..a5f47172eebc 100644 --- a/models/user/user_test.go +++ b/models/user/user_test.go @@ -235,6 +235,20 @@ func TestCreateUserInvalidEmail(t *testing.T) { assert.True(t, IsErrEmailInvalid(err)) } +func TestCreateUserEmailAlreadyUsed(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + user := unittest.AssertExistsAndLoadBean(t, &User{ID: 2}).(*User) + + // add new user with user2's email + user.Name = "testuser" + user.LowerName = strings.ToLower(user.Name) + user.ID = 0 + err := CreateUser(user) + assert.Error(t, err) + assert.True(t, IsErrEmailAlreadyUsed(err)) +} + func TestGetUserIDsByNames(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) diff --git a/services/auth/signin.go b/services/auth/signin.go index 4392e861f997..a6f2d14b5c60 100644 --- a/services/auth/signin.go +++ b/services/auth/signin.go @@ -24,17 +24,18 @@ import ( func UserSignIn(username, password string) (*user_model.User, *auth.Source, error) { var user *user_model.User if strings.Contains(username, "@") { - user = &user_model.User{Email: strings.ToLower(strings.TrimSpace(username))} + emailAddress := user_model.EmailAddress{LowerEmail: strings.ToLower(strings.TrimSpace(username))} // check same email - cnt, err := db.Count(user) + has, err := db.GetEngine(db.DefaultContext).Where("is_activated=?", true).Get(&emailAddress) if err != nil { return nil, nil, err } - if cnt > 1 { - return nil, nil, user_model.ErrEmailAlreadyUsed{ + if !has { + return nil, nil, user_model.ErrEmailAddressNotExist{ Email: user.Email, } } + user = &user_model.User{ID: emailAddress.UID} } else { trimmedUsername := strings.TrimSpace(username) if len(trimmedUsername) == 0 { diff --git a/services/auth/source/ldap/source_authenticate.go b/services/auth/source/ldap/source_authenticate.go index 52971bb87e58..bc35bc20f656 100644 --- a/services/auth/source/ldap/source_authenticate.go +++ b/services/auth/source/ldap/source_authenticate.go @@ -19,10 +19,14 @@ import ( // Authenticate queries if login/password is valid against the LDAP directory pool, // and create a local user if success when enabled. func (source *Source) Authenticate(user *user_model.User, userName, password string) (*user_model.User, error) { - sr := source.SearchEntry(userName, password, source.authSource.Type == auth.DLDAP) + loginName := userName + if user != nil { + loginName = user.LoginName + } + sr := source.SearchEntry(loginName, password, source.authSource.Type == auth.DLDAP) if sr == nil { // User not in LDAP, do nothing - return nil, user_model.ErrUserNotExist{Name: userName} + return nil, user_model.ErrUserNotExist{Name: loginName} } isAttributeSSHPublicKeySet := len(strings.TrimSpace(source.AttributeSSHPublicKey)) > 0