Various user fixes (#38)

Featurecreep.md

@@ -237,9 +237,9 @@ Teams sind global, d.h. Ein Team kann mehrere Namespaces verwalten.
 
 ### Bugfixes
 
-* [ ] Panic wenn mailer nicht erreichbar -> Als workaround mailer deaktivierbar machen, bzw keine mails verschicken
+* [x] Panic wenn mailer nicht erreichbar -> Als workaround mailer deaktivierbar machen, bzw keine mails verschicken
 * [x] "unexpected EOF"
-* [ ] Beim Login & Password reset gibt die API zurück dass der Nutzer nicht existiert
+* [x] Beim Login & Password reset gibt die API zurück dass der Nutzer nicht existiert
 
 ### Docs
 

REST-Tests/users.http

@@ -27,7 +27,7 @@ Content-Type: application/json
 Accept: application/json
 
 {
-  "user_name": "user"
+  "email": "k@knt.li"
 }
 
 ### Request a token to reset a password

config.yml.sample

@@ -54,6 +54,8 @@ redis:
   db: 0
 
 mailer:
+  # Whether to enable the mailer or not. If it is disabled, all users are enabled right away and password reset is not possible.
+  enabled: false
   # SMTP Host
   host: ""
   # SMTP Host port

docs/config.md

@@ -78,6 +78,8 @@ redis:
   db: 0
 
 mailer:
+  # Whether to enable the mailer or not. If it is disabled, all users are enabled right away and password reset is not possible.
+  enabled: false
   # SMTP Host
   host: ""
   # SMTP Host port

pkg/config/config.go

@@ -63,6 +63,7 @@ func init() {
 	viper.SetDefault("cache.type", "memory")
 	viper.SetDefault("cache.maxelementsize", 1000)
 	// Mailer
+	viper.SetDefault("mailer.enabled", false)
 	viper.SetDefault("mailer.host", "")
 	viper.SetDefault("mailer.port", "587")
 	viper.SetDefault("mailer.user", "user")

pkg/mail/mail.go

@@ -31,6 +31,10 @@ var Queue chan *gomail.Message
 func StartMailDaemon() {
 	Queue = make(chan *gomail.Message, viper.GetInt("mailer.queuelength"))
 
+	if !viper.GetBool("mailer.enabled") {
+		return
+	}
+
 	if viper.GetString("mailer.host") == "" {
 		log.Log.Warning("Mailer seems to be not configured! Please see the config docs for more details.")
 		return

pkg/models/fixtures/users.yml

@@ -2,20 +2,27 @@
   id: 1
   username: 'user1'
   password: '1234'
-  email: 'johndoe@example.com'
+  email: 'user1@example.com'
 -
   id: 2
   username: 'user2'
   password: '1234'
-  email: 'johndoe@example.com'
+  email: 'user2@example.com'
 -
   id: 3
   username: 'user3'
   password: '1234'
-  email: 'johndoe@example.com'
+  email: 'user3@example.com'
 -
   id: 4
   username: 'user4'
   password: '1234'
-  email: 'johndoe@example.com'
+  email: 'user4@example.com'
-  email_confirm_token: tiepiQueed8ahc7zeeFe1eveiy4Ein8osooxegiephauph2Ael
+  email_confirm_token: tiepiQueed8ahc7zeeFe1eveiy4Ein8osooxegiephauph2Ael
+-
+  id: 5
+  username: 'user5'
+  password: '1234'
+  email: 'user4@example.com'
+  email_confirm_token: tiepiQueed8ahc7zeeFe1eveiy4Ein8osooxegiephauph2Ael
+  is_active: false

pkg/models/list_users_test.go

@@ -160,7 +160,7 @@ func TestListUser_ReadAll(t *testing.T) {
 						ID:       1,
 						Username: "user1",
 						Password: "1234",
-						Email:    "johndoe@example.com",
+						Email:    "user1@example.com",
 					},
 					Right: UserRightRead,
 				},
@@ -169,7 +169,7 @@ func TestListUser_ReadAll(t *testing.T) {
 						ID:       2,
 						Username: "user2",
 						Password: "1234",
-						Email:    "johndoe@example.com",
+						Email:    "user2@example.com",
 					},
 					Right: UserRightRead,
 				},

pkg/models/namespace_users_test.go

@@ -161,7 +161,7 @@ func TestNamespaceUser_ReadAll(t *testing.T) {
 						ID:       1,
 						Username: "user1",
 						Password: "1234",
-						Email:    "johndoe@example.com",
+						Email:    "user1@example.com",
 					},
 					Right: UserRightRead,
 				},
@@ -170,7 +170,7 @@ func TestNamespaceUser_ReadAll(t *testing.T) {
 						ID:       2,
 						Username: "user2",
 						Password: "1234",
-						Email:    "johndoe@example.com",
+						Email:    "user2@example.com",
 					},
 					Right: UserRightRead,
 				},

