Skip to content

feat(setting): add validation for auth-user-session-idle-ttl-minutes #510

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions pkg/resources/management.cattle.io/v3/setting/Setting.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
50 changes: 50 additions & 0 deletions pkg/resources/management.cattle.io/v3/setting/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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:
}

Expand Down Expand Up @@ -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
Expand Down
116 changes: 116 additions & 0 deletions pkg/resources/management.cattle.io/v3/setting/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}
Loading