Group members lose access to push or see projects on Harbor

Handle the case if there is duplicate user group name when onboard ldap user group
   Continue to attach groups when it fail on one item
   Fixes #16220

Signed-off-by: stonezdj <stonezdj@gmail.com>

src/pkg/usergroup/manager.go

@@ -19,6 +19,7 @@ import (
 	"errors"
 	"github.com/goharbor/harbor/src/common"
 	"github.com/goharbor/harbor/src/common/utils"
+	"github.com/goharbor/harbor/src/lib/log"
 	"github.com/goharbor/harbor/src/lib/q"
 	"github.com/goharbor/harbor/src/pkg/usergroup/dao"
 	"github.com/goharbor/harbor/src/pkg/usergroup/model"
@@ -83,7 +84,9 @@ func (m *manager) Populate(ctx context.Context, userGroups []model.UserGroup) ([
 	for _, group := range userGroups {
 		err := m.Onboard(ctx, &group)
 		if err != nil {
-			return ugList, err
+			// log the current error and continue
+			log.Warningf("failed to onboard user group %+v, error %v, continue with other user groups", group, err)
+			continue
 		}
 		if group.ID > 0 {
 			ugList = append(ugList, group.ID)
@@ -102,10 +105,35 @@ func (m *manager) UpdateName(ctx context.Context, id int, groupName string) erro
 
 func (m *manager) Onboard(ctx context.Context, g *model.UserGroup) error {
 	if g.GroupType == common.LDAPGroupType {
-		return m.onBoardCommonUserGroup(ctx, g, "LdapGroupDN", "GroupType")
+		return m.onBoardLdapUserGroup(ctx, g)
 	}
 	return m.onBoardCommonUserGroup(ctx, g, "GroupName", "GroupType")
 }
+
+// onBoardLdapUserGroup -- Check if the ldap group name duplicated and onboard the ldap group
+func (m *manager) onBoardLdapUserGroup(ctx context.Context, g *model.UserGroup) error {
+	g.LdapGroupDN = utils.TrimLower(g.LdapGroupDN)
+	// check if any duplicate ldap group name exist
+	ug, err := m.dao.Query(ctx, q.New(q.KeyWords{"GroupName": g.GroupName, "GroupType": g.GroupType}))
+	if err != nil {
+		return err
+	}
+	if len(ug) > 0 {
+		if g.LdapGroupDN == ug[0].LdapGroupDN {
+			g.ID = ug[0].ID
+			return nil
+		}
+		// if duplicated with name, fall back to ldap group dn
+		if len(g.LdapGroupDN) <= 255 {
+			g.GroupName = g.LdapGroupDN
+		} else {
+			g.GroupName = g.LdapGroupDN[:254]
+		}
+		log.Warningf("existing duplicate user group with the same name, name the current user group with ldap group DN %v", g.GroupName)
+	}
+	return m.onBoardCommonUserGroup(ctx, g, "LdapGroupDN", "GroupType")
+}
+
 func (m *manager) onBoardCommonUserGroup(ctx context.Context, g *model.UserGroup, keyAttribute string, combinedKeyAttributes ...string) error {
 	g.LdapGroupDN = utils.TrimLower(g.LdapGroupDN)
 	created, ID, err := m.dao.ReadOrCreate(ctx, g, keyAttribute, combinedKeyAttributes...)

src/pkg/usergroup/manager_test.go

@@ -47,6 +47,46 @@ func (s *ManagerTestSuite) TestOnboardGroup() {
 	s.True(len(ugs) > 0)
 }
 
+func (s *ManagerTestSuite) TestOnboardGroupWithDuplicatedName() {
+	ctx := s.Context()
+	ugs := []*model.UserGroup{
+		{
+			GroupName:   "harbor_dev",
+			GroupType:   1,
+			LdapGroupDN: "cn=harbor_dev,ou=groups,dc=example,dc=com",
+		},
+		{
+			GroupName:   "harbor_dev",
+			GroupType:   1,
+			LdapGroupDN: "cn=harbor_dev,ou=groups,dc=example2,dc=com",
+		},
+		{
+			GroupName:   "harbor_dev",
+			GroupType:   1,
+			LdapGroupDN: "cn=harbor_dev,ou=groups,dc=example3,dc=com,dc=verylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcname",
+		},
+	}
+	for _, ug := range ugs {
+		err := s.mgr.Onboard(ctx, ug)
+		s.Nil(err)
+	}
+	// both user group should be onboard to user group
+	ugs, err := s.mgr.List(ctx, q.New(q.KeyWords{"GroupType": 1, "LdapGroupDN": "cn=harbor_dev,ou=groups,dc=example,dc=com"}))
+	s.Nil(err)
+	s.True(len(ugs) > 0)
+
+	ugs, err = s.mgr.List(ctx, q.New(q.KeyWords{"GroupType": 1, "LdapGroupDN": "cn=harbor_dev,ou=groups,dc=example2,dc=com"}))
+	s.Nil(err)
+	s.True(len(ugs) > 0)
+	s.Equal("cn=harbor_dev,ou=groups,dc=example2,dc=com", ugs[0].GroupName)
+
+	ugs, err = s.mgr.List(ctx, q.New(q.KeyWords{"GroupType": 1, "LdapGroupDN": "cn=harbor_dev,ou=groups,dc=example3,dc=com,dc=verylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcname"}))
+	s.Nil(err)
+	s.True(len(ugs) > 0)
+	s.Equal("cn=harbor_dev,ou=groups,dc=example3,dc=com,dc=verylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcnameverylongdcna", ugs[0].GroupName)
+
+}
+
 func (s *ManagerTestSuite) TestPopulateGroup() {
 	ctx := s.Context()
 	ugs := []model.UserGroup{