From da3cfae56ee30a98ec750aa53586db3ceaa24503 Mon Sep 17 00:00:00 2001 From: bsaurusrex <82356519+bsaurusrex@users.noreply.github.com> Date: Fri, 9 Oct 2026 09:55:40 +0800 Subject: [PATCH 1/2] fix: accept comma-separated users on one users file line again v4 split all users on commas, including the users file, so files like user1:hash:totp,user2:hash worked. v5 reads the users file line by line and fails on such lines with "invalid user format". An entry is now split on commas again, but only when it holds at least two users and every part is a valid user with a bcrypt hash, so usernames that contain a comma keep working and a malformed line can never be split into different users (e.g. dropping one user's TOTP secret). A line that is not a valid list and whose hash or TOTP secret contains a comma is rejected; such lines were accepted before. The user verify command parses entries the same way, and errors now say which entry is invalid and the expected format. Refs #685 Co-Authored-By: Claude Opus 5.5 --- .env.example | 2 +- cmd/tinyauth/verify_user.go | 11 +++- internal/model/config.go | 2 +- internal/utils/user_utils.go | 84 +++++++++++++++++++++++++++---- internal/utils/user_utils_test.go | 60 ++++++++++++++++++++++ 5 files changed, 146 insertions(+), 13 deletions(-) diff --git a/.env.example b/.env.example index 45d488e17..08b0a52fe 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 fe9ad0fe0..a3c3c271d 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 9d5143904..0815ba090 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 d94b3a20d..0bf229d89 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,88 @@ 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) + } + if len(users) > 1 { + 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 +116,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 973be9183..458a90802 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,65 @@ 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 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) From 8a0f3062117e299c5a45ae1383e04aec84fd2cdf Mon Sep 17 00:00:00 2001 From: bsaurusrex <82356519+bsaurusrex@users.noreply.github.com> Date: Fri, 9 Oct 2026 18:16:14 +0800 Subject: [PATCH 2/2] fix: accept a single users-file line with a trailing comma A users file written by v4 can have one user per line with a trailing comma. The comma-split path produced exactly one user for such a line, failed the len(users) > 1 guard, and fell through to the whole-entry parse, which rejects the trailing comma. Return the single parsed user when the only extra part is the tolerated trailing comma. Refs #685 Co-Authored-By: Claude Opus 4.8 --- internal/utils/user_utils.go | 5 ++++- internal/utils/user_utils_test.go | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/internal/utils/user_utils.go b/internal/utils/user_utils.go index 0bf229d89..1216e85b5 100644 --- a/internal/utils/user_utils.go +++ b/internal/utils/user_utils.go @@ -60,7 +60,10 @@ func ParseUserEntry(entry string) ([]model.LocalUser, error) { } users = append(users, *user) } - if len(users) > 1 { + // 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 } } diff --git a/internal/utils/user_utils_test.go b/internal/utils/user_utils_test.go index 458a90802..ae0cca014 100644 --- a/internal/utils/user_utils_test.go +++ b/internal/utils/user_utils_test.go @@ -129,6 +129,22 @@ func TestGetUsers(t *testing.T) { 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)