diff --git a/pkg/i18n/lang/en.json b/pkg/i18n/lang/en.json
index eaf22b241..9cf69f1c6 100644
--- a/pkg/i18n/lang/en.json
+++ b/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": {
diff --git a/pkg/models/api_tokens_expiry_cron.go b/pkg/models/api_tokens_expiry_cron.go
index 218a98f02..25090faaa 100644
--- a/pkg/models/api_tokens_expiry_cron.go
+++ b/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
diff --git a/pkg/models/api_tokens_expiry_cron_test.go b/pkg/models/api_tokens_expiry_cron_test.go
new file mode 100644
index 000000000..3c0ce6063
--- /dev/null
+++ b/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 .
+
+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{})
+ })
+}
diff --git a/pkg/models/api_tokens_expiry_notification.go b/pkg/models/api_tokens_expiry_notification.go
index 656ffe279..7ee60a11f 100644
--- a/pkg/models/api_tokens_expiry_notification.go
+++ b/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"))
}
diff --git a/pkg/models/api_tokens_expiry_notification_test.go b/pkg/models/api_tokens_expiry_notification_test.go
new file mode 100644
index 000000000..0f7327771
--- /dev/null
+++ b/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 .
+
+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)
+ })
+}
diff --git a/pkg/notifications/testing.go b/pkg/notifications/testing.go
index 2fd301f8f..59826278b 100644
--- a/pkg/notifications/testing.go
+++ b/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