diff --git a/docs.md b/docs.md index 0e9541da9..72767602f 100644 --- a/docs.md +++ b/docs.md @@ -418,6 +418,7 @@ When settings are created or updated, the following common checks take place: - If set, `user-last-login-default` must be a date time according to RFC3339 (e.g. `2023-11-29T00:00:00Z`). - If set, `user-retention-cron` must be a valid standard cron expression (e.g. `0 0 * * 0`). - The `auth-user-session-ttl-minutes` must be a positive integer and can't be greater than `disable-inactive-user-after` or `delete-inactive-user-after` if those values are set. +- The `auth-user-session-idle-ttl-minutes` must be a positive integer and can't be greater than `auth-user-session-ttl-minutes`. #### Update diff --git a/pkg/resources/management.cattle.io/v3/setting/Setting.md b/pkg/resources/management.cattle.io/v3/setting/Setting.md index 935017162..a0c5c5ecf 100644 --- a/pkg/resources/management.cattle.io/v3/setting/Setting.md +++ b/pkg/resources/management.cattle.io/v3/setting/Setting.md @@ -9,6 +9,7 @@ When settings are created or updated, the following common checks take place: - If set, `user-last-login-default` must be a date time according to RFC3339 (e.g. `2023-11-29T00:00:00Z`). - If set, `user-retention-cron` must be a valid standard cron expression (e.g. `0 0 * * 0`). - The `auth-user-session-ttl-minutes` must be a positive integer and can't be greater than `disable-inactive-user-after` or `delete-inactive-user-after` if those values are set. +- The `auth-user-session-idle-ttl-minutes` must be a positive integer and can't be greater than `auth-user-session-ttl-minutes`. ### Update diff --git a/pkg/resources/management.cattle.io/v3/setting/validator.go b/pkg/resources/management.cattle.io/v3/setting/validator.go index 7b5bded7e..832757085 100644 --- a/pkg/resources/management.cattle.io/v3/setting/validator.go +++ b/pkg/resources/management.cattle.io/v3/setting/validator.go @@ -28,6 +28,7 @@ const ( DeleteInactiveUserAfter = "delete-inactive-user-after" DisableInactiveUserAfter = "disable-inactive-user-after" AuthUserSessionTTLMinutes = "auth-user-session-ttl-minutes" + AuthUserSessionIdleTTLMinutes = "auth-user-session-idle-ttl-minutes" UserLastLoginDefault = "user-last-login-default" UserRetentionCron = "user-retention-cron" AgentTLSMode = "agent-tls-mode" @@ -148,6 +149,8 @@ func (a *admitter) admitCommonCreateUpdate(_, newSetting *v3.Setting) (*admissio err = a.validateUserRetentionCron(newSetting) case AuthUserSessionTTLMinutes: err = a.validateAuthUserSessionTTLMinutes(newSetting) + case AuthUserSessionIdleTTLMinutes: + err = a.validateAuthUserSessionIdleTTLMinutes(newSetting) default: } @@ -203,6 +206,53 @@ func (a *admitter) validateAuthUserSessionTTLMinutes(s *v3.Setting) error { return nil } +// validateAuthUserSessionIdleTTLMinutes validates the auth-user-session-idle-ttl-minutes setting +// to make sure it's a positive integer and that duration is not greater than +// auth-user-session-ttl-minutes settings if they are set. +// If it encounters an error fetching or parsing auth-user-session-ttl-minutes settings +// it logs but doesn't return the error to avoid rejecting the request. +func (a *admitter) validateAuthUserSessionIdleTTLMinutes(s *v3.Setting) error { + if s.Value == "" { + return nil + } + + userSessionIdleDuration, err := parseMinutes(s.Value) + if err != nil { + return field.TypeInvalid(valuePath, s.Value, err.Error()) + } + if userSessionIdleDuration < 1 { + return field.TypeInvalid(valuePath, s.Value, "negative value or less than 1 minute") + } + + isGreaterThanSetting := func(name string) bool { + setting, err := a.settingCache.Get(name) + if err != nil { + logrus.Warnf("[settingValidator] Failed to get %s: %s", name, err) + return false // Deliberately allow to proceed. + } + + // auth-user-session-ttl-minutes is expressed as minutes, + // so we use parseMinutes to compare it with the new + // auth-user-session-idle-ttl-minutes setting. + settingDur, err := parseMinutes(effectiveValue(setting)) + if err != nil { + logrus.Warnf("[settingValidator] Failed to parse %s: %s", name, err) + return false // Deliberately allow to proceed. + } + + // since auth-user-session-ttl-minutes = 0 is an available value, + // we check it as: settingDur >= 0. + return settingDur >= 0 && userSessionIdleDuration > settingDur + } + + // if auth-user-session-idle-ttl-minutes > auth-user-usesison-ttl-minutes + if isGreaterThanSetting(AuthUserSessionTTLMinutes) { + return field.Forbidden(valuePath, "can't be greater than "+AuthUserSessionTTLMinutes) + } + + return nil +} + var errLessThanAuthUserSessionTTL = fmt.Errorf("can't be less than %s", AuthUserSessionTTLMinutes) // isLessThanUserSessionTTL checks if the given duration is less than the value of diff --git a/pkg/resources/management.cattle.io/v3/setting/validator_test.go b/pkg/resources/management.cattle.io/v3/setting/validator_test.go index cfb5fe12f..397448c6b 100644 --- a/pkg/resources/management.cattle.io/v3/setting/validator_test.go +++ b/pkg/resources/management.cattle.io/v3/setting/validator_test.go @@ -1217,3 +1217,119 @@ func TestValidateAgentTLSMode(t *testing.T) { }) } } + +func (s *SettingSuite) TestValidateAuthUserSessionTTLIdleMinutesOnUpdate() { + s.validateAuthUserSessionTTLIdleMinutes(v1.Update, s.T()) +} + +func (s *SettingSuite) TestValidateAuthUserSessionTTLIdleMinutesOnCreate() { + s.validateAuthUserSessionTTLIdleMinutes(v1.Create, s.T()) +} + +func (s *SettingSuite) validateAuthUserSessionTTLIdleMinutes(op v1.Operation, t *testing.T) { + ctrl := gomock.NewController(t) + settingCache := fake.NewMockNonNamespacedCacheInterface[*v3.Setting](ctrl) + + tests := []struct { + name string + value string + mockSetup func() + allowed bool + }{ + { + name: "valid value", + value: "10", + mockSetup: func() { + settingCache.EXPECT().Get(gomock.Any()).DoAndReturn(func(_ string) (*v3.Setting, error) { + return &v3.Setting{ + Value: "", + Default: "960", + }, nil + }).Times(1) + }, + allowed: true, + }, + { + name: "value is too high", + value: "10000", + mockSetup: func() { + settingCache.EXPECT().Get(gomock.Any()).DoAndReturn(func(_ string) (*v3.Setting, error) { + return &v3.Setting{ + Value: "", + Default: "960", + }, nil + }).Times(1) + }, + allowed: false, + }, + { + name: "value is too low", + value: "-10", + mockSetup: func() {}, + allowed: false, + }, + { + name: "value cannot be 0", + value: "0", + mockSetup: func() {}, + allowed: false, + }, + { + name: "value cannot be 0.5", + value: "0.5", + mockSetup: func() {}, + allowed: false, + }, + { + name: "value cannot be a char", + value: "A", + mockSetup: func() {}, + allowed: false, + }, + { + name: "invalid value due to auth-session-user-ttl-minutes", + value: "12", + mockSetup: func() { + settingCache.EXPECT().Get(gomock.Any()).DoAndReturn(func(_ string) (*v3.Setting, error) { + return &v3.Setting{ + Value: "10", + Default: "", + }, nil + }).Times(1) + }, + allowed: false, + }, + { + name: "valid because auth-user-session-ttl-minutes equal 0 means token lives forever", + value: "1", + mockSetup: func() { + settingCache.EXPECT().Get(gomock.Any()).DoAndReturn(func(_ string) (*v3.Setting, error) { + return &v3.Setting{ + Value: "0", + Default: "", + }, nil + }).Times(1) + }, + allowed: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + tt.mockSetup() + + validator := setting.NewValidator(nil, settingCache) + s.testAdmit(t, validator, &v3.Setting{ + ObjectMeta: metav1.ObjectMeta{ + Name: setting.AuthUserSessionIdleTTLMinutes, + }, + }, &v3.Setting{ + ObjectMeta: metav1.ObjectMeta{ + Name: setting.AuthUserSessionIdleTTLMinutes, + }, + Value: tt.value, + }, op, tt.allowed) + }) + } +}