Refactor user email confirmation + password reset handling (#919)

Co-authored-by: kolaente <k@knt.li>
Reviewed-on: https://kolaente.dev/vikunja/api/pulls/919
Co-authored-by: konrad <konrad@kola-entertainments.de>
Co-committed-by: konrad <konrad@kola-entertainments.de>

pkg/cmd/user.go

@@ -135,7 +135,7 @@ var userListCmd = &cobra.Command{
 			"ID",
 			"Username",
 			"Email",
-			"Active",
+			"Status",
 			"Created",
 			"Updated",
 		})
@@ -145,7 +145,7 @@ var userListCmd = &cobra.Command{
 				strconv.FormatInt(u.ID, 10),
 				u.Username,
 				u.Email,
-				strconv.FormatBool(u.IsActive),
+				u.Status.String(),
 				u.Created.Format(time.RFC3339),
 				u.Updated.Format(time.RFC3339),
 			})
@@ -277,11 +277,15 @@ var userChangeEnabledCmd = &cobra.Command{
 		u := getUserFromArg(s, args[0])
 
 		if userFlagEnableUser {
-			u.IsActive = true
+			u.Status = user.StatusActive
 		} else if userFlagDisableUser {
-			u.IsActive = false
+			u.Status = user.StatusDisabled
 		} else {
-			u.IsActive = !u.IsActive
+			if u.Status == user.StatusActive {
+				u.Status = user.StatusDisabled
+			} else {
+				u.Status = user.StatusActive
+			}
 		}
 		_, err := user.UpdateUser(s, u)
 		if err != nil {
@@ -293,6 +297,6 @@ var userChangeEnabledCmd = &cobra.Command{
 			log.Fatalf("Error saving everything: %s", err)
 		}
 
-		fmt.Printf("User status successfully changed, user is now active: %t.\n", u.IsActive)
+		fmt.Printf("User status successfully changed, status is now \"%s\"\n", u.Status)
 	},
 }

pkg/db/fixtures/user_tokens.yml

@@ -0,0 +1,18 @@
+-
+  id: 1
+  user_id: 3
+  token: 'passwordresettesttoken'
+  kind: 1
+  created: 2021-07-12 00:00:11
+-
+  id: 2
+  user_id: 4
+  token: 'tiepiQueed8ahc7zeeFe1eveiy4Ein8osooxegiephauph2Ael'
+  kind: 2
+  created: 2021-07-12 00:00:12
+-
+  id: 3
+  user_id: 5
+  token: 'tiepiQueed8ahc7zeeFe1eveiy4Ein8osooxegiephauph2Aei'
+  kind: 2
+  created: 2021-07-12 00:00:13

pkg/db/fixtures/users.yml

@@ -3,7 +3,6 @@
   username: 'user1'
   password: '$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.' # 1234
   email: 'user1@example.com'
-  is_active: true
   issuer: local
   updated: 2018-12-02 15:13:12
   created: 2018-12-01 15:13:12
@@ -20,7 +19,6 @@
   username: 'user3'
   password: '$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.' # 1234
   email: 'user3@example.com'
-  password_reset_token: passwordresettesttoken
   issuer: local
   updated: 2018-12-02 15:13:12
   created: 2018-12-01 15:13:12
@@ -29,7 +27,7 @@
   username: 'user4'
   password: '$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.' # 1234
   email: 'user4@example.com'
-  email_confirm_token: tiepiQueed8ahc7zeeFe1eveiy4Ein8osooxegiephauph2Ael
+  status: 1
   issuer: local
   updated: 2018-12-02 15:13:12
   created: 2018-12-01 15:13:12
@@ -38,8 +36,7 @@
   username: 'user5'
   password: '$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.' # 1234
   email: 'user5@example.com'
-  email_confirm_token: tiepiQueed8ahc7zeeFe1eveiy4Ein8osooxegiephauph2Ael
+  status: 1
-  is_active: false
   issuer: local
   updated: 2018-12-02 15:13:12
   created: 2018-12-01 15:13:12
@@ -48,7 +45,6 @@
   username: 'user6'
   password: '$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.' # 1234
   email: 'user6@example.com'
-  is_active: true
   issuer: local
   updated: 2018-12-02 15:13:12
   created: 2018-12-01 15:13:12
@@ -56,7 +52,6 @@
   username: 'user7'
   password: '$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.' # 1234
   email: 'user7@example.com'
-  is_active: true
   issuer: local
   discoverable_by_email: true
   updated: 2018-12-02 15:13:12
