From 9e2230cc33af189a95f047eddad9dfe5ce41373a Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Sat, 26 Apr 2025 14:29:36 +0200 Subject: [PATCH] Support leading and trailing spaces in user passwords This improves compatibility with external authentication providers that allow such characters in passwords. Passwords created via the WebAdmin UI are still sanitized to prevent user confusion. Signed-off-by: Nicola Murino --- internal/dataprovider/admin.go | 3 +- internal/dataprovider/dataprovider.go | 2 +- internal/httpd/httpd_test.go | 51 +++++++++++++++++++++++++++ internal/httpd/server.go | 4 +-- 4 files changed, 56 insertions(+), 4 deletions(-) diff --git a/internal/dataprovider/admin.go b/internal/dataprovider/admin.go index fb8d2774..cad903dd 100644 --- a/internal/dataprovider/admin.go +++ b/internal/dataprovider/admin.go @@ -366,6 +366,7 @@ func (a *Admin) validateGroups() error { func (a *Admin) validate() error { a.SetEmptySecretsIfNil() + a.Password = strings.TrimSpace(a.Password) if a.Username == "" { return util.NewI18nError(util.NewValidationError("username is mandatory"), util.I18nErrorUsernameRequired) } @@ -481,7 +482,7 @@ func (a *Admin) checkUserAndPass(password, ip string) error { if err := a.CanLogin(ip); err != nil { return err } - if a.Password == "" || password == "" { + if a.Password == "" || strings.TrimSpace(password) == "" { return errors.New("credentials cannot be null or empty") } match, err := a.CheckPassword(password) diff --git a/internal/dataprovider/dataprovider.go b/internal/dataprovider/dataprovider.go index 53a96294..5713479a 100644 --- a/internal/dataprovider/dataprovider.go +++ b/internal/dataprovider/dataprovider.go @@ -3537,7 +3537,7 @@ func checkUserAndPass(user *User, password, ip, protocol string) (User, error) { if err != nil { return *user, ErrInvalidCredentials } - if user.Password == "" || password == "" { + if user.Password == "" || strings.TrimSpace(password) == "" { return *user, errors.New("credentials cannot be null or empty") } if !user.Filters.Hooks.CheckPasswordDisabled { diff --git a/internal/httpd/httpd_test.go b/internal/httpd/httpd_test.go index 79ff65e7..879ac9d5 100644 --- a/internal/httpd/httpd_test.go +++ b/internal/httpd/httpd_test.go @@ -6908,6 +6908,57 @@ func TestAdminGenerateRecoveryCodesSaveError(t *testing.T) { assert.NoError(t, err) } +func TestAdminCredentialsWithSpaces(t *testing.T) { + a := getTestAdmin() + a.Username = xid.New().String() + a.Password = " " + xid.New().String() + " " + admin, _, err := httpdtest.AddAdmin(a, http.StatusCreated) + assert.NoError(t, err) + // For admins the password is always trimmed. + _, err = getJWTAPITokenFromTestServer(a.Username, a.Password) + assert.Error(t, err) + _, err = getJWTAPITokenFromTestServer(a.Username, strings.TrimSpace(a.Password)) + assert.NoError(t, err) + // The password sent from the WebAdmin UI is automatically trimmed + _, err = getJWTWebToken(a.Username, a.Password) + assert.NoError(t, err) + _, err = getJWTWebToken(a.Username, strings.TrimSpace(a.Password)) + assert.NoError(t, err) + + _, err = httpdtest.RemoveAdmin(admin, http.StatusOK) + assert.NoError(t, err) +} + +func TestUserCredentialsWithSpaces(t *testing.T) { + u := getTestUser() + u.Password = " " + xid.New().String() + " " + user, _, err := httpdtest.AddUser(u, http.StatusCreated) + assert.NoError(t, err) + // For users the password is not trimmed + _, err = getJWTAPIUserTokenFromTestServer(u.Username, u.Password) + assert.NoError(t, err) + _, err = getJWTAPIUserTokenFromTestServer(u.Username, strings.TrimSpace(u.Password)) + assert.Error(t, err) + + _, err = getJWTWebClientTokenFromTestServer(u.Username, u.Password) + assert.NoError(t, err) + _, err = getJWTWebClientTokenFromTestServer(u.Username, strings.TrimSpace(u.Password)) + assert.Error(t, err) + + user.Password = u.Password + conn, sftpClient, err := getSftpClient(user) + if assert.NoError(t, err) { + conn.Close() + sftpClient.Close() + } + user.Password = strings.TrimSpace(u.Password) + _, _, err = getSftpClient(user) + assert.Error(t, err) + + _, err = httpdtest.RemoveUser(user, http.StatusOK) + assert.NoError(t, err) +} + func TestNamingRules(t *testing.T) { smtpCfg := smtp.Config{ Host: "127.0.0.1", diff --git a/internal/httpd/server.go b/internal/httpd/server.go index 381adede..1ae9084a 100644 --- a/internal/httpd/server.go +++ b/internal/httpd/server.go @@ -244,7 +244,7 @@ func (s *httpdServer) handleWebClientLoginPost(w http.ResponseWriter, r *http.Re } protocol := common.ProtocolHTTP username := strings.TrimSpace(r.Form.Get("username")) - password := strings.TrimSpace(r.Form.Get("password")) + password := r.Form.Get("password") if username == "" || password == "" { updateLoginMetrics(&dataprovider.User{BaseUser: sdk.BaseUser{Username: username}}, dataprovider.LoginMethodPassword, ipAddr, common.ErrNoCredentials, r) @@ -840,7 +840,7 @@ func (s *httpdServer) getUserToken(w http.ResponseWriter, r *http.Request) { sendAPIResponse(w, r, nil, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized) return } - if username == "" || password == "" { + if username == "" || strings.TrimSpace(password) == "" { updateLoginMetrics(&dataprovider.User{BaseUser: sdk.BaseUser{Username: username}}, dataprovider.LoginMethodPassword, ipAddr, common.ErrNoCredentials, r) w.Header().Set(common.HTTPAuthenticationHeader, basicRealm)