actions: add a generic hook to define external commands and HTTP URL

We can only define a single hook now and it can be an HTTP notification
or an external command, not both

config/config.go

@@ -53,9 +53,8 @@ func init() {
 			Umask:        "0022",
 			UploadMode:   0,
 			Actions: sftpd.Actions{
-				ExecuteOn:           []string{},
-				Command:             "",
-				HTTPNotificationURL: "",
+				ExecuteOn: []string{},
+				Hook:      "",
 			},
 			HostKeys:                []string{},
 			KexAlgorithms:           []string{},
@@ -83,9 +82,8 @@ func init() {
 			PoolSize:         0,
 			UsersBaseDir:     "",
 			Actions: dataprovider.Actions{
-				ExecuteOn:           []string{},
-				Command:             "",
-				HTTPNotificationURL: "",
+				ExecuteOn: []string{},
+				Hook:      "",
 			},
 			ExternalAuthHook:  "",
 			ExternalAuthScope: 0,
@@ -240,6 +238,28 @@ func checkHooksCompatibility() {
 		logger.WarnToConsole("keyboard_interactive_auth_program is deprecated, please use keyboard_interactive_auth_hook")
 		globalConf.SFTPD.KeyboardInteractiveHook = globalConf.SFTPD.KeyboardInteractiveProgram //nolint:staticcheck
 	}
+	if len(globalConf.SFTPD.Actions.Hook) == 0 {
+		if len(globalConf.SFTPD.Actions.HTTPNotificationURL) > 0 { //nolint:staticcheck
+			logger.Warn(logSender, "", "http_notification_url is deprecated, please use hook")
+			logger.WarnToConsole("http_notification_url is deprecated, please use hook")
+			globalConf.SFTPD.Actions.Hook = globalConf.SFTPD.Actions.HTTPNotificationURL //nolint:staticcheck
+		} else if len(globalConf.SFTPD.Actions.Command) > 0 { //nolint:staticcheck
+			logger.Warn(logSender, "", "command is deprecated, please use hook")
+			logger.WarnToConsole("command is deprecated, please use hook")
+			globalConf.SFTPD.Actions.Hook = globalConf.SFTPD.Actions.Command //nolint:staticcheck
+		}
+	}
+	if len(globalConf.ProviderConf.Actions.Hook) == 0 {
+		if len(globalConf.ProviderConf.Actions.HTTPNotificationURL) > 0 { //nolint:staticcheck
+			logger.Warn(logSender, "", "http_notification_url is deprecated, please use hook")
+			logger.WarnToConsole("http_notification_url is deprecated, please use hook")
+			globalConf.ProviderConf.Actions.Hook = globalConf.ProviderConf.Actions.HTTPNotificationURL //nolint:staticcheck
+		} else if len(globalConf.ProviderConf.Actions.Command) > 0 { //nolint:staticcheck
+			logger.Warn(logSender, "", "command is deprecated, please use hook")
+			logger.WarnToConsole("command is deprecated, please use hook")
+			globalConf.ProviderConf.Actions.Hook = globalConf.ProviderConf.Actions.Command //nolint:staticcheck
+		}
+	}
 }
 
 func checkHostKeyCompatibility() {

config/config_test.go

@@ -177,6 +177,7 @@ func TestHookCompatibity(t *testing.T) {
 	providerConf := config.GetProviderConf()
 	providerConf.ExternalAuthProgram = "ext_auth_program" //nolint:staticcheck
 	providerConf.PreLoginProgram = "pre_login_program"    //nolint:staticcheck
+	providerConf.Actions.Command = "/tmp/test_cmd"        //nolint:staticcheck
 	c := make(map[string]dataprovider.Config)
 	c["data_provider"] = providerConf
 	jsonConf, err := json.Marshal(c)
@@ -188,10 +189,26 @@ func TestHookCompatibity(t *testing.T) {
 	providerConf = config.GetProviderConf()
 	assert.Equal(t, "ext_auth_program", providerConf.ExternalAuthHook)
 	assert.Equal(t, "pre_login_program", providerConf.PreLoginHook)
+	assert.Equal(t, "/tmp/test_cmd", providerConf.Actions.Hook)
+	err = os.Remove(configFilePath)
+	assert.NoError(t, err)
+	providerConf.Actions.Hook = ""
+	providerConf.Actions.HTTPNotificationURL = "http://example.com/notify" //nolint:staticcheck
+	c = make(map[string]dataprovider.Config)
+	c["data_provider"] = providerConf
+	jsonConf, err = json.Marshal(c)
+	assert.NoError(t, err)
+	err = ioutil.WriteFile(configFilePath, jsonConf, 0666)
+	assert.NoError(t, err)
+	err = config.LoadConfig(configDir, tempConfigName)
+	assert.NoError(t, err)
+	providerConf = config.GetProviderConf()
+	assert.Equal(t, "http://example.com/notify", providerConf.Actions.Hook)
 	err = os.Remove(configFilePath)
 	assert.NoError(t, err)
 	sftpdConf := config.GetSFTPDConfig()
 	sftpdConf.KeyboardInteractiveProgram = "key_int_program" //nolint:staticcheck
+	sftpdConf.Actions.Command = "/tmp/sftp_cmd"              //nolint:staticcheck
 	cnf := make(map[string]sftpd.Configuration)
 	cnf["sftpd"] = sftpdConf
 	jsonConf, err = json.Marshal(cnf)
@@ -202,6 +219,21 @@ func TestHookCompatibity(t *testing.T) {
 	assert.NoError(t, err)
 	sftpdConf = config.GetSFTPDConfig()
 	assert.Equal(t, "key_int_program", sftpdConf.KeyboardInteractiveHook)
+	assert.Equal(t, "/tmp/sftp_cmd", sftpdConf.Actions.Hook)
+	err = os.Remove(configFilePath)
+	assert.NoError(t, err)
+	sftpdConf.Actions.Hook = ""
+	sftpdConf.Actions.HTTPNotificationURL = "http://example.com/sftp" //nolint:staticcheck
+	cnf = make(map[string]sftpd.Configuration)
+	cnf["sftpd"] = sftpdConf
+	jsonConf, err = json.Marshal(cnf)
+	assert.NoError(t, err)
+	err = ioutil.WriteFile(configFilePath, jsonConf, 0666)
+	assert.NoError(t, err)
+	err = config.LoadConfig(configDir, tempConfigName)
+	assert.NoError(t, err)
+	sftpdConf = config.GetSFTPDConfig()
+	assert.Equal(t, "http://example.com/sftp", sftpdConf.Actions.Hook)
 	err = os.Remove(configFilePath)
 	assert.NoError(t, err)
 }

dataprovider/dataprovider.go

@@ -111,13 +111,12 @@ type schemaVersion struct {
 type Actions struct {
 	// Valid values are add, update, delete. Empty slice to disable
 	ExecuteOn []string `json:"execute_on" mapstructure:"execute_on"`
-	// Absolute path to the command to execute, empty to disable
+	// Deprecated: please use Hook
 	Command string `json:"command" mapstructure:"command"`
-	// The URL to notify using an HTTP POST.
-	// The action is added to the query string. For example <url>?action=update.
-	// The user is sent serialized as json inside the POST body.
-	// Empty to disable
+	// Deprecated: please use Hook
 	HTTPNotificationURL string `json:"http_notification_url" mapstructure:"http_notification_url"`
+	// Absolute path to an external program or an HTTP URL
+	Hook string `json:"hook" mapstructure:"hook"`
 }
 
 // Config provider configuration
@@ -1520,15 +1519,20 @@ func providerLog(level logger.LogLevel, format string, v ...interface{}) {
 }
 
 func executeNotificationCommand(operation string, user User) error {
+	if !filepath.IsAbs(config.Actions.Hook) {
+		err := fmt.Errorf("invalid notification command %#v", config.Actions.Hook)
+		logger.Warn(logSender, "", "unable to execute notification command: %v", err)
+		return err
+	}
 	ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
 	defer cancel()
 	commandArgs := user.getNotificationFieldsAsSlice(operation)
-	cmd := exec.CommandContext(ctx, config.Actions.Command, commandArgs...)
+	cmd := exec.CommandContext(ctx, config.Actions.Hook, commandArgs...)
 	cmd.Env = append(os.Environ(), user.getNotificationFieldsAsEnvVars(operation)...)
 	startTime := time.Now()
 	err := cmd.Run()
 	providerLog(logger.LevelDebug, "executed command %#v with arguments: %+v, elapsed: %v, error: %v",
-		config.Actions.Command, commandArgs, time.Since(startTime), err)
+		config.Actions.Hook, commandArgs, time.Since(startTime), err)
 	return err
 }
 
@@ -1537,6 +1541,9 @@ func executeAction(operation string, user User) {
 	if !utils.IsStringInSlice(operation, config.Actions.ExecuteOn) {
 		return
 	}
+	if len(config.Actions.Hook) == 0 {
+		return
+	}
 	if operation != operationDelete {
 		var err error
 		user, err = provider.userExists(user.Username)
@@ -1545,21 +1552,11 @@ func executeAction(operation string, user User) {
 			return
 		}
 	}
-	if len(config.Actions.Command) > 0 && filepath.IsAbs(config.Actions.Command) {
-		// we are in a goroutine but if we have to send an HTTP notification we don't want to wait for the
-		// end of the command
-		if len(config.Actions.HTTPNotificationURL) > 0 {
-			go executeNotificationCommand(operation, user) //nolint:errcheck // the error is used in test cases only
-		} else {
-			executeNotificationCommand(operation, user) //nolint:errcheck // the error is used in test cases only
-		}
-	}
-	if len(config.Actions.HTTPNotificationURL) > 0 {
+	if strings.HasPrefix(config.Actions.Hook, "http") {
 		var url *url.URL
-		url, err := url.Parse(config.Actions.HTTPNotificationURL)
+		url, err := url.Parse(config.Actions.Hook)
 		if err != nil {
-			providerLog(logger.LevelWarn, "Invalid http_notification_url %#v for operation %#v: %v", config.Actions.HTTPNotificationURL,
-				operation, err)
+			providerLog(logger.LevelWarn, "Invalid http_notification_url %#v for operation %#v: %v", config.Actions.Hook, operation, err)
 			return
 		}
 		q := url.Query()
@@ -1580,5 +1577,7 @@ func executeAction(operation string, user User) {
 		}
 		providerLog(logger.LevelDebug, "notified operation %#v to URL: %v status code: %v, elapsed: %v err: %v",
 			operation, url.String(), respCode, time.Since(startTime), err)
+	} else {
+		executeNotificationCommand(operation, user) //nolint:errcheck // the error is used in test cases only
 	}
 }

docs/custom-actions.md

@@ -1,11 +1,12 @@
 # Custom Actions
 
 The `actions` struct inside the "sftpd" configuration section allows to configure the actions for file operations and SSH commands.
+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 `command`, if defined, is invoked with the following arguments:
+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`
 - `username`
@@ -13,7 +14,7 @@ The `command`, if defined, is invoked with the following arguments:
 - `target_path`, non-empty for `rename` action
 - `ssh_cmd`, non-empty for `ssh_cmd` action
 
-The `command` can also read the following environment variables:
+The external program can also read the following environment variables:
 
 - `SFTPGO_ACTION`
 - `SFTPGO_ACTION_USERNAME`
@@ -27,9 +28,9 @@ The `command` can also read the following environment variables:
 - `SFTPGO_ACTION_STATUS`, integer. 0 means an error occurred. 1 means no error
 
 Previous global environment variables aren't cleared when the script is called.
-The `command` must finish within 30 seconds.
+The program must finish within 30 seconds.
 
-The `http_notification_url`, if defined, will be invoked as HTTP POST. The request body will contain a JSON serialized struct with the following fields:
+If the `hook` defines an HTTP URL then this URL will be invoked as HTTP POST. The request body will contain a JSON serialized struct with the following fields:
 
 - `action`
 - `username`
@@ -49,7 +50,7 @@ The `actions` struct inside the "data_provider" configuration section allows you
 
 Actions will not be fired for internal updates, such as the last login or the user quota fields, or after external authentication.
 
-The `command`, if defined, is invoked with the following arguments:
+If the `hook` defines a path to an external program, then this program is invoked with the following arguments:
 
 - `action`, string, possible values are: `add`, `update`, `delete`
 - `username`
@@ -60,7 +61,7 @@ The `command`, if defined, is invoked with the following arguments:
 - `uid`
 - `gid`
 
-The `command` can also read the following environment variables:
+The external program can also read the following environment variables:
 
 - `SFTPGO_USER_ACTION`
 - `SFTPGO_USER_USERNAME`
@@ -79,8 +80,8 @@ The `command` can also read the following environment variables:
 - `SFTPGO_USER_FS_PROVIDER`
 
 Previous global environment variables aren't cleared when the script is called.
-The `command` must finish within 15 seconds.
+The program must finish within 15 seconds.
 
-The `http_notification_url`, if defined, will be invoked as HTTP POST. The action is added to the query string, for example `<http_notification_url>?action=update`, and the user is sent serialized as JSON inside the POST body with sensitive fields removed.
+If the `hook` defines an HTTP URL then this URL will be invoked as HTTP POST. The action is added to the query string, for example `<hook>?action=update`, and the user is sent serialized as JSON inside the POST body with sensitive fields removed.
 
 The HTTP request will use the global configuration for HTTP clients.

sftpd/internal_test.go

@@ -159,9 +159,8 @@ func TestWrongActions(t *testing.T) {
 		badCommand = "C:\\bad\\command"
 	}
 	actions = Actions{
-		ExecuteOn:           []string{operationDownload},
-		Command:             badCommand,
-		HTTPNotificationURL: "",
+		ExecuteOn: []string{operationDownload},
+		Hook:      badCommand,
 	}
 	user := dataprovider.User{
 		Username: "username",
@@ -170,21 +169,27 @@ func TestWrongActions(t *testing.T) {
 	assert.Error(t, err, "action with bad command must fail")
 
 	err = executeAction(newActionNotification(user, operationDelete, "path", "", "", 0, nil))
-	assert.NoError(t, err)
-	actions.Command = ""
-	actions.HTTPNotificationURL = "http://foo\x7f.com/"
+	assert.EqualError(t, err, errUnconfiguredAction.Error())
+	actions.Hook = "http://foo\x7f.com/"
 	err = executeAction(newActionNotification(user, operationDownload, "path", "", "", 0, nil))
 	assert.Error(t, err, "action with bad url must fail")
 
+	actions.Hook = ""
+	err = executeAction(newActionNotification(user, operationDownload, "path", "", "", 0, nil))
+	assert.Error(t, err, errNoHook.Error())
+
+	actions.Hook = "relative path"
+	err = executeNotificationCommand(newActionNotification(user, operationDownload, "path", "", "", 0, nil))
+	assert.EqualError(t, err, fmt.Sprintf("invalid notification command %#v", actions.Hook))
+
 	actions = actionsCopy
 }
 
 func TestActionHTTP(t *testing.T) {
 	actionsCopy := actions
 	actions = Actions{
-		ExecuteOn:           []string{operationDownload},
-		Command:             "",
-		HTTPNotificationURL: "http://127.0.0.1:8080/",
+		ExecuteOn: []string{operationDownload},
+		Hook:      "http://127.0.0.1:8080/",
 	}
 	user := dataprovider.User{
 		Username: "username",

sftpd/sftpd.go

@@ -7,11 +7,13 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net/url"
 	"os"
 	"os/exec"
 	"path/filepath"
+	"strings"
 	"sync"
 	"time"
 
@@ -66,9 +68,11 @@ 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"}
+	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")
 )
 
 type connectionTransfer struct {
@@ -92,10 +96,12 @@ type ActiveQuotaScan struct {
 type Actions struct {
 	// Valid values are download, upload, delete, rename, ssh_cmd. Empty slice to disable
 	ExecuteOn []string `json:"execute_on" mapstructure:"execute_on"`
-	// Absolute path to the command to execute, empty to disable
+	// Deprecated: please use Hook
 	Command string `json:"command" mapstructure:"command"`
-	// The URL to notify using an HTTP GET, empty to disable
+	// Deprecated: please use Hook
 	HTTPNotificationURL string `json:"http_notification_url" mapstructure:"http_notification_url"`
+	// Absolute path to an external program or an HTTP URL
+	Hook string `json:"hook" mapstructure:"hook"`
 }
 
 // ConnectionStatus status for an active connection
@@ -474,38 +480,36 @@ func isAtomicUploadEnabled() bool {
 }
 
 func executeNotificationCommand(a actionNotification) error {
+	if !filepath.IsAbs(actions.Hook) {
+		err := fmt.Errorf("invalid notification command %#v", actions.Hook)
+		logger.Warn(logSender, "", "unable to execute notification command: %v", err)
+		return err
+	}
 	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
 	defer cancel()
-	cmd := exec.CommandContext(ctx, actions.Command, a.Action, a.Username, a.Path, a.TargetPath, a.SSHCmd)
+	cmd := exec.CommandContext(ctx, actions.Hook, a.Action, a.Username, a.Path, a.TargetPath, a.SSHCmd)
 	cmd.Env = append(os.Environ(), a.AsEnvVars()...)
 	startTime := time.Now()
 	err := cmd.Run()
 	logger.Debug(logSender, "", "executed command %#v with arguments: %#v, %#v, %#v, %#v, %#v, elapsed: %v, error: %v",
-		actions.Command, a.Action, a.Username, a.Path, a.TargetPath, a.SSHCmd, time.Since(startTime), err)
+		actions.Hook, a.Action, a.Username, a.Path, a.TargetPath, a.SSHCmd, time.Since(startTime), err)
 	return err
 }
 
 // executed in a goroutine
 func executeAction(a actionNotification) error {
 	if !utils.IsStringInSlice(a.Action, actions.ExecuteOn) {
-		return nil
+		return errUnconfiguredAction
 	}
-	var err error
-	if len(actions.Command) > 0 && filepath.IsAbs(actions.Command) {
-		// we are in a goroutine but if we have to send an HTTP notification we don't want to wait for the
-		// end of the command
-		if len(actions.HTTPNotificationURL) > 0 {
-			go executeNotificationCommand(a) //nolint:errcheck
-		} else {
-			err = executeNotificationCommand(a) //nolint:errcheck
-		}
+	if len(actions.Hook) == 0 {
+		logger.Warn(logSender, "", "Unable to send notification, no hook is defined")
+		return errNoHook
 	}
-	if len(actions.HTTPNotificationURL) > 0 {
+	if strings.HasPrefix(actions.Hook, "http") {
 		var url *url.URL
-		url, err = url.Parse(actions.HTTPNotificationURL)
+		url, err := url.Parse(actions.Hook)
 		if err != nil {
-			logger.Warn(logSender, "", "Invalid http_notification_url %#v for operation %#v: %v", actions.HTTPNotificationURL,
-				a.Action, err)
+			logger.Warn(logSender, "", "Invalid hook %#v for operation %#v: %v", actions.Hook, a.Action, err)
 			return err
 		}
 		startTime := time.Now()
@@ -518,6 +522,7 @@ func executeAction(a actionNotification) error {
 		}
 		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)
+		return err
 	}
-	return err
+	return executeNotificationCommand(a)
 }

sftpd/sftpd_test.go

@@ -174,8 +174,7 @@ func TestMain(m *testing.M) {
 		scriptArgs = "%*"
 	} else {
 		sftpdConf.Actions.ExecuteOn = []string{"download", "upload", "rename", "delete", "ssh_cmd"}
-		sftpdConf.Actions.Command = "/bin/true"
-		sftpdConf.Actions.HTTPNotificationURL = "http://127.0.0.1:8083/"
+		sftpdConf.Actions.Hook = "/bin/true"
 		scriptArgs = "$@"
 		scpPath, err = exec.LookPath("scp")
 		if err != nil {

sftpgo.json

@@ -9,8 +9,7 @@
     "upload_mode": 0,
     "actions": {
       "execute_on": [],
-      "command": "",
-      "http_notification_url": ""
+      "hook": ""
     },
     "host_keys": [],
     "kex_algorithms": [],
@@ -46,8 +45,7 @@
     "users_base_dir": "",
     "actions": {
       "execute_on": [],
-      "command": "",
-      "http_notification_url": ""
+      "hook": ""
     },
     "external_auth_hook": "",
     "external_auth_scope": 0,