From 96f68e595af97af484e350d959a9e7fa9b8605d0 Mon Sep 17 00:00:00 2001 From: kolaente Date: Tue, 13 Jul 2021 21:58:18 +0200 Subject: [PATCH] Replace user is_active by status field --- pkg/cmd/user.go | 16 ++++--- pkg/db/fixtures/users.yml | 13 +----- pkg/integrations/integrations.go | 3 +- pkg/migration/20210713213622.go | 71 ++++++++++++++++++++++++++++++ pkg/models/label_test.go | 2 - pkg/models/list_users_test.go | 1 - pkg/models/namespace_users_test.go | 1 - pkg/models/task_collection_test.go | 2 - pkg/models/users_list_test.go | 13 +----- pkg/modules/auth/openid/openid.go | 2 +- pkg/user/update_email.go | 2 +- pkg/user/user.go | 28 ++++++++++-- pkg/user/user_create.go | 5 +-- pkg/user/user_email_confirm.go | 3 +- pkg/user/user_password_reset.go | 2 +- 15 files changed, 117 insertions(+), 47 deletions(-) create mode 100644 pkg/migration/20210713213622.go diff --git a/pkg/cmd/user.go b/pkg/cmd/user.go index e7c9a295f..2c2e99008 100644 --- a/pkg/cmd/user.go +++ b/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) }, } diff --git a/pkg/db/fixtures/users.yml b/pkg/db/fixtures/users.yml index 5ea137758..7ad372811 100644 --- a/pkg/db/fixtures/users.yml +++ b/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 @@ -28,6 +27,7 @@ username: 'user4' password: '$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.' # 1234 email: 'user4@example.com' + status: 1 issuer: local updated: 2018-12-02 15:13:12 created: 2018-12-01 15:13:12 @@ -36,7 +36,7 @@ username: 'user5' password: '$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.' # 1234 email: 'user5@example.com' - is_active: false + status: 1 issuer: local updated: 2018-12-02 15:13:12 created: 2018-12-01 15:13:12 @@ -45,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 @@ -53,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 @@ -62,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 @@ -70,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 @@ -78,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 @@ -87,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 @@ -96,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 @@ -105,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 @@ -113,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 diff --git a/pkg/integrations/integrations.go b/pkg/integrations/integrations.go index 30badc69d..9da74da8b 100644 --- a/pkg/integrations/integrations.go +++ b/pkg/integrations/integrations.go @@ -46,7 +46,6 @@ var ( Username: "user1", Password: "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.", Email: "user1@example.com", - IsActive: true, } testuser2 = user.User{ ID: 2, @@ -71,7 +70,7 @@ var ( Username: "user5", Password: "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.", Email: "user5@example.com", - IsActive: false, + Status: user.StatusDisabled, } ) diff --git a/pkg/migration/20210713213622.go b/pkg/migration/20210713213622.go new file mode 100644 index 000000000..30ac6ebe8 --- /dev/null +++ b/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 . + +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 + }, + }) +} diff --git a/pkg/models/label_test.go b/pkg/models/label_test.go index 9666695c8..52151d01f 100644 --- a/pkg/models/label_test.go +++ b/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, diff --git a/pkg/models/list_users_test.go b/pkg/models/list_users_test.go index f36858b27..f8c3968af 100644 --- a/pkg/models/list_users_test.go +++ b/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, diff --git a/pkg/models/namespace_users_test.go b/pkg/models/namespace_users_test.go index f9fe51d99..6f75140ab 100644 --- a/pkg/models/namespace_users_test.go +++ b/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, diff --git a/pkg/models/task_collection_test.go b/pkg/models/task_collection_test.go index 14e986066..28b70dbd9 100644 --- a/pkg/models/task_collection_test.go +++ b/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, diff --git a/pkg/models/users_list_test.go b/pkg/models/users_list_test.go index 24f770233..cf0ed846e 100644 --- a/pkg/models/users_list_test.go +++ b/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, @@ -60,7 +59,7 @@ func TestListUsersFromList(t *testing.T) { ID: 4, Username: "user4", Password: "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.", - IsActive: false, + Status: user.StatusEmailConfirmationRequired, Issuer: "local", EmailRemindersEnabled: true, OverdueTasksRemindersEnabled: true, @@ -71,7 +70,7 @@ func TestListUsersFromList(t *testing.T) { ID: 5, Username: "user5", Password: "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.", - IsActive: false, + Status: user.StatusEmailConfirmationRequired, Issuer: "local", EmailRemindersEnabled: true, OverdueTasksRemindersEnabled: true, @@ -82,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, @@ -93,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, @@ -105,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, @@ -116,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, @@ -127,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, @@ -139,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, @@ -151,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, @@ -163,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, diff --git a/pkg/modules/auth/openid/openid.go b/pkg/modules/auth/openid/openid.go index a86fd88c6..3268c664a 100644 --- a/pkg/modules/auth/openid/openid.go +++ b/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, } diff --git a/pkg/user/update_email.go b/pkg/user/update_email.go index 22f9477a8..6b4b35d9b 100644 --- a/pkg/user/update_email.go +++ b/pkg/user/update_email.go @@ -62,7 +62,7 @@ func UpdateEmail(s *xorm.Session, update *EmailUpdate) (err error) { return } - update.User.IsActive = false + update.User.Status = StatusEmailConfirmationRequired token, err := generateNewToken(s, update.User, TokenEmailConfirm) if err != nil { return diff --git a/pkg/user/user.go b/pkg/user/user.go index 8a634f95b..87c1d3607 100644 --- a/pkg/user/user.go +++ b/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,8 +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"` - IsActive bool `xorm:"null" json:"-"` + Email string `xorm:"varchar(250) null" json:"email,omitempty" valid:"email,length(0|250)" maxLength:"250"` + + Status Status `xorm:"default 0" json:"-"` AvatarProvider string `xorm:"varchar(255) null" json:"-"` AvatarFileID int64 `xorm:"null" json:"-"` @@ -252,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} } diff --git a/pkg/user/user_create.go b/pkg/user/user_create.go index 77ff3bb49..f445eb884 100644 --- a/pkg/user/user_create.go +++ b/pkg/user/user_create.go @@ -53,7 +53,7 @@ func CreateUser(s *xorm.Session, user *User) (newUser *User, err error) { } } - user.IsActive = true + user.Status = StatusActive user.AvatarProvider = "initials" // Insert it @@ -80,8 +80,7 @@ func CreateUser(s *xorm.Session, user *User) (newUser *User, err error) { return newUserOut, err } - // TODO: Proper status change - user.IsActive = false + user.Status = StatusEmailConfirmationRequired token, err := generateNewToken(s, user, TokenEmailConfirm) if err != nil { return nil, err diff --git a/pkg/user/user_email_confirm.go b/pkg/user/user_email_confirm.go index b8f9dcc46..374e5beff 100644 --- a/pkg/user/user_email_confirm.go +++ b/pkg/user/user_email_confirm.go @@ -47,8 +47,7 @@ func ConfirmEmail(s *xorm.Session, c *EmailConfirm) (err error) { return } - // TODO: proper user status - user.IsActive = true + user.Status = StatusActive err = removeTokens(s, user, TokenEmailConfirm) if err != nil { return diff --git a/pkg/user/user_password_reset.go b/pkg/user/user_password_reset.go index 00869a4f5..b03a7807a 100644 --- a/pkg/user/user_password_reset.go +++ b/pkg/user/user_password_reset.go @@ -116,7 +116,7 @@ func RequestUserPasswordResetToken(s *xorm.Session, user *User) (err error) { return } - // Dont send a mail if we're testing + // Dont send a mail if no mailer is configured if !config.MailerEnabled.GetBool() { return }