@@ -65,7 +60,6 @@
   username: 'user8'
   password: '$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.' # 1234
   email: 'user8@example.com'
-  is_active: true
   issuer: local
   updated: 2018-12-02 15:13:12
   created: 2018-12-01 15:13:12
@@ -73,7 +67,6 @@
   username: 'user9'
   password: '$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.' # 1234
   email: 'user9@example.com'
-  is_active: true
   issuer: local
   updated: 2018-12-02 15:13:12
   created: 2018-12-01 15:13:12
@@ -81,7 +74,6 @@
   username: 'user10'
   password: '$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.' # 1234
   email: 'user10@example.com'
-  is_active: true
   issuer: local
   updated: 2018-12-02 15:13:12
   created: 2018-12-01 15:13:12
@@ -90,7 +82,6 @@
   name: 'Some one else'
   password: '$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.' # 1234
   email: 'user11@example.com'
-  is_active: true
   issuer: local
   updated: 2018-12-02 15:13:12
   created: 2018-12-01 15:13:12
@@ -99,7 +90,6 @@
   name: 'Name with spaces'
   password: '$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.' # 1234
   email: 'user12@example.com'
-  is_active: true
   issuer: local
   discoverable_by_name: true
   updated: 2018-12-02 15:13:12
@@ -108,7 +98,6 @@
   username: 'user13'
   password: '$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.' # 1234
   email: 'user14@example.com'
-  is_active: true
   issuer: local
   updated: 2018-12-02 15:13:12
   created: 2018-12-01 15:13:12
@@ -116,7 +105,6 @@
   username: 'user14'
   password: '$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.' # 1234
   email: 'user15@some.service.com'
-  is_active: true
   issuer: 'https://some.service.com'
   subject: '12345'
   updated: 2018-12-02 15:13:12

pkg/db/test_fixtures.go

@@ -59,10 +59,6 @@ func InitFixtures(tablenames ...string) (err error) {
 	}
 
 	fixtures, err = testfixtures.New(loaderOptions...)
-	if err != nil {
-		return err
-	}
-
 	return err
 }
 
@@ -106,7 +102,7 @@ func LoadFixtures() error {
 			}
 		}
 	}
-	return err
+	return nil
 }
 
 // LoadAndAssertFixtures loads all fixtures defined before and asserts they are correctly loaded

pkg/initialize/init.go

