From 9dca49ba6e4978a6ed22e471e105533322b68e5d Mon Sep 17 00:00:00 2001 From: stonezdj Date: Wed, 5 Sep 2018 16:54:12 +0800 Subject: [PATCH] LDAP group DN should be case insensitive Fix issue #5776, LDAP servers are case insensitive. because only LDAP group DN is used to compare/equal operation, lowercase all LDAP group DN when retrieves it from LDAP server, and lowercase them before save in DB Signed-off-by: stonezdj --- src/common/dao/config.go | 5 +++++ src/common/dao/group/usergroup.go | 5 +++-- src/common/dao/group/usergroup_test.go | 2 +- src/common/utils/utils.go | 5 +++++ src/common/utils/utils_test.go | 22 ++++++++++++++++++++++ src/ui/auth/ldap/ldap.go | 4 +++- 6 files changed, 39 insertions(+), 4 deletions(-) diff --git a/src/common/dao/config.go b/src/common/dao/config.go index 461be1348..466865d40 100644 --- 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 index eaddf0a6c..5e9504b08 100644 --- 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 index 6455708fb..1cc268b57 100644 --- 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 index f6bf117bc..40de5c00c 100644 --- a/src/common/utils/utils.go +++ b/src/common/utils/utils.go @@ -209,3 +209,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 index fdd19f744..4b50e3576 100644 --- 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 index dbb76262e..47ce5b5dd 100644 --- 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 }