Merge pull request #13500 from thechristschn/fix-searchgroupbyname

Fix api ldap group search by name

src/common/utils/ldap/ldap.go

@@ -424,7 +424,7 @@ func (session *Session) searchGroup(groupDN, filter, gName, groupNameAttribute s
 		groupName = result.Entries[0].Attributes[0].Values[0]
 	}
 	group := models.LdapGroup{
-		GroupDN:   groupDN,
+		GroupDN:   result.Entries[0].DN,
 		GroupName: groupName,
 	}
 	ldapGroups = append(ldapGroups, group)

src/common/utils/ldap/ldap_test.go

@@ -423,6 +423,104 @@ func TestSession_SearchGroupByDN(t *testing.T) {
 	}
 }
 
+func TestSession_SearchGroupByName(t *testing.T) {
+	ldapConfig := models.LdapConf{
+		LdapURL:            ldapTestConfig[common.LDAPURL].(string) + ":389",
+		LdapSearchDn:       ldapTestConfig[common.LDAPSearchDN].(string),
+		LdapScope:          2,
+		LdapSearchPassword: ldapTestConfig[common.LDAPSearchPwd].(string),
+		LdapBaseDn:         ldapTestConfig[common.LDAPBaseDN].(string),
+	}
+	ldapGroupConfig := models.LdapGroupConf{
+		LdapGroupBaseDN:        "dc=example,dc=com",
+		LdapGroupFilter:        "objectclass=groupOfNames",
+		LdapGroupNameAttribute: "cn",
+		LdapGroupSearchScope:   2,
+	}
+	ldapGroupConfig2 := models.LdapGroupConf{
+		LdapGroupBaseDN:        "dc=example,dc=com",
+		LdapGroupFilter:        "objectclass=groupOfNames",
+		LdapGroupNameAttribute: "o",
+		LdapGroupSearchScope:   2,
+	}
+	groupConfigWithFilter := models.LdapGroupConf{
+		LdapGroupBaseDN:        "dc=example,dc=com",
+		LdapGroupFilter:        "(cn=*admin*)",
+		LdapGroupNameAttribute: "cn",
+		LdapGroupSearchScope:   2,
+	}
+	groupConfigWithDifferentGroupDN := models.LdapGroupConf{
+		LdapGroupBaseDN:        "dc=harbor,dc=example,dc=com",
+		LdapGroupFilter:        "(objectclass=groupOfNames)",
+		LdapGroupNameAttribute: "cn",
+		LdapGroupSearchScope:   2,
+	}
+
+	type fields struct {
+		ldapConfig      models.LdapConf
+		ldapGroupConfig models.LdapGroupConf
+		ldapConn        *goldap.Conn
+	}
+	type args struct {
+		groupName string
+	}
+	tests := []struct {
+		name    string
+		fields  fields
+		args    args
+		want    []models.LdapGroup
+		wantErr bool
+	}{
+		{"normal search",
+			fields{ldapConfig: ldapConfig, ldapGroupConfig: ldapGroupConfig},
+			args{groupName: "harbor_users"},
+			[]models.LdapGroup{{GroupName: "harbor_users", GroupDN: "cn=harbor_users,ou=groups,dc=example,dc=com"}}, false},
+		{"search non-exist group",
+			fields{ldapConfig: ldapConfig, ldapGroupConfig: ldapGroupConfig},
+			args{groupName: "harbor_non_users"},
+			[]models.LdapGroup{}, false},
+		{"search with gid = o",
+			fields{ldapConfig: ldapConfig, ldapGroupConfig: ldapGroupConfig2},
+			args{groupName: "hgroup"},
+			[]models.LdapGroup{{GroupName: "hgroup", GroupDN: "cn=harbor_group,ou=groups,dc=example,dc=com"}}, false},
+		{"search with group filter success",
+			fields{ldapConfig: ldapConfig, ldapGroupConfig: groupConfigWithFilter},
+			args{groupName: "harbor_admin"},
+			[]models.LdapGroup{{GroupName: "harbor_admin", GroupDN: "cn=harbor_admin,ou=groups,dc=example,dc=com"}}, false},
+		{"search with group filter fail",
+			fields{ldapConfig: ldapConfig, ldapGroupConfig: groupConfigWithFilter},
+			args{groupName: "harbor_users"},
+			[]models.LdapGroup{}, false},
+		{"search with different group base dn success",
+			fields{ldapConfig: ldapConfig, ldapGroupConfig: groupConfigWithDifferentGroupDN},
+			args{groupName: "harbor_root"},
+			[]models.LdapGroup{{GroupName: "harbor_root", GroupDN: "cn=harbor_root,dc=harbor,dc=example,dc=com"}}, false},
+		{"search with different group base dn fail",
+			fields{ldapConfig: ldapConfig, ldapGroupConfig: groupConfigWithDifferentGroupDN},
+			args{groupName: "harbor_guest"},
+			[]models.LdapGroup{}, false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			session := &Session{
+				ldapConfig:      tt.fields.ldapConfig,
+				ldapGroupConfig: tt.fields.ldapGroupConfig,
+				ldapConn:        tt.fields.ldapConn,
+			}
+			session.Open()
+			defer session.Close()
+			got, err := session.SearchGroupByName(tt.args.groupName)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("Session.SearchGroupByName() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("Session.SearchGroupByName() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
 func TestCreateUserSearchFilter(t *testing.T) {
 	type args struct {
 		origFilter string

src/core/api/harborapi_test.go

@@ -791,6 +791,26 @@ func (a testapi) LdapPost(authInfo usrInfo, ldapConf apilib.LdapConf) (int, erro
 	return httpStatusCode, err
 }
 
+// Search Ldap Groups
+func (a testapi) LdapGroupsSearch(groupName, groupDN string, authInfo ...usrInfo) (int, []apilib.LdapGroupsSearch, error) {
+	_sling := sling.New().Get(a.basePath)
+	// create path and map variables
+	path := "/api/ldap/groups/search"
+	_sling = _sling.Path(path)
+	// body params
+	type QueryParams struct {
+		GroupName string `url:"groupname, omitempty"`
+		GroupDN   string `url:"groupdn, omitempty"`
+	}
+	_sling = _sling.QueryStruct(&QueryParams{GroupName: groupName, GroupDN: groupDN})
+	httpStatusCode, body, err := request(_sling, jsonAcceptHeader, authInfo...)
+	var successPayLoad []apilib.LdapGroupsSearch
+	if 200 == httpStatusCode && nil == err {
+		err = json.Unmarshal(body, &successPayLoad)
+	}
+	return httpStatusCode, successPayLoad, err
+}
+
 func (a testapi) GetConfig(authInfo usrInfo) (int, map[string]*value, error) {
 	_sling := sling.New().Base(a.basePath).Get("/api/configurations")
 

src/core/api/ldap_test.go

@@ -0,0 +1,89 @@
+// Copyright 2018 Project Harbor Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package api
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/goharbor/harbor/src/common"
+	"github.com/goharbor/harbor/src/common/utils/test"
+	"github.com/goharbor/harbor/src/core/config"
+	"github.com/goharbor/harbor/src/testing/apitests/apilib"
+	"github.com/stretchr/testify/assert"
+)
+
+var ldapTestConfig = map[string]interface{}{
+	common.ExtEndpoint:            "host01.com",
+	common.AUTHMode:               "ldap_auth",
+	common.DatabaseType:           "postgresql",
+	common.PostGreSQLHOST:         "127.0.0.1",
+	common.PostGreSQLPort:         5432,
+	common.PostGreSQLUsername:     "postgres",
+	common.PostGreSQLPassword:     "root123",
+	common.PostGreSQLDatabase:     "registry",
+	common.LDAPURL:                "ldap://127.0.0.1",
+	common.LDAPSearchDN:           "cn=admin,dc=example,dc=com",
+	common.LDAPSearchPwd:          "admin",
+	common.LDAPBaseDN:             "dc=example,dc=com",
+	common.LDAPUID:                "uid",
+	common.LDAPFilter:             "",
+	common.LDAPScope:              2,
+	common.LDAPTimeout:            30,
+	common.AdminInitialPassword:   "password",
+	common.LDAPGroupSearchFilter:  "objectclass=groupOfNames",
+	common.LDAPGroupBaseDN:        "dc=example,dc=com",
+	common.LDAPGroupAttributeName: "cn",
+	common.LDAPGroupSearchScope:   2,
+	common.LDAPGroupAdminDn:       "cn=harbor_users,ou=groups,dc=example,dc=com",
+}
+
+func TestLdapGroupsSearch(t *testing.T) {
+
+	fmt.Println("Testing Ldap Groups Search")
+	assert := assert.New(t)
+	config.InitWithSettings(ldapTestConfig)
+	apiTest := newHarborAPI()
+
+	ldapGroup := apilib.LdapGroupsSearch{
+		GroupName: "harbor_users",
+		GroupDN:   "cn=harbor_users,ou=groups,dc=example,dc=com",
+	}
+
+	// case 1: search group by name
+	code, groups, err := apiTest.LdapGroupsSearch(ldapGroup.GroupName, "", *admin)
+	if err != nil {
+		t.Error("Error occurred while search ldap groups", err.Error())
+		t.Log(err)
+	} else {
+		assert.Equal(200, code, "Search ldap group status should be 200")
+		assert.Equal(1, len(groups), "Search ldap groups record should be 1")
+		assert.Equal(ldapGroup.GroupDN, groups[0].GroupDN, "Group DNs should be equal")
+		assert.Equal(ldapGroup.GroupName, groups[0].GroupName, "Group names should be equal")
+	}
+
+	// case 2: search group by DN
+	code, groups, err = apiTest.LdapGroupsSearch("", ldapGroup.GroupDN, *admin)
+	if err != nil {
+		t.Error("Error occurred while search ldap groups", err.Error())
+		t.Log(err)
+	} else {
+		assert.Equal(200, code, "Search ldap groups status should be 200")
+		assert.Equal(1, len(groups), "Search ldap groups record should be 1 ")
+		assert.Equal(ldapGroup.GroupDN, groups[0].GroupDN, "Group DNs should be equal")
+		assert.Equal(ldapGroup.GroupName, groups[0].GroupName, "Group names should be equal")
+	}
+
+	config.InitWithSettings(test.GetDefaultConfigMap())
+}

src/testing/apitests/apilib/ldap.go

@@ -32,3 +32,9 @@ type LdapConf struct {
 	LdapScope             int    `json:"ldap_scope"`
 	LdapConnectionTimeout int    `json:"ldap_connection_timeout"`
 }
+
+// LdapGroupsSearch the ldap group search type
+type LdapGroupsSearch struct {
+	GroupName string `json:"group_name,omitempty"`
+	GroupDN   string `json:"ldap_group_dn,omitempty"`
+}