Merge pull request #5828 from stonezdj/ldap_caseinsense

LDAP group DN should be case insensitively
goharbor · Sep 7, 2018 · cd31cbf · cd31cbf
2 parents f0f1c4e + 9dca49b
commit cd31cbf
Show file tree

Hide file tree

Showing 6 changed files with 39 additions and 4 deletions.
diff --git a/src/common/dao/config.go b/src/common/dao/config.go
@@ -15,7 +15,9 @@
 package dao
 
 import (
+	"github.com/goharbor/harbor/src/common"
 	"github.com/goharbor/harbor/src/common/models"
+	"github.com/goharbor/harbor/src/common/utils"
 )
 
 // AuthModeCanBeModified determines whether auth mode can be
@@ -51,6 +53,9 @@ func GetConfigEntries() ([]*models.ConfigEntry, error) {
 func SaveConfigEntries(entries []models.ConfigEntry) error {
 	o := GetOrmer()
 	for _, entry := range entries {
+		if entry.Key == common.LdapGroupAdminDn {
+			entry.Value = utils.TrimLower(entry.Value)
+		}
 		tempEntry := models.ConfigEntry{}
 		tempEntry.Key = entry.Key
 		tempEntry.Value = entry.Value

diff --git a/src/common/dao/group/usergroup.go b/src/common/dao/group/usergroup.go
@@ -19,6 +19,7 @@ import (
 	"time"
 
 	"github.com/goharbor/harbor/src/common"
+	"github.com/goharbor/harbor/src/common/utils"
 
 	"github.com/goharbor/harbor/src/common/dao"
 	"github.com/goharbor/harbor/src/common/models"
@@ -33,7 +34,7 @@ func AddUserGroup(userGroup models.UserGroup) (int, error) {
 	var id int
 	now := time.Now()
 
-	err := o.Raw(sql, userGroup.GroupName, userGroup.GroupType, userGroup.LdapGroupDN, now, now).QueryRow(&id)
+	err := o.Raw(sql, userGroup.GroupName, userGroup.GroupType, utils.TrimLower(userGroup.LdapGroupDN), now, now).QueryRow(&id)
 	if err != nil {
 		return 0, err
 	}
@@ -59,7 +60,7 @@ func QueryUserGroup(query models.UserGroup) ([]*models.UserGroup, error) {
 
 	if len(query.LdapGroupDN) != 0 {
 		sql += ` and ldap_group_dn = ? `
-		sqlParam = append(sqlParam, query.LdapGroupDN)
+		sqlParam = append(sqlParam, utils.TrimLower(query.LdapGroupDN))
 	}
 	if query.ID != 0 {
 		sql += ` and id = ? `

diff --git a/src/common/dao/group/usergroup_test.go b/src/common/dao/group/usergroup_test.go
@@ -47,7 +47,7 @@ func TestMain(m *testing.M) {
 		initSqls := []string{
 			"insert into harbor_user (username, email, password, realname)  values ('member_test_01', 'member_test_01@example.com', '123456', 'member_test_01')",
 			"insert into project (name, owner_id) values ('member_test_01', 1)",
-			"insert into user_group (group_name, group_type, ldap_group_dn) values ('test_group_01', 1, 'CN=harbor_users,OU=sample,OU=vmware,DC=harbor,DC=com')",
+			"insert into user_group (group_name, group_type, ldap_group_dn) values ('test_group_01', 1, 'cn=harbor_users,ou=sample,ou=vmware,dc=harbor,dc=com')",
 			"update project set owner_id = (select user_id from harbor_user where username = 'member_test_01') where name = 'member_test_01'",
 			"insert into project_member (project_id, entity_id, entity_type, role) values ( (select project_id from project where name = 'member_test_01') , (select user_id from harbor_user where username = 'member_test_01'), 'u', 1)",
 			"insert into project_member (project_id, entity_id, entity_type, role) values ( (select project_id from project where name = 'member_test_01') , (select id from user_group where group_name = 'test_group_01'), 'g', 1)",

diff --git a/src/common/utils/utils.go b/src/common/utils/utils.go
@@ -211,3 +211,8 @@ func ParseOfftime(offtime int64) (hour, minite, second int) {
 	second = int(offtime % 60)
 	return
 }
+
+// TrimLower ...
+func TrimLower(str string) string {
+	return strings.TrimSpace(strings.ToLower(str))
+}
diff --git a/src/common/utils/utils_test.go b/src/common/utils/utils_test.go
@@ -359,3 +359,25 @@ func TestParseOfftime(t *testing.T) {
 		assert.Equal(t, c.second, s)
 	}
 }
+
+func TestTrimLower(t *testing.T) {
+	type args struct {
+		str string
+	}
+	tests := []struct {
+		name string
+		args args
+		want string
+	}{
+		{"normal", args{" CN=example,DC=test,DC=com "}, "cn=example,dc=test,dc=com"},
+		{"empty", args{" "}, ""},
+		{"empty2", args{""}, ""},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := TrimLower(tt.args.str); got != tt.want {
+				t.Errorf("TrimLower() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
diff --git a/src/ui/auth/ldap/ldap.go b/src/ui/auth/ldap/ldap.go
@@ -20,6 +20,7 @@ import (
 	"strings"
 
 	"github.com/goharbor/harbor/src/common"
+	"github.com/goharbor/harbor/src/common/utils"
 	goldap "gopkg.in/ldap.v2"
 
 	"github.com/goharbor/harbor/src/common/dao"
@@ -89,10 +90,11 @@ func (l *Auth) Authenticate(m models.AuthModel) (*models.User, error) {
 	// Retrieve ldap related info in login to avoid too many traffic with LDAP server.
 	// Get group admin dn
 	groupCfg, err := config.LDAPGroupConf()
-	groupAdminDN := strings.TrimSpace(groupCfg.LdapGroupAdminDN)
+	groupAdminDN := utils.TrimLower(groupCfg.LdapGroupAdminDN)
 	// Attach user group
 	for _, groupDN := range ldapUsers[0].GroupDNList {
 
+		groupDN = utils.TrimLower(groupDN)
 		if len(groupAdminDN) > 0 && groupAdminDN == groupDN {
 			u.HasAdminRole = true
 		}