sftpd actions: add support for pre-delete action

Fixes #121

dataprovider/dataprovider.go

@@ -503,7 +503,7 @@ func AddUser(p Provider, user User) error {
 	}
 	err := p.addUser(user)
 	if err == nil {
-		go executeAction(operationAdd, user) //nolint:errcheck // the error is used in test cases only
+		go executeAction(operationAdd, user)
 	}
 	return err
 }
@@ -516,7 +516,7 @@ func UpdateUser(p Provider, user User) error {
 	}
 	err := p.updateUser(user)
 	if err == nil {
-		go executeAction(operationUpdate, user) //nolint:errcheck // the error is used in test cases only
+		go executeAction(operationUpdate, user)
 	}
 	return err
 }
@@ -529,7 +529,7 @@ func DeleteUser(p Provider, user User) error {
 	}
 	err := p.deleteUser(user)
 	if err == nil {
-		go executeAction(operationDelete, user) //nolint:errcheck // the error is used in test cases only
+		go executeAction(operationDelete, user)
 	}
 	return err
 }

docs/custom-actions.md

@@ -5,10 +5,11 @@ The `hook` can be defined as the absolute path of your program or an HTTP URL.
 
 The `upload` condition includes both uploads to new files and overwrite of existing files. The `ssh_cmd` condition will be triggered after a command is successfully executed via SSH. `scp` will trigger the `download` and `upload` conditions and not `ssh_cmd`.
 The notification will indicate if an error is detected and so, for example, a partial file is uploaded.
+The `pre-delete` action, if defined, will be called just before files deletion. If the external command completes with a zero exit status or the HTTP notification response code is `200` then SFTPGo will assume that the file was already deleted/moved and so it will not try to remove the file and it will not execute the hook defined for the `delete` action.
 
 If the `hook` defines a path to an external program, then this program is invoked with the following arguments:
 
-- `action`, string, possible values are: `download`, `upload`, `delete`, `rename`, `ssh_cmd`
+- `action`, string, possible values are: `download`, `upload`, `pre-delete`,`delete`, `rename`, `ssh_cmd`
 - `username`
 - `path` is the full filesystem path, can be empty for some ssh commands
 - `target_path`, non-empty for `rename` action

docs/full-configuration.md

@@ -47,9 +47,10 @@ The configuration file contains the following sections:
   - `banner`, string. Identification string used by the server. Leave empty to use the default banner. Default `SFTPGo_<version>`, for example `SSH-2.0-SFTPGo_0.9.5`
   - `upload_mode` integer. 0 means standard: the files are uploaded directly to the requested path. 1 means atomic: files are uploaded to a temporary path and renamed to the requested path when the client ends the upload. Atomic mode avoids problems such as a web server that serves partial files when the files are being uploaded. In atomic mode, if there is an upload error, the temporary file is deleted and so the requested upload path will not contain a partial file. 2 means atomic with resume support: same as atomic but if there is an upload error, the temporary file is renamed to the requested path and not deleted. This way, a client can reconnect and resume the upload.
   - `actions`, struct. It contains the command to execute and/or the HTTP URL to notify and the trigger conditions. See the "Custom Actions" paragraph for more details
-    - `execute_on`, list of strings. Valid values are `download`, `upload`, `delete`, `rename`, `ssh_cmd`. Leave empty to disable actions.
+    - `execute_on`, list of strings. Valid values are `download`, `upload`, `pre-delete`, `delete`, `rename`, `ssh_cmd`. Leave empty to disable actions.
-    - `command`, string. Absolute path to the command to execute. Leave empty to disable.
+    - `command`, string. Deprecated please use `hook`.
-    - `http_notification_url`, a valid URL. An HTTP GET request will be executed to this URL. Leave empty to disable.
+    - `http_notification_url`, a valid URL. Deprecated please use `hook`.
+    - `hook`, string. Absolute path to the command to execute or HTTP URL to notify.
   - `keys`, struct array. Deprecated, please use `host_keys`.
     - `private_key`, path to the private key file. It can be a path relative to the config dir or an absolute one.
   - `host_keys`, list of strings. It contains the daemon's private host keys. Each host key can be defined as a path relative to the configuration directory or an absolute one. If empty or missing, the daemon will search or try to generate `id_rsa` and `id_ecdsa` keys inside the configuration directory.
@@ -93,8 +94,9 @@ The configuration file contains the following sections:
   - `users_base_dir`, string. Users default base directory. If no home dir is defined while adding a new user, and this value is a valid absolute path, then the user home dir will be automatically defined as the path obtained joining the base dir and the username
   - `actions`, struct. It contains the command to execute and/or the HTTP URL to notify and the trigger conditions. See the "Custom Actions" paragraph for more details
     - `execute_on`, list of strings. Valid values are `add`, `update`, `delete`. `update` action will not be fired for internal updates such as the last login or the user quota fields.
-    - `command`, string. Absolute path to the command to execute. Leave empty to disable.
+    - `command`, string. Deprecated please use `hook`.
-    - `http_notification_url`, a valid URL. Leave empty to disable.
+    - `http_notification_url`, a valid URL. Deprecated please use `hook`.
+    - `hook`, string. Absolute path to the command to execute or HTTP URL to notify.
   - `external_auth_program`, string. Deprecated, please use `external_auth_hook`.
   - `external_auth_hook`, string. Absolute path to an external program or an HTTP URL to invoke for users authentication. See the "External Authentication" paragraph for more details. Leave empty to disable.
   - `external_auth_scope`, integer. 0 means all supported authetication scopes (passwords, public keys and keyboard interactive). 1 means passwords only. 2 means public keys only. 4 means key keyboard interactive only. The flags can be combined, for example 6 means public keys and keyboard interactive

sftpd/handler.go

@@ -425,9 +425,14 @@ func (c Connection) handleSFTPRemove(filePath string, request *sftp.Request) err
 	}
 
 	size = fi.Size()
-	if err := c.fs.Remove(filePath, false); err != nil {
+	actionErr := executeAction(newActionNotification(c.User, operationPreDelete, filePath, "", "", fi.Size(), nil))
-		c.Log(logger.LevelWarn, logSender, "failed to remove a file/symlink %#v: %+v", filePath, err)
+	if actionErr == nil {
-		return vfs.GetSFTPError(c.fs, err)
+		c.Log(logger.LevelDebug, logSender, "remove for path %#v handled by pre-delete action", filePath)
+	} else {
+		if err := c.fs.Remove(filePath, false); err != nil {
+			c.Log(logger.LevelWarn, logSender, "failed to remove a file/symlink %#v: %+v", filePath, err)
+			return vfs.GetSFTPError(c.fs, err)
+		}
 	}
 
 	logger.CommandLog(removeLogSender, filePath, "", c.User.Username, "", c.ID, c.protocol, -1, -1, "", "", "")
@@ -436,7 +441,9 @@ func (c Connection) handleSFTPRemove(filePath string, request *sftp.Request) err
 			dataprovider.UpdateUserQuota(dataProvider, c.User, -1, -size, false) //nolint:errcheck
 		}
 	}
-	go executeAction(newActionNotification(c.User, operationDelete, filePath, "", "", fi.Size(), nil)) //nolint:errcheck
+	if actionErr != nil {
+		go executeAction(newActionNotification(c.User, operationDelete, filePath, "", "", fi.Size(), nil)) //nolint:errcheck
+	}
 
 	return sftp.ErrSSHFxOk
 }

sftpd/internal_test.go

@@ -8,6 +8,7 @@ import (
 	"io/ioutil"
 	"net"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"runtime"
 	"sync"
@@ -187,6 +188,7 @@ func TestWrongActions(t *testing.T) {
 
 func TestActionHTTP(t *testing.T) {
 	actionsCopy := actions
+
 	actions = Actions{
 		ExecuteOn: []string{operationDownload},
 		Hook:      "http://127.0.0.1:8080/",
@@ -195,7 +197,42 @@ func TestActionHTTP(t *testing.T) {
 		Username: "username",
 	}
 	err := executeAction(newActionNotification(user, operationDownload, "path", "", "", 0, nil))
+	assert.EqualError(t, err, errUnexpectedHTTResponse.Error())
+
+	actions = actionsCopy
+}
+
+func TestPreDeleteAction(t *testing.T) {
+	actionsCopy := actions
+
+	hookCmd, err := exec.LookPath("true")
 	assert.NoError(t, err)
+	actions = Actions{
+		ExecuteOn: []string{operationPreDelete},
+		Hook:      hookCmd,
+	}
+	homeDir := filepath.Join(os.TempDir(), "test_user")
+	err = os.MkdirAll(homeDir, 0777)
+	assert.NoError(t, err)
+	user := dataprovider.User{
+		Username: "username",
+		HomeDir:  homeDir,
+	}
+	user.Permissions = make(map[string][]string)
+	user.Permissions["/"] = []string{dataprovider.PermAny}
+	c := Connection{
+		fs:   vfs.NewOsFs("id", homeDir, nil),
+		User: user,
+	}
+	testfile := filepath.Join(user.HomeDir, "testfile")
+	request := sftp.NewRequest("Remove", "/testfile")
+	err = ioutil.WriteFile(testfile, []byte("test"), 0666)
+	assert.NoError(t, err)
+	err = c.handleSFTPRemove(testfile, request)
+	assert.EqualError(t, err, sftp.ErrSSHFxOk.Error())
+	assert.FileExists(t, testfile)
+
+	os.RemoveAll(homeDir)
 
 	actions = actionsCopy
 }

sftpd/sftpd.go

@@ -9,6 +9,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"net/http"
 	"net/url"
 	"os"
 	"os/exec"
@@ -42,6 +43,7 @@ const (
 	operationDownload   = "download"
 	operationUpload     = "upload"
 	operationDelete     = "delete"
+	operationPreDelete  = "pre-delete"
 	operationRename     = "rename"
 	operationSSHCmd     = "ssh_cmd"
 	protocolSFTP        = "SFTP"
@@ -68,11 +70,12 @@ var (
 	setstatMode          int
 	supportedSSHCommands = []string{"scp", "md5sum", "sha1sum", "sha256sum", "sha384sum", "sha512sum", "cd", "pwd",
 		"git-receive-pack", "git-upload-pack", "git-upload-archive", "rsync"}
-	defaultSSHCommands    = []string{"md5sum", "sha1sum", "cd", "pwd", "scp"}
+	defaultSSHCommands       = []string{"md5sum", "sha1sum", "cd", "pwd", "scp"}
-	sshHashCommands       = []string{"md5sum", "sha1sum", "sha256sum", "sha384sum", "sha512sum"}
+	sshHashCommands          = []string{"md5sum", "sha1sum", "sha256sum", "sha384sum", "sha512sum"}
-	systemCommands        = []string{"git-receive-pack", "git-upload-pack", "git-upload-archive", "rsync"}
+	systemCommands           = []string{"git-receive-pack", "git-upload-pack", "git-upload-archive", "rsync"}
-	errUnconfiguredAction = errors.New("no hook is configured for this action")
+	errUnconfiguredAction    = errors.New("no hook is configured for this action")
-	errNoHook             = errors.New("unable to execute action, no hook defined")
+	errNoHook                = errors.New("unable to execute action, no hook defined")
+	errUnexpectedHTTResponse = errors.New("unexpected HTTP response code")
 )
 
 type connectionTransfer struct {
@@ -496,7 +499,6 @@ func executeNotificationCommand(a actionNotification) error {
 	return err
 }
 
-// executed in a goroutine
 func executeAction(a actionNotification) error {
 	if !utils.IsStringInSlice(a.Action, actions.ExecuteOn) {
 		return errUnconfiguredAction
@@ -519,6 +521,9 @@ func executeAction(a actionNotification) error {
 		if err == nil {
 			respCode = resp.StatusCode
 			resp.Body.Close()
+			if respCode != http.StatusOK {
+				err = errUnexpectedHTTResponse
+			}
 		}
 		logger.Debug(logSender, "", "notified operation %#v to URL: %v status code: %v, elapsed: %v err: %v",
 			a.Action, url.String(), respCode, time.Since(startTime), err)

sftpd/sftpd_test.go

@@ -116,6 +116,7 @@ var (
 	scpPath          string
 	gitPath          string
 	sshPath          string
+	hookCmdPath      string
 	pubKeyPath       string
 	privateKeyPath   string
 	trustedCAUserKey string
@@ -134,7 +135,6 @@ func TestMain(m *testing.M) {
 	err := ioutil.WriteFile(loginBannerFile, []byte("simple login banner\n"), 0777)
 	if err != nil {
 		logger.WarnToConsole("error creating login banner: %v", err)
-		os.Exit(1)
 	}
 	err = config.LoadConfig(configDir, "")
 	if err != nil {
@@ -169,21 +169,15 @@ func TestMain(m *testing.M) {
 	// work in non atomic mode too
 	sftpdConf.UploadMode = 2
 	homeBasePath = os.TempDir()
+	checkSystemCommands()
 	var scriptArgs string
 	if runtime.GOOS == osWindows {
 		scriptArgs = "%*"
 	} else {
 		sftpdConf.Actions.ExecuteOn = []string{"download", "upload", "rename", "delete", "ssh_cmd"}
-		sftpdConf.Actions.Hook = "/bin/true"
+		sftpdConf.Actions.Hook = hookCmdPath
 		scriptArgs = "$@"
-		scpPath, err = exec.LookPath("scp")
-		if err != nil {
-			logger.Warn(logSender, "", "unable to get scp command. SCP tests will be skipped, err: %v", err)
-			logger.WarnToConsole("unable to get scp command. SCP tests will be skipped, err: %v", err)
-			scpPath = ""
-		}
 	}
-	checkGitCommand()
 
 	keyIntAuthPath = filepath.Join(homeBasePath, "keyintauth.sh")
 	err = ioutil.WriteFile(keyIntAuthPath, getKeyboardInteractiveScriptContent([]string{"1", "2"}, 0, false, 1), 0755)
@@ -4447,7 +4441,7 @@ func waitQuotaScans() error {
 	return nil
 }
 
-func checkGitCommand() {
+func checkSystemCommands() {
 	var err error
 	gitPath, err = exec.LookPath("git")
 	if err != nil {
@@ -4462,6 +4456,21 @@ func checkGitCommand() {
 		logger.WarnToConsole("unable to get ssh command. GIT tests will be skipped, err: %v", err)
 		gitPath = ""
 	}
+	hookCmdPath, err = exec.LookPath("true")
+	if err != nil {
+		logger.Warn(logSender, "", "unable to get hook command: %v", err)
+		logger.WarnToConsole("unable to get hook command: %v", err)
+	}
+	if runtime.GOOS == osWindows {
+		scpPath = ""
+	} else {
+		scpPath, err = exec.LookPath("scp")
+		if err != nil {
+			logger.Warn(logSender, "", "unable to get scp command. SCP tests will be skipped, err: %v", err)
+			logger.WarnToConsole("unable to get scp command. SCP tests will be skipped, err: %v", err)
+			scpPath = ""
+		}
+	}
 }
 
 func initGitRepo(path string) ([]byte, error) {