From e3f92ce4fcc455e4057e06b6aef107f615e6ef57 Mon Sep 17 00:00:00 2001 From: Stavros Date: Thu, 8 Jan 2026 16:03:37 +0200 Subject: [PATCH] refactor: simplify user parsing (#571) --- internal/bootstrap/router_bootstrap.go | 3 +- internal/config/config.go | 12 +-- internal/controller/proxy_controller_test.go | 2 +- internal/controller/user_controller_test.go | 2 +- internal/service/auth_service.go | 4 +- internal/utils/user_utils.go | 83 ++++++++++---------- internal/utils/user_utils_test.go | 14 ++-- 7 files changed, 59 insertions(+), 61 deletions(-) diff --git a/internal/bootstrap/router_bootstrap.go b/internal/bootstrap/router_bootstrap.go index 531f5e0..abcd81b 100644 --- a/internal/bootstrap/router_bootstrap.go +++ b/internal/bootstrap/router_bootstrap.go @@ -2,7 +2,6 @@ package bootstrap import ( "fmt" - "strings" "github.com/steveiliop56/tinyauth/internal/controller" "github.com/steveiliop56/tinyauth/internal/middleware" @@ -15,7 +14,7 @@ func (app *BootstrapApp) setupRouter() (*gin.Engine, error) { engine.Use(gin.Recovery()) if len(app.config.Server.TrustedProxies) > 0 { - err := engine.SetTrustedProxies(strings.Split(app.config.Server.TrustedProxies, ",")) + err := engine.SetTrustedProxies(app.config.Server.TrustedProxies) if err != nil { return nil, fmt.Errorf("failed to set trusted proxies: %w", err) diff --git a/internal/config/config.go b/internal/config/config.go index 61dda81..920288d 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -33,15 +33,15 @@ type Config struct { } type ServerConfig struct { - Port int `description:"The port on which the server listens." yaml:"port"` - Address string `description:"The address on which the server listens." yaml:"address"` - SocketPath string `description:"The path to the Unix socket." yaml:"socketPath"` - TrustedProxies string `description:"Comma-separated list of trusted proxy addresses." yaml:"trustedProxies"` + Port int `description:"The port on which the server listens." yaml:"port"` + Address string `description:"The address on which the server listens." yaml:"address"` + SocketPath string `description:"The path to the Unix socket." yaml:"socketPath"` + TrustedProxies []string `description:"Comma-separated list of trusted proxy addresses." yaml:"trustedProxies"` } type AuthConfig struct { IP IPConfig `description:"IP whitelisting config options." yaml:"ip"` - Users string `description:"Comma-separated list of users (username:hashed_password)." yaml:"users"` + Users []string `description:"Comma-separated list of users (username:hashed_password)." yaml:"users"` UsersFile string `description:"Path to the users file." yaml:"usersFile"` SecureCookie bool `description:"Enable secure cookies." yaml:"secureCookie"` SessionExpiry int `description:"Session expiry time in seconds." yaml:"sessionExpiry"` @@ -56,7 +56,7 @@ type IPConfig struct { } type OAuthConfig struct { - Whitelist string `description:"Comma-separated list of allowed OAuth domains." yaml:"whitelist"` + Whitelist []string `description:"Comma-separated list of allowed OAuth domains." yaml:"whitelist"` AutoRedirect string `description:"The OAuth provider to use for automatic redirection." yaml:"autoRedirect"` Providers map[string]OAuthServiceConfig `description:"OAuth providers configuration." yaml:"providers"` } diff --git a/internal/controller/proxy_controller_test.go b/internal/controller/proxy_controller_test.go index 5f44382..57711fc 100644 --- a/internal/controller/proxy_controller_test.go +++ b/internal/controller/proxy_controller_test.go @@ -57,7 +57,7 @@ func setupProxyController(t *testing.T, middlewares *[]gin.HandlerFunc) (*gin.En Password: "$2a$10$ne6z693sTgzT3ePoQ05PgOecUHnBjM7sSNj6M.l5CLUP.f6NyCnt.", // test }, }, - OauthWhitelist: "", + OauthWhitelist: []string{}, SessionExpiry: 3600, SessionMaxLifetime: 0, SecureCookie: false, diff --git a/internal/controller/user_controller_test.go b/internal/controller/user_controller_test.go index dc8ec5c..99768c7 100644 --- a/internal/controller/user_controller_test.go +++ b/internal/controller/user_controller_test.go @@ -60,7 +60,7 @@ func setupUserController(t *testing.T, middlewares *[]gin.HandlerFunc) (*gin.Eng TotpSecret: totpSecret, }, }, - OauthWhitelist: "", + OauthWhitelist: []string{}, SessionExpiry: 3600, SessionMaxLifetime: 0, SecureCookie: false, diff --git a/internal/service/auth_service.go b/internal/service/auth_service.go index bde054a..6d829c6 100644 --- a/internal/service/auth_service.go +++ b/internal/service/auth_service.go @@ -27,7 +27,7 @@ type LoginAttempt struct { type AuthServiceConfig struct { Users []config.User - OauthWhitelist string + OauthWhitelist []string SessionExpiry int SessionMaxLifetime int SecureCookie bool @@ -187,7 +187,7 @@ func (auth *AuthService) RecordLoginAttempt(identifier string, success bool) { } func (auth *AuthService) IsEmailWhitelisted(email string) bool { - return utils.CheckFilter(auth.config.OauthWhitelist, email) + return utils.CheckFilter(strings.Join(auth.config.OauthWhitelist, ","), email) } func (auth *AuthService) CreateSessionCookie(c *gin.Context, data *config.SessionCookie) error { diff --git a/internal/utils/user_utils.go b/internal/utils/user_utils.go index 8ee8e94..a56bd6e 100644 --- a/internal/utils/user_utils.go +++ b/internal/utils/user_utils.go @@ -7,22 +7,14 @@ import ( "github.com/steveiliop56/tinyauth/internal/config" ) -func ParseUsers(users string) ([]config.User, error) { - var usersParsed []config.User +func ParseUsers(usersStr []string) ([]config.User, error) { + var users []config.User - users = strings.TrimSpace(users) - - if users == "" { + if len(usersStr) == 0 { return []config.User{}, nil } - userList := strings.Split(users, ",") - - if len(userList) == 0 { - return []config.User{}, errors.New("invalid user format") - } - - for _, user := range userList { + for _, user := range usersStr { if strings.TrimSpace(user) == "" { continue } @@ -30,64 +22,71 @@ func ParseUsers(users string) ([]config.User, error) { if err != nil { return []config.User{}, err } - usersParsed = append(usersParsed, parsed) + users = append(users, parsed) } - return usersParsed, nil + return users, nil } -func GetUsers(conf string, file string) ([]config.User, error) { - var users string +func GetUsers(usersCfg []string, usersPath string) ([]config.User, error) { + var usersStr []string - if conf == "" && file == "" { + if len(usersCfg) == 0 && usersPath == "" { return []config.User{}, nil } - if conf != "" { - users += conf + if len(usersCfg) > 0 { + usersStr = append(usersStr, usersCfg...) } - if file != "" { - contents, err := ReadFile(file) + if usersPath != "" { + contents, err := ReadFile(usersPath) + if err != nil { return []config.User{}, err } - if users != "" { - users += "," + + lines := strings.SplitSeq(contents, "\n") + + for line := range lines { + lineTrimmed := strings.TrimSpace(line) + if lineTrimmed == "" { + continue + } + usersStr = append(usersStr, lineTrimmed) } - users += ParseFileToLine(contents) } - return ParseUsers(users) + return ParseUsers(usersStr) } -func ParseUser(user string) (config.User, error) { - if strings.Contains(user, "$$") { - user = strings.ReplaceAll(user, "$$", "$") +func ParseUser(userStr string) (config.User, error) { + if strings.Contains(userStr, "$$") { + userStr = strings.ReplaceAll(userStr, "$$", "$") } - userSplit := strings.Split(user, ":") + parts := strings.SplitN(userStr, ":", 4) - if len(userSplit) < 2 || len(userSplit) > 3 { + if len(parts) < 2 || len(parts) > 3 { return config.User{}, errors.New("invalid user format") } - for _, userPart := range userSplit { - if strings.TrimSpace(userPart) == "" { + for i, part := range parts { + trimmed := strings.TrimSpace(part) + if trimmed == "" { return config.User{}, errors.New("invalid user format") } + parts[i] = trimmed } - if len(userSplit) == 2 { - return config.User{ - Username: strings.TrimSpace(userSplit[0]), - Password: strings.TrimSpace(userSplit[1]), - }, nil + user := config.User{ + Username: parts[0], + Password: parts[1], } - return config.User{ - Username: strings.TrimSpace(userSplit[0]), - Password: strings.TrimSpace(userSplit[1]), - TotpSecret: strings.TrimSpace(userSplit[2]), - }, nil + if len(parts) == 3 { + user.TotpSecret = parts[2] + } + + return user, nil } diff --git a/internal/utils/user_utils_test.go b/internal/utils/user_utils_test.go index 3cc23c3..658fbdc 100644 --- a/internal/utils/user_utils_test.go +++ b/internal/utils/user_utils_test.go @@ -22,7 +22,7 @@ func TestGetUsers(t *testing.T) { defer os.Remove("/tmp/tinyauth_users_test.txt") // Test file - users, err := utils.GetUsers("", "/tmp/tinyauth_users_test.txt") + users, err := utils.GetUsers([]string{}, "/tmp/tinyauth_users_test.txt") assert.NilError(t, err) @@ -34,7 +34,7 @@ func TestGetUsers(t *testing.T) { assert.Equal(t, "$2a$10$Mz5xhkfSJUtPWkzCd/TdaePh9CaXc5QcGII5wIMPLSR46eTwma30G", users[1].Password) // Test config - users, err = utils.GetUsers("user3:$2a$10$Mz5xhkfSJUtPWkzCd/TdaePh9CaXc5QcGII5wIMPLSR46eTwma30G,user4:$2a$10$Mz5xhkfSJUtPWkzCd/TdaePh9CaXc5QcGII5wIMPLSR46eTwma30G", "") + users, err = utils.GetUsers([]string{"user3:$2a$10$Mz5xhkfSJUtPWkzCd/TdaePh9CaXc5QcGII5wIMPLSR46eTwma30G", "user4:$2a$10$Mz5xhkfSJUtPWkzCd/TdaePh9CaXc5QcGII5wIMPLSR46eTwma30G"}, "") assert.NilError(t, err) @@ -46,7 +46,7 @@ func TestGetUsers(t *testing.T) { assert.Equal(t, "$2a$10$Mz5xhkfSJUtPWkzCd/TdaePh9CaXc5QcGII5wIMPLSR46eTwma30G", users[1].Password) // Test both - users, err = utils.GetUsers("user5:$2a$10$Mz5xhkfSJUtPWkzCd/TdaePh9CaXc5QcGII5wIMPLSR46eTwma30G", "/tmp/tinyauth_users_test.txt") + users, err = utils.GetUsers([]string{"user5:$2a$10$Mz5xhkfSJUtPWkzCd/TdaePh9CaXc5QcGII5wIMPLSR46eTwma30G"}, "/tmp/tinyauth_users_test.txt") assert.NilError(t, err) @@ -60,14 +60,14 @@ func TestGetUsers(t *testing.T) { assert.Equal(t, "$2a$10$Mz5xhkfSJUtPWkzCd/TdaePh9CaXc5QcGII5wIMPLSR46eTwma30G", users[2].Password) // Test empty - users, err = utils.GetUsers("", "") + users, err = utils.GetUsers([]string{}, "") assert.NilError(t, err) assert.Equal(t, 0, len(users)) // Test non-existent file - users, err = utils.GetUsers("", "/tmp/non_existent_file.txt") + users, err = utils.GetUsers([]string{}, "/tmp/non_existent_file.txt") assert.ErrorContains(t, err, "no such file or directory") @@ -76,7 +76,7 @@ func TestGetUsers(t *testing.T) { func TestParseUsers(t *testing.T) { // Valid users - users, err := utils.ParseUsers("user1:$2a$10$Mz5xhkfSJUtPWkzCd/TdaePh9CaXc5QcGII5wIMPLSR46eTwma30G,user2:$2a$10$Mz5xhkfSJUtPWkzCd/TdaePh9CaXc5QcGII5wIMPLSR46eTwma30G:ABCDEF") // user2 has TOTP + users, err := utils.ParseUsers([]string{"user1:$2a$10$Mz5xhkfSJUtPWkzCd/TdaePh9CaXc5QcGII5wIMPLSR46eTwma30G", "user2:$2a$10$Mz5xhkfSJUtPWkzCd/TdaePh9CaXc5QcGII5wIMPLSR46eTwma30G:ABCDEF"}) // user2 has TOTP assert.NilError(t, err) @@ -90,7 +90,7 @@ func TestParseUsers(t *testing.T) { assert.Equal(t, "ABCDEF", users[1].TotpSecret) // Valid weirdly spaced users - users, err = utils.ParseUsers(" user1:$2a$10$Mz5xhkfSJUtPWkzCd/TdaePh9CaXc5QcGII5wIMPLSR46eTwma30G , user2:$2a$10$Mz5xhkfSJUtPWkzCd/TdaePh9CaXc5QcGII5wIMPLSR46eTwma30G:ABCDEF ") // Spacing is on purpose + users, err = utils.ParseUsers([]string{" user1:$2a$10$Mz5xhkfSJUtPWkzCd/TdaePh9CaXc5QcGII5wIMPLSR46eTwma30G ", " user2:$2a$10$Mz5xhkfSJUtPWkzCd/TdaePh9CaXc5QcGII5wIMPLSR46eTwma30G:ABCDEF "}) // Spacing is on purpose assert.NilError(t, err) assert.Equal(t, 2, len(users))