Merge pull request #8333 from stonezdj/fix_onboard_group

Fix OnBoardGroup issue

src/common/dao/group/usergroup.go

@@ -144,11 +144,7 @@ func UpdateUserGroupName(id int, groupName string) error {
 	return err
 }
 
-// OnBoardUserGroup will check if a usergroup exists in usergroup table, if not insert the usergroup and
+func onBoardCommonUserGroup(g *models.UserGroup, keyAttribute string, combinedKeyAttributes ...string) error {
-// put the id in the pointer of usergroup model, if it does exist, return the usergroup's profile.
-// This is used for ldap and uaa authentication, such the usergroup can have an ID in Harbor.
-// the keyAttribute and combinedKeyAttribute are key columns used to check duplicate usergroup in harbor
-func OnBoardUserGroup(g *models.UserGroup, keyAttribute string, combinedKeyAttributes ...string) error {
 	g.LdapGroupDN = utils.TrimLower(g.LdapGroupDN)
 
 	o := dao.GetOrmer()
@@ -172,3 +168,12 @@ func OnBoardUserGroup(g *models.UserGroup, keyAttribute string, combinedKeyAttri
 
 	return nil
 }
+
+// OnBoardUserGroup will check if a usergroup exists in usergroup table, if not insert the usergroup and
+// put the id in the pointer of usergroup model, if it does exist, return the usergroup's profile.
+func OnBoardUserGroup(g *models.UserGroup) error {
+	if g.GroupType == common.LDAPGroupType {
+		return onBoardCommonUserGroup(g, "LdapGroupDN", "GroupType")
+	}
+	return onBoardCommonUserGroup(g, "GroupName", "GroupType")
+}

src/common/dao/group/usergroup_test.go

@@ -256,7 +256,7 @@ func TestOnBoardUserGroup(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if err := OnBoardUserGroup(tt.args.g, "LdapGroupDN", "GroupType"); (err != nil) != tt.wantErr {
+			if err := OnBoardUserGroup(tt.args.g); (err != nil) != tt.wantErr {
 				t.Errorf("OnBoardUserGroup() error = %v, wantErr %v", err, tt.wantErr)
 			}
 		})

src/core/auth/authproxy/auth.go

@@ -17,6 +17,7 @@ package authproxy
 import (
 	"crypto/tls"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"net/http"
@@ -190,12 +191,14 @@ func (a *Auth) SearchGroup(groupKey string) (*models.UserGroup, error) {
 // OnBoardGroup create user group entity in Harbor DB, altGroupName is not used.
 func (a *Auth) OnBoardGroup(u *models.UserGroup, altGroupName string) error {
 	// if group name provided, on board the user group
-	userGroup := &models.UserGroup{GroupName: u.GroupName, GroupType: common.HTTPGroupType}
+	if len(u.GroupName) == 0 {
-	err := group.OnBoardUserGroup(u, "GroupName", "GroupType")
+		return errors.New("Should provide a group name")
+	}
+	u.GroupType = common.HTTPGroupType
+	err := group.OnBoardUserGroup(u)
 	if err != nil {
 		return err
 	}
-	u.ID = userGroup.ID
 	return nil
 }
 

src/core/auth/authproxy/auth_test.go

@@ -43,6 +43,7 @@ func TestMain(m *testing.M) {
 	}
 	mockSvr = test.NewMockServer(map[string]string{"jt": "pp", "Admin@vsphere.local": "Admin!23"})
 	defer mockSvr.Close()
+	defer dao.ExecuteBatchSQL([]string{"delete from user_group where group_name='OnBoardTest'"})
 	a = &Auth{
 		Endpoint:            mockSvr.URL + "/test/login",
 		TokenReviewEndpoint: mockSvr.URL + "/test/tokenreview",
@@ -50,10 +51,17 @@ func TestMain(m *testing.M) {
 		// So it won't require mocking the cfgManager
 		settingTimeStamp: time.Now(),
 	}
+	cfgMap := cut.GetUnitTestConfig()
 	conf := map[string]interface{}{
 		common.HTTPAuthProxyEndpoint:            a.Endpoint,
 		common.HTTPAuthProxyTokenReviewEndpoint: a.TokenReviewEndpoint,
 		common.HTTPAuthProxyVerifyCert:          !a.SkipCertVerify,
+		common.PostGreSQLSSLMode:                cfgMap[common.PostGreSQLSSLMode],
+		common.PostGreSQLUsername:               cfgMap[common.PostGreSQLUsername],
+		common.PostGreSQLPort:                   cfgMap[common.PostGreSQLPort],
+		common.PostGreSQLHOST:                   cfgMap[common.PostGreSQLHOST],
+		common.PostGreSQLPassword:               cfgMap[common.PostGreSQLPassword],
+		common.PostGreSQLDatabase:               cfgMap[common.PostGreSQLDatabase],
 	}
 
 	config.InitWithSettings(conf)
@@ -174,3 +182,19 @@ func TestAuth_PostAuthenticate(t *testing.T) {
 	}
 
 }
+
+func TestAuth_OnBoardGroup(t *testing.T) {
+	input := &models.UserGroup{
+		GroupName: "OnBoardTest",
+		GroupType: common.HTTPGroupType,
+	}
+	a.OnBoardGroup(input, "")
+
+	assert.True(t, input.ID > 0, "The OnBoardGroup should have a valid group ID")
+
+	emptyGroup := &models.UserGroup{}
+	err := a.OnBoardGroup(emptyGroup, "")
+	if err == nil {
+		t.Fatal("Empty user group should failed to OnBoard")
+	}
+}

src/core/auth/ldap/ldap.go

@@ -214,7 +214,7 @@ func (l *Auth) OnBoardGroup(u *models.UserGroup, altGroupName string) error {
 	if len(userGroupList) > 0 {
 		return auth.ErrDuplicateLDAPGroup
 	}
-	return group.OnBoardUserGroup(u, "LdapGroupDN", "GroupType")
+	return group.OnBoardUserGroup(u)
 }
 
 // PostAuthenticate -- If user exist in harbor DB, sync email address, if not exist, call OnBoardUser