fix(auth): correctly delete older password reset tokens in cron

pkg/user/token.go

@@ -122,6 +122,14 @@ func removeTokenByID(s *xorm.Session, u *User, kind TokenKind, id int64) (err er
 	return
 }
 
+// CleanupOldTokens removes all password reset and account deletion tokens older than 24 hours.
+func CleanupOldTokens(s *xorm.Session) (deleted int64, err error) {
+	deleted, err = s.
+		Where("created < ? AND (kind = ? OR kind = ?)", time.Now().Add(time.Hour*24*-1), TokenPasswordReset, TokenAccountDeletion).
+		Delete(&Token{})
+	return
+}
+
 // RegisterTokenCleanupCron registers a cron function to clean up all password reset tokens older than 24 hours
 func RegisterTokenCleanupCron() {
 	const logPrefix = "[User Token Cleanup Cron] "
@@ -130,9 +138,7 @@ func RegisterTokenCleanupCron() {
 		s := db.NewSession()
 		defer s.Close()
 
-		deleted, err := s.
-			Where("created > ? AND (kind = ? OR kind = ?)", time.Now().Add(time.Hour*24*-1), TokenPasswordReset, TokenAccountDeletion).
-			Delete(&Token{})
+		deleted, err := CleanupOldTokens(s)
 		if err != nil {
 			log.Errorf(logPrefix+"Error removing old password reset tokens: %s", err)
 			return

pkg/user/user_test.go

@@ -620,6 +620,68 @@ func TestUserPasswordReset(t *testing.T) {
 	})
 }
 
+func TestCleanupOldTokens(t *testing.T) {
+	t.Run("deletes old tokens and keeps recent ones", func(t *testing.T) {
+		db.LoadAndAssertFixtures(t)
+		s := db.NewSession()
+		defer s.Close()
+
+		// Insert a recent password reset token that should NOT be deleted
+		recentToken := &Token{
+			UserID: 1,
+			Token:  "recenttoken",
+			Kind:   TokenPasswordReset,
+		}
+		_, err := s.Insert(recentToken)
+		require.NoError(t, err)
+
+		deleted, err := CleanupOldTokens(s)
+		require.NoError(t, err)
+
+		// Fixtures have two old tokens that should be cleaned up:
+		// id=1 (kind=1, TokenPasswordReset, created 2021) and id=4 (kind=3, TokenAccountDeletion, created 2021)
+		assert.Equal(t, int64(2), deleted)
+
+		err = s.Commit()
+		require.NoError(t, err)
+
+		// The old password reset token from fixtures should be gone
+		db.AssertMissing(t, "user_tokens", map[string]interface{}{
+			"id": 1,
+		})
+		// The old account deletion token from fixtures should be gone
+		db.AssertMissing(t, "user_tokens", map[string]interface{}{
+			"id": 4,
+		})
+		// The recent token should still exist
+		db.AssertExists(t, "user_tokens", map[string]interface{}{
+			"token": "recenttoken",
+			"kind":  TokenPasswordReset,
+		}, false)
+	})
+	t.Run("does not delete email confirm tokens", func(t *testing.T) {
+		db.LoadAndAssertFixtures(t)
+		s := db.NewSession()
+		defer s.Close()
+
+		_, err := CleanupOldTokens(s)
+		require.NoError(t, err)
+
+		err = s.Commit()
+		require.NoError(t, err)
+
+		// The old email confirm tokens (kind=2) from fixtures should still exist
+		db.AssertExists(t, "user_tokens", map[string]interface{}{
+			"id":   2,
+			"kind": TokenEmailConfirm,
+		}, false)
+		db.AssertExists(t, "user_tokens", map[string]interface{}{
+			"id":   3,
+			"kind": TokenEmailConfirm,
+		}, false)
+	})
+}
+
 func TestConfirmDeletion(t *testing.T) {
 	t.Run("normal", func(t *testing.T) {
 		db.LoadAndAssertFixtures(t)