Fix LDAP group related issues

User groups should not have same DN
Should not import an LDAP group if there is a user group with same DN

docs/swagger.yaml

@@ -513,13 +513,15 @@ paths:
         '201':
           description: Project member created successfully.
         '400':
-          description: Illegal format of project member or project id is invalid.
+          description: Illegal format of project member or project id is invalid, or LDAP DN is invalid.
         '401':
           description: User need to log in first.
         '403':
           description: User in session does not have permission to the project.
         '404':
           description: Project does not exist, or the username does not found, or the user group does not found.
+        '409':
+          description: An LDAP user group with same DN already exist.           
         '500':
           description: Unexpected internal errors.  
   '/projects/{project_id}/members/{mid}':

src/adminserver/systemcfg/store/database/driver_db.go

@@ -31,14 +31,15 @@ const (
 
 var (
 	numKeys = map[string]bool{
-		common.EmailPort:       true,
+		common.EmailPort:            true,
-		common.LDAPScope:       true,
+		common.LDAPScope:            true,
-		common.LDAPTimeout:     true,
+		common.LDAPGroupSearchScope: true,
-		common.TokenExpiration: true,
+		common.LDAPTimeout:          true,
-		common.MaxJobWorkers:   true,
+		common.TokenExpiration:      true,
-		common.CfgExpiration:   true,
+		common.MaxJobWorkers:        true,
-		common.ClairDBPort:     true,
+		common.CfgExpiration:        true,
-		common.PostGreSQLPort:  true,
+		common.ClairDBPort:          true,
+		common.PostGreSQLPort:       true,
 	}
 	boolKeys = map[string]bool{
 		common.WithClair:        true,

src/ui/api/projectmember.go

@@ -15,9 +15,11 @@
 package api
 
 import (
+	"errors"
 	"fmt"
 	"net/http"
 	"strconv"
+	"strings"
 
 	"github.com/vmware/harbor/src/common"
 	"github.com/vmware/harbor/src/common/dao/project"
@@ -35,6 +37,12 @@ type ProjectMemberAPI struct {
 	project    *models.Project
 }
 
+// ErrDuplicateProjectMember ...
+var ErrDuplicateProjectMember = errors.New("The project member specified already exist")
+
+// ErrInvalidRole ...
+var ErrInvalidRole = errors.New("Failed to update project member, role is not in 1,2,3")
+
 // Prepare validates the URL and parms
 func (pma *ProjectMemberAPI) Prepare() {
 	pma.BaseController.Prepare()
@@ -121,12 +129,25 @@ func (pma *ProjectMemberAPI) Post() {
 	projectID := pma.project.ProjectID
 	var request models.MemberReq
 	pma.DecodeJSONReq(&request)
-	pmid, err := AddOrUpdateProjectMember(projectID, request)
+	request.MemberGroup.LdapGroupDN = strings.TrimSpace(request.MemberGroup.LdapGroupDN)
+
+	pmid, err := AddProjectMember(projectID, request)
 	if err == auth.ErrorGroupNotExist || err == auth.ErrorUserNotExist {
 		pma.HandleNotFound(fmt.Sprintf("Failed to add project member, error: %v", err))
 		return
-	}
+	} else if err == auth.ErrDuplicateLDAPGroup {
-	if err != nil {
+		pma.HandleConflict(fmt.Sprintf("Failed to add project member, already exist LDAP group or project member, groupDN:%v", request.MemberGroup.LdapGroupDN))
+		return
+	} else if err == ErrDuplicateProjectMember {
+		pma.HandleConflict(fmt.Sprintf("Failed to add project member, already exist LDAP group or project member, groupMemberID:%v", request.MemberGroup.ID))
+		return
+	} else if err == ErrInvalidRole {
+		pma.HandleBadRequest(fmt.Sprintf("Invalid role ID, role ID %v", request.Role))
+		return
+	} else if err == auth.ErrInvalidLDAPGroupDN {
+		pma.HandleBadRequest(fmt.Sprintf("Invalid LDAP DN: %v", request.MemberGroup.LdapGroupDN))
+		return
+	} else if err != nil {
 		pma.HandleInternalServerError(fmt.Sprintf("Failed to add project member, error: %v", err))
 		return
 	}
@@ -160,8 +181,8 @@ func (pma *ProjectMemberAPI) Delete() {
 	}
 }
 
-// AddOrUpdateProjectMember ... If the project member relationship does not exist, create it. if exist, update it
+// AddProjectMember ...
-func AddOrUpdateProjectMember(projectID int64, request models.MemberReq) (int, error) {
+func AddProjectMember(projectID int64, request models.MemberReq) (int, error) {
 	var member models.Member
 	member.ProjectID = projectID
 	member.Role = request.Role
@@ -179,18 +200,20 @@ func AddOrUpdateProjectMember(projectID int64, request models.MemberReq) (int, e
 		}
 		member.EntityID = userID
 	} else if len(request.MemberGroup.LdapGroupDN) > 0 {
-		member.EntityType = common.GroupMember
+
-		//If groupname provided, use the provided groupname
+		//If groupname provided, use the provided groupname to name this group
-		//If ldap group already exist in harbor, use the previous group name
 		groupID, err := auth.SearchAndOnBoardGroup(request.MemberGroup.LdapGroupDN, request.MemberGroup.GroupName)
 		if err != nil {
 			return 0, err
 		}
 		member.EntityID = groupID
+		member.EntityType = common.GroupMember
 	}
 	if member.EntityID <= 0 {
 		return 0, fmt.Errorf("Can not get valid member entity, request: %+v", request)
 	}
+
+	//Check if member already exist in current project
 	memberList, err := project.GetProjectMember(models.Member{
 		ProjectID:  member.ProjectID,
 		EntityID:   member.EntityID,
@@ -200,12 +223,12 @@ func AddOrUpdateProjectMember(projectID int64, request models.MemberReq) (int, e
 		return 0, err
 	}
 	if len(memberList) > 0 {
-		project.UpdateProjectMemberRole(memberList[0].ID, member.Role)
+		return 0, ErrDuplicateProjectMember
-		return 0, nil
 	}
 
 	if member.Role < 1 || member.Role > 3 {
-		return 0, fmt.Errorf("Failed to update project member, role is not in 1,2,3 role:%v", member.Role)
+		//Return invalid role error
+		return 0, ErrInvalidRole
 	}
 	return project.AddProjectMember(member)
 }

src/ui/api/usergroup.go

@@ -18,6 +18,7 @@ import (
 	"fmt"
 	"net/http"
 	"strconv"
+	"strings"
 
 	"github.com/vmware/harbor/src/common"
 	"github.com/vmware/harbor/src/common/dao/group"
@@ -94,6 +95,7 @@ func (uga *UserGroupAPI) Post() {
 	uga.DecodeJSONReq(&userGroup)
 	userGroup.ID = 0
 	userGroup.GroupType = common.LdapGroupType
+	userGroup.LdapGroupDN = strings.TrimSpace(userGroup.LdapGroupDN)
 	query := models.UserGroup{GroupType: userGroup.GroupType, LdapGroupDN: userGroup.LdapGroupDN}
 	result, err := group.QueryUserGroup(query)
 	if err != nil {

src/ui/auth/authenticator.go

@@ -37,6 +37,12 @@ var ErrorUserNotExist = errors.New("User does not exist")
 // ErrorGroupNotExist ...
 var ErrorGroupNotExist = errors.New("Group does not exist")
 
+// ErrDuplicateLDAPGroup ...
+var ErrDuplicateLDAPGroup = errors.New("An LDAP user group with same DN already exist")
+
+// ErrInvalidLDAPGroupDN ...
+var ErrInvalidLDAPGroupDN = errors.New("The LDAP group DN is invalid")
+
 //ErrAuth is the type of error to indicate a failed authentication due to user's error.
 type ErrAuth struct {
 	details string

src/ui/auth/ldap/ldap.go

@@ -20,6 +20,7 @@ import (
 	"strings"
 
 	"github.com/vmware/harbor/src/common"
+	goldap "gopkg.in/ldap.v2"
 
 	"github.com/vmware/harbor/src/common/dao"
 	"github.com/vmware/harbor/src/common/dao/group"
@@ -162,6 +163,9 @@ func (l *Auth) SearchUser(username string) (*models.User, error) {
 
 //SearchGroup -- Search group in ldap authenticator, groupKey is LDAP group DN.
 func (l *Auth) SearchGroup(groupKey string) (*models.UserGroup, error) {
+	if _, err := goldap.ParseDN(groupKey); err != nil {
+		return nil, auth.ErrInvalidLDAPGroupDN
+	}
 	ldapSession, err := ldapUtils.LoadSystemLdapConfig()
 
 	if err != nil {
@@ -192,10 +196,21 @@ func (l *Auth) SearchGroup(groupKey string) (*models.UserGroup, error) {
 
 // OnBoardGroup -- Create Group in harbor DB, if altGroupName is not empty, take the altGroupName as groupName in harbor DB.
 func (l *Auth) OnBoardGroup(u *models.UserGroup, altGroupName string) error {
+	if _, err := goldap.ParseDN(u.LdapGroupDN); err != nil {
+		return auth.ErrInvalidLDAPGroupDN
+	}
 	if len(altGroupName) > 0 {
 		u.GroupName = altGroupName
 	}
 	u.GroupType = common.LdapGroupType
+	//Check duplicate LDAP DN in usergroup, if usergroup exist, return error
+	userGroupList, err := group.QueryUserGroup(models.UserGroup{LdapGroupDN: u.LdapGroupDN})
+	if err != nil {
+		return err
+	}
+	if len(userGroupList) > 0 {
+		return auth.ErrDuplicateLDAPGroup
+	}
 	return group.OnBoardUserGroup(u, "LdapGroupDN", "GroupType")
 }
 

src/ui/auth/ldap/ldap_test.go

@@ -27,6 +27,7 @@ import (
 	"github.com/vmware/harbor/src/common/utils/log"
 	"github.com/vmware/harbor/src/common/utils/test"
 	"github.com/vmware/harbor/src/ui/api"
+
 	"github.com/vmware/harbor/src/ui/auth"
 	uiConfig "github.com/vmware/harbor/src/ui/config"
 )
@@ -389,8 +390,7 @@ func TestSearchAndOnBoardUser(t *testing.T) {
 		t.Errorf("Can not search and onboard user %v", "mike")
 	}
 }
-func TestAddOrUpdateProjectMemberWithLdapUser(t *testing.T) {
+func TestAddProjectMemberWithLdapUser(t *testing.T) {
-
 	currentProject, err := dao.GetProjectByName("member_test_01")
 	if err != nil {
 		t.Errorf("Error occurred when GetProjectByName: %v", err)
@@ -402,18 +402,15 @@ func TestAddOrUpdateProjectMemberWithLdapUser(t *testing.T) {
 		},
 		Role: models.PROJECTADMIN,
 	}
-
+	pmid, err := api.AddProjectMember(currentProject.ProjectID, member)
-	pmid, err := api.AddOrUpdateProjectMember(currentProject.ProjectID, member)
 	if err != nil {
 		t.Errorf("Error occurred in AddOrUpdateProjectMember: %v", err)
 	}
 	if pmid == 0 {
 		t.Errorf("Error occurred in AddOrUpdateProjectMember: pmid:%v", pmid)
 	}
-
 }
 func TestAddProjectMemberWithLdapGroup(t *testing.T) {
-
 	currentProject, err := dao.GetProjectByName("member_test_01")
 	if err != nil {
 		t.Errorf("Error occurred when GetProjectByName: %v", err)
@@ -425,8 +422,7 @@ func TestAddProjectMemberWithLdapGroup(t *testing.T) {
 		},
 		Role: models.PROJECTADMIN,
 	}
-
+	pmid, err := api.AddProjectMember(currentProject.ProjectID, member)
-	pmid, err := api.AddOrUpdateProjectMember(currentProject.ProjectID, member)
 	if err != nil {
 		t.Errorf("Error occurred in AddOrUpdateProjectMember: %v", err)
 	}
@@ -440,7 +436,6 @@ func TestAddProjectMemberWithLdapGroup(t *testing.T) {
 	if err != nil {
 		t.Errorf("Failed to query project member, %v, error: %v", queryMember, err)
 	}
-
 	if len(memberList) == 0 {
 		t.Errorf("Failed to query project member, %v", queryMember)
 	}