pkg/models/unit_tests.go

@@ -30,10 +30,6 @@ import (
 	"testing"
 )
 
-// IsTesting is set to true when we're running tests.
-// We don't have a good solution to test email sending yet, so we disable email sending when testing
-var IsTesting bool
-
 // MainTest creates the test engine
 func MainTest(m *testing.M, pathToRoot string) {
 	var err error
@@ -42,8 +38,6 @@ func MainTest(m *testing.M, pathToRoot string) {
 		log.Log.Fatalf("Error creating test engine: %v\n", err)
 	}
 
-	IsTesting = true
-
 	// Start the pseudo mail queue
 	mail.StartMailDaemon()
 

pkg/models/user.go

@@ -78,9 +78,9 @@ func getUserWithError(a web.Auth) (*User, error) {
 // APIUserPassword represents a user object without timestamps and a json password field.
 type APIUserPassword struct {
 	ID       int64  `json:"id"`
-	Username string `json:"username"`
+	Username string `json:"username" valid:"length(3|250)"`
-	Password string `json:"password"`
+	Password string `json:"password" valid:"length(8|250)"`
-	Email    string `json:"email"`
+	Email    string `json:"email" valid:"email,length(0|250)"`
 }
 
 // APIFormat formats an API User into a normal user struct
@@ -125,7 +125,9 @@ func CheckUserCredentials(u *UserLogin) (User, error) {
 	// Check if the user exists
 	user, err := GetUser(User{Username: u.Username})
 	if err != nil {
-		return User{}, err
+		// hashing the password takes a long time, so we hash something to not make it clear if the username was wrong
+		bcrypt.GenerateFromPassword([]byte(u.Username), 14)
+		return User{}, ErrWrongUsernameOrPassword{}
 	}
 
 	// User is invalid if it needs to verify its email address

pkg/models/user_add_update.go

@@ -20,6 +20,7 @@ import (
 	"code.vikunja.io/api/pkg/mail"
 	"code.vikunja.io/api/pkg/metrics"
 	"code.vikunja.io/api/pkg/utils"
+	"github.com/spf13/viper"
 	"golang.org/x/crypto/bcrypt"
 )
 
@@ -67,11 +68,13 @@ func CreateUser(user User) (newUser User, err error) {
 		return User{}, err
 	}
 
-	// Generate a confirm token
+	newUser.IsActive = true
-	newUser.EmailConfirmToken = utils.MakeRandomString(400)
+	if viper.GetBool("mailer.enabled") {
-
+		// The new user should not be activated until it confirms his mail address
-	// The new user should not be activated until it confirms his mail address
+		newUser.IsActive = false
-	newUser.IsActive = false
+		// Generate a confirm token
+		newUser.EmailConfirmToken = utils.MakeRandomString(400)
+	}
 
 	// Insert it
 	_, err = x.Insert(newUser)
@@ -96,7 +99,7 @@ func CreateUser(user User) (newUser User, err error) {
 	}
 
 	// Dont send a mail if we're testing
-	if IsTesting {
+	if !viper.GetBool("mailer.enabled") {
 		return newUserOut, err
 	}
 

pkg/models/user_password_reset.go

@@ -19,6 +19,7 @@ package models
 import (
 	"code.vikunja.io/api/pkg/mail"
 	"code.vikunja.io/api/pkg/utils"
+	"github.com/spf13/viper"
 )
 
 // PasswordReset holds the data to reset a password
@@ -59,7 +60,7 @@ func UserPasswordReset(reset *PasswordReset) (err error) {
 	}
 
 	// Dont send a mail if we're testing
-	if IsTesting {
+	if !viper.GetBool("mailer.enabled") {
 		return
 	}
 
@@ -75,13 +76,13 @@ func UserPasswordReset(reset *PasswordReset) (err error) {
 
 // PasswordTokenRequest defines the request format for password reset resqest
 type PasswordTokenRequest struct {
-	Username string `json:"user_name"`
+	Email string `json:"email" valid:"email,length(0|250)"`
 }
 
 // RequestUserPasswordResetToken inserts a random token to reset a users password into the databsse
 func RequestUserPasswordResetToken(tr *PasswordTokenRequest) (err error) {
 	// Check if the user exists
-	user, err := GetUser(User{Username: tr.Username})
+	user, err := GetUser(User{Email: tr.Email})
 	if err != nil {
 		return
 	}
@@ -96,7 +97,7 @@ func RequestUserPasswordResetToken(tr *PasswordTokenRequest) (err error) {
 	}
 
 	// Dont send a mail if we're testing
-	if IsTesting {
+	if !viper.GetBool("mailer.enabled") {
 		return
 	}
 

pkg/models/user_test.go

@@ -78,7 +78,7 @@ func TestCreateUser(t *testing.T) {
 	assert.True(t, IsErrUserDoesNotExist(err))
 
 	// Check the user credentials with an unverified email
-	user, err := CheckUserCredentials(&UserLogin{"testuu", "1234"})
+	user, err := CheckUserCredentials(&UserLogin{"user5", "1234"})
 	assert.Error(t, err)
 	assert.True(t, IsErrEmailNotConfirmed(err))
 
@@ -97,7 +97,7 @@ func TestCreateUser(t *testing.T) {
 	// Check usercredentials for a nonexistent user (should fail)
 	_, err = CheckUserCredentials(&UserLogin{"dfstestuu", "1234"})
 	assert.Error(t, err)
-	assert.True(t, IsErrUserDoesNotExist(err))
+	assert.True(t, IsErrWrongUsernameOrPassword(err))
 
 	// Update the user
 	uuser, err := UpdateUser(User{ID: theuser.ID, Password: "444444"})
@@ -146,7 +146,7 @@ func TestCreateUser(t *testing.T) {
 func TestUserPasswordReset(t *testing.T) {
 	// Request a new token
 	tr := &PasswordTokenRequest{
-		Username: "user1",
+		Email: "user1@example.com",
 	}
 	err := RequestUserPasswordResetToken(tr)
 	assert.NoError(t, err)

pkg/routes/api/v1/user_password_reset.go

@@ -67,6 +67,10 @@ func UserRequestResetPasswordToken(c echo.Context) error {
 		return echo.NewHTTPError(http.StatusBadRequest, "No username provided.")
 	}
 
+	if err := c.Validate(pwTokenReset); err != nil {
+		return echo.NewHTTPError(http.StatusBadRequest, err)
+	}
+
 	err := models.RequestUserPasswordResetToken(&pwTokenReset)
 	if err != nil {
 		return handler.HandleHTTPError(err, c)

pkg/routes/routes.go

@@ -14,22 +14,6 @@
 //  You should have received a copy of the GNU General Public License
 //  along with this program.  If not, see <https://www.gnu.org/licenses/>.
 
-//  Vikunja is a todo-list application to facilitate your life.
-//  Copyright 2018 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 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 General Public License for more details.
-//
-//  You should have received a copy of the GNU General Public License
-//  along with this program.  If not, see <https://www.gnu.org/licenses/>.
-
 // @title Vikunja API
 // @license.name GPLv3
 // @BasePath /api/v1