Merge pull request #14717 from stonezdj/21pr16_verify_ldap_group_settings

Add the ldap group config validation

src/core/auth/ldap/ldap.go

@@ -115,6 +115,10 @@ func (l *Auth) attachLDAPGroup(ctx context.Context, ldapUsers []model.User, u *m
 		}
 
 	}
+	// skip to attach group when ldap_group_search_filter is empty
+	if len(groupCfg.Filter) == 0 {
+		return
+	}
 	userGroups := make([]ugModel.UserGroup, 0)
 	for _, dn := range ldapUsers[0].GroupDNList {
 		lGroups, err := sess.SearchGroupByDN(dn)

src/lib/config/config.go

@@ -97,6 +97,7 @@ func Init() {
 	log.Info("init secret store")
 	// init secret store
 	initSecretStore()
+	DefaultCfgManager = common.DBCfgManager
 }
 
 // InitWithSettings init config with predefined configs, and optionally overwrite the keyprovider

src/pkg/config/manager.go

@@ -23,6 +23,7 @@ import (
 	"github.com/goharbor/harbor/src/lib/config/metadata"
 	"github.com/goharbor/harbor/src/lib/log"
 	"github.com/goharbor/harbor/src/pkg/config/store"
+	"github.com/goharbor/harbor/src/pkg/config/validate"
 	"os"
 )
 
@@ -31,6 +32,10 @@ type CfgManager struct {
 	Store *store.ConfigStore
 }
 
+var validateRules = []validate.Rule{
+	&validate.LdapGroupValidateRule{},
+}
+
 // LoadDefault ...
 func (c *CfgManager) LoadDefault() {
 	// Init Default Value
@@ -178,5 +183,11 @@ func (c *CfgManager) ValidateCfg(ctx context.Context, cfgs map[string]interface{
 			return fmt.Errorf("%v, item name: %v", err, key)
 		}
 	}
+
+	for _, r := range validateRules {
+		if err := r.Validate(ctx, c, cfgs); err != nil {
+			return err
+		}
+	}
 	return nil
 }

src/pkg/config/validate/ldapgroup.go

@@ -0,0 +1,65 @@
+//  Copyright 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 validate
+
+import (
+	"context"
+	"github.com/goharbor/harbor/src/common"
+	"github.com/goharbor/harbor/src/lib/config"
+	cfgModels "github.com/goharbor/harbor/src/lib/config/models"
+	"github.com/goharbor/harbor/src/lib/errors"
+)
+
+// LdapGroupValidateRule validate the ldap group configuration
+type LdapGroupValidateRule struct {
+}
+
+// Validate validate the ldap group config settings, cfgs is the config items need to be updated, the final config should be merged with the cfgMgr
+func (l LdapGroupValidateRule) Validate(ctx context.Context, cfgMgr config.Manager, cfgs map[string]interface{}) error {
+	cfg := &cfgModels.GroupConf{
+		Filter:              cfgMgr.Get(ctx, common.LDAPGroupSearchFilter).GetString(),
+		NameAttribute:       cfgMgr.Get(ctx, common.LDAPGroupAttributeName).GetString(),
+		MembershipAttribute: cfgMgr.Get(ctx, common.LDAPGroupMembershipAttribute).GetString(),
+	}
+	updated := false
+	// Merge the cfgs and the cfgMgr to get the final GroupConf
+	if val, exist := cfgs[common.LDAPGroupSearchFilter]; exist {
+		cfg.Filter = val.(string)
+		updated = true
+	}
+	if val, exist := cfgs[common.LDAPGroupAttributeName]; exist {
+		cfg.NameAttribute = val.(string)
+		updated = true
+	}
+	if val, exist := cfgs[common.LDAPGroupMembershipAttribute]; exist {
+		cfg.MembershipAttribute = val.(string)
+		updated = true
+	}
+	if !updated {
+		return nil
+	}
+
+	if len(cfg.Filter) == 0 {
+		// skip to validate group config
+		return nil
+	}
+	if len(cfg.NameAttribute) == 0 {
+		return errors.New("ldap group name attribute can not be empty")
+	}
+	if len(cfg.MembershipAttribute) == 0 {
+		return errors.New("ldap group membership attribute can not be empty")
+	}
+	return nil
+}

src/pkg/config/validate/test/ldapgroup_test.go

@@ -0,0 +1,83 @@
+//  Copyright 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 test
+
+import (
+	"context"
+	"errors"
+	"github.com/goharbor/harbor/src/common"
+	"github.com/goharbor/harbor/src/common/utils/test"
+	"github.com/goharbor/harbor/src/lib/config"
+	"github.com/goharbor/harbor/src/lib/orm"
+	_ "github.com/goharbor/harbor/src/pkg/config/inmemory"
+	"github.com/goharbor/harbor/src/pkg/config/validate"
+	"os"
+	"testing"
+)
+
+func TestMain(m *testing.M) {
+	test.InitDatabaseFromEnv()
+	config.InitWithSettings(map[string]interface{}{
+		"ldap_group_search_filter":        "objectClass=groupOfNames",
+		"ldap_group_attribute_name":       "",
+		"ldap_group_membership_attribute": "memberof",
+	})
+	// do some initialization
+	os.Exit(m.Run())
+}
+func TestLdapGroupValidateRule_Validate(t *testing.T) {
+	mgr, err := config.GetManager(common.InMemoryCfgManager)
+	if err != nil {
+		t.Error(err)
+	}
+	rule := validate.LdapGroupValidateRule{}
+	type args struct {
+		ctx    context.Context
+		cfgMgr config.Manager
+		cfgs   map[string]interface{}
+	}
+	cases := []struct {
+		name string
+		in   args
+		want error
+	}{
+		{
+			name: `nothing updated, no error`,
+			in:   args{ctx: orm.Context(), cfgMgr: mgr, cfgs: map[string]interface{}{}},
+			want: nil,
+		},
+		{
+			name: `empty ldap group membership attribute, update all`,
+			in:   args{ctx: orm.Context(), cfgMgr: mgr, cfgs: map[string]interface{}{"ldap_group_search_filter": "objectClass=groupOfNames", "ldap_group_attribute_name": "cn", "ldap_group_membership_attribute": ""}},
+			want: errors.New("ldap group membership attribute can not be empty"),
+		},
+		{
+			name: `empty ldap group attribute name, update partially`,
+			in:   args{ctx: orm.Context(), cfgMgr: mgr, cfgs: map[string]interface{}{"ldap_group_search_filter": "objectClass=groupOfNames", "ldap_group_membership_attribute": "memberof"}},
+			want: errors.New("ldap group name attribute can not be empty"),
+		},
+	}
+	for _, tt := range cases {
+		t.Run(tt.name, func(t *testing.T) {
+			got := rule.Validate(tt.in.ctx, tt.in.cfgMgr, tt.in.cfgs)
+			if got == nil && tt.want == nil {
+				return
+			}
+			if got != nil && got.Error() != tt.want.Error() {
+				t.Errorf(`(%v) = %v; want "%v"`, tt.in, got, tt.want)
+			}
+		})
+	}
+}

src/pkg/config/validate/validate.go

@@ -0,0 +1,30 @@
+//  Copyright 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 validate
+
+import (
+	"context"
+	"github.com/goharbor/harbor/src/lib/config"
+)
+
+// Rule rules to validate the configure input
+// Steps to add validation rule for configure parameter
+// 1. Create/find the model need to validate, add validate tag in the field.
+// 2. Implement the Rule interface like LdapGroupValidateRule
+// 3. Add the rule to validateRules in pkg/config/manager.go
+type Rule interface {
+	// Validate validates a specific group of configuration items, cfgs contains the config need to be updated, the final config should merged with the cfgMgr
+	Validate(ctx context.Context, cfgMgr config.Manager, cfgs map[string]interface{}) error
+}