Merge pull request #16260 from stonezdj/22jan21_ldap_group_duplicate

Group members lose access to push or see projects on Harbor

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{