From d9b4b1894318e04ea2200d9cdb01130a8bbf80fb Mon Sep 17 00:00:00 2001 From: stonezdj Date: Fri, 3 Aug 2018 16:05:09 +0800 Subject: [PATCH] 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 | 4 +- .../systemcfg/store/database/driver_db.go | 17 +++---- src/ui/api/projectmember.go | 45 ++++++++++++++----- src/ui/api/usergroup.go | 2 + src/ui/auth/authenticator.go | 6 +++ src/ui/auth/ldap/ldap.go | 15 +++++++ src/ui/auth/ldap/ldap_test.go | 13 ++---- 7 files changed, 73 insertions(+), 29 deletions(-) diff --git a/docs/swagger.yaml b/docs/swagger.yaml index 0c1dcb1ed..fdcbbc7ac 100644 --- a/docs/swagger.yaml +++ b/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}': diff --git a/src/adminserver/systemcfg/store/database/driver_db.go b/src/adminserver/systemcfg/store/database/driver_db.go index 3a922eceb..a5ab833fc 100644 --- a/src/adminserver/systemcfg/store/database/driver_db.go +++ b/src/adminserver/systemcfg/store/database/driver_db.go @@ -31,14 +31,15 @@ const ( var ( numKeys = map[string]bool{ - common.EmailPort: true, - common.LDAPScope: true, - common.LDAPTimeout: true, - common.TokenExpiration: true, - common.MaxJobWorkers: true, - common.CfgExpiration: true, - common.ClairDBPort: true, - common.PostGreSQLPort: true, + common.EmailPort: true, + common.LDAPScope: true, + common.LDAPGroupSearchScope: true, + common.LDAPTimeout: true, + common.TokenExpiration: true, + common.MaxJobWorkers: true, + common.CfgExpiration: true, + common.ClairDBPort: true, + common.PostGreSQLPort: true, } boolKeys = map[string]bool{ common.WithClair: true, diff --git a/src/ui/api/projectmember.go b/src/ui/api/projectmember.go index 19000c808..861e1264d 100644 --- a/src/ui/api/projectmember.go +++ b/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 - } - if err != nil { + } else if err == auth.ErrDuplicateLDAPGroup { + 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 -func AddOrUpdateProjectMember(projectID int64, request models.MemberReq) (int, error) { +// AddProjectMember ... +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 ldap group already exist in harbor, use the previous group name + + //If groupname provided, use the provided groupname to name this group 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, nil + return 0, ErrDuplicateProjectMember } 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) } diff --git a/src/ui/api/usergroup.go b/src/ui/api/usergroup.go index 2eae58644..e3d9c9672 100644 --- a/src/ui/api/usergroup.go +++ b/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 { diff --git a/src/ui/auth/authenticator.go b/src/ui/auth/authenticator.go index 3cfb75e29..5bbd1058f 100644 --- a/src/ui/auth/authenticator.go +++ b/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 diff --git a/src/ui/auth/ldap/ldap.go b/src/ui/auth/ldap/ldap.go index d0e21e2b8..3d81a16f8 100644 --- a/src/ui/auth/ldap/ldap.go +++ b/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") } diff --git a/src/ui/auth/ldap/ldap_test.go b/src/ui/auth/ldap/ldap_test.go index 2e65b2fab..80d2197fb 100644 --- a/src/ui/auth/ldap/ldap_test.go +++ b/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.AddOrUpdateProjectMember(currentProject.ProjectID, member) + pmid, err := api.AddProjectMember(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.AddOrUpdateProjectMember(currentProject.ProjectID, member) + pmid, err := api.AddProjectMember(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) }