test: add tests for API token expiry notifications and cron

pkg/i18n/lang/en.json

@@ -136,7 +136,7 @@
         "api_token": {
             "expiring": {
                 "week": {
-                    "subject": "Your API token \"%[1]s\" expires in 7 days",
+                    "subject": "Your API token \"%[1]s\" expires soon",
                     "message": "Your API token \"%[1]s\" will expire on %[2]s. If you still need it, please create a new token before it expires."
                 },
                 "day": {

pkg/models/api_tokens_expiry_cron.go

@@ -37,16 +37,12 @@ func RegisterAPITokenExpiryCheckCron() {
 		return
 	}
 
-	err := cron.Schedule("0 * * * *", checkForExpiringAPITokens)
+	err := cron.Schedule("0 * * * *", func() { checkForExpiringAPITokensAt(time.Now()) })
 	if err != nil {
 		log.Fatalf("Could not register API token expiry check cron: %s", err)
 	}
 }
 
-func checkForExpiringAPITokens() {
-	checkForExpiringAPITokensAt(time.Now())
-}
-
 func checkForExpiringAPITokensAt(now time.Time) {
 	const logPrefix = "[API Token Expiry Check] "
 
@@ -92,32 +88,34 @@ func checkForExpiringAPITokensAt(now time.Time) {
 			continue
 		}
 
-		// Determine which thresholds apply
-		expiresWithinOneDay := token.ExpiresAt.Before(oneDay) || token.ExpiresAt.Equal(oneDay)
-
-		if expiresWithinOneDay {
-			if err := sendTokenExpiryNotificationIfNew(s, u, token, &APITokenExpiringDayNotification{
+		// Send only the most urgent notification: 1-day if within 24h, otherwise 7-day
+		if token.ExpiresAt.Before(oneDay) || token.ExpiresAt.Equal(oneDay) {
+			if err := sendTokenExpiryNotificationIfNew(s, u, &APITokenExpiringDayNotification{
 				User:  u,
 				Token: token,
 			}); err != nil {
 				log.Errorf(logPrefix+"Error sending 1-day notification for token %d: %s", token.ID, err)
 			}
+			continue
 		}
 
-		// Always check the 7-day notification (token is within 7 days by the query)
-		if err := sendTokenExpiryNotificationIfNew(s, u, token, &APITokenExpiringWeekNotification{
+		if err := sendTokenExpiryNotificationIfNew(s, u, &APITokenExpiringWeekNotification{
 			User:  u,
 			Token: token,
 		}); err != nil {
 			log.Errorf(logPrefix+"Error sending 7-day notification for token %d: %s", token.ID, err)
 		}
 	}
+
+	if err := s.Commit(); err != nil {
+		log.Errorf(logPrefix+"Error committing session: %s", err)
+	}
 }
 
 // sendTokenExpiryNotificationIfNew checks whether a notification with the same
 // name and subject (token ID) has already been sent for this user. If not, it
 // sends the notification (both email and DB).
-func sendTokenExpiryNotificationIfNew(s *xorm.Session, u *user.User, _ *APIToken, n notifications.NotificationWithSubject) error {
+func sendTokenExpiryNotificationIfNew(s *xorm.Session, u *user.User, n notifications.NotificationWithSubject) error {
 	existing, err := notifications.GetNotificationsForNameAndUser(s, u.ID, n.Name(), n.SubjectID())
 	if err != nil {
 		return err

pkg/models/api_tokens_expiry_cron_test.go

@@ -0,0 +1,142 @@
+// Vikunja is a to-do list application to facilitate your life.
+// Copyright 2018-present Vikunja and contributors. All rights reserved.
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+package models
+
+import (
+	"testing"
+	"time"
+
+	"code.vikunja.io/api/pkg/db"
+	"code.vikunja.io/api/pkg/notifications"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestCheckForExpiringAPITokens(t *testing.T) {
+	t.Run("sends 7-day notification", func(t *testing.T) {
+		db.LoadAndAssertFixtures(t)
+		notifications.Fake()
+		t.Cleanup(notifications.Unfake)
+
+		now := time.Now()
+		s := db.NewSession()
+		defer s.Close()
+
+		token := &APIToken{
+			Title:          "Test 7-day token",
+			TokenSalt:      "salt1",
+			TokenHash:      "uniquehash7day",
+			TokenLastEight: "test1234",
+			APIPermissions: APIPermissions{"tasks": {"read"}},
+			ExpiresAt:      now.Add(6 * 24 * time.Hour),
+			OwnerID:        1,
+		}
+		_, err := s.Insert(token)
+		require.NoError(t, err)
+		require.NoError(t, s.Commit())
+
+		checkForExpiringAPITokensAt(now)
+
+		notifications.AssertSent(t, &APITokenExpiringWeekNotification{})
+	})
+
+	t.Run("sends only 1-day notification for token expiring within 24h", func(t *testing.T) {
+		db.LoadAndAssertFixtures(t)
+		notifications.Fake()
+		t.Cleanup(notifications.Unfake)
+
+		now := time.Now()
+		s := db.NewSession()
+		defer s.Close()
+
+		token := &APIToken{
+			Title:          "Test 1-day token",
+			TokenSalt:      "salt2",
+			TokenHash:      "uniquehash1day",
+			TokenLastEight: "test5678",
+			APIPermissions: APIPermissions{"tasks": {"read"}},
+			ExpiresAt:      now.Add(20 * time.Hour),
+			OwnerID:        1,
+		}
+		_, err := s.Insert(token)
+		require.NoError(t, err)
+		require.NoError(t, s.Commit())
+
+		checkForExpiringAPITokensAt(now)
+
+		notifications.AssertSent(t, &APITokenExpiringDayNotification{})
+		notifications.AssertNotSent(t, &APITokenExpiringWeekNotification{})
+	})
+
+	t.Run("does not send for tokens expiring in 30 days", func(t *testing.T) {
+		db.LoadAndAssertFixtures(t)
+		notifications.Fake()
+		t.Cleanup(notifications.Unfake)
+
+		now := time.Now()
+		s := db.NewSession()
+		defer s.Close()
+
+		token := &APIToken{
+			Title:          "Far future token",
+			TokenSalt:      "salt3",
+			TokenHash:      "uniquehash30day",
+			TokenLastEight: "test9012",
+			APIPermissions: APIPermissions{"tasks": {"read"}},
+			ExpiresAt:      now.Add(30 * 24 * time.Hour),
+			OwnerID:        1,
+		}
+		_, err := s.Insert(token)
+		require.NoError(t, err)
+		require.NoError(t, s.Commit())
+
+		checkForExpiringAPITokensAt(now)
+
+		// The existing fixture tokens expire in 2099, so no notifications should be sent
+		// for our 30-day token either
+		notifications.AssertNotSent(t, &APITokenExpiringWeekNotification{})
+		notifications.AssertNotSent(t, &APITokenExpiringDayNotification{})
+	})
+
+	t.Run("does not send for already expired tokens", func(t *testing.T) {
+		db.LoadAndAssertFixtures(t)
+		notifications.Fake()
+		t.Cleanup(notifications.Unfake)
+
+		now := time.Now()
+		s := db.NewSession()
+		defer s.Close()
+
+		token := &APIToken{
+			Title:          "Expired token",
+			TokenSalt:      "salt4",
+			TokenHash:      "uniquehashexpired",
+			TokenLastEight: "testexp1",
+			APIPermissions: APIPermissions{"tasks": {"read"}},
+			ExpiresAt:      now.Add(-24 * time.Hour),
+			OwnerID:        1,
+		}
+		_, err := s.Insert(token)
+		require.NoError(t, err)
+		require.NoError(t, s.Commit())
+
+		checkForExpiringAPITokensAt(now)
+
+		notifications.AssertNotSent(t, &APITokenExpiringWeekNotification{})
+		notifications.AssertNotSent(t, &APITokenExpiringDayNotification{})
+	})
+}

pkg/models/api_tokens_expiry_notification.go

@@ -33,7 +33,7 @@ func (n *APITokenExpiringWeekNotification) ToMail(lang string) *notifications.Ma
 	return notifications.NewMail().
 		Subject(i18n.T(lang, "notifications.api_token.expiring.week.subject", n.Token.Title)).
 		Greeting(i18n.T(lang, "notifications.greeting", n.User.GetName())).
-		Line(i18n.T(lang, "notifications.api_token.expiring.week.message", n.Token.Title, n.Token.ExpiresAt.Format("January 2, 2006"))).
+		Line(i18n.T(lang, "notifications.api_token.expiring.week.message", n.Token.Title, n.Token.ExpiresAt.Format("2006-01-02"))).
 		Action(i18n.T(lang, "notifications.api_token.expiring.action"), config.ServicePublicURL.GetString()+"user/settings/api-tokens").
 		Line(i18n.T(lang, "notifications.common.have_nice_day"))
 }
@@ -60,7 +60,7 @@ func (n *APITokenExpiringDayNotification) ToMail(lang string) *notifications.Mai
 	return notifications.NewMail().
 		Subject(i18n.T(lang, "notifications.api_token.expiring.day.subject", n.Token.Title)).
 		Greeting(i18n.T(lang, "notifications.greeting", n.User.GetName())).
-		Line(i18n.T(lang, "notifications.api_token.expiring.day.message", n.Token.Title, n.Token.ExpiresAt.Format("January 2, 2006"))).
+		Line(i18n.T(lang, "notifications.api_token.expiring.day.message", n.Token.Title, n.Token.ExpiresAt.Format("2006-01-02"))).
 		Action(i18n.T(lang, "notifications.api_token.expiring.action"), config.ServicePublicURL.GetString()+"user/settings/api-tokens").
 		Line(i18n.T(lang, "notifications.common.have_nice_day"))
 }

pkg/models/api_tokens_expiry_notification_test.go

@@ -0,0 +1,74 @@
+// Vikunja is a to-do list application to facilitate your life.
+// Copyright 2018-present Vikunja and contributors. All rights reserved.
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+package models
+
+import (
+	"testing"
+	"time"
+
+	"code.vikunja.io/api/pkg/user"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestAPITokenExpiringWeekNotification(t *testing.T) {
+	u := &user.User{ID: 1, Name: "Test User"}
+	token := &APIToken{ID: 42, Title: "My Token", ExpiresAt: time.Now().Add(7 * 24 * time.Hour)}
+
+	n := &APITokenExpiringWeekNotification{User: u, Token: token}
+
+	t.Run("Name", func(t *testing.T) {
+		assert.Equal(t, "api_token.expiring.week", n.Name())
+	})
+
+	t.Run("SubjectID", func(t *testing.T) {
+		assert.Equal(t, int64(42), n.SubjectID())
+	})
+
+	t.Run("ToDB", func(t *testing.T) {
+		assert.NotNil(t, n.ToDB())
+	})
+
+	t.Run("ToMail", func(t *testing.T) {
+		mail := n.ToMail("en")
+		assert.NotNil(t, mail)
+	})
+}
+
+func TestAPITokenExpiringDayNotification(t *testing.T) {
+	u := &user.User{ID: 1, Name: "Test User"}
+	token := &APIToken{ID: 99, Title: "CI Token", ExpiresAt: time.Now().Add(24 * time.Hour)}
+
+	n := &APITokenExpiringDayNotification{User: u, Token: token}
+
+	t.Run("Name", func(t *testing.T) {
+		assert.Equal(t, "api_token.expiring.day", n.Name())
+	})
+
+	t.Run("SubjectID", func(t *testing.T) {
+		assert.Equal(t, int64(99), n.SubjectID())
+	})
+
+	t.Run("ToDB", func(t *testing.T) {
+		assert.NotNil(t, n.ToDB())
+	})
+
+	t.Run("ToMail", func(t *testing.T) {
+		mail := n.ToMail("en")
+		assert.NotNil(t, mail)
+	})
+}

pkg/notifications/testing.go

@@ -32,6 +32,13 @@ func Fake() {
 	sentTestNotifications = nil
 }
 
+// Unfake disables test mode so that subsequent calls to Notify write to the
+// database again. Call this (or use t.Cleanup) after tests that use Fake().
+func Unfake() {
+	isUnderTest = false
+	sentTestNotifications = nil
+}
+
 // AssertSent asserts a notification has been sent
 func AssertSent(t *testing.T, n Notification) {
 	var found bool