Skip to content

Commit

Permalink
Fix ldap loginname (#18789)
Browse files Browse the repository at this point in the history
* 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 <xiaolunwen@gmail.com>
Co-authored-by: zeripath <art27@cantab.net>
  • Loading branch information
3 people authored Feb 18, 2022
1 parent 1ab88da commit 0cc2675
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 8 deletions.
2 changes: 0 additions & 2 deletions integrations/signin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
14 changes: 14 additions & 0 deletions models/user/user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())

Expand Down
9 changes: 5 additions & 4 deletions services/auth/signin.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
8 changes: 6 additions & 2 deletions services/auth/source/ldap/source_authenticate.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,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
Expand Down

0 comments on commit 0cc2675

Please sign in to comment.