diff --git a/dataprovider/dataprovider.go b/dataprovider/dataprovider.go index 0d064bf6..b4031ad8 100644 --- a/dataprovider/dataprovider.go +++ b/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 } diff --git a/docs/custom-actions.md b/docs/custom-actions.md index 3ae9bbdf..0d7f6ab8 100644 --- a/docs/custom-actions.md +++ b/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 diff --git a/docs/full-configuration.md b/docs/full-configuration.md index 91586ec2..0f65f173 100644 --- a/docs/full-configuration.md +++ b/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_`, 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. - - `command`, string. Absolute path to the command to execute. Leave empty to disable. - - `http_notification_url`, a valid URL. An HTTP GET request will be executed to this URL. Leave empty to disable. + - `execute_on`, list of strings. Valid values are `download`, `upload`, `pre-delete`, `delete`, `rename`, `ssh_cmd`. Leave empty to disable actions. + - `command`, string. Deprecated please use `hook`. + - `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. - - `http_notification_url`, a valid URL. Leave empty to disable. + - `command`, string. Deprecated please use `hook`. + - `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 diff --git a/sftpd/handler.go b/sftpd/handler.go index b39eacd8..a6277452 100644 --- a/sftpd/handler.go +++ b/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 { - c.Log(logger.LevelWarn, logSender, "failed to remove a file/symlink %#v: %+v", filePath, err) - return vfs.GetSFTPError(c.fs, err) + actionErr := executeAction(newActionNotification(c.User, operationPreDelete, filePath, "", "", fi.Size(), nil)) + if actionErr == nil { + 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 } diff --git a/sftpd/internal_test.go b/sftpd/internal_test.go index e3796b58..4de3a78f 100644 --- a/sftpd/internal_test.go +++ b/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 } diff --git a/sftpd/sftpd.go b/sftpd/sftpd.go index 394636a1..01e05eda 100644 --- a/sftpd/sftpd.go +++ b/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"} - sshHashCommands = []string{"md5sum", "sha1sum", "sha256sum", "sha384sum", "sha512sum"} - systemCommands = []string{"git-receive-pack", "git-upload-pack", "git-upload-archive", "rsync"} - errUnconfiguredAction = errors.New("no hook is configured for this action") - errNoHook = errors.New("unable to execute action, no hook defined") + defaultSSHCommands = []string{"md5sum", "sha1sum", "cd", "pwd", "scp"} + sshHashCommands = []string{"md5sum", "sha1sum", "sha256sum", "sha384sum", "sha512sum"} + systemCommands = []string{"git-receive-pack", "git-upload-pack", "git-upload-archive", "rsync"} + errUnconfiguredAction = errors.New("no hook is configured for this action") + 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) diff --git a/sftpd/sftpd_test.go b/sftpd/sftpd_test.go index c5b5f925..0c55fff8 100644 --- a/sftpd/sftpd_test.go +++ b/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) {