fix: remove panic() and relay password hashing errors to UI

api/session_test.go

@@ -14,6 +14,7 @@ import (
 	"github.com/gotify/server/v2/model"
 	"github.com/gotify/server/v2/test/testdb"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 )
 
@@ -39,9 +40,12 @@ func (s *SessionSuite) BeforeTest(suiteName, testName string) {
 	s.notified = false
 	s.a = &SessionAPI{DB: s.db, NotifyDeleted: s.notify}
 
+	pw, err := password.CreatePassword("testpass", 5)
+	require.NoError(s.T(), err)
+
 	s.db.CreateUser(&model.User{
 		Name: "testuser",
-		Pass: password.CreatePassword("testpass", 5),
+		Pass: pw,
 	})
 }
 

api/user.go

@@ -188,10 +188,19 @@ func (a *UserAPI) GetCurrentUser(ctx *gin.Context) {
 func (a *UserAPI) CreateUser(ctx *gin.Context) {
 	user := model.CreateUserExternal{}
 	if err := ctx.Bind(&user); err == nil {
+		if err := password.ValidateNewPassword(user.Pass); err != nil {
+			ctx.AbortWithError(http.StatusBadRequest, err)
+			return
+		}
+		pw, err := password.CreatePassword(user.Pass, a.PasswordStrength)
+		if err != nil {
+			ctx.AbortWithError(http.StatusInternalServerError, fmt.Errorf("failed to prepare password: %s", err))
+			return
+		}
 		internal := &model.User{
 			Name:  user.Name,
 			Admin: user.Admin,
-			Pass:  password.CreatePassword(user.Pass, a.PasswordStrength),
+			Pass:  pw,
 		}
 		existingUser, err := a.DB.GetUserByName(internal.Name)
 		if success := successOrAbort(ctx, 500, err); !success {
@@ -389,11 +398,20 @@ func (a *UserAPI) DeleteUserByID(ctx *gin.Context) {
 func (a *UserAPI) ChangePassword(ctx *gin.Context) {
 	pw := model.UserExternalPass{}
 	if err := ctx.Bind(&pw); err == nil {
+		if err := password.ValidateNewPassword(pw.Pass); err != nil {
+			ctx.AbortWithError(http.StatusBadRequest, err)
+			return
+		}
 		user, err := a.DB.GetUserByID(auth.GetUserID(ctx))
 		if success := successOrAbort(ctx, 500, err); !success {
 			return
 		}
-		user.Pass = password.CreatePassword(pw.Pass, a.PasswordStrength)
+		pw, err := password.CreatePassword(pw.Pass, a.PasswordStrength)
+		if err != nil {
+			ctx.AbortWithError(http.StatusInternalServerError, fmt.Errorf("failed to prepare password: %s", err))
+			return
+		}
+		user.Pass = pw
 		successOrAbort(ctx, 500, a.DB.UpdateUser(user))
 	}
 }
@@ -465,7 +483,16 @@ func (a *UserAPI) UpdateUserByID(ctx *gin.Context) {
 				dbUser.Admin = updatedUser.Admin
 
 				if updatedUser.Pass != "" {
-					dbUser.Pass = password.CreatePassword(updatedUser.Pass, a.PasswordStrength)
+					if err := password.ValidateNewPassword(updatedUser.Pass); err != nil {
+						ctx.AbortWithError(http.StatusBadRequest, err)
+						return
+					}
+					pw, err := password.CreatePassword(updatedUser.Pass, a.PasswordStrength)
+					if err != nil {
+						ctx.AbortWithError(http.StatusInternalServerError, fmt.Errorf("failed to prepare password: %s", err))
+						return
+					}
+					dbUser.Pass = pw
 				}
 				if success := successOrAbort(ctx, 500, a.DB.UpdateUser(dbUser)); !success {
 					return

api/user_test.go

@@ -14,6 +14,7 @@ import (
 	"github.com/gotify/server/v2/test"
 	"github.com/gotify/server/v2/test/testdb"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 )
 
@@ -320,6 +321,24 @@ func (s *UserSuite) Test_CreateUser_NameAlreadyExists() {
 	assert.Equal(s.T(), 400, s.recorder.Code)
 }
 
+func (s *UserSuite) Test_CreateUser_EmptyPassword_Expect400() {
+	s.loginAdmin()
+
+	s.ctx.Request = httptest.NewRequest("POST", "/user", strings.NewReader(`{"name": "admin", "pass": "", "admin": false}`))
+	s.ctx.Request.Header.Set("Content-Type", "application/json")
+	s.a.CreateUser(s.ctx)
+	assert.Equal(s.T(), 400, s.recorder.Code)
+}
+
+func (s *UserSuite) Test_CreateUser_TooLongPassword_Expect400() {
+	s.loginAdmin()
+
+	s.ctx.Request = httptest.NewRequest("POST", "/user", strings.NewReader(`{"name": "admin", "pass": "`+strings.Repeat("a", 100)+`", "admin": false}`))
+	s.ctx.Request.Header.Set("Content-Type", "application/json")
+	s.a.CreateUser(s.ctx)
+	assert.Equal(s.T(), 400, s.recorder.Code)
+}
+
 func (s *UserSuite) Test_UpdateUserByID_InvalidID() {
 	s.ctx.Params = gin.Params{{Key: "id", Value: "abc"}}
 
@@ -331,6 +350,28 @@ func (s *UserSuite) Test_UpdateUserByID_InvalidID() {
 	assert.Equal(s.T(), 400, s.recorder.Code)
 }
 
+func (s *UserSuite) Test_UpdateUserByID_EmptyPassword_Expect400() {
+	s.loginAdmin()
+
+	s.ctx.Params = gin.Params{{Key: "id", Value: "1"}}
+
+	s.ctx.Request = httptest.NewRequest("POST", "/user/1", strings.NewReader(`{"name": "admin", "pass": "", "admin": false}`))
+	s.ctx.Request.Header.Set("Content-Type", "application/json")
+	s.a.UpdateUserByID(s.ctx)
+	assert.Equal(s.T(), 400, s.recorder.Code)
+}
+
+func (s *UserSuite) Test_UpdateUserByID_TooLongPassword_Expect400() {
+	s.loginAdmin()
+
+	s.ctx.Params = gin.Params{{Key: "id", Value: "1"}}
+
+	s.ctx.Request = httptest.NewRequest("POST", "/user/1", strings.NewReader(`{"name": "admin", "pass": "`+strings.Repeat("a", 100)+`", "admin": false}`))
+	s.ctx.Request.Header.Set("Content-Type", "application/json")
+	s.a.UpdateUserByID(s.ctx)
+	assert.Equal(s.T(), 400, s.recorder.Code)
+}
+
 func (s *UserSuite) Test_UpdateUserByID_LastAdmin_Expect400() {
 	s.db.CreateUser(&model.User{
 		ID:    7,
@@ -359,7 +400,9 @@ func (s *UserSuite) Test_UpdateUserByID_UnknownUser() {
 }
 
 func (s *UserSuite) Test_UpdateUserByID_UpdateNotPassword() {
-	s.db.CreateUser(&model.User{ID: 2, Name: "nico", Pass: password.CreatePassword("old", 5)})
+	pw, err := password.CreatePassword("old", 5)
+	require.NoError(s.T(), err)
+	s.db.CreateUser(&model.User{ID: 2, Name: "nico", Pass: pw})
 
 	s.ctx.Params = gin.Params{{Key: "id", Value: "2"}}
 
@@ -376,7 +419,9 @@ func (s *UserSuite) Test_UpdateUserByID_UpdateNotPassword() {
 }
 
 func (s *UserSuite) Test_UpdateUserByID_UpdatePassword() {
-	s.db.CreateUser(&model.User{ID: 2, Name: "tom", Pass: password.CreatePassword("old", 5)})
+	pw, err := password.CreatePassword("old", 5)
+	require.NoError(s.T(), err)
+	s.db.CreateUser(&model.User{ID: 2, Name: "tom", Pass: pw})
 
 	s.ctx.Params = gin.Params{{Key: "id", Value: "2"}}
 
@@ -413,7 +458,9 @@ func (s *UserSuite) Test_UpdateUserByID_PreservesOIDCID() {
 }
 
 func (s *UserSuite) Test_UpdatePassword() {
-	s.db.CreateUser(&model.User{ID: 1, Name: "jmattheis", Pass: password.CreatePassword("old", 5)})
+	pw, err := password.CreatePassword("old", 5)
+	require.NoError(s.T(), err)
+	s.db.CreateUser(&model.User{ID: 1, Name: "jmattheis", Pass: pw})
 
 	test.WithUser(s.ctx, 1)
 	s.ctx.Request = httptest.NewRequest("POST", "/user/current/password", strings.NewReader(`{"pass": "new"}`))
@@ -429,7 +476,9 @@ func (s *UserSuite) Test_UpdatePassword() {
 }
 
 func (s *UserSuite) Test_UpdatePassword_EmptyPassword() {
-	s.db.CreateUser(&model.User{ID: 1, Name: "jmattheis", Pass: password.CreatePassword("old", 5)})
+	pw, err := password.CreatePassword("old", 5)
+	require.NoError(s.T(), err)
+	s.db.CreateUser(&model.User{ID: 1, Name: "jmattheis", Pass: pw})
 
 	test.WithUser(s.ctx, 1)
 	s.ctx.Request = httptest.NewRequest("POST", "/user/current/password", strings.NewReader(`{"pass":""}`))
@@ -444,6 +493,18 @@ func (s *UserSuite) Test_UpdatePassword_EmptyPassword() {
 	assert.True(s.T(), password.ComparePassword(user.Pass, []byte("old")))
 }
 
+func (s *UserSuite) Test_UpdatePassword_TooLongPassword_Expect400() {
+	pw, err := password.CreatePassword("old", 5)
+	require.NoError(s.T(), err)
+	s.db.CreateUser(&model.User{ID: 1, Name: "jmattheis", Pass: pw})
+
+	test.WithUser(s.ctx, 1)
+	s.ctx.Request = httptest.NewRequest("POST", "/user/current/password", strings.NewReader(`{"pass": "`+strings.Repeat("a", 100)+`"}`))
+	s.ctx.Request.Header.Set("Content-Type", "application/json")
+	s.a.ChangePassword(s.ctx)
+	assert.Equal(s.T(), 400, s.recorder.Code)
+}
+
 func (s *UserSuite) loginAdmin() {
 	s.db.CreateUser(&model.User{ID: 1, Name: "admin", Admin: true})
 	auth.RegisterUser(s.ctx, &model.User{ID: 1, Admin: true})

auth/authentication_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/gotify/server/v2/model"
 	"github.com/gotify/server/v2/test/testdb"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 )
 
@@ -37,9 +38,12 @@ func (s *AuthenticationSuite) SetupSuite() {
 	elevated := now.Add(time.Hour)
 	expired := now.Add(-time.Hour)
 
+	pw, err := password.CreatePassword("pw", 5)
+	require.NoError(s.T(), err)
+
 	s.DB.CreateUser(&model.User{
 		Name:         "existing",
-		Pass:         password.CreatePassword("pw", 5),
+		Pass:         pw,
 		Admin:        false,
 		Applications: []model.Application{{Token: "apptoken", Name: "backup server1", Description: "irrelevant"}},
 		Clients: []model.Client{
@@ -51,7 +55,7 @@ func (s *AuthenticationSuite) SetupSuite() {
 
 	s.DB.CreateUser(&model.User{
 		Name:         "admin",
-		Pass:         password.CreatePassword("pw", 5),
+		Pass:         pw,
 		Admin:        true,
 		Applications: []model.Application{{Token: "apptoken_admin", Name: "backup server2", Description: "irrelevant"}},
 		Clients: []model.Client{

auth/password/password.go

@@ -1,14 +1,30 @@
 package password
 
-import "golang.org/x/crypto/bcrypt"
+import (
+	"errors"
+
+	"golang.org/x/crypto/bcrypt"
+)
+
+var ErrUnexpectedError = errors.New("unexpected error")
+
+func ValidateNewPassword(pw string) error {
+	if pw == "" {
+		return errors.New("password must not be empty")
+	}
+	if len([]byte(pw)) > 72 {
+		return bcrypt.ErrPasswordTooLong
+	}
+	return nil
+}
 
 // CreatePassword returns a hashed version of the given password.
-func CreatePassword(pw string, strength int) []byte {
+func CreatePassword(pw string, strength int) ([]byte, error) {
 	hashedPassword, err := bcrypt.GenerateFromPassword([]byte(pw), strength)
-	if err != nil {
-		panic(err)
+	if err != nil && err != bcrypt.ErrPasswordTooLong {
+		err = ErrUnexpectedError
 	}
-	return hashedPassword
+	return hashedPassword, err
 }
 
 // ComparePassword compares a hashed password with its possible plaintext equivalent.

auth/password/password_test.go

@@ -1,21 +1,32 @@
 package password
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"golang.org/x/crypto/bcrypt"
 )
 
 func TestPasswordSuccess(t *testing.T) {
-	password := CreatePassword("secret", 5)
+	password, err := CreatePassword("secret", 5)
+	require.NoError(t, err)
 	assert.Equal(t, true, ComparePassword(password, []byte("secret")))
 }
 
 func TestPasswordFailure(t *testing.T) {
-	password := CreatePassword("secret", 5)
+	password, err := CreatePassword("secret", 5)
+	require.NoError(t, err)
 	assert.Equal(t, false, ComparePassword(password, []byte("secretx")))
 }
 
-func TestBCryptFailure(t *testing.T) {
-	assert.Panics(t, func() { CreatePassword("secret", 12312) })
+func TestBCryptoTooLongErrorIsReturned(t *testing.T) {
+	_, err := CreatePassword(strings.Repeat("a", 100), 5)
+	assert.ErrorIs(t, err, bcrypt.ErrPasswordTooLong)
+}
+
+func TestBCryptErrorIsMasked(t *testing.T) {
+	_, err := CreatePassword("secret", 12312)
+	assert.ErrorIs(t, err, ErrUnexpectedError)
 }

config/config_test.go

@@ -13,7 +13,8 @@ func TestConfigEnv(t *testing.T) {
 	mode.Set(mode.TestDev)
 	os.Setenv("GOTIFY_DEFAULTUSER_NAME", "jmattheis")
 	os.Setenv("GOTIFY_SERVER_SSL_LETSENCRYPT_HOSTS", "push.example.tld,push.other.tld")
-	os.Setenv("GOTIFY_SERVER_RESPONSEHEADERS",
+	os.Setenv(
+		"GOTIFY_SERVER_RESPONSEHEADERS",
 		`{"Access-Control-Allow-Origin":"*","Access-Control-Allow-Methods":"GET,POST"}`,
 	)
 	os.Setenv("GOTIFY_SERVER_CORS_ALLOWORIGINS", ".+.example.com,otherdomain.com")

database/database.go

@@ -94,7 +94,11 @@ func New(dialect, connection, defaultUser, defaultPass string, strength int, cre
 	userCount := int64(0)
 	db.Find(new(model.User)).Count(&userCount)
 	if createDefaultUserIfNotExist && userCount == 0 {
-		db.Create(&model.User{Name: defaultUser, Pass: password.CreatePassword(defaultPass, strength), Admin: true})
+		pass, err := password.CreatePassword(defaultPass, strength)
+		if err != nil {
+			return nil, err
+		}
+		db.Create(&model.User{Name: defaultUser, Pass: pass, Admin: true})
 	}
 
 	if err := db.Transaction(fillMissingSortKeys, &sql.TxOptions{Isolation: sql.LevelSerializable}); err != nil {

plugin/testing/mock/mock.go

@@ -151,7 +151,7 @@ func (c *PluginInstance) DefaultConfig() any {
 
 // ValidateAndSetConfig implements compat.Configuror
 func (c *PluginInstance) ValidateAndSetConfig(config any) error {
-	if (config.(*PluginConfig)).IsNotValid {
+	if config.(*PluginConfig).IsNotValid {
 		return errors.New("conf is not valid")
 	}
 	c.Config = config.(*PluginConfig)

router/router.go

@@ -66,7 +66,8 @@ func Create(db *database.GormDatabase, vInfo *model.VersionInfo, conf *config.Co
 		})
 	}
 	streamHandler := stream.New(
-		time.Duration(conf.Server.Stream.PingPeriodSeconds)*time.Second, 15*time.Second, conf.Server.Stream.AllowedOrigins)
+		time.Duration(conf.Server.Stream.PingPeriodSeconds)*time.Second, 15*time.Second, conf.Server.Stream.AllowedOrigins,
+	)
 	go func() {
 		ticker := time.NewTicker(5 * time.Minute)
 		for range ticker.C {