diff --git a/cmd/serve.go b/cmd/serve.go index 8b5672cc..d71c40eb 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -555,7 +555,7 @@ func parseUsers(usersRaw []string) ([]*user.User, error) { role := user.Role(strings.TrimSpace(parts[2])) if !user.AllowedUsername(username) { return nil, fmt.Errorf("invalid auth-users: %s, username invalid", userLine) - } else if err := user.ValidPasswordHash(passwordHash); err != nil { + } else if err := user.ValidPasswordHash(passwordHash, user.DefaultUserPasswordBcryptCost); err != nil { return nil, fmt.Errorf("invalid auth-users: %s, password hash invalid, %s", userLine, err.Error()) } else if !user.AllowedRole(role) { return nil, fmt.Errorf("invalid auth-users: %s, role %s is not allowed, allowed roles are 'admin' or 'user'", userLine, role) diff --git a/cmd/serve_test.go b/cmd/serve_test.go index 339423b6..b89efa8a 100644 --- a/cmd/serve_test.go +++ b/cmd/serve_test.go @@ -26,11 +26,11 @@ func TestParseUsers_Success(t *testing.T) { }{ { name: "single user", - input: []string{"alice:$2a$10$abcdefghijklmnopqrstuvwxyz:user"}, + input: []string{"alice:$2a$10$320YlQeaMghYZsvtu9jzfOQZS32FysWY/T9qu5NWqcIh.DN.u5P5S:user"}, expected: []*user.User{ { Name: "alice", - Hash: "$2a$10$abcdefghijklmnopqrstuvwxyz", + Hash: "$2a$10$320YlQeaMghYZsvtu9jzfOQZS32FysWY/T9qu5NWqcIh.DN.u5P5S", Role: user.RoleUser, Provisioned: true, }, @@ -39,19 +39,19 @@ func TestParseUsers_Success(t *testing.T) { { name: "multiple users with different roles", input: []string{ - "alice:$2a$10$abcdefghijklmnopqrstuvwxyz:user", - "bob:$2b$10$abcdefghijklmnopqrstuvwxyz:admin", + "alice:$2a$10$320YlQeaMghYZsvtu9jzfOQZS32FysWY/T9qu5NWqcIh.DN.u5P5S:user", + "bob:$2a$10$jIcuBWcbxd6oW1aPvoJ5iOShzu3/UJ2kSxKbTZtDypG06nBflQagq:admin", }, expected: []*user.User{ { Name: "alice", - Hash: "$2a$10$abcdefghijklmnopqrstuvwxyz", + Hash: "$2a$10$320YlQeaMghYZsvtu9jzfOQZS32FysWY/T9qu5NWqcIh.DN.u5P5S", Role: user.RoleUser, Provisioned: true, }, { Name: "bob", - Hash: "$2b$10$abcdefghijklmnopqrstuvwxyz", + Hash: "$2a$10$jIcuBWcbxd6oW1aPvoJ5iOShzu3/UJ2kSxKbTZtDypG06nBflQagq", Role: user.RoleAdmin, Provisioned: true, }, @@ -64,11 +64,11 @@ func TestParseUsers_Success(t *testing.T) { }, { name: "user with special characters in name", - input: []string{"alice.test+123@example.com:$2y$10$abcdefghijklmnopqrstuvwxyz:user"}, + input: []string{"alice.test+123@example.com:$2a$10$RYUYAsl5zOnAIp6fH7BPX.Eug0rUfEUk92r8WiVusb0VK.vGojWBe:user"}, expected: []*user.User{ { Name: "alice.test+123@example.com", - Hash: "$2y$10$abcdefghijklmnopqrstuvwxyz", + Hash: "$2a$10$RYUYAsl5zOnAIp6fH7BPX.Eug0rUfEUk92r8WiVusb0VK.vGojWBe", Role: user.RoleUser, Provisioned: true, }, @@ -110,23 +110,23 @@ func TestParseUsers_Errors(t *testing.T) { }, { name: "invalid username", - input: []string{"alice@#$%:$2a$10$abcdefghijklmnopqrstuvwxyz:user"}, - error: "invalid auth-users: alice@#$%:$2a$10$abcdefghijklmnopqrstuvwxyz:user, username invalid", + input: []string{"alice@#$%:$2a$10$320YlQeaMghYZsvtu9jzfOQZS32FysWY/T9qu5NWqcIh.DN.u5P5S:user"}, + error: "invalid auth-users: alice@#$%:$2a$10$320YlQeaMghYZsvtu9jzfOQZS32FysWY/T9qu5NWqcIh.DN.u5P5S:user, username invalid", }, { name: "invalid password hash - wrong prefix", input: []string{"alice:plaintext:user"}, - error: "invalid auth-users: alice:plaintext:user, password hash but be a bcrypt hash, use 'ntfy user hash' to generate", + error: "invalid auth-users: alice:plaintext:user, password hash invalid, password hash must be a bcrypt hash, use 'ntfy user hash' to generate", }, { name: "invalid role", - input: []string{"alice:$2a$10$abcdefghijklmnopqrstuvwxyz:invalid"}, - error: "invalid auth-users: alice:$2a$10$abcdefghijklmnopqrstuvwxyz:invalid, role invalid is not allowed, allowed roles are 'admin' or 'user'", + input: []string{"alice:$2a$10$320YlQeaMghYZsvtu9jzfOQZS32FysWY/T9qu5NWqcIh.DN.u5P5S:invalid"}, + error: "invalid auth-users: alice:$2a$10$320YlQeaMghYZsvtu9jzfOQZS32FysWY/T9qu5NWqcIh.DN.u5P5S:invalid, role invalid is not allowed, allowed roles are 'admin' or 'user'", }, { name: "empty username", - input: []string{":$2a$10$abcdefghijklmnopqrstuvwxyz:user"}, - error: "invalid auth-users: :$2a$10$abcdefghijklmnopqrstuvwxyz:user, username invalid", + input: []string{":$2a$10$320YlQeaMghYZsvtu9jzfOQZS32FysWY/T9qu5NWqcIh.DN.u5P5S:user"}, + error: "invalid auth-users: :$2a$10$320YlQeaMghYZsvtu9jzfOQZS32FysWY/T9qu5NWqcIh.DN.u5P5S:user, username invalid", }, } diff --git a/user/manager.go b/user/manager.go index dc9bbaee..1d900604 100644 --- a/user/manager.go +++ b/user/manager.go @@ -1066,7 +1066,7 @@ func (a *Manager) addUserTx(tx *sql.Tx, username, password string, role Role, ha var err error = nil if hashed { hash = password - if err := ValidPasswordHash(hash); err != nil { + if err := ValidPasswordHash(hash, a.config.BcryptCost); err != nil { return err } } else { @@ -1434,7 +1434,7 @@ func (a *Manager) changePasswordTx(tx *sql.Tx, username, password string, hashed var err error if hashed { hash = password - if err := ValidPasswordHash(hash); err != nil { + if err := ValidPasswordHash(hash, a.config.BcryptCost); err != nil { return err } } else { diff --git a/user/manager_test.go b/user/manager_test.go index b8817682..26a70942 100644 --- a/user/manager_test.go +++ b/user/manager_test.go @@ -1162,7 +1162,7 @@ func TestManager_WithProvisionedUsers(t *testing.T) { // Re-open the DB (second app start) require.Nil(t, a.db.Close()) conf.Users = []*User{ - {Name: "philuser", Hash: "$2a$10$AAAU21sX1uhZamTLJXHuxgVC0Z/GKISibrKCLohPgtG7yIxSk4C", Role: RoleUser}, + {Name: "philuser", Hash: "$2a$10$AAAAU21sX1uhZamTLJXHuxgVC0Z/GKISibrKCLohPgtG7yIxSk4C", Role: RoleUser}, } conf.Access = map[string][]*Grant{ "philuser": { @@ -1292,7 +1292,7 @@ func TestManager_UpdateNonProvisionedUsersToProvisionedUsers(t *testing.T) { // Re-open the DB (second app start) require.Nil(t, a.db.Close()) conf.Users = []*User{ - {Name: "philuser", Hash: "$2a$10$AAAU21sX1uhZamTLJXHuxgVC0Z/GKISibrKCLohPgtG7yIxSk4C", Role: RoleUser}, + {Name: "philuser", Hash: "$2a$10$AAAAU21sX1uhZamTLJXHuxgVC0Z/GKISibrKCLohPgtG7yIxSk4C", Role: RoleUser}, } conf.Access = map[string][]*Grant{ "philuser": { @@ -1308,7 +1308,7 @@ func TestManager_UpdateNonProvisionedUsersToProvisionedUsers(t *testing.T) { require.Len(t, users, 2) require.Equal(t, "philuser", users[0].Name) require.Equal(t, RoleUser, users[0].Role) - require.Equal(t, "$2a$10$AAAU21sX1uhZamTLJXHuxgVC0Z/GKISibrKCLohPgtG7yIxSk4C", users[0].Hash) + require.Equal(t, "$2a$10$AAAAU21sX1uhZamTLJXHuxgVC0Z/GKISibrKCLohPgtG7yIxSk4C", users[0].Hash) require.True(t, users[0].Provisioned) // Updated to provisioned! grants, err = a.Grants("philuser") diff --git a/user/util.go b/user/util.go index 2e425e19..91230fae 100644 --- a/user/util.go +++ b/user/util.go @@ -41,14 +41,14 @@ func AllowedTier(tier string) bool { } // ValidPasswordHash checks if the given password hash is a valid bcrypt hash -func ValidPasswordHash(hash string) error { +func ValidPasswordHash(hash string, minCost int) error { if !strings.HasPrefix(hash, "$2a$") && !strings.HasPrefix(hash, "$2b$") && !strings.HasPrefix(hash, "$2y$") { return ErrPasswordHashInvalid } cost, err := bcrypt.Cost([]byte(hash)) - if err != nil { + if err != nil { // Check if the hash is valid (length, format, etc.) return err - } else if cost < DefaultUserPasswordBcryptCost { + } else if cost < minCost { return ErrPasswordHashWeak } return nil