From 8d971ab2f2a834d7dd1b48073d5a67480a6a9809 Mon Sep 17 00:00:00 2001 From: Hansuuuuuuuuuu <14076998+Hansuuuuuuuuuu@users.noreply.github.com> Date: Mon, 14 Sep 2020 21:57:38 +0800 Subject: [PATCH] Auth: Replace maximum inactive/lifetime settings of days to duration (#27150) Allows login_maximum_inactive_lifetime_duration and login_maximum_lifetime_duration to be configured using time.Duration-compatible values while retaining backward compatibility. Fixes #17554 Co-authored-by: Marcus Efraimsson --- conf/defaults.ini | 8 ++--- conf/sample.ini | 8 ++--- docs/sources/auth/overview.md | 10 +++--- pkg/api/login.go | 2 +- pkg/middleware/middleware.go | 9 +++-- pkg/middleware/middleware_test.go | 12 +++---- pkg/services/auth/auth_token.go | 6 ++-- pkg/services/auth/auth_token_test.go | 7 ++-- pkg/services/auth/token_cleanup.go | 9 ++--- pkg/services/auth/token_cleanup_test.go | 10 +++--- pkg/setting/setting.go | 43 ++++++++++++++++------ pkg/setting/setting_test.go | 47 +++++++++++++++++++++++++ 12 files changed, 122 insertions(+), 49 deletions(-) diff --git a/conf/defaults.ini b/conf/defaults.ini index 4f51c7073f1..81b32540198 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -279,11 +279,11 @@ editors_can_admin = false # Login cookie name login_cookie_name = grafana_session -# The lifetime (days) an authenticated user can be inactive before being required to login at next visit. Default is 7 days. -login_maximum_inactive_lifetime_days = 7 +# The maximum lifetime (duration) an authenticated user can be inactive before being required to login at next visit. Default is 7 days (7d). This setting should be expressed as a duration, e.g. 5m (minutes), 6h (hours), 10d (days), 2w (weeks), 1M (month). The lifetime resets at each successful token rotation (token_rotation_interval_minutes). +login_maximum_inactive_lifetime_duration = -# The maximum lifetime (days) an authenticated user can be logged in since login time before being required to login. Default is 30 days. -login_maximum_lifetime_days = 30 +# The maximum lifetime (duration) an authenticated user can be logged in since login time before being required to login. Default is 30 days (30d). This setting should be expressed as a duration, e.g. 5m (minutes), 6h (hours), 10d (days), 2w (weeks), 1M (month). +login_maximum_lifetime_duration = # How often should auth tokens be rotated for authenticated users when being active. The default is each 10 minutes. token_rotation_interval_minutes = 10 diff --git a/conf/sample.ini b/conf/sample.ini index 89b1a5edae3..f0254e24ed4 100644 --- a/conf/sample.ini +++ b/conf/sample.ini @@ -278,11 +278,11 @@ # Login cookie name ;login_cookie_name = grafana_session -# The lifetime (days) an authenticated user can be inactive before being required to login at next visit. Default is 7 days, -;login_maximum_inactive_lifetime_days = 7 +# The maximum lifetime (duration) an authenticated user can be inactive before being required to login at next visit. Default is 7 days (7d). This setting should be expressed as a duration, e.g. 5m (minutes), 6h (hours), 10d (days), 2w (weeks), 1M (month). The lifetime resets at each successful token rotation +;login_maximum_inactive_lifetime_duration = -# The maximum lifetime (days) an authenticated user can be logged in since login time before being required to login. Default is 30 days. -;login_maximum_lifetime_days = 30 +# The maximum lifetime (duration) an authenticated user can be logged in since login time before being required to login. Default is 30 days (30d). This setting should be expressed as a duration, e.g. 5m (minutes), 6h (hours), 10d (days), 2w (weeks), 1M (month). +;login_maximum_lifetime_duration = # How often should auth tokens be rotated for authenticated users when being active. The default is each 10 minutes. ;token_rotation_interval_minutes = 10 diff --git a/docs/sources/auth/overview.md b/docs/sources/auth/overview.md index 53ef293b2c2..40813252680 100644 --- a/docs/sources/auth/overview.md +++ b/docs/sources/auth/overview.md @@ -59,11 +59,13 @@ Example: # Login cookie name login_cookie_name = grafana_session -# The lifetime (days) an authenticated user can be inactive before being required to login at next visit. Default is 7 days. -login_maximum_inactive_lifetime_days = 7 -# The maximum lifetime (days) an authenticated user can be logged in since login time before being required to login. Default is 30 days. -login_maximum_lifetime_days = 30 +# The maximum lifetime (duration) an authenticated user can be inactive before being required to login at next visit. Default is 7 days (7d). This setting should be expressed as a duration, e.g. 5m (minutes), 6h (hours), 10d (days), 2w (weeks), 1M (month). The lifetime resets at each successful token rotation (token_rotation_interval_minutes). +login_maximum_inactive_lifetime_duration = + + +# The maximum lifetime (duration) an authenticated user can be logged in since login time before being required to login. Default is 30 days (30d). This setting should be expressed as a duration, e.g. 5m (minutes), 6h (hours), 10d (days), 2w (weeks), 1M (month). +login_maximum_lifetime_duration = # How often should auth tokens be rotated for authenticated users when being active. The default is each 10 minutes. token_rotation_interval_minutes = 10 diff --git a/pkg/api/login.go b/pkg/api/login.go index 96c2b3e26ad..9719fb5bdd9 100644 --- a/pkg/api/login.go +++ b/pkg/api/login.go @@ -249,7 +249,7 @@ func (hs *HTTPServer) loginUserWithUser(user *models.User, c *models.ReqContext) } hs.log.Info("Successful Login", "User", user.Email) - middleware.WriteSessionCookie(c, userToken.UnhashedToken, hs.Cfg.LoginMaxLifetimeDays) + middleware.WriteSessionCookie(c, userToken.UnhashedToken, hs.Cfg.LoginMaxLifetime) return nil } diff --git a/pkg/middleware/middleware.go b/pkg/middleware/middleware.go index 72bf1d95ce2..7ce4fa7ff34 100644 --- a/pkg/middleware/middleware.go +++ b/pkg/middleware/middleware.go @@ -261,22 +261,21 @@ func rotateEndOfRequestFunc(ctx *models.ReqContext, authTokenService models.User } if rotated { - WriteSessionCookie(ctx, token.UnhashedToken, setting.LoginMaxLifetimeDays) + WriteSessionCookie(ctx, token.UnhashedToken, setting.LoginMaxLifetime) } } } -func WriteSessionCookie(ctx *models.ReqContext, value string, maxLifetimeDays int) { +func WriteSessionCookie(ctx *models.ReqContext, value string, maxLifetime time.Duration) { if setting.Env == setting.DEV { ctx.Logger.Info("New token", "unhashed token", value) } var maxAge int - if maxLifetimeDays <= 0 { + if maxLifetime <= 0 { maxAge = -1 } else { - maxAgeHours := (time.Duration(setting.LoginMaxLifetimeDays) * 24 * time.Hour) + time.Hour - maxAge = int(maxAgeHours.Seconds()) + maxAge = int(maxLifetime.Seconds()) } WriteCookie(ctx.Resp, setting.LoginCookieName, url.QueryEscape(value), maxAge, newCookieOptions) diff --git a/pkg/middleware/middleware_test.go b/pkg/middleware/middleware_test.go index fcc91b6e1e2..2ba5eec4662 100644 --- a/pkg/middleware/middleware_test.go +++ b/pkg/middleware/middleware_test.go @@ -17,6 +17,7 @@ import ( "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/bus" + "github.com/grafana/grafana/pkg/components/gtime" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/remotecache" authproxy "github.com/grafana/grafana/pkg/middleware/auth_proxy" @@ -253,8 +254,7 @@ func TestMiddlewareContext(t *testing.T) { return true, nil } - maxAgeHours := (time.Duration(setting.LoginMaxLifetimeDays) * 24 * time.Hour) - maxAge := (maxAgeHours + time.Hour).Seconds() + maxAge := int(setting.LoginMaxLifetime.Seconds()) sameSitePolicies := []http.SameSite{ http.SameSiteNoneMode, @@ -272,7 +272,7 @@ func TestMiddlewareContext(t *testing.T) { Value: "rotated", Path: expectedCookiePath, HttpOnly: true, - MaxAge: int(maxAge), + MaxAge: maxAge, Secure: setting.CookieSecure, SameSite: sameSitePolicy, } @@ -303,7 +303,7 @@ func TestMiddlewareContext(t *testing.T) { Value: "rotated", Path: expectedCookiePath, HttpOnly: true, - MaxAge: int(maxAge), + MaxAge: maxAge, Secure: setting.CookieSecure, } @@ -546,7 +546,7 @@ func middlewareScenario(t *testing.T, desc string, fn scenarioFunc) { defer bus.ClearBusHandlers() setting.LoginCookieName = "grafana_session" - setting.LoginMaxLifetimeDays = 30 + setting.LoginMaxLifetime, _ = gtime.ParseInterval("30d") sc := &scenarioContext{} @@ -637,7 +637,7 @@ func TestTokenRotationAtEndOfRequest(t *testing.T) { func initTokenRotationTest(ctx context.Context) (*models.ReqContext, *httptest.ResponseRecorder, error) { setting.LoginCookieName = "login_token" - setting.LoginMaxLifetimeDays = 7 + setting.LoginMaxLifetime, _ = gtime.ParseInterval("7d") rr := httptest.NewRecorder() req, err := http.NewRequestWithContext(ctx, "", "", nil) diff --git a/pkg/services/auth/auth_token.go b/pkg/services/auth/auth_token.go index 4a4d0e9e637..6b742b35644 100644 --- a/pkg/services/auth/auth_token.go +++ b/pkg/services/auth/auth_token.go @@ -397,13 +397,11 @@ func (s *UserAuthTokenService) GetUserTokens(ctx context.Context, userId int64) } func (s *UserAuthTokenService) createdAfterParam() int64 { - tokenMaxLifetime := time.Duration(s.Cfg.LoginMaxLifetimeDays) * 24 * time.Hour - return getTime().Add(-tokenMaxLifetime).Unix() + return getTime().Add(-s.Cfg.LoginMaxLifetime).Unix() } func (s *UserAuthTokenService) rotatedAfterParam() int64 { - tokenMaxInactiveLifetime := time.Duration(s.Cfg.LoginMaxInactiveLifetimeDays) * 24 * time.Hour - return getTime().Add(-tokenMaxInactiveLifetime).Unix() + return getTime().Add(-s.Cfg.LoginMaxInactiveLifetime).Unix() } func hashToken(token string) string { diff --git a/pkg/services/auth/auth_token_test.go b/pkg/services/auth/auth_token_test.go index c898a68a302..99f737411d1 100644 --- a/pkg/services/auth/auth_token_test.go +++ b/pkg/services/auth/auth_token_test.go @@ -494,13 +494,14 @@ func TestUserAuthToken(t *testing.T) { func createTestContext(t *testing.T) *testContext { t.Helper() - + maxInactiveDurationVal, _ := time.ParseDuration("168h") + maxLifetimeDurationVal, _ := time.ParseDuration("720h") sqlstore := sqlstore.InitTestDB(t) tokenService := &UserAuthTokenService{ SQLStore: sqlstore, Cfg: &setting.Cfg{ - LoginMaxInactiveLifetimeDays: 7, - LoginMaxLifetimeDays: 30, + LoginMaxInactiveLifetime: maxInactiveDurationVal, + LoginMaxLifetime: maxLifetimeDurationVal, TokenRotationIntervalMinutes: 10, }, log: log.New("test-logger"), diff --git a/pkg/services/auth/token_cleanup.go b/pkg/services/auth/token_cleanup.go index 683a9ee47b6..9658eff9ab4 100644 --- a/pkg/services/auth/token_cleanup.go +++ b/pkg/services/auth/token_cleanup.go @@ -8,11 +8,12 @@ import ( ) func (srv *UserAuthTokenService) Run(ctx context.Context) error { + var err error ticker := time.NewTicker(time.Hour) - maxInactiveLifetime := time.Duration(srv.Cfg.LoginMaxInactiveLifetimeDays) * 24 * time.Hour - maxLifetime := time.Duration(srv.Cfg.LoginMaxLifetimeDays) * 24 * time.Hour + maxInactiveLifetime := srv.Cfg.LoginMaxInactiveLifetime + maxLifetime := srv.Cfg.LoginMaxLifetime - err := srv.ServerLockService.LockAndExecute(ctx, "cleanup expired auth tokens", time.Hour*12, func() { + err = srv.ServerLockService.LockAndExecute(ctx, "cleanup expired auth tokens", time.Hour*12, func() { if _, err := srv.deleteExpiredTokens(ctx, maxInactiveLifetime, maxLifetime); err != nil { srv.log.Error("An error occurred while deleting expired tokens", "err", err) } @@ -24,7 +25,7 @@ func (srv *UserAuthTokenService) Run(ctx context.Context) error { for { select { case <-ticker.C: - err := srv.ServerLockService.LockAndExecute(ctx, "cleanup expired auth tokens", time.Hour*12, func() { + err = srv.ServerLockService.LockAndExecute(ctx, "cleanup expired auth tokens", time.Hour*12, func() { if _, err := srv.deleteExpiredTokens(ctx, maxInactiveLifetime, maxLifetime); err != nil { srv.log.Error("An error occurred while deleting expired tokens", "err", err) } diff --git a/pkg/services/auth/token_cleanup_test.go b/pkg/services/auth/token_cleanup_test.go index bb80d91a955..86105d550c1 100644 --- a/pkg/services/auth/token_cleanup_test.go +++ b/pkg/services/auth/token_cleanup_test.go @@ -12,8 +12,10 @@ import ( func TestUserAuthTokenCleanup(t *testing.T) { Convey("Test user auth token cleanup", t, func() { ctx := createTestContext(t) - ctx.tokenService.Cfg.LoginMaxInactiveLifetimeDays = 7 - ctx.tokenService.Cfg.LoginMaxLifetimeDays = 30 + maxInactiveLifetime, _ := time.ParseDuration("168h") + maxLifetime, _ := time.ParseDuration("720h") + ctx.tokenService.Cfg.LoginMaxInactiveLifetime = maxInactiveLifetime + ctx.tokenService.Cfg.LoginMaxLifetime = maxLifetime insertToken := func(token string, prev string, createdAt, rotatedAt int64) { ut := userAuthToken{AuthToken: token, PrevAuthToken: prev, CreatedAt: createdAt, RotatedAt: rotatedAt, UserAgent: "", ClientIp: ""} @@ -27,7 +29,7 @@ func TestUserAuthTokenCleanup(t *testing.T) { } Convey("should delete tokens where token rotation age is older than or equal 7 days", func() { - from := t.Add(-7 * 24 * time.Hour) + from := t.Add(-168 * time.Hour) // insert three old tokens that should be deleted for i := 0; i < 3; i++ { @@ -40,7 +42,7 @@ func TestUserAuthTokenCleanup(t *testing.T) { insertToken(fmt.Sprintf("newA%d", i), fmt.Sprintf("newB%d", i), from.Unix(), from.Unix()) } - affected, err := ctx.tokenService.deleteExpiredTokens(context.Background(), 7*24*time.Hour, 30*24*time.Hour) + affected, err := ctx.tokenService.deleteExpiredTokens(context.Background(), 168*time.Hour, 30*24*time.Hour) So(err, ShouldBeNil) So(affected, ShouldEqual, 3) }) diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index 4b76871202c..207d698b541 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -140,10 +140,10 @@ var ( ViewersCanEdit bool // Http auth - AdminUser string - AdminPassword string - LoginCookieName string - LoginMaxLifetimeDays int + AdminUser string + AdminPassword string + LoginCookieName string + LoginMaxLifetime time.Duration AnonymousEnabled bool AnonymousOrgName string @@ -278,8 +278,8 @@ type Cfg struct { // Auth LoginCookieName string - LoginMaxInactiveLifetimeDays int - LoginMaxLifetimeDays int + LoginMaxInactiveLifetime time.Duration + LoginMaxLifetime time.Duration TokenRotationIntervalMinutes int // OAuth @@ -946,15 +946,38 @@ func readSecuritySettings(iniFile *ini.File, cfg *Cfg) error { return nil } -func readAuthSettings(iniFile *ini.File, cfg *Cfg) error { +func readAuthSettings(iniFile *ini.File, cfg *Cfg) (err error) { auth := iniFile.Section("auth") LoginCookieName = valueAsString(auth, "login_cookie_name", "grafana_session") cfg.LoginCookieName = LoginCookieName - cfg.LoginMaxInactiveLifetimeDays = auth.Key("login_maximum_inactive_lifetime_days").MustInt(7) + maxInactiveDaysVal := auth.Key("login_maximum_inactive_lifetime_days").MustString("") + if maxInactiveDaysVal != "" { + maxInactiveDaysVal = fmt.Sprintf("%sd", maxInactiveDaysVal) + cfg.Logger.Warn("[Deprecated] the configuration setting 'login_maximum_inactive_lifetime_days' is deprecated, please use 'login_maximum_inactive_lifetime_duration' instead") + } else { + maxInactiveDaysVal = "7d" + } + maxInactiveDurationVal := valueAsString(auth, "login_maximum_inactive_lifetime_duration", maxInactiveDaysVal) + cfg.LoginMaxInactiveLifetime, err = gtime.ParseInterval(maxInactiveDurationVal) + if err != nil { + return err + } + + maxLifetimeDaysVal := auth.Key("login_maximum_lifetime_days").MustString("") + if maxLifetimeDaysVal != "" { + maxLifetimeDaysVal = fmt.Sprintf("%sd", maxLifetimeDaysVal) + cfg.Logger.Warn("[Deprecated] the configuration setting 'login_maximum_lifetime_days' is deprecated, please use 'login_maximum_lifetime_duration' instead") + } else { + maxLifetimeDaysVal = "7d" + } + maxLifetimeDurationVal := valueAsString(auth, "login_maximum_lifetime_duration", maxLifetimeDaysVal) + cfg.LoginMaxLifetime, err = gtime.ParseInterval(maxLifetimeDurationVal) + if err != nil { + return err + } + LoginMaxLifetime = cfg.LoginMaxLifetime - LoginMaxLifetimeDays = auth.Key("login_maximum_lifetime_days").MustInt(30) - cfg.LoginMaxLifetimeDays = LoginMaxLifetimeDays cfg.ApiKeyMaxSecondsToLive = auth.Key("api_key_max_seconds_to_live").MustInt64(-1) cfg.TokenRotationIntervalMinutes = auth.Key("token_rotation_interval_minutes").MustInt(10) diff --git a/pkg/setting/setting_test.go b/pkg/setting/setting_test.go index 3b01d6cd9b8..3434c46ed58 100644 --- a/pkg/setting/setting_test.go +++ b/pkg/setting/setting_test.go @@ -8,6 +8,7 @@ import ( "runtime" "strings" "testing" + "time" "github.com/stretchr/testify/require" @@ -314,3 +315,49 @@ func TestParseAppUrlAndSubUrl(t *testing.T) { require.Equal(t, tc.expectedAppSubURL, appSubURL) } } +func TestAuthDurationSettings(t *testing.T) { + f := ini.Empty() + cfg := NewCfg() + sec, err := f.NewSection("auth") + require.NoError(t, err) + _, err = sec.NewKey("login_maximum_inactive_lifetime_days", "10") + require.NoError(t, err) + _, err = sec.NewKey("login_maximum_inactive_lifetime_duration", "") + require.NoError(t, err) + maxInactiveDaysTest, _ := time.ParseDuration("240h") + err = readAuthSettings(f, cfg) + require.NoError(t, err) + require.Equal(t, maxInactiveDaysTest, cfg.LoginMaxInactiveLifetime) + + f = ini.Empty() + sec, err = f.NewSection("auth") + require.NoError(t, err) + _, err = sec.NewKey("login_maximum_inactive_lifetime_duration", "824h") + require.NoError(t, err) + maxInactiveDurationTest, _ := time.ParseDuration("824h") + err = readAuthSettings(f, cfg) + require.NoError(t, err) + require.Equal(t, maxInactiveDurationTest, cfg.LoginMaxInactiveLifetime) + + f = ini.Empty() + sec, err = f.NewSection("auth") + require.NoError(t, err) + _, err = sec.NewKey("login_maximum_lifetime_days", "24") + require.NoError(t, err) + _, err = sec.NewKey("login_maximum_lifetime_duration", "") + require.NoError(t, err) + maxLifetimeDaysTest, _ := time.ParseDuration("576h") + err = readAuthSettings(f, cfg) + require.NoError(t, err) + require.Equal(t, maxLifetimeDaysTest, cfg.LoginMaxLifetime) + + f = ini.Empty() + sec, err = f.NewSection("auth") + require.NoError(t, err) + _, err = sec.NewKey("login_maximum_lifetime_duration", "824h") + require.NoError(t, err) + maxLifetimeDurationTest, _ := time.ParseDuration("824h") + err = readAuthSettings(f, cfg) + require.NoError(t, err) + require.Equal(t, maxLifetimeDurationTest, cfg.LoginMaxLifetime) +}