Merge pull request #3870 from stonezdj/ldap_syncuser2

Sync user email in ldap #3663

src/common/dao/config_test.go

@@ -46,7 +46,7 @@ func TestAuthModeCanBeModified(t *testing.T) {
 			t.Fatalf("failed to register user: %v", err)
 		}
 		defer func(id int64) {
-			if err := deleteUser(id); err != nil {
+			if err := CleanUser(id); err != nil {
 				t.Fatalf("failed to delete user %d: %v", id, err)
 			}
 		}(id)
@@ -68,4 +68,4 @@ func TestAuthModeCanBeModified(t *testing.T) {
 			t.Errorf("unexpected result: %t != %t", flag, false)
 		}
 	}
-}
+}

src/common/dao/user.go

@@ -258,9 +258,12 @@ func DeleteUser(userID int) error {
 }
 
 // ChangeUserProfile ...
-func ChangeUserProfile(user models.User) error {
+func ChangeUserProfile(user models.User, cols ...string) error {
 	o := GetOrmer()
-	if _, err := o.Update(&user, "Email", "Realname", "Comment"); err != nil {
+	if len(cols) == 0 {
+		cols = []string{"Email", "Realname", "Comment"}
+	}
+	if _, err := o.Update(&user, cols...); err != nil {
 		log.Errorf("update user failed, error: %v", err)
 		return err
 	}
@@ -290,3 +293,12 @@ func OnBoardUser(u *models.User) error {
 	}
 	return nil
 }
+
+//CleanUser - Clean this user information from DB
+func CleanUser(id int64) error {
+	if _, err := GetOrmer().QueryTable(&models.User{}).
+		Filter("UserID", id).Delete(); err != nil {
+		return err
+	}
+	return nil
+}

src/common/dao/user_test.go

@@ -39,7 +39,7 @@ func TestDeleteUser(t *testing.T) {
 		t.Fatalf("failed to register user: %v", err)
 	}
 	defer func(id int64) {
-		if err := deleteUser(id); err != nil {
+		if err := CleanUser(id); err != nil {
 			t.Fatalf("failed to delete user %d: %v", id, err)
 		}
 	}(id)
@@ -88,13 +88,5 @@ func TestOnBoardUser(t *testing.T) {
 	err = OnBoardUser(u)
 	assert.Nil(err)
 	assert.True(u.UserID == id)
-	deleteUser(int64(id))
-}
-
-func deleteUser(id int64) error {
-	if _, err := GetOrmer().QueryTable(&models.User{}).
-		Filter("UserID", id).Delete(); err != nil {
-		return err
-	}
-	return nil
+	CleanUser(int64(id))
 }

src/ui/auth/authenticator.go

@@ -39,6 +39,34 @@ type AuthenticateHelper interface {
 	OnBoardUser(u *models.User) error
 	// Get user information from account repository
 	SearchUser(username string) (*models.User, error)
+	// Update user information after authenticate, such as Onboard or sync info etc
+	PostAuthenticate(u *models.User) error
+}
+
+// DefaultAuthenticateHelper - default AuthenticateHelper implementation
+type DefaultAuthenticateHelper struct {
+}
+
+// Authenticate ...
+func (d *DefaultAuthenticateHelper) Authenticate(m models.AuthModel) (*models.User, error) {
+	return nil, nil
+}
+
+// OnBoardUser will check if a user exists in user table, if not insert the user and
+// put the id in the pointer of user model, if it does exist, fill in the user model based
+// on the data record of the user
+func (d *DefaultAuthenticateHelper) OnBoardUser(u *models.User) error {
+	return nil
+}
+
+//SearchUser - Get user information from account repository
+func (d *DefaultAuthenticateHelper) SearchUser(username string) (*models.User, error) {
+	return nil, nil
+}
+
+//PostAuthenticate - Update user information after authenticate, such as Onboard or sync info etc
+func (d *DefaultAuthenticateHelper) PostAuthenticate(u *models.User) error {
+	return nil
 }
 
 var registry = make(map[string]AuthenticateHelper)
@@ -79,6 +107,9 @@ func Login(m models.AuthModel) (*models.User, error) {
 		lock.Lock(m.Principal)
 		time.Sleep(frozenTime)
 	}
+
+	authenticator.PostAuthenticate(user)
+
 	return user, err
 }
 
@@ -112,3 +143,12 @@ func SearchUser(username string) (*models.User, error) {
 	}
 	return helper.SearchUser(username)
 }
+
+// PostAuthenticate -
+func PostAuthenticate(u *models.User) error {
+	helper, err := getHelper()
+	if err != nil {
+		return err
+	}
+	return helper.PostAuthenticate(u)
+}

src/ui/auth/db/db.go

@@ -21,7 +21,9 @@ import (
 )
 
 // Auth implements Authenticator interface to authenticate user against DB.
-type Auth struct{}
+type Auth struct {
+	auth.DefaultAuthenticateHelper
+}
 
 // Authenticate calls dao to authenticate user.
 func (d *Auth) Authenticate(m models.AuthModel) (*models.User, error) {
@@ -32,12 +34,6 @@ func (d *Auth) Authenticate(m models.AuthModel) (*models.User, error) {
 	return u, nil
 }
 
-// OnBoardUser - Dummy implementation when auth_mod is db_auth
-func (d *Auth) OnBoardUser(user *models.User) error {
-	//No need to create user in local database
-	return nil
-}
-
 // SearchUser - Check if user exist in local db
 func (d *Auth) SearchUser(username string) (*models.User, error) {
 	var queryCondition = models.User{

src/ui/auth/ldap/ldap.go

@@ -16,6 +16,7 @@ package ldap
 
 import (
 	"fmt"
+	"regexp"
 	"strings"
 
 	"github.com/vmware/harbor/src/common/dao"
@@ -26,7 +27,9 @@ import (
 )
 
 // Auth implements AuthenticateHelper interface to authenticate against LDAP
-type Auth struct{}
+type Auth struct {
+	auth.DefaultAuthenticateHelper
+}
 
 // Authenticate checks user's credential against LDAP based on basedn template and LDAP URL,
 // if the check is successful a dummy record will be inserted into DB, such that this user can
@@ -68,7 +71,7 @@ func (l *Auth) Authenticate(m models.AuthModel) (*models.User, error) {
 
 	u := models.User{}
 	u.Username = ldapUsers[0].Username
-	u.Email = ldapUsers[0].Email
+	u.Email = strings.TrimSpace(ldapUsers[0].Email)
 	u.Realname = ldapUsers[0].Realname
 
 	dn := ldapUsers[0].DN
@@ -78,34 +81,7 @@ func (l *Auth) Authenticate(m models.AuthModel) (*models.User, error) {
 		log.Warningf("Failed to bind user, username: %s, dn: %s, error: %v", u.Username, dn, err)
 		return nil, nil
 	}
-	exist, err := dao.UserExists(u, "username")
-	if err != nil {
-		return nil, err
-	}
-
-	if exist {
-		currentUser, err := dao.GetUser(u)
-		if err != nil {
-			return nil, err
-		}
-		u.UserID = currentUser.UserID
-		u.HasAdminRole = currentUser.HasAdminRole
-	} else {
-		var user models.User
-		user.Username = ldapUsers[0].Username
-		user.Email = ldapUsers[0].Email
-		user.Realname = ldapUsers[0].Realname
-
-		err = auth.OnBoardUser(&user)
-		if err != nil || user.UserID <= 0 {
-			log.Errorf("Can't import user %s, error: %v", ldapUsers[0].Username, err)
-			return nil, fmt.Errorf("can't import user %s, error: %v", ldapUsers[0].Username, err)
-		}
-		u.UserID = user.UserID
-	}
-
 	return &u, nil
-
 }
 
 // OnBoardUser will check if a user exists in user table, if not insert the user and
@@ -153,6 +129,52 @@ func (l *Auth) SearchUser(username string) (*models.User, error) {
 	return &user, nil
 }
 
+//PostAuthenticate -- If user exist in harbor DB, sync email address, if not exist, call OnBoardUser
+func (l *Auth) PostAuthenticate(u *models.User) error {
+
+	exist, err := dao.UserExists(*u, "username")
+	if err != nil {
+		return err
+	}
+
+	if exist {
+		queryCondition := models.User{
+			Username: u.Username,
+		}
+		dbUser, err := dao.GetUser(queryCondition)
+		if err != nil {
+			return err
+		}
+		if dbUser == nil {
+			fmt.Printf("User not found in DB %+v", u)
+			return nil
+		}
+		u.UserID = dbUser.UserID
+		u.HasAdminRole = dbUser.HasAdminRole
+
+		if dbUser.Email != u.Email {
+			Re := regexp.MustCompile(`^[a-z0-9._%+\-]+@[a-z0-9.\-]+\.[a-z]{2,4}$`)
+			if !Re.MatchString(u.Email) {
+				log.Debugf("Not a valid email address: %v, skip to sync", u.Email)
+			} else {
+				dao.ChangeUserProfile(*u, "Email")
+			}
+			u.Email = dbUser.Email
+		}
+
+		return nil
+	}
+
+	err = auth.OnBoardUser(u)
+	if err != nil {
+		return err
+	}
+	if u.UserID <= 0 {
+		return fmt.Errorf("Can not OnBoardUser %v", u)
+	}
+	return nil
+}
+
 func init() {
 	auth.Register("ldap_auth", &Auth{})
 }

src/ui/auth/ldap/ldap_test.go

@@ -19,6 +19,7 @@ import (
 	"os"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/vmware/harbor/src/common"
 	"github.com/vmware/harbor/src/common/dao"
 	"github.com/vmware/harbor/src/common/models"
@@ -204,3 +205,59 @@ func TestAuthenticateHelperSearchUser(t *testing.T) {
 		t.Error("Failed to search user test")
 	}
 }
+
+func TestPostAuthentication(t *testing.T) {
+
+	assert := assert.New(t)
+	user1 := &models.User{
+		Username: "test003",
+		Email:    "test003@vmware.com",
+		Realname: "test003",
+	}
+
+	queryCondition := models.User{
+		Username: "test003",
+		Realname: "test003",
+	}
+
+	err := auth.OnBoardUser(user1)
+	assert.Nil(err)
+
+	user2 := &models.User{
+		Username: "test003",
+		Email:    "234invalidmail@@@@@",
+	}
+
+	auth.PostAuthenticate(user2)
+
+	dbUser, err := dao.GetUser(queryCondition)
+	if err != nil {
+		t.Fatalf("Failed to get user, error %v", err)
+	}
+	assert.EqualValues("test003@vmware.com", dbUser.Email)
+
+	user3 := &models.User{
+		Username: "test003",
+	}
+
+	auth.PostAuthenticate(user3)
+	dbUser, err = dao.GetUser(queryCondition)
+	if err != nil {
+		t.Fatalf("Failed to get user, error %v", err)
+	}
+	assert.EqualValues("test003@vmware.com", dbUser.Email)
+
+	user4 := &models.User{
+		Username: "test003",
+		Email:    "test003@example.com",
+	}
+
+	auth.PostAuthenticate(user4)
+
+	dbUser, err = dao.GetUser(queryCondition)
+	if err != nil {
+		t.Fatalf("Failed to get user, error %v", err)
+	}
+	assert.EqualValues("test003@example.com", dbUser.Email)
+	dao.CleanUser(int64(dbUser.UserID))
+}

src/ui/auth/uaa/uaa.go

@@ -48,6 +48,7 @@ func CreateClient() (uaa.Client, error) {
 type Auth struct {
 	sync.Mutex
 	client uaa.Client
+	auth.DefaultAuthenticateHelper
 }
 
 //Authenticate ...