@@ -94,6 +94,7 @@ func FullInit() {
 	cron.Init()
 	models.RegisterReminderCron()
 	models.RegisterOverdueReminderCron()
+	user.RegisterTokenCleanupCron()
 
 	// Start processing events
 	go func() {

pkg/integrations/integrations.go

@@ -24,11 +24,11 @@ import (
 	"strings"
 	"testing"
 
-	"code.vikunja.io/api/pkg/events"
+	"code.vikunja.io/api/pkg/files"
 
 	"code.vikunja.io/api/pkg/config"
 	"code.vikunja.io/api/pkg/db"
-	"code.vikunja.io/api/pkg/files"
+	"code.vikunja.io/api/pkg/events"
 	"code.vikunja.io/api/pkg/models"
 	"code.vikunja.io/api/pkg/modules/auth"
 	"code.vikunja.io/api/pkg/routes"
@@ -47,7 +47,6 @@ var (
 		Username: "user1",
 		Password: "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.",
 		Email:    "user1@example.com",
-		IsActive: true,
 	}
 	testuser2 = user.User{
 		ID:       2,
@@ -56,26 +55,23 @@ var (
 		Email:    "user2@example.com",
 	}
 	testuser3 = user.User{
-		ID:                 3,
+		ID:       3,
-		Username:           "user3",
+		Username: "user3",
-		Password:           "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.",
+		Password: "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.",
-		Email:              "user3@example.com",
+		Email:    "user3@example.com",
-		PasswordResetToken: "passwordresettesttoken",
 	}
 	testuser4 = user.User{
-		ID:                4,
+		ID:       4,
-		Username:          "user4",
+		Username: "user4",
-		Password:          "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.",
+		Password: "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.",
-		Email:             "user4@example.com",
+		Email:    "user4@example.com",
-		EmailConfirmToken: "tiepiQueed8ahc7zeeFe1eveiy4Ein8osooxegiephauph2Ael",
 	}
 	testuser5 = user.User{
-		ID:                4,
+		ID:       4,
-		Username:          "user5",
+		Username: "user5",
-		Password:          "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.",
+		Password: "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.",
-		Email:             "user5@example.com",
+		Email:    "user5@example.com",
-		EmailConfirmToken: "tiepiQueed8ahc7zeeFe1eveiy4Ein8osooxegiephauph2Ael",
+		Status:   user.StatusDisabled,
-		IsActive:          false,
 	}
 )
 

pkg/migration/20210711173657.go

@@ -0,0 +1,101 @@
+// Vikunja is a to-do list application to facilitate your life.
+// Copyright 2018-2021 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 Licensee 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 Licensee for more details.
+//
+// You should have received a copy of the GNU Affero General Public Licensee
+// along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+package migration
+
+import (
+	"time"
+
+	"src.techknowlogick.com/xormigrate"
+	"xorm.io/xorm"
+)
+
+type user20210711173657 struct {
+	ID                 int64  `xorm:"bigint autoincr not null unique pk" json:"id"`
+	PasswordResetToken string `xorm:"varchar(450) null" json:"-"`
+	EmailConfirmToken  string `xorm:"varchar(450) null" json:"-"`
+}
+
+func (u user20210711173657) TableName() string {
+	return "users"
+}
+
+type userTokens20210711173657 struct {
+	ID      int64     `xorm:"bigint autoincr not null unique pk"`
+	UserID  int64     `xorm:"not null"`
+	Token   string    `xorm:"not null"`
+	Kind    int       `xorm:"not null"`
+	Created time.Time `xorm:"created not null"`
+}
+
+func (userTokens20210711173657) TableName() string {
+	return "user_tokens"
+}
+
+func init() {
+	migrations = append(migrations, &xormigrate.Migration{
+		ID:          "20210711173657",
+		Description: "Add user tokens table",
+		Migrate: func(tx *xorm.Engine) error {
+			err := tx.Sync2(userTokens20210711173657{})
+			if err != nil {
+				return err
+			}
+
+			users := []*user20210711173657{}
+			err = tx.Where(`password_reset_token != '' OR email_confirm_token != ''`).Find(&users)
+			if err != nil {
+				return err
+			}
+
+			const tokenPasswordReset = 1
+			const tokenEmailConfirm = 2
+
+			for _, user := range users {
+				if user.PasswordResetToken != "" {
+					_, err = tx.Insert(&userTokens20210711173657{
+						UserID: user.ID,
+						Token:  user.PasswordResetToken,
+						Kind:   tokenPasswordReset,
+					})
+					if err != nil {
+						return err
+					}
+				}
+
+				if user.EmailConfirmToken != "" {
+					_, err = tx.Insert(&userTokens20210711173657{
+						UserID: user.ID,
+						Token:  user.EmailConfirmToken,
+						Kind:   tokenEmailConfirm,
+					})
+					if err != nil {
+						return err
+					}
+				}
+			}
+
+			err = dropTableColum(tx, "users", "password_reset_token")
+			if err != nil {
+				return err
+			}
+			return dropTableColum(tx, "users", "email_confirm_token")
+		},
+		Rollback: func(tx *xorm.Engine) error {
+			return nil
+		},
+	})
+}

pkg/migration/20210713213622.go

@@ -0,0 +1,71 @@
+// Vikunja is a to-do list application to facilitate your life.
+// Copyright 2018-2021 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 Licensee 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 Licensee for more details.
+//
+// You should have received a copy of the GNU Affero General Public Licensee
+// along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+package migration
+
+import (
+	"src.techknowlogick.com/xormigrate"
+	"xorm.io/xorm"
+)
+
+type users20210713213622 struct {
+	ID       int64 `xorm:"bigint autoincr not null unique pk" json:"id"`
+	IsActive bool  `xorm:"null" json:"-"`
+	Status   int   `xorm:"default 0" json:"-"`
+}
+
+func (users20210713213622) TableName() string {
+	return "users"
+}
+
+func init() {
+	migrations = append(migrations, &xormigrate.Migration{
+		ID:          "20210713213622",
+		Description: "Add users status instead of is_active",
+		Migrate: func(tx *xorm.Engine) error {
+			err := tx.Sync2(users20210713213622{})
+			if err != nil {
+				return err
+			}
+
+			users := []*users20210713213622{}
+			err = tx.Find(&users)
+			if err != nil {
+				return err
+			}
+
+			for _, user := range users {
+				if user.IsActive {
+					continue
+				}
+
+				user.Status = 1 // 1 is "email confirmation required" - as that's the only way is_active was used before we'll use that
+				_, err := tx.
+					Where("id = ?", user.ID).
+					Cols("status").
+					Update(user)
+				if err != nil {
+					return err
+				}
+			}
+
+			return dropTableColum(tx, "users", "is_active")
+		},
+		Rollback: func(tx *xorm.Engine) error {
+			return nil
+		},
+	})
+}

pkg/models/label_test.go

@@ -51,7 +51,6 @@ func TestLabel_ReadAll(t *testing.T) {
 		ID:                           1,
 		Username:                     "user1",
 		Password:                     "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.",
-		IsActive:                     true,
 		Issuer:                       "local",
 		EmailRemindersEnabled:        true,
 		OverdueTasksRemindersEnabled: true,
@@ -166,7 +165,6 @@ func TestLabel_ReadOne(t *testing.T) {
 		ID:                           1,
 		Username:                     "user1",
 		Password:                     "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.",
-		IsActive:                     true,
 		Issuer:                       "local",
 		EmailRemindersEnabled:        true,
 		OverdueTasksRemindersEnabled: true,

pkg/models/list_users_test.go

@@ -180,7 +180,6 @@ func TestListUser_ReadAll(t *testing.T) {
 						ID:                           1,
 						Username:                     "user1",
 						Password:                     "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.",
-						IsActive:                     true,
 						Issuer:                       "local",
 						EmailRemindersEnabled:        true,
 						OverdueTasksRemindersEnabled: true,

pkg/models/namespace_users_test.go

@@ -179,7 +179,6 @@ func TestNamespaceUser_ReadAll(t *testing.T) {
 						ID:                           1,
 						Username:                     "user1",
 						Password:                     "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.",
-						IsActive:                     true,
 						Issuer:                       "local",
 						EmailRemindersEnabled:        true,
 						OverdueTasksRemindersEnabled: true,

pkg/models/task_collection_test.go

@@ -34,7 +34,6 @@ func TestTaskCollection_ReadAll(t *testing.T) {
 		ID:                           1,
 		Username:                     "user1",
 		Password:                     "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.",
-		IsActive:                     true,
 		Issuer:                       "local",
 		EmailRemindersEnabled:        true,
 		OverdueTasksRemindersEnabled: true,
@@ -56,7 +55,6 @@ func TestTaskCollection_ReadAll(t *testing.T) {
 		Username:                     "user6",
 		Password:                     "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.",
 		Issuer:                       "local",
-		IsActive:                     true,
 		EmailRemindersEnabled:        true,
 		OverdueTasksRemindersEnabled: true,
 		Created:                      testCreatedTime,

pkg/models/unit_tests.go

@@ -55,6 +55,7 @@ func SetupTests() {
 		"team_namespaces",
 		"teams",
 		"users",
+		"user_tokens",
 		"users_lists",
 		"users_namespaces",
 		"buckets",

pkg/models/users_list_test.go

@@ -29,7 +29,6 @@ func TestListUsersFromList(t *testing.T) {
 		ID:                           1,
 		Username:                     "user1",
 		Password:                     "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.",
-		IsActive:                     true,
 		Issuer:                       "local",
 		EmailRemindersEnabled:        true,
 		OverdueTasksRemindersEnabled: true,
@@ -50,7 +49,6 @@ func TestListUsersFromList(t *testing.T) {
 		ID:                           3,
 		Username:                     "user3",
 		Password:                     "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.",
-		PasswordResetToken:           "passwordresettesttoken",
 		Issuer:                       "local",
 		EmailRemindersEnabled:        true,
 		OverdueTasksRemindersEnabled: true,
@@ -61,8 +59,7 @@ func TestListUsersFromList(t *testing.T) {
 		ID:                           4,
 		Username:                     "user4",
 		Password:                     "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.",
-		IsActive:                     false,
+		Status:                       user.StatusEmailConfirmationRequired,
-		EmailConfirmToken:            "tiepiQueed8ahc7zeeFe1eveiy4Ein8osooxegiephauph2Ael",
 		Issuer:                       "local",
 		EmailRemindersEnabled:        true,
 		OverdueTasksRemindersEnabled: true,
@@ -73,8 +70,7 @@ func TestListUsersFromList(t *testing.T) {
 		ID:                           5,
 		Username:                     "user5",
 		Password:                     "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.",
-		IsActive:                     false,
+		Status:                       user.StatusEmailConfirmationRequired,
-		EmailConfirmToken:            "tiepiQueed8ahc7zeeFe1eveiy4Ein8osooxegiephauph2Ael",
 		Issuer:                       "local",
 		EmailRemindersEnabled:        true,
 		OverdueTasksRemindersEnabled: true,
@@ -85,7 +81,6 @@ func TestListUsersFromList(t *testing.T) {
 		ID:                           6,
 		Username:                     "user6",
 		Password:                     "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.",
-		IsActive:                     true,
 		Issuer:                       "local",
 		EmailRemindersEnabled:        true,
 		OverdueTasksRemindersEnabled: true,
@@ -96,7 +91,6 @@ func TestListUsersFromList(t *testing.T) {
 		ID:                           7,
 		Username:                     "user7",
 		Password:                     "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.",
-		IsActive:                     true,
 		Issuer:                       "local",
 		EmailRemindersEnabled:        true,
 		DiscoverableByEmail:          true,
@@ -108,7 +102,6 @@ func TestListUsersFromList(t *testing.T) {
 		ID:                           8,
 		Username:                     "user8",
 		Password:                     "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.",
-		IsActive:                     true,
 		Issuer:                       "local",
 		EmailRemindersEnabled:        true,
 		OverdueTasksRemindersEnabled: true,
@@ -119,7 +112,6 @@ func TestListUsersFromList(t *testing.T) {
 		ID:                           9,
 		Username:                     "user9",
 		Password:                     "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.",
-		IsActive:                     true,
 		Issuer:                       "local",
 		EmailRemindersEnabled:        true,
 		OverdueTasksRemindersEnabled: true,
@@ -130,7 +122,6 @@ func TestListUsersFromList(t *testing.T) {
 		ID:                           10,
 		Username:                     "user10",
 		Password:                     "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.",
-		IsActive:                     true,
 		Issuer:                       "local",
 		EmailRemindersEnabled:        true,
 		OverdueTasksRemindersEnabled: true,
@@ -142,7 +133,6 @@ func TestListUsersFromList(t *testing.T) {
 		Username:                     "user11",
 		Name:                         "Some one else",
 		Password:                     "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.",
-		IsActive:                     true,
 		Issuer:                       "local",
 		EmailRemindersEnabled:        true,
 		OverdueTasksRemindersEnabled: true,
@@ -154,7 +144,6 @@ func TestListUsersFromList(t *testing.T) {
 		Username:                     "user12",
 		Name:                         "Name with spaces",
 		Password:                     "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.",
-		IsActive:                     true,
 		Issuer:                       "local",
 		EmailRemindersEnabled:        true,
 		DiscoverableByName:           true,
@@ -166,7 +155,6 @@ func TestListUsersFromList(t *testing.T) {
 		ID:                           13,
 		Username:                     "user13",
 		Password:                     "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.",
-		IsActive:                     true,
 		Issuer:                       "local",
 		EmailRemindersEnabled:        true,
 		OverdueTasksRemindersEnabled: true,

pkg/modules/auth/openid/openid.go

@@ -216,7 +216,7 @@ func getOrCreateUser(s *xorm.Session, cl *claims, issuer, subject string) (u *us
 		uu := &user.User{
 			Username: cl.PreferredUsername,
 			Email:    cl.Email,
-			IsActive: true,
+			Status:   user.StatusActive,
 			Issuer:   issuer,
 			Subject:  subject,
 		}

pkg/user/db.go

@@ -36,5 +36,6 @@ func GetTables() []interface{} {
 	return []interface{}{
 		&User{},
 		&TOTP{},
+		&Token{},
 	}
 }

pkg/user/notifications.go

@@ -23,8 +23,9 @@ import (
 
 // EmailConfirmNotification represents a EmailConfirmNotification notification
 type EmailConfirmNotification struct {
-	User  *User
+	User         *User
-	IsNew bool
+	IsNew        bool
+	ConfirmToken string
 }
 
 // ToMail returns the mail notification for EmailConfirmNotification
@@ -45,7 +46,7 @@ func (n *EmailConfirmNotification) ToMail() *notifications.Mail {
 
 	return nn.
 		Line("To confirm your email address, click the link below:").
-		Action("Confirm your email address", config.ServiceFrontendurl.GetString()+"?userEmailConfirm="+n.User.EmailConfirmToken).
+		Action("Confirm your email address", config.ServiceFrontendurl.GetString()+"?userEmailConfirm="+n.ConfirmToken).
 		Line("Have a nice day!")
 }
 
@@ -85,7 +86,8 @@ func (n *PasswordChangedNotification) Name() string {
 
 // ResetPasswordNotification represents a ResetPasswordNotification notification
 type ResetPasswordNotification struct {
-	User *User
+	User  *User
+	Token *Token
 }
 
 // ToMail returns the mail notification for ResetPasswordNotification
@@ -94,7 +96,8 @@ func (n *ResetPasswordNotification) ToMail() *notifications.Mail {
 		Subject("Reset your password on Vikunja").
 		Greeting("Hi "+n.User.GetName()+",").
 		Line("To reset your password, click the link below:").
-		Action("Reset your password", config.ServiceFrontendurl.GetString()+"?userPasswordReset="+n.User.PasswordResetToken).
+		Action("Reset your password", config.ServiceFrontendurl.GetString()+"?userPasswordReset="+n.Token.Token).
+		Line("This link will be valid for 24 hours.").
 		Line("Have a nice day!")
 }
 

pkg/user/test.go

@@ -34,7 +34,7 @@ func InitTests() {
 		log.Fatal(err)
 	}
 
-	err = db.InitTestFixtures("users")
+	err = db.InitTestFixtures("users", "user_tokens")
 	if err != nil {
 		log.Fatal(err)
 	}

pkg/user/token.go

@@ -0,0 +1,104 @@
+// Vikunja is a to-do list application to facilitate your life.
+// Copyright 2018-2021 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 Licensee 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 Licensee for more details.
+//
+// You should have received a copy of the GNU Affero General Public Licensee
+// along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+package user
+
+import (
+	"time"
+
+	"code.vikunja.io/api/pkg/cron"
+	"code.vikunja.io/api/pkg/db"
+	"code.vikunja.io/api/pkg/log"
+	"code.vikunja.io/api/pkg/utils"
+	"xorm.io/xorm"
+)
+
+// TokenKind represents a user token kind
+type TokenKind int
+
+const (
+	TokenUnknown TokenKind = iota
+	TokenPasswordReset
+	TokenEmailConfirm
+
+	tokenSize = 64
+)
+
+// Token is a token a user can use to do things like verify their email or resetting their password
+type Token struct {
+	ID      int64     `xorm:"bigint autoincr not null unique pk"`
+	UserID  int64     `xorm:"not null"`
+	Token   string    `xorm:"not null"`
+	Kind    TokenKind `xorm:"not null"`
+	Created time.Time `xorm:"created not null"`
+}
+
+// TableName returns the real table name for user tokens
+func (t *Token) TableName() string {
+	return "user_tokens"
+}
+
+func generateNewToken(s *xorm.Session, u *User, kind TokenKind) (token *Token, err error) {
+	token = &Token{
+		UserID: u.ID,
+		Kind:   kind,
+		Token:  utils.MakeRandomString(tokenSize),
+	}
+
+	_, err = s.Insert(token)
+	return
+}
+
+func getToken(s *xorm.Session, token string, kind TokenKind) (t *Token, err error) {
+	t = &Token{}
+	has, err := s.Where("kind = ? AND token = ?", kind, token).
+		Get(t)
+	if err != nil || !has {
+		return nil, err
+	}
+
+	return
+}
+
+func removeTokens(s *xorm.Session, u *User, kind TokenKind) (err error) {
+	_, err = s.Where("user_id = ? AND kind = ?", u.ID, kind).
+		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] "
+
+	err := cron.Schedule("0 * * * *", func() {
+		s := db.NewSession()
+		defer s.Close()
+
+		deleted, err := s.
+			Where("created > ? AND kind = ?", time.Now().Add(time.Hour*24*-1), TokenPasswordReset).
+			Delete(&Token{})
+		if err != nil {
+			log.Errorf(logPrefix+"Error removing old password reset tokens: %s", err)
+			return
+		}
+		if deleted > 0 {
+			log.Debugf(logPrefix+"Deleted %d old password reset tokens", deleted)
+		}
+	})
+	if err != nil {
+		log.Fatalf("Could not register token cleanup cron: %s", err)
+	}
+}

pkg/user/update_email.go

@@ -19,7 +19,6 @@ package user
 import (
 	"code.vikunja.io/api/pkg/config"
 	"code.vikunja.io/api/pkg/notifications"
-	"code.vikunja.io/api/pkg/utils"
 	"xorm.io/xorm"
 )
 
@@ -52,26 +51,35 @@ func UpdateEmail(s *xorm.Session, update *EmailUpdate) (err error) {
 		return
 	}
 
-	update.User.IsActive = false
 	update.User.Email = update.NewEmail
-	update.User.EmailConfirmToken = utils.MakeRandomString(64)
+
+	// Send the confirmation mail
+	if !config.MailerEnabled.GetBool() {
+		_, err = s.
+			Where("id = ?", update.User.ID).
+			Cols("email").
+			Update(update.User)
+		return
+	}
+
+	update.User.Status = StatusEmailConfirmationRequired
+	token, err := generateNewToken(s, update.User, TokenEmailConfirm)
+	if err != nil {
+		return
+	}
 	_, err = s.
 		Where("id = ?", update.User.ID).
-		Cols("email", "is_active", "email_confirm_token").
+		Cols("email", "is_active"). // TODO: Status change
 		Update(update.User)
 	if err != nil {
 		return
 	}
 
-	// Send the confirmation mail
-	if !config.MailerEnabled.GetBool() {
-		return
-	}
-
 	// Send the user a mail with a link to confirm the mail
 	n := &EmailConfirmNotification{
-		User:  update.User,
+		User:         update.User,
-		IsNew: false,
+		IsNew:        false,
+		ConfirmToken: token.Token,
 	}
 
 	err = notifications.Notify(update.User, n)

pkg/user/user.go

@@ -44,6 +44,27 @@ type Login struct {
 	TOTPPasscode string `json:"totp_passcode"`
 }
 
+type Status int
+
+func (s Status) String() string {
+	switch s {
+	case StatusActive:
+		return "Active"
+	case StatusEmailConfirmationRequired:
+		return "Email Confirmation required"
+	case StatusDisabled:
+		return "Disabled"
+	}
+
+	return "Unknown"
+}
+
+const (
+	StatusActive = iota
+	StatusEmailConfirmationRequired
+	StatusDisabled
+)
+
 // User holds information about an user
 type User struct {
 	// The unique, numeric id of this user.
@@ -54,11 +75,9 @@ type User struct {
 	Username string `xorm:"varchar(250) not null unique" json:"username" valid:"length(1|250)" minLength:"1" maxLength:"250"`
 	Password string `xorm:"varchar(250) null" json:"-"`
 	// The user's email address.
-	Email    string `xorm:"varchar(250) null" json:"email,omitempty" valid:"email,length(0|250)" maxLength:"250"`
+	Email string `xorm:"varchar(250) null" json:"email,omitempty" valid:"email,length(0|250)" maxLength:"250"`
-	IsActive bool   `xorm:"null" json:"-"`
 
-	PasswordResetToken string `xorm:"varchar(450) null" json:"-"`
+	Status Status `xorm:"default 0" json:"-"`
-	EmailConfirmToken  string `xorm:"varchar(450) null" json:"-"`
 
 	AvatarProvider string `xorm:"varchar(255) null" json:"-"`
 	AvatarFileID   int64  `xorm:"null" json:"-"`
@@ -255,7 +274,7 @@ func CheckUserCredentials(s *xorm.Session, u *Login) (*User, error) {
 	}
 
 	// The user is invalid if they need to verify their email address
-	if !user.IsActive {
+	if user.Status == StatusEmailConfirmationRequired {
 		return &User{}, ErrEmailNotConfirmed{UserID: user.ID}
 	}
 

pkg/user/user_create.go

@@ -20,7 +20,6 @@ import (
 	"code.vikunja.io/api/pkg/config"
 	"code.vikunja.io/api/pkg/events"
 	"code.vikunja.io/api/pkg/notifications"
-	"code.vikunja.io/api/pkg/utils"
 	"golang.org/x/crypto/bcrypt"
 	"xorm.io/xorm"
 )
@@ -54,14 +53,7 @@ func CreateUser(s *xorm.Session, user *User) (newUser *User, err error) {
 		}
 	}
 
-	user.IsActive = true
+	user.Status = StatusActive
-	if config.MailerEnabled.GetBool() && user.Issuer == issuerLocal {
-		// The new user should not be activated until it confirms his mail address
-		user.IsActive = false
-		// Generate a confirm token
-		user.EmailConfirmToken = utils.MakeRandomString(60)
-	}
-
 	user.AvatarProvider = "initials"
 
 	// Insert it
@@ -84,13 +76,28 @@ func CreateUser(s *xorm.Session, user *User) (newUser *User, err error) {
 	}
 
 	// Dont send a mail if no mailer is configured
-	if !config.MailerEnabled.GetBool() {
+	if !config.MailerEnabled.GetBool() || user.Issuer != issuerLocal {
 		return newUserOut, err
 	}
 
+	user.Status = StatusEmailConfirmationRequired
+	token, err := generateNewToken(s, user, TokenEmailConfirm)
+	if err != nil {
+		return nil, err
+	}
+
+	_, err = s.
+		Where("id = ?", user.ID).
+		Cols("email", "is_active").
+		Update(user)
+	if err != nil {
+		return
+	}
+
 	n := &EmailConfirmNotification{
-		User:  user,
+		User:         user,
-		IsNew: false,
+		IsNew:        true,
+		ConfirmToken: token.Token,
 	}
 
 	err = notifications.Notify(user, n)

pkg/user/user_email_confirm.go

@@ -16,7 +16,9 @@
 
 package user
 
-import "xorm.io/xorm"
+import (
+	"xorm.io/xorm"
+)
 
 // EmailConfirm holds the token to confirm a mail address
 type EmailConfirm struct {
@@ -32,24 +34,27 @@ func ConfirmEmail(s *xorm.Session, c *EmailConfirm) (err error) {
 		return ErrInvalidEmailConfirmToken{}
 	}
 
-	// Check if the token is valid
+	token, err := getToken(s, c.Token, TokenEmailConfirm)
-	user := User{}
+	if err != nil {
-	has, err := s.
+		return
-		Where("email_confirm_token = ?", c.Token).
+	}
-		Get(&user)
+	if token == nil {
+		return ErrInvalidEmailConfirmToken{Token: c.Token}
+	}
+
+	user, err := GetUserByID(s, token.UserID)
 	if err != nil {
 		return
 	}
 
-	if !has {
+	user.Status = StatusActive
-		return ErrInvalidEmailConfirmToken{Token: c.Token}
+	err = removeTokens(s, user, TokenEmailConfirm)
+	if err != nil {
+		return
 	}
-
-	user.IsActive = true
-	user.EmailConfirmToken = ""
 	_, err = s.
 		Where("id = ?", user.ID).
-		Cols("is_active", "email_confirm_token").
+		Cols("is_active").
-		Update(&user)
+		Update(user)
 	return
 }

pkg/user/user_password_reset.go

@@ -19,7 +19,6 @@ package user
 import (
 	"code.vikunja.io/api/pkg/config"
 	"code.vikunja.io/api/pkg/notifications"
-	"code.vikunja.io/api/pkg/utils"
 	"xorm.io/xorm"
 )
 
@@ -44,16 +43,17 @@ func ResetPassword(s *xorm.Session, reset *PasswordReset) (err error) {
 	}
 
 	// Check if we have a token
-	user := &User{}
+	token, err := getToken(s, reset.Token, TokenPasswordReset)
-	exists, err := s.
-		Where("password_reset_token = ?", reset.Token).
-		Get(user)
 	if err != nil {
-		return
+		return err
+	}
+	if token == nil {
+		return ErrInvalidPasswordResetToken{Token: reset.Token}
 	}
 
-	if !exists {
+	user, err := GetUserByID(s, token.UserID)
-		return ErrInvalidPasswordResetToken{Token: reset.Token}
+	if err != nil {
+		return
 	}
 
 	// Hash the password
@@ -62,17 +62,20 @@ func ResetPassword(s *xorm.Session, reset *PasswordReset) (err error) {
 		return
 	}
 
-	// Save it
+	err = removeTokens(s, user, TokenEmailConfirm)
-	user.PasswordResetToken = ""
+	if err != nil {
+		return
+	}
+
 	_, err = s.
-		Cols("password", "password_reset_token").
+		Cols("password").
 		Where("id = ?", user.ID).
 		Update(user)
 	if err != nil {
 		return
 	}
 
-	// Dont send a mail if we're testing
+	// Dont send a mail if no mailer is configured
 	if !config.MailerEnabled.GetBool() {
 		return
 	}
@@ -108,24 +111,19 @@ func RequestUserPasswordResetTokenByEmail(s *xorm.Session, tr *PasswordTokenRequ
 
 // RequestUserPasswordResetToken sends a user a password reset email.
 func RequestUserPasswordResetToken(s *xorm.Session, user *User) (err error) {
-	// Generate a token and save it
+	token, err := generateNewToken(s, user, TokenPasswordReset)
-	user.PasswordResetToken = utils.MakeRandomString(400)
-
-	// Save it
-	_, err = s.
-		Where("id = ?", user.ID).
-		Update(user)
 	if err != nil {
 		return
 	}
 
-	// Dont send a mail if we're testing
+	// Dont send a mail if no mailer is configured
 	if !config.MailerEnabled.GetBool() {
 		return
 	}
 
 	n := &ResetPasswordNotification{
-		User: user,
+		User:  user,
+		Token: token,
 	}
 
 	err = notifications.Notify(user, n)

pkg/user/user_test.go

@@ -322,7 +322,6 @@ func TestUpdateUser(t *testing.T) {
 }
 
 func TestUpdateUserPassword(t *testing.T) {
-
 	t.Run("normal", func(t *testing.T) {
 		db.LoadAndAssertFixtures(t)
 		s := db.NewSession()