Browse Source

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 <[email protected]>
Nicola Murino 6 months ago
parent
commit
9e2230cc33

+ 2 - 1
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)

+ 1 - 1
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 {

+ 51 - 0
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",

+ 2 - 2
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)