From 948350659094f140806c829a52b23eac172ec400 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Tue, 5 Feb 2019 21:20:11 +0100 Subject: [PATCH] auth token clean up job now runs on schedule and deletes all expired tokens delete tokens having created_at <= LoginMaxLifetimeDays or rotated_at <= LoginMaxInactiveLifetimeDays --- .../auth/authtoken/session_cleanup.go | 34 ++++++++--- .../auth/authtoken/session_cleanup_test.go | 56 +++++++++++++++---- 2 files changed, 69 insertions(+), 21 deletions(-) diff --git a/pkg/services/auth/authtoken/session_cleanup.go b/pkg/services/auth/authtoken/session_cleanup.go index ecee82767e4..2b8dfb7b4e2 100644 --- a/pkg/services/auth/authtoken/session_cleanup.go +++ b/pkg/services/auth/authtoken/session_cleanup.go @@ -6,14 +6,23 @@ import ( ) func (srv *UserAuthTokenServiceImpl) Run(ctx context.Context) error { - ticker := time.NewTicker(time.Hour * 12) - deleteSessionAfter := time.Hour * 24 * time.Duration(srv.Cfg.ExpiredTokensCleanupIntervalDays) + if srv.Cfg.ExpiredTokensCleanupIntervalDays <= 0 { + srv.log.Debug("cleanup of expired auth tokens are disabled") + return nil + } + + jobInterval := time.Duration(srv.Cfg.ExpiredTokensCleanupIntervalDays) * 24 * time.Hour + srv.log.Debug("cleanup of expired auth tokens are enabled", "intervalDays", srv.Cfg.ExpiredTokensCleanupIntervalDays) + + ticker := time.NewTicker(jobInterval) + maxInactiveLifetime := time.Duration(srv.Cfg.LoginMaxInactiveLifetimeDays) * 24 * time.Hour + maxLifetime := time.Duration(srv.Cfg.LoginMaxLifetimeDays) * 24 * time.Hour for { select { case <-ticker.C: - srv.ServerLockService.LockAndExecute(ctx, "delete expired auth tokens", time.Hour*12, func() { - srv.deleteOldSession(deleteSessionAfter) + srv.ServerLockService.LockAndExecute(ctx, "cleanup expired auth tokens", time.Hour*12, func() { + srv.deleteExpiredTokens(maxInactiveLifetime, maxLifetime) }) case <-ctx.Done(): @@ -22,17 +31,24 @@ func (srv *UserAuthTokenServiceImpl) Run(ctx context.Context) error { } } -func (srv *UserAuthTokenServiceImpl) deleteOldSession(deleteSessionAfter time.Duration) (int64, error) { - sql := `DELETE from user_auth_token WHERE rotated_at < ?` +func (srv *UserAuthTokenServiceImpl) deleteExpiredTokens(maxInactiveLifetime, maxLifetime time.Duration) (int64, error) { + createdBefore := getTime().Add(-maxLifetime) + rotatedBefore := getTime().Add(-maxInactiveLifetime) - deleteBefore := getTime().Add(-deleteSessionAfter) - res, err := srv.SQLStore.NewSession().Exec(sql, deleteBefore.Unix()) + srv.log.Debug("starting cleanup of expired auth tokens", "createdBefore", createdBefore, "rotatedBefore", rotatedBefore) + + sql := `DELETE from user_auth_token WHERE created_at <= ? OR rotated_at <= ?` + res, err := srv.SQLStore.NewSession().Exec(sql, createdBefore.Unix(), rotatedBefore.Unix()) if err != nil { return 0, err } affected, err := res.RowsAffected() - srv.log.Info("deleted old sessions", "count", affected) + if err != nil { + srv.log.Error("failed to cleanup expired auth tokens", "error", err) + return 0, nil + } + srv.log.Info("cleanup of expired auth tokens done", "count", affected) return affected, err } diff --git a/pkg/services/auth/authtoken/session_cleanup_test.go b/pkg/services/auth/authtoken/session_cleanup_test.go index bca1aa824eb..7b611b3263c 100644 --- a/pkg/services/auth/authtoken/session_cleanup_test.go +++ b/pkg/services/auth/authtoken/session_cleanup_test.go @@ -12,25 +12,57 @@ 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 - insertToken := func(token string, prev string, rotatedAt int64) { - ut := userAuthToken{AuthToken: token, PrevAuthToken: prev, CreatedAt: rotatedAt, RotatedAt: rotatedAt, UserAgent: "", ClientIp: ""} + insertToken := func(token string, prev string, createdAt, rotatedAt int64) { + ut := userAuthToken{AuthToken: token, PrevAuthToken: prev, CreatedAt: createdAt, RotatedAt: rotatedAt, UserAgent: "", ClientIp: ""} _, err := ctx.sqlstore.NewSession().Insert(&ut) So(err, ShouldBeNil) } - // insert three old tokens that should be deleted - for i := 0; i < 3; i++ { - insertToken(fmt.Sprintf("oldA%d", i), fmt.Sprintf("oldB%d", i), int64(i)) + t := time.Date(2018, 12, 13, 13, 45, 0, 0, time.UTC) + getTime = func() time.Time { + return t } - // insert three active tokens that should not be deleted - for i := 0; i < 3; i++ { - insertToken(fmt.Sprintf("newA%d", i), fmt.Sprintf("newB%d", i), getTime().Unix()) - } + Convey("should delete tokens where token rotation age is older than or equal 7 days", func() { + from := t.Add(-7 * 24 * time.Hour) - affected, err := ctx.tokenService.deleteOldSession(time.Hour) - So(err, ShouldBeNil) - So(affected, ShouldEqual, 3) + // insert three old tokens that should be deleted + for i := 0; i < 3; i++ { + insertToken(fmt.Sprintf("oldA%d", i), fmt.Sprintf("oldB%d", i), from.Unix(), from.Unix()) + } + + // insert three active tokens that should not be deleted + for i := 0; i < 3; i++ { + from = from.Add(time.Second) + insertToken(fmt.Sprintf("newA%d", i), fmt.Sprintf("newB%d", i), from.Unix(), from.Unix()) + } + + affected, err := ctx.tokenService.deleteExpiredTokens(7*24*time.Hour, 30*24*time.Hour) + So(err, ShouldBeNil) + So(affected, ShouldEqual, 3) + }) + + Convey("should delete tokens where token age is older than or equal 30 days", func() { + from := t.Add(-30 * 24 * time.Hour) + fromRotate := t.Add(-time.Second) + + // insert three old tokens that should be deleted + for i := 0; i < 3; i++ { + insertToken(fmt.Sprintf("oldA%d", i), fmt.Sprintf("oldB%d", i), from.Unix(), fromRotate.Unix()) + } + + // insert three active tokens that should not be deleted + for i := 0; i < 3; i++ { + from = from.Add(time.Second) + insertToken(fmt.Sprintf("newA%d", i), fmt.Sprintf("newB%d", i), from.Unix(), fromRotate.Unix()) + } + + affected, err := ctx.tokenService.deleteExpiredTokens(7*24*time.Hour, 30*24*time.Hour) + So(err, ShouldBeNil) + So(affected, ShouldEqual, 3) + }) }) }