From 354bcd9f073e6ad7d07e8d13c869eac34ea2f21b Mon Sep 17 00:00:00 2001 From: Qiu Jian Date: Sat, 31 Oct 2020 18:38:10 +0800 Subject: [PATCH] fix: save service config changes may send invalid action log --- pkg/apis/identity/consts.go | 16 +++++++++ pkg/cloudcommon/options/manager.go | 7 ++-- pkg/cloudcommon/options/mergeconf.go | 10 +++--- pkg/compute/options/options.go | 7 ++++ pkg/image/options/options.go | 6 ++++ pkg/keystone/models/configs.go | 27 +++++++-------- pkg/keystone/models/identity_provider.go | 4 +-- pkg/keystone/models/passwords.go | 10 ++++++ pkg/keystone/models/services.go | 4 +-- pkg/keystone/options/options.go | 1 + pkg/util/stringutils2/stringutils.go | 26 ++++++++++++++ pkg/util/stringutils2/stringutils_test.go | 42 +++++++++++++++++++++++ 12 files changed, 135 insertions(+), 25 deletions(-) diff --git a/pkg/apis/identity/consts.go b/pkg/apis/identity/consts.go index 5fe371ecc6..02503e7f1b 100644 --- a/pkg/apis/identity/consts.go +++ b/pkg/apis/identity/consts.go @@ -203,3 +203,19 @@ var ( }, } ) + +func mergeConfigOptionsFrom(opt1, opt2 map[string][]string) map[string][]string { + for opt, values := range opt2 { + ovalues, _ := opt1[opt] + opt1[opt] = append(ovalues, values...) + } + return opt1 +} + +func MergeServiceConfigOptions(opts ...map[string][]string) map[string][]string { + ret := make(map[string][]string) + for i := range opts { + ret = mergeConfigOptionsFrom(ret, opts[i]) + } + return ret +} diff --git a/pkg/cloudcommon/options/manager.go b/pkg/cloudcommon/options/manager.go index bd691b7b88..c9eed0553d 100644 --- a/pkg/cloudcommon/options/manager.go +++ b/pkg/cloudcommon/options/manager.go @@ -116,7 +116,7 @@ func optionsEquals(newOpts interface{}, oldOpts interface{}) bool { func (manager *SOptionManager) DoSync(first bool) (time.Duration, error) { newOpts := manager.newOptions() copyOptions(newOpts, manager.options) - merged := manager.session.Merge(newOpts, manager.serviceType, manager.serviceVersion, first) + merged := manager.session.Merge(newOpts, manager.serviceType, manager.serviceVersion) if merged && !optionsEquals(newOpts, manager.options) { if manager.onOptionsChange != nil && manager.onOptionsChange(manager.options, newOpts) && !first { @@ -124,7 +124,10 @@ func (manager *SOptionManager) DoSync(first bool) (time.Duration, error) { appsrv.SetExitFlag() } copyOptions(manager.options, newOpts) - manager.session.Upload(first) + if first { + // upload config for the first time ONLY + manager.session.Upload() + } } return manager.refreshInterval, nil } diff --git a/pkg/cloudcommon/options/mergeconf.go b/pkg/cloudcommon/options/mergeconf.go index fb3fbe44c3..befe02e72c 100644 --- a/pkg/cloudcommon/options/mergeconf.go +++ b/pkg/cloudcommon/options/mergeconf.go @@ -57,8 +57,8 @@ func getServiceConfig(s *mcclient.ClientSession, serviceId string) (jsonutils.JS } type IServiceConfigSession interface { - Merge(opts interface{}, serviceType string, serviceVersion string, isFirst bool) bool - Upload(isFirst bool) + Merge(opts interface{}, serviceType string, serviceVersion string) bool + Upload() IsRemote() bool } @@ -72,7 +72,7 @@ func newServiceConfigSession() IServiceConfigSession { return &mcclientServiceConfigSession{} } -func (s *mcclientServiceConfigSession) Merge(opts interface{}, serviceType string, serviceVersion string, isFirst bool) bool { +func (s *mcclientServiceConfigSession) Merge(opts interface{}, serviceType string, serviceVersion string) bool { merged := false s.config = jsonutils.Marshal(opts).(*jsonutils.JSONDict) region, _ := s.config.GetString("region") @@ -88,7 +88,7 @@ func (s *mcclientServiceConfigSession) Merge(opts interface{}, serviceType strin merged = true } else { // not initialized - s.Upload(isFirst) + s.Upload() } } commonServiceId, _ := getServiceIdByType(s.session, consts.COMMON_SERVICE, "") @@ -113,7 +113,7 @@ func (s *mcclientServiceConfigSession) Merge(opts interface{}, serviceType strin return false } -func (s *mcclientServiceConfigSession) Upload(isFirst bool) { +func (s *mcclientServiceConfigSession) Upload() { // upload service config if len(s.serviceId) > 0 { nconf := jsonutils.NewDict() diff --git a/pkg/compute/options/options.go b/pkg/compute/options/options.go index d55ed9149e..5a2547a913 100644 --- a/pkg/compute/options/options.go +++ b/pkg/compute/options/options.go @@ -188,5 +188,12 @@ func OnOptionsChange(oldO, newO interface{}) bool { if common_options.OnCommonOptionsChange(&oldOpts.CommonOptions, &newOpts.CommonOptions) { changed = true } + + if oldOpts.PendingDeleteCheckSeconds != newOpts.PendingDeleteCheckSeconds { + if !oldOpts.IsSlaveNode { + changed = true + } + } + return changed } diff --git a/pkg/image/options/options.go b/pkg/image/options/options.go index ad087ddd19..dbd601c820 100644 --- a/pkg/image/options/options.go +++ b/pkg/image/options/options.go @@ -56,5 +56,11 @@ func OnOptionsChange(oldO, newO interface{}) bool { changed = true } + if oldOpts.PendingDeleteCheckSeconds != newOpts.PendingDeleteCheckSeconds { + if !oldOpts.IsSlaveNode { + changed = true + } + } + return changed } diff --git a/pkg/keystone/models/configs.go b/pkg/keystone/models/configs.go index 521dabd96d..fd119ad245 100644 --- a/pkg/keystone/models/configs.go +++ b/pkg/keystone/models/configs.go @@ -367,7 +367,7 @@ func GetConfigs(model db.IModel, sensitive bool, whiteList, blackList map[string return config2map(opts), nil } -func saveConfigs(userCred mcclient.TokenCredential, action string, model db.IModel, opts api.TConfigs, whiteList map[string][]string, blackList map[string][]string, sensitiveConfs map[string][]string, skipLog bool) error { +func saveConfigs(userCred mcclient.TokenCredential, action string, model db.IModel, opts api.TConfigs, whiteList map[string][]string, blackList map[string][]string, sensitiveConfs map[string][]string) error { var err error changed := make([]sChangeOption, 0) changedSensitive := make([]sChangeOption, 0) @@ -401,9 +401,6 @@ func saveConfigs(userCred mcclient.TokenCredential, action string, model db.IMod return errors.Wrap(err, "SensitiveConfigManager.syncConfig") } } - if userCred == nil { - userCred = getDefaultAdminCred() - } maskedValue := jsonutils.NewString("*") for i := range changedSensitive { if changedSensitive[i].OValue != nil { @@ -416,10 +413,12 @@ func saveConfigs(userCred mcclient.TokenCredential, action string, model db.IMod changed = append(changed, changedSensitive...) if len(changed) > 0 { notes := jsonutils.Marshal(changed) - db.OpsLog.LogEvent(model, db.ACT_CHANGE_CONFIG, notes, userCred) - if !skipLog { + if userCred == nil { + userCred = getDefaultAdminCred() + } else { logclient.AddSimpleActionLog(model, logclient.ACT_CHANGE_CONFIG, notes, userCred, true) } + db.OpsLog.LogEvent(model, db.ACT_CHANGE_CONFIG, notes, userCred) } return nil } @@ -433,7 +432,7 @@ func NewServiceConfigSession() common_options.IServiceConfigSession { return &dbServiceConfigSession{} } -func (s *dbServiceConfigSession) Merge(opts interface{}, serviceType string, serviceVersion string, isFirst bool) bool { +func (s *dbServiceConfigSession) Merge(opts interface{}, serviceType string, serviceVersion string) bool { merged := false s.config = jsonutils.Marshal(opts).(*jsonutils.JSONDict) s.service, _ = ServiceManager.fetchServiceByType(serviceType) @@ -447,7 +446,7 @@ func (s *dbServiceConfigSession) Merge(opts interface{}, serviceType string, ser merged = true } else { // not initialized - uploadConfig(s.service, s.config, isFirst) + uploadConfig(s.service, s.config) } } commonService, _ := ServiceManager.fetchServiceByType(consts.COMMON_SERVICE) @@ -461,7 +460,7 @@ func (s *dbServiceConfigSession) Merge(opts interface{}, serviceType string, ser merged = true } else { // common not initialized - uploadConfig(commonService, s.config, isFirst) + uploadConfig(commonService, s.config) } } if merged { @@ -474,18 +473,18 @@ func (s *dbServiceConfigSession) Merge(opts interface{}, serviceType string, ser return false } -func (s *dbServiceConfigSession) Upload(isFirst bool) { +func (s *dbServiceConfigSession) Upload() { if s.service == nil { return } - uploadConfig(s.service, s.config, isFirst) + uploadConfig(s.service, s.config) } func (s *dbServiceConfigSession) IsRemote() bool { return false } -func uploadConfig(service *SService, config jsonutils.JSONObject, isFirst bool) { +func uploadConfig(service *SService, config jsonutils.JSONObject) { nconf := jsonutils.NewDict() nconf.Add(config, "default") tconf := api.TConfigs{} @@ -495,9 +494,9 @@ func uploadConfig(service *SService, config jsonutils.JSONObject, isFirst bool) return } if service.isCommonService() { - err = saveConfigs(nil, "", service, tconf, api.CommonWhitelistOptionMap, nil, nil, isFirst) + err = saveConfigs(nil, "", service, tconf, api.CommonWhitelistOptionMap, nil, nil) } else { - err = saveConfigs(nil, "", service, tconf, nil, api.ServiceBlacklistOptionMap, nil, isFirst) + err = saveConfigs(nil, "", service, tconf, nil, api.MergeServiceConfigOptions(api.CommonWhitelistOptionMap, api.ServiceBlacklistOptionMap), nil) } if err != nil { log.Errorf("saveConfigs fail %s", err) diff --git a/pkg/keystone/models/identity_provider.go b/pkg/keystone/models/identity_provider.go index 1e99b50c71..40c3f30371 100644 --- a/pkg/keystone/models/identity_provider.go +++ b/pkg/keystone/models/identity_provider.go @@ -340,7 +340,7 @@ func (ident *SIdentityProvider) PerformConfig(ctx context.Context, userCred mccl opts := input.Config action := input.Action - err = saveConfigs(userCred, action, ident, opts, nil, nil, api.SensitiveDomainConfigMap, false) + err = saveConfigs(userCred, action, ident, opts, nil, nil, api.SensitiveDomainConfigMap) if err != nil { return nil, httperrors.NewInternalServerError("saveConfig fail %s", err) } @@ -492,7 +492,7 @@ func (ident *SIdentityProvider) PostCreate(ctx context.Context, userCred mcclien log.Errorf("parse config error %s", err) return } - err = saveConfigs(userCred, "", ident, opts, nil, nil, api.SensitiveDomainConfigMap, false) + err = saveConfigs(userCred, "", ident, opts, nil, nil, api.SensitiveDomainConfigMap) if err != nil { log.Errorf("saveConfig fail %s", err) return diff --git a/pkg/keystone/models/passwords.go b/pkg/keystone/models/passwords.go index e316086575..3d2eef72bd 100644 --- a/pkg/keystone/models/passwords.go +++ b/pkg/keystone/models/passwords.go @@ -27,6 +27,7 @@ import ( "yunion.io/x/onecloud/pkg/httperrors" o "yunion.io/x/onecloud/pkg/keystone/options" "yunion.io/x/onecloud/pkg/util/seclib2" + "yunion.io/x/onecloud/pkg/util/stringutils2" ) // +onecloud:swagger-gen-ignore @@ -111,6 +112,15 @@ func validatePasswordComplexity(password string) error { if o.Options.PasswordMinimalLength > 0 && len(password) < o.Options.PasswordMinimalLength { return errors.Wrap(httperrors.ErrWeakPassword, "too simple password") } + if o.Options.PasswordCharComplexity > 0 { + complexity := o.Options.PasswordCharComplexity + if complexity > 4 { + complexity = 4 + } + if stringutils2.GetCharTypeCount(password) < complexity { + return errors.Wrap(httperrors.ErrWeakPassword, "too simple password") + } + } return nil } diff --git a/pkg/keystone/models/services.go b/pkg/keystone/models/services.go index 50a743b6fa..335d281a12 100644 --- a/pkg/keystone/models/services.go +++ b/pkg/keystone/models/services.go @@ -201,9 +201,9 @@ func (service *SService) PerformConfig(ctx context.Context, userCred mcclient.To action := input.Action opts := input.Config if service.isCommonService() { - err = saveConfigs(userCred, action, service, opts, api.CommonWhitelistOptionMap, nil, nil, false) + err = saveConfigs(userCred, action, service, opts, api.CommonWhitelistOptionMap, nil, nil) } else { - err = saveConfigs(userCred, action, service, opts, nil, api.ServiceBlacklistOptionMap, nil, false) + err = saveConfigs(userCred, action, service, opts, nil, api.MergeServiceConfigOptions(api.CommonWhitelistOptionMap, api.ServiceBlacklistOptionMap), nil) } if err != nil { return nil, httperrors.NewInternalServerError("saveConfig fail %s", err) diff --git a/pkg/keystone/options/options.go b/pkg/keystone/options/options.go index 8b7d8f464b..fee4d78acf 100644 --- a/pkg/keystone/options/options.go +++ b/pkg/keystone/options/options.go @@ -41,6 +41,7 @@ type SKeystoneOptions struct { PasswordExpirationSeconds int `help:"password expires after the duration in seconds"` PasswordMinimalLength int `help:"password minimal length" default:"6"` PasswordUniqueHistoryCheck int `help:"password must be unique in last N passwords"` + PasswordCharComplexity int `help:"password complexity policy" default:"0"` PasswordErrorLockCount int `help:"lock user account if given number of failed auth"` diff --git a/pkg/util/stringutils2/stringutils.go b/pkg/util/stringutils2/stringutils.go index 0ad1c5215f..4368fbf08a 100644 --- a/pkg/util/stringutils2/stringutils.go +++ b/pkg/util/stringutils2/stringutils.go @@ -185,3 +185,29 @@ func GenerateHostName(name string, osType string) string { } return forOther(name) } + +func GetCharTypeCount(str string) int { + digitIdx := 0 + lowerIdx := 1 + upperIdx := 2 + otherIdx := 3 + complexity := make([]int, 4) + for _, b := range []byte(str) { + if b >= '0' && b <= '9' { + complexity[digitIdx] += 1 + } else if b >= 'a' && b <= 'z' { + complexity[lowerIdx] += 1 + } else if b >= 'A' && b <= 'Z' { + complexity[upperIdx] += 1 + } else { + complexity[otherIdx] += 1 + } + } + ret := 0 + for i := range complexity { + if complexity[i] > 0 { + ret += 1 + } + } + return ret +} diff --git a/pkg/util/stringutils2/stringutils_test.go b/pkg/util/stringutils2/stringutils_test.go index f2ed89377e..6315d192e6 100644 --- a/pkg/util/stringutils2/stringutils_test.go +++ b/pkg/util/stringutils2/stringutils_test.go @@ -166,3 +166,45 @@ func TestGenerateHostName(t *testing.T) { } } } + +func TestGetCharTypeCount(t *testing.T) { + cases := []struct { + in string + want int + }{ + { + in: "", + want: 0, + }, + { + in: "123", + want: 1, + }, + { + in: "abc", + want: 1, + }, + { + in: "abcAbc", + want: 2, + }, + { + in: "123dbA", + want: 3, + }, + { + in: "123@Acv", + want: 4, + }, + { + in: "中文", + want: 1, + }, + } + for _, c := range cases { + got := GetCharTypeCount(c.in) + if got != c.want { + t.Errorf("GetCharTypeCount %s want %d got %d", c.in, c.want, got) + } + } +}