web admin: allow to set an empty password for SFTPGo users

dataprovider/dataprovider.go

@@ -1379,7 +1379,7 @@ func validateBaseParams(user *User) error {
 }
 
 func createUserPasswordHash(user *User) error {
-	if user.Password != "" && !utils.IsStringPrefixInSlice(user.Password, hashPwdPrefixes) {
+	if user.Password != "" && !user.IsPasswordHashed() {
 		pwd, err := argon2id.CreateHash(user.Password, argon2Params)
 		if err != nil {
 			return err

dataprovider/user.go

@@ -261,6 +261,11 @@ func (u *User) HideConfidentialData() {
 	}
 }
 
+// IsPasswordHashed returns true if the password is hashed
+func (u *User) IsPasswordHashed() bool {
+	return utils.IsStringPrefixInSlice(u.Password, hashPwdPrefixes)
+}
+
 // SetEmptySecrets sets to empty any user secret
 func (u *User) SetEmptySecrets() {
 	u.FsConfig.S3Config.AccessSecret = kms.NewEmptySecret()

docs/howto/README.md

@@ -3,4 +3,3 @@
 Here we collect step-to-step tutorials. SFTPGo users are encouraged to contribute!
 
 - [SFTPGo with PostgreSQL data provider and S3 backend](./postgresql-s3.md)
-- [Expose Web Admin and REST API over HTTPS and password protected](./rest-api-https-auth.md)

docs/howto/rest-api-https-auth.md

@@ -1,81 +0,0 @@
-# Expose Web Admin and REST API over HTTPS
-
-This tutorial shows how to expose the SFTPGo web interface and REST API over HTTPS.
-
-## Preliminary Note
-
-Before proceeding further you need to have a SFTPGo instance already configured and running.
-
-We assume:
-
-- you are running SFTPGo as service using the dedicated `sftpgo` system user
-- the SFTPGo configuration directory is `/etc/sftpgo`
-- you are running SFTPGo on Ubuntu 20.04, however this instructions can be easily adapted for other Linux variants.
-
-## Creation of a Self-Signed Certificate
-
-For demostration purpose we use a self-signed certificate here. These certificates are easy to make and do not cost money. However, they do not provide all of the security properties that certificates signed by a Public Certificate Authority (CA) aim to provide, you are encouraged to use a certificate signed by a Public CA.
-
-When creating a new SSL certificate, one needs to specify the duration validity of the same by changing the value 365 (as appearing in the message below) to the preferred number of days. It is important to mention here that the certificate so created stands to auto-expire upon completion of one year.
-
-```shell
-sudo mkdir /etc/sftpgo/ssl
-sudo openssl req -x509 -nodes -days 365 -newkey rsa:2048 -keyout /etc/sftpgo/ssl/sftpgo.key -out /etc/sftpgo/ssl/sftpgo.crt
-```
-
-The above command is rather versatile, and lets you create both the self-signed SSL certificate and the server key to safeguard it, in addition to placing both of these into the `etc/sftpgo/ssl` directory. Answer to the questions to create the certificate and the key for HTTPS.
-
-Assign the proper permissions to the generated certificates.
-
-```shell
-sudo chown -R sftpgo:sftpgo /etc/sftpgo/ssl
-```
-
-## HTTPS Setup
-
-Open the SFTPGo configuration.
-
-```shell
-sudo vi /etc/sftpgo/sftpgo.json
-```
-
-Search for the `httpd` section and change it as follow.
-
-```json
-  "httpd": {
-    "bindings": [
-      {
-        "port": 8080,
-        "address": "",
-        "enable_web_admin": true,
-        "enable_https": false,
-        "client_auth_type": 0
-      }
-    ],
-    "templates_path": "/usr/share/sftpgo/templates",
-    "static_files_path": "/usr/share/sftpgo/static",
-    "backups_path": "/srv/sftpgo/backups",
-    "certificate_file": "/etc/sftpgo/ssl/sftpgo.crt",
-    "certificate_key_file": "/etc/sftpgo/ssl/sftpgo.key",
-    "ca_certificates": [],
-    "ca_revocation_lists": []
-  },
-```
-
-The configuration keys `certificate_file` and `certificate_key_file` point to the certificate and key we previously created. Setting an empty `address` means that the service will listen on all available network interfaces.
-
-Now restart the SFTPGo service to apply the changes.
-
-```shell
-sudo systemctl restart sftpgo
-```
-
-You are done! Now SFTPGo web admin and REST API are exposed over HTTPS.
-
-You can easily replace the self-signed certificate used here with a properly signed certificate.
-
-The certificate could frequently change if you use something like [let's encrypt](https://letsencrypt.org/). SFTPGo allows hot-certificate reloading using the following command.
-
-```shell
-sudo systemctl reload sftpgo
-```

httpd/httpd_test.go

@@ -115,6 +115,7 @@ AAAEA0E24gi8ab/XRSvJ85TGZJMe6HVmwxSG4ExPfTMwwe2n5EHjI1NnP2Yc6RrDBSJs11
 6aKNVXcSsx4vFZQGUI3+AAAACW5pY29sYUBwMQECAwQ=
 -----END OPENSSH PRIVATE KEY-----`
 	sftpPkeyFingerprint = "SHA256:QVQ06XHZZbYZzqfrsZcf3Yozy2WTnqQPeLOkcJCdbP0"
+	redactedSecret      = "[**redacted**]"
 )
 
 var (
@@ -899,20 +900,62 @@ func TestAddUserInvalidVirtualFolders(t *testing.T) {
 
 func TestUserPublicKey(t *testing.T) {
 	u := getTestUser()
+	u.Password = ""
 	invalidPubKey := "invalid"
-	validPubKey := "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQC03jj0D+djk7pxIf/0OhrxrchJTRZklofJ1NoIu4752Sq02mdXmarMVsqJ1cAjV5LBVy3D1F5U6XW4rppkXeVtd04Pxb09ehtH0pRRPaoHHlALiJt8CoMpbKYMA8b3KXPPriGxgGomvtU2T2RMURSwOZbMtpsugfjYSWenyYX+VORYhylWnSXL961LTyC21ehd6d6QnW9G7E5hYMITMY9TuQZz3bROYzXiTsgN0+g6Hn7exFQp50p45StUMfV/SftCMdCxlxuyGny2CrN/vfjO7xxOo2uv7q1qm10Q46KPWJQv+pgZ/OfL+EDjy07n5QVSKHlbx+2nT4Q0EgOSQaCTYwn3YjtABfIxWwgAFdyj6YlPulCL22qU4MYhDcA6PSBwDdf8hvxBfvsiHdM+JcSHvv8/VeJhk6CmnZxGY0fxBupov27z3yEO8nAg8k+6PaUiW1MSUfuGMF/ktB8LOstXsEPXSszuyXiOv4DaryOXUiSn7bmRqKcEFlJusO6aZP0= nicola@p1"
 	u.PublicKeys = []string{invalidPubKey}
 	_, _, err := httpdtest.AddUser(u, http.StatusBadRequest)
 	assert.NoError(t, err)
-	u.PublicKeys = []string{validPubKey}
+	u.PublicKeys = []string{testPubKey}
 	user, _, err := httpdtest.AddUser(u, http.StatusCreated)
 	assert.NoError(t, err)
-	user.PublicKeys = []string{validPubKey, invalidPubKey}
+
+	dbUser, err := dataprovider.UserExists(u.Username)
+	assert.NoError(t, err)
+	assert.Empty(t, dbUser.Password)
+	assert.False(t, dbUser.IsPasswordHashed())
+
+	user.PublicKeys = []string{testPubKey, invalidPubKey}
 	_, _, err = httpdtest.UpdateUser(user, http.StatusBadRequest, "")
 	assert.NoError(t, err)
-	user.PublicKeys = []string{validPubKey, validPubKey, validPubKey}
+	user.PublicKeys = []string{testPubKey, testPubKey, testPubKey}
+	user.Password = defaultPassword
 	_, _, err = httpdtest.UpdateUser(user, http.StatusOK, "")
 	assert.NoError(t, err)
+
+	dbUser, err = dataprovider.UserExists(u.Username)
+	assert.NoError(t, err)
+	assert.NotEmpty(t, dbUser.Password)
+	assert.True(t, dbUser.IsPasswordHashed())
+
+	_, err = httpdtest.RemoveUser(user, http.StatusOK)
+	assert.NoError(t, err)
+}
+
+func TestUpdateUserEmptyPassword(t *testing.T) {
+	u := getTestUser()
+	u.PublicKeys = []string{testPubKey}
+	user, _, err := httpdtest.AddUser(u, http.StatusCreated)
+	assert.NoError(t, err)
+
+	// the password is not empty
+	dbUser, err := dataprovider.UserExists(u.Username)
+	assert.NoError(t, err)
+	assert.NotEmpty(t, dbUser.Password)
+	assert.True(t, dbUser.IsPasswordHashed())
+	// now update the user and set an empty password
+	customUser := make(map[string]interface{})
+	customUser["password"] = ""
+	asJSON, err := json.Marshal(customUser)
+	assert.NoError(t, err)
+	userNoPwd, _, err := httpdtest.UpdateUserWithJSON(user, http.StatusOK, "", asJSON)
+	assert.NoError(t, err)
+	assert.Equal(t, user, userNoPwd) // the password is hidden so the user must be equal
+	// check the password within the data provider
+	dbUser, err = dataprovider.UserExists(u.Username)
+	assert.NoError(t, err)
+	assert.Empty(t, dbUser.Password)
+	assert.False(t, dbUser.IsPasswordHashed())
+
 	_, err = httpdtest.RemoveUser(user, http.StatusOK)
 	assert.NoError(t, err)
 }
@@ -4634,6 +4677,11 @@ func TestWebUserAddMock(t *testing.T) {
 	req.Header.Set("Content-Type", contentType)
 	rr = executeRequest(req)
 	checkResponseCode(t, http.StatusSeeOther, rr)
+
+	dbUser, err := dataprovider.UserExists(user.Username)
+	assert.NoError(t, err)
+	assert.NotEmpty(t, dbUser.Password)
+	assert.True(t, dbUser.IsPasswordHashed())
 	// the user already exists, was created with the above request
 	b, contentType, _ = getMultipartFormData(form, "", "")
 	req, _ = http.NewRequest(http.MethodPost, webUserPath, &b)
@@ -4730,6 +4778,10 @@ func TestWebUserUpdateMock(t *testing.T) {
 	setBearerForReq(req, apiToken)
 	rr := executeRequest(req)
 	checkResponseCode(t, http.StatusCreated, rr)
+	dbUser, err := dataprovider.UserExists(user.Username)
+	assert.NoError(t, err)
+	assert.NotEmpty(t, dbUser.Password)
+	assert.True(t, dbUser.IsPasswordHashed())
 	err = render.DecodeJSON(rr.Body, &user)
 	assert.NoError(t, err)
 	user.MaxSessions = 1
@@ -4739,6 +4791,8 @@ func TestWebUserUpdateMock(t *testing.T) {
 	user.AdditionalInfo = "new additional info"
 	form := make(url.Values)
 	form.Set("username", user.Username)
+	form.Set("password", "")
+	form.Set("public_keys", testPubKey)
 	form.Set("home_dir", user.HomeDir)
 	form.Set("uid", "0")
 	form.Set("gid", strconv.FormatInt(int64(user.GID), 10))
@@ -4774,6 +4828,36 @@ func TestWebUserUpdateMock(t *testing.T) {
 	req.Header.Set("Content-Type", contentType)
 	rr = executeRequest(req)
 	checkResponseCode(t, http.StatusSeeOther, rr)
+	dbUser, err = dataprovider.UserExists(user.Username)
+	assert.NoError(t, err)
+	assert.Empty(t, dbUser.Password)
+	assert.False(t, dbUser.IsPasswordHashed())
+
+	form.Set("password", defaultPassword)
+	b, contentType, _ = getMultipartFormData(form, "", "")
+	req, _ = http.NewRequest(http.MethodPost, path.Join(webUserPath, user.Username), &b)
+	setJWTCookieForReq(req, webToken)
+	req.Header.Set("Content-Type", contentType)
+	rr = executeRequest(req)
+	checkResponseCode(t, http.StatusSeeOther, rr)
+	dbUser, err = dataprovider.UserExists(user.Username)
+	assert.NoError(t, err)
+	assert.NotEmpty(t, dbUser.Password)
+	assert.True(t, dbUser.IsPasswordHashed())
+	prevPwd := dbUser.Password
+
+	form.Set("password", redactedSecret)
+	b, contentType, _ = getMultipartFormData(form, "", "")
+	req, _ = http.NewRequest(http.MethodPost, path.Join(webUserPath, user.Username), &b)
+	setJWTCookieForReq(req, webToken)
+	req.Header.Set("Content-Type", contentType)
+	rr = executeRequest(req)
+	checkResponseCode(t, http.StatusSeeOther, rr)
+	dbUser, err = dataprovider.UserExists(user.Username)
+	assert.NoError(t, err)
+	assert.NotEmpty(t, dbUser.Password)
+	assert.True(t, dbUser.IsPasswordHashed())
+	assert.Equal(t, prevPwd, dbUser.Password)
 
 	req, _ = http.NewRequest(http.MethodGet, path.Join(userPath, user.Username), nil)
 	setBearerForReq(req, apiToken)
@@ -5177,6 +5261,7 @@ func TestWebUserS3Mock(t *testing.T) {
 	form := make(url.Values)
 	form.Set(csrfFormToken, csrfToken)
 	form.Set("username", user.Username)
+	form.Set("password", redactedSecret)
 	form.Set("home_dir", user.HomeDir)
 	form.Set("uid", "0")
 	form.Set("gid", strconv.FormatInt(int64(user.GID), 10))
@@ -5249,7 +5334,7 @@ func TestWebUserS3Mock(t *testing.T) {
 	assert.Empty(t, updateUser.FsConfig.S3Config.AccessSecret.GetKey())
 	assert.Empty(t, updateUser.FsConfig.S3Config.AccessSecret.GetAdditionalData())
 	// now check that a redacted password is not saved
-	form.Set("s3_access_secret", "[**redacted**] ")
+	form.Set("s3_access_secret", redactedSecret)
 	b, contentType, _ = getMultipartFormData(form, "", "")
 	req, _ = http.NewRequest(http.MethodPost, path.Join(webUserPath, user.Username), &b)
 	setJWTCookieForReq(req, webToken)
@@ -5318,6 +5403,7 @@ func TestWebUserGCSMock(t *testing.T) {
 	form := make(url.Values)
 	form.Set(csrfFormToken, csrfToken)
 	form.Set("username", user.Username)
+	form.Set("password", redactedSecret)
 	form.Set("home_dir", user.HomeDir)
 	form.Set("uid", "0")
 	form.Set("gid", strconv.FormatInt(int64(user.GID), 10))
@@ -5420,6 +5506,7 @@ func TestWebUserAzureBlobMock(t *testing.T) {
 	form := make(url.Values)
 	form.Set(csrfFormToken, csrfToken)
 	form.Set("username", user.Username)
+	form.Set("password", redactedSecret)
 	form.Set("home_dir", user.HomeDir)
 	form.Set("uid", "0")
 	form.Set("gid", strconv.FormatInt(int64(user.GID), 10))
@@ -5491,7 +5578,7 @@ func TestWebUserAzureBlobMock(t *testing.T) {
 	assert.Empty(t, updateUser.FsConfig.AzBlobConfig.AccountKey.GetKey())
 	assert.Empty(t, updateUser.FsConfig.AzBlobConfig.AccountKey.GetAdditionalData())
 	// now check that a redacted password is not saved
-	form.Set("az_account_key", "[**redacted**] ")
+	form.Set("az_account_key", redactedSecret+" ")
 	b, contentType, _ = getMultipartFormData(form, "", "")
 	req, _ = http.NewRequest(http.MethodPost, path.Join(webUserPath, user.Username), &b)
 	setJWTCookieForReq(req, webToken)
@@ -5535,6 +5622,7 @@ func TestWebUserCryptMock(t *testing.T) {
 	form := make(url.Values)
 	form.Set(csrfFormToken, csrfToken)
 	form.Set("username", user.Username)
+	form.Set("password", redactedSecret)
 	form.Set("home_dir", user.HomeDir)
 	form.Set("uid", "0")
 	form.Set("gid", strconv.FormatInt(int64(user.GID), 10))
@@ -5582,7 +5670,7 @@ func TestWebUserCryptMock(t *testing.T) {
 	assert.Empty(t, updateUser.FsConfig.CryptConfig.Passphrase.GetKey())
 	assert.Empty(t, updateUser.FsConfig.CryptConfig.Passphrase.GetAdditionalData())
 	// now check that a redacted password is not saved
-	form.Set("crypt_passphrase", "[**redacted**] ")
+	form.Set("crypt_passphrase", redactedSecret+" ")
 	b, contentType, _ = getMultipartFormData(form, "", "")
 	req, _ = http.NewRequest(http.MethodPost, path.Join(webUserPath, user.Username), &b)
 	setJWTCookieForReq(req, webToken)
@@ -5631,6 +5719,7 @@ func TestWebUserSFTPFsMock(t *testing.T) {
 	form := make(url.Values)
 	form.Set(csrfFormToken, csrfToken)
 	form.Set("username", user.Username)
+	form.Set("password", redactedSecret)
 	form.Set("home_dir", user.HomeDir)
 	form.Set("uid", "0")
 	form.Set("gid", strconv.FormatInt(int64(user.GID), 10))
@@ -5692,8 +5781,8 @@ func TestWebUserSFTPFsMock(t *testing.T) {
 	assert.Len(t, updateUser.FsConfig.SFTPConfig.Fingerprints, 1)
 	assert.Contains(t, updateUser.FsConfig.SFTPConfig.Fingerprints, sftpPkeyFingerprint)
 	// now check that a redacted credentials are not saved
-	form.Set("sftp_password", "[**redacted**] ")
+	form.Set("sftp_password", redactedSecret+" ")
-	form.Set("sftp_private_key", "[**redacted**]")
+	form.Set("sftp_private_key", redactedSecret)
 	b, contentType, _ = getMultipartFormData(form, "", "")
 	req, _ = http.NewRequest(http.MethodPost, path.Join(webUserPath, user.Username), &b)
 	setJWTCookieForReq(req, webToken)

httpd/web.go

@@ -389,6 +389,9 @@ func renderUserPage(w http.ResponseWriter, r *http.Request, user *dataprovider.U
 		title = "User template"
 		currentURL = webTemplateUser
 	}
+	if user.Password != "" && user.IsPasswordHashed() && mode == userPageModeUpdate {
+		user.Password = redactedSecret
+	}
 	data := userPage{
 		basePage:             getBasePageData(title, currentURL, r),
 		Mode:                 mode,
@@ -1334,6 +1337,7 @@ func handleWebAddUserGet(w http.ResponseWriter, r *http.Request) {
 		if err == nil {
 			user.ID = 0
 			user.Username = ""
+			user.Password = ""
 			user.SetEmptySecrets()
 			renderUserPage(w, r, &user, userPageModeAdd, "")
 		} else if _, ok := err.(*dataprovider.RecordNotFoundError); ok {
@@ -1401,7 +1405,7 @@ func handleWebUpdateUserPost(w http.ResponseWriter, r *http.Request) {
 	updatedUser.ID = user.ID
 	updatedUser.Username = user.Username
 	updatedUser.SetEmptySecretsIfNil()
-	if updatedUser.Password == "" {
+	if updatedUser.Password == redactedSecret {
 		updatedUser.Password = user.Password
 	}
 	updateEncryptedSecrets(&updatedUser, user.FsConfig.S3Config.AccessSecret, user.FsConfig.AzBlobConfig.AccountKey,

httpdtest/httpdtest.go

@@ -152,15 +152,14 @@ func AddUser(user dataprovider.User, expectedStatusCode int) (dataprovider.User,
 	return newUser, body, err
 }
 
-// UpdateUser updates an existing user and checks the received HTTP Status code against expectedStatusCode.
+// UpdateUserWithJSON update a user using the provided JSON as POST body
-func UpdateUser(user dataprovider.User, expectedStatusCode int, disconnect string) (dataprovider.User, []byte, error) {
+func UpdateUserWithJSON(user dataprovider.User, expectedStatusCode int, disconnect string, userAsJSON []byte) (dataprovider.User, []byte, error) {
 	var newUser dataprovider.User
 	var body []byte
 	url, err := addDisconnectQueryParam(buildURLRelativeToBase(userPath, url.PathEscape(user.Username)), disconnect)
 	if err != nil {
 		return user, body, err
 	}
-	userAsJSON, _ := json.Marshal(user)
 	resp, err := sendHTTPRequest(http.MethodPut, url.String(), bytes.NewBuffer(userAsJSON), "application/json",
 		getDefaultToken())
 	if err != nil {
@@ -181,6 +180,12 @@ func UpdateUser(user dataprovider.User, expectedStatusCode int, disconnect strin
 	return newUser, body, err
 }
 
+// UpdateUser updates an existing user and checks the received HTTP Status code against expectedStatusCode.
+func UpdateUser(user dataprovider.User, expectedStatusCode int, disconnect string) (dataprovider.User, []byte, error) {
+	userAsJSON, _ := json.Marshal(user)
+	return UpdateUserWithJSON(user, expectedStatusCode, disconnect, userAsJSON)
+}
+
 // RemoveUser removes an existing user and checks the received HTTP Status code against expectedStatusCode.
 func RemoveUser(user dataprovider.User, expectedStatusCode int) ([]byte, error) {
 	var body []byte

templates/user.html

@@ -85,12 +85,7 @@
             <div class="form-group row">
                 <label for="idPassword" class="col-sm-2 col-form-label">Password</label>
                 <div class="col-sm-10">
-                    <input type="password" class="form-control" id="idPassword" name="password" placeholder="" {{if eq .Mode 2}}aria-describedby="pwdHelpBlock" {{end}}>
+                    <input type="password" class="form-control" id="idPassword" name="password" value="{{.User.Password}}" placeholder="">
-                    {{if eq .Mode 2}}
-                    <small id="pwdHelpBlock" class="form-text text-muted">
-                        If empty the current password will not be changed
-                    </small>
-                    {{end}}
                 </div>
             </div>