auth token clean up job now runs on schedule and deletes all expired tokens

delete tokens having created_at <= LoginMaxLifetimeDays or
rotated_at <= LoginMaxInactiveLifetimeDays

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
 }

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)
 		}
 
+		t := time.Date(2018, 12, 13, 13, 45, 0, 0, time.UTC)
+		getTime = func() time.Time {
+			return t
+		}
+
+		Convey("should delete tokens where token rotation age is older than or equal 7 days", func() {
+			from := t.Add(-7 * 24 * time.Hour)
+
 			// 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))
+				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++ {
-			insertToken(fmt.Sprintf("newA%d", i), fmt.Sprintf("newB%d", i), getTime().Unix())
+				from = from.Add(time.Second)
+				insertToken(fmt.Sprintf("newA%d", i), fmt.Sprintf("newB%d", i), from.Unix(), from.Unix())
 			}
 
-		affected, err := ctx.tokenService.deleteOldSession(time.Hour)
+			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)
+		})
+	})
 }