From 0cc2675c44fb2ce7580c16dec74cb76b9e7cb1e1 Mon Sep 17 00:00:00 2001
From: Johan Van de Wauw <johan@gisky.be>
Date: Fri, 18 Feb 2022 10:33:00 +0100
Subject: [PATCH] Fix ldap loginname (#18789)
* 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>
---
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 3ea8866150..a6e4b7d4d2 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 70591c8c12..a5f47172ee 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 4392e861f9..a6f2d14b5c 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 e804e32e84..2c1bcc29cc 100644
--- a/services/auth/source/ldap/source_authenticate.go
+++ b/services/auth/source/ldap/source_authenticate.go
@@ -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