From 412215ee2f1e59878e5f54343edd20d3fc7356ba Mon Sep 17 00:00:00 2001 From: kolaente Date: Fri, 27 Feb 2026 14:12:51 +0100 Subject: [PATCH] fix(auth): correctly delete older password reset tokens in cron --- pkg/user/token.go | 12 ++++++--- pkg/user/user_test.go | 62 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 3 deletions(-) diff --git a/pkg/user/token.go b/pkg/user/token.go index a2e3dd60f..565270289 100644 --- a/pkg/user/token.go +++ b/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 diff --git a/pkg/user/user_test.go b/pkg/user/user_test.go index 488632f68..24aa27c58 100644 --- a/pkg/user/user_test.go +++ b/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)