diff --git a/.env.example b/.env.example index 45d488e1..08b0a52f 100644 --- a/.env.example +++ b/.env.example @@ -89,7 +89,7 @@ TINYAUTH_AUTH_USERATTRIBUTES_name_ADDRESS_REGION= TINYAUTH_AUTH_USERATTRIBUTES_name_ADDRESS_POSTALCODE= # Country. TINYAUTH_AUTH_USERATTRIBUTES_name_ADDRESS_COUNTRY= -# Path to the users file. +# Path to the users file, one username:password_hash[:totp_secret] user per line (comma-separated users on one line are also accepted). TINYAUTH_AUTH_USERSFILE= # Enable secure cookies. TINYAUTH_AUTH_SECURECOOKIE=false diff --git a/cmd/tinyauth/verify_user.go b/cmd/tinyauth/verify_user.go index fe9ad0fe..a3c3c271 100644 --- a/cmd/tinyauth/verify_user.go +++ b/cmd/tinyauth/verify_user.go @@ -4,7 +4,9 @@ import ( "errors" "fmt" "os" + "slices" + "github.com/tinyauthapp/tinyauth/internal/model" "github.com/tinyauthapp/tinyauth/internal/utils" "charm.land/huh/v2" @@ -86,16 +88,21 @@ func verifyUserCmd() *cli.Command { return fmt.Errorf("user, username, and password are required") } - user, err := utils.ParseUser(tCfg.User) + // parse like the server does, a users file line may hold several comma-separated users + users, err := utils.ParseUserEntry(tCfg.User) if err != nil { return fmt.Errorf("failed to parse user: %w", err) } - if user.Username != tCfg.Username { + idx := slices.IndexFunc(users, func(u model.LocalUser) bool { return u.Username == tCfg.Username }) + + if idx == -1 { return fmt.Errorf("username is incorrect") } + user := users[idx] + err = bcrypt.CompareHashAndPassword([]byte(user.Password), []byte(tCfg.Password)) if err != nil { diff --git a/internal/model/config.go b/internal/model/config.go index 9d514390..0815ba09 100644 --- a/internal/model/config.go +++ b/internal/model/config.go @@ -145,7 +145,7 @@ type AuthConfig struct { Users []string `description:"Comma-separated list of users (username:hashed_password)." yaml:"users,omitempty"` SubdomainsEnabled bool `description:"Enable subdomains support." yaml:"subdomainsEnabled,omitempty"` UserAttributes map[string]UserAttributes `description:"Map of per-user OIDC attributes (username -> attributes)." yaml:"userAttributes,omitempty"` - UsersFile string `description:"Path to the users file." yaml:"usersFile,omitempty"` + UsersFile string `description:"Path to the users file, one username:password_hash[:totp_secret] user per line (comma-separated users on one line are also accepted)." yaml:"usersFile,omitempty"` SecureCookie bool `description:"Enable secure cookies." yaml:"secureCookie,omitempty"` SessionExpiry int `description:"Session expiry time in seconds." yaml:"sessionExpiry,omitempty"` SessionMaxLifetime int `description:"Maximum session lifetime in seconds." yaml:"sessionMaxLifetime,omitempty"` diff --git a/internal/utils/user_utils.go b/internal/utils/user_utils.go index d94b3a20..1216e85b 100644 --- a/internal/utils/user_utils.go +++ b/internal/utils/user_utils.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/tinyauthapp/tinyauth/internal/model" + "golang.org/x/crypto/bcrypt" ) func ParseUsers(usersStr []string, userAttributes map[string]model.UserAttributes) (*[]model.LocalUser, error) { @@ -16,23 +17,91 @@ func ParseUsers(usersStr []string, userAttributes map[string]model.UserAttribute return nil, nil } - for _, user := range usersStr { - if strings.TrimSpace(user) == "" { + for i, entry := range usersStr { + if strings.TrimSpace(entry) == "" { continue } - parsed, err := ParseUser(strings.TrimSpace(user)) + parsed, err := ParseUserEntry(entry) if err != nil { - return nil, err + return nil, fmt.Errorf("user entry %d: %w", i+1, err) } - if attrs, ok := userAttributes[parsed.Username]; ok { - parsed.Attributes = attrs + for _, user := range parsed { + if attrs, ok := userAttributes[user.Username]; ok { + user.Attributes = attrs + } + users = append(users, user) } - users = append(users, *parsed) } return &users, nil } +// ParseUserEntry parses one entry (e.g. a users file line). Like v4, an entry may hold several comma-separated +// users. It is only split when there are at least two users and every part is a valid user with a bcrypt hash, +// so usernames containing a comma keep working and a malformed line is never split into different users. +func ParseUserEntry(entry string) ([]model.LocalUser, error) { + if strings.Contains(entry, ",") { + var users []model.LocalUser + parts := strings.Split(entry, ",") + for i, part := range parts { + if strings.TrimSpace(part) == "" { + // a single trailing comma is tolerated (v4 wrote one), any other empty + // part means this is not a clean list, so do not reinterpret usernames + if i == len(parts)-1 { + continue + } + users = nil + break + } + user, err := ParseUser(strings.TrimSpace(part)) + if err != nil || !isBcryptHash(user.Password) { + users = nil + break + } + users = append(users, *user) + } + // A single user with only a tolerated trailing comma is still a clean list; return it + // so a v4 file with one user per line (each ending in a comma) keeps working. + trailingOnly := len(users) == 1 && len(parts) == 2 && strings.TrimSpace(parts[1]) == "" + if len(users) > 1 || trailingOnly { + return users, nil + } + } + + user, err := ParseUser(strings.TrimSpace(entry)) + if err != nil { + return nil, err + } + + // password hashes and TOTP secrets never contain a comma, this is a list with an invalid user in it + if strings.Contains(user.Password, ",") || strings.Contains(user.TOTPSecret, ",") { + return nil, errors.New("invalid user format, expected username:password_hash[:totp_secret] separated by commas") + } + + return []model.LocalUser{*user}, nil +} + +// isBcryptHash reports whether s is exactly one bcrypt hash (bcrypt ignores trailing bytes, so the length is checked too) +func isBcryptHash(s string) bool { + if len(s) != 60 { + return false + } + if _, err := bcrypt.Cost([]byte(s)); err != nil { + return false + } + // bcrypt.Cost only validates the "$2x$cost$" prefix, not the body, so a value like + // "$2a$10$" + strings.Repeat("!", 53) would pass. Check the 22-char salt and 31-char + // hash that follow use the bcrypt base64 alphabet so a malformed body is not mistaken + // for a real hash and used to split an entry into separate users. + for i := 7; i < len(s); i++ { + c := s[i] + if c != '.' && c != '/' && (c < '0' || c > '9') && (c < 'A' || c > 'Z') && (c < 'a' || c > 'z') { + return false + } + } + return true +} + func GetUsers(usersCfg []string, usersPath string, userAttributes map[string]model.UserAttributes) (*[]model.LocalUser, error) { usersStr, err := GetStringList(usersCfg, usersPath) if err != nil { @@ -50,13 +119,13 @@ func ParseUser(userStr string) (*model.LocalUser, error) { parts := strings.SplitN(userStr, ":", 4) if len(parts) < 2 || len(parts) > 3 { - return nil, errors.New("invalid user format") + return nil, errors.New("invalid user format, expected username:password_hash[:totp_secret]") } for i, part := range parts { trimmed := strings.TrimSpace(part) if trimmed == "" { - return nil, errors.New("invalid user format") + return nil, errors.New("invalid user format, expected username:password_hash[:totp_secret]") } parts[i] = trimmed } diff --git a/internal/utils/user_utils_test.go b/internal/utils/user_utils_test.go index 973be918..ae0cca01 100644 --- a/internal/utils/user_utils_test.go +++ b/internal/utils/user_utils_test.go @@ -2,6 +2,7 @@ package utils_test import ( "os" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -83,6 +84,81 @@ func TestGetUsers(t *testing.T) { } } + // Test comma-separated users on a single file line (v4 format) + err = os.WriteFile(tmpDir+"/tinyauth_users_comma.txt", []byte("user6:"+hash+":JBSWY3DPEHPK3PXP,user7:"+hash+",\r\nuser8:"+hash+"\r\n"), 0600) + require.NoError(t, err) + + users, err = utils.GetUsers([]string{}, tmpDir+"/tinyauth_users_comma.txt", noAttrs) + + assert.NoError(t, err) + assert.Len(t, *users, 3) + assert.Equal(t, "user6", (*users)[0].Username) + assert.Equal(t, "JBSWY3DPEHPK3PXP", (*users)[0].TOTPSecret) + assert.Equal(t, "user7", (*users)[1].Username) + assert.Equal(t, hash, (*users)[1].Password) + assert.Equal(t, "", (*users)[1].TOTPSecret) + assert.Equal(t, "user8", (*users)[2].Username) + assert.Equal(t, hash, (*users)[2].Password) + + // Test usernames containing a comma are not split + users, err = utils.GetUsers([]string{"Doe, John:" + hash}, "", noAttrs) + + assert.NoError(t, err) + assert.Len(t, *users, 1) + assert.Equal(t, "Doe, John", (*users)[0].Username) + + // Test a comma-separated list with an invalid user is rejected instead of misparsed + _, err = utils.GetUsers([]string{"user6:" + hash + ":JBSWY3DPEHPK3PXP,user7"}, "", noAttrs) + + assert.ErrorContains(t, err, "user entry 1: invalid user format") + + // Test a malformed line is never split into different users (would drop the TOTP of user6) + _, err = utils.GetUsers([]string{"user6:" + hash + ",x:JBSWY3DPEHPK3PXP"}, "", noAttrs) + + assert.ErrorContains(t, err, "user entry 1: invalid user format") + + // Test a single user with a stray comma is not renamed + users, err = utils.GetUsers([]string{",user9:" + hash}, "", noAttrs) + + assert.NoError(t, err) + assert.Len(t, *users, 1) + assert.Equal(t, ",user9", (*users)[0].Username) + + // Test a leading comma in a multi-user entry does not rename the first user by dropping the blank part + _, err = utils.GetUsers([]string{",user9:" + hash + ",user10:" + hash}, "", noAttrs) + + assert.ErrorContains(t, err, "user entry 1: invalid user format") + + // Test a single user with a tolerated trailing comma (v4 wrote one) still parses + users, err = utils.GetUsers([]string{"user13:" + hash + ","}, "", noAttrs) + + assert.NoError(t, err) + assert.Len(t, *users, 1) + assert.Equal(t, "user13", (*users)[0].Username) + assert.Equal(t, hash, (*users)[0].Password) + + // Test a single user with TOTP and a trailing comma keeps the TOTP intact + users, err = utils.GetUsers([]string{"user14:" + hash + ":JBSWY3DPEHPK3PXP,"}, "", noAttrs) + + assert.NoError(t, err) + assert.Len(t, *users, 1) + assert.Equal(t, "user14", (*users)[0].Username) + assert.Equal(t, "JBSWY3DPEHPK3PXP", (*users)[0].TOTPSecret) + + // Test a well-shaped but malformed bcrypt body is not accepted as a hash that would split the entry + badHash := "$2a$10$" + strings.Repeat("!", 53) + + assert.Len(t, badHash, 60) + + _, err = utils.GetUsers([]string{"user11:" + badHash + ",user12:" + hash}, "", noAttrs) + + assert.ErrorContains(t, err, "user entry 1: invalid user format") + + // Test invalid entry reports its position + _, err = utils.GetUsers([]string{"user8:" + hash, "user9"}, "", noAttrs) + + assert.ErrorContains(t, err, "user entry 2: invalid user format") + // Test empty users, err = utils.GetUsers([]string{}, "", noAttrs)