Implementing all 4 typical "messages" as in proftpd and other commercial FTP servers by @syncplify (#435)

* Implemented all 4 "msgs" as in proftpd and other commercial FTP servers
* Fixed 2 out of 3 issues as per Drakkan's request
* Added GetPostAuthMessage via MainDriverExtensionPostAuthMessage interface
* Trying to make wsl happy
* Rewriting some _old_ code around the handlePASS method
* Cleaner QUIT handler
---------

Co-authored-by: Syncplify <fjodr@syncplify.me>
Co-authored-by: syncplify <info@syncplify.me>

Author: fclairamb

driver.go

@@ -58,6 +58,19 @@ type MainDriverExtensionUserVerifier interface {
 	PreAuthUser(cc ClientContext, user string) error
 }
 
+// MainDriverExtensionPostAuthMessage is an extension that allows to send a message
+// after the authentication
+type MainDriverExtensionPostAuthMessage interface {
+	// PostAuthMessage is called after the authentication
+	PostAuthMessage(cc ClientContext, user string, authErr error) string
+}
+
+// MainDriverExtensionQuitMessage is an extension that allows to control the quit message
+type MainDriverExtensionQuitMessage interface {
+	// QuitMessage returns the message to display when the user quits the server
+	QuitMessage() string
+}
+
 // ClientDriver is the base FS implementation that allows to manipulate files
 type ClientDriver interface {
 	afero.Fs

driver_test.go

@@ -103,6 +103,8 @@ type TestServerDriver struct {
 	TLSVerificationReply tlsVerificationReply
 	errPassiveListener   error
 	TLSRequirement       TLSRequirement
+	customAuthMessage    bool
+	customQuitMessage    bool
 }
 
 // TestClientDriver defines a minimal serverftp client driver
@@ -229,11 +231,36 @@ func (driver *TestServerDriver) AuthUser(_ ClientContext, user, pass string) (Cl
 		clientdriver := NewTestClientDriver(driver)
 
 		return clientdriver, nil
+	} else if user == "nil" && pass == "nil" {
+		// Definitely a bad behavior (but can be done on the driver side)
+		return nil, nil
 	}
 
 	return nil, errBadUserNameOrPassword
 }
 
+// PostAuthMessage returns a message displayed after authentication
+func (driver *TestServerDriver) PostAuthMessage(_ ClientContext, _ string, authErr error) string {
+	if !driver.customAuthMessage {
+		return ""
+	}
+
+	if authErr != nil {
+		return "You are not welcome here"
+	}
+
+	return "Welcome to the FTP Server"
+}
+
+// QuitMessage returns a goodbye message
+func (driver *TestServerDriver) QuitMessage() string {
+	if !driver.customQuitMessage {
+		return ""
+	}
+
+	return "Sayonara, bye bye!"
+}
+
 // ClientDisconnected is called when the user disconnects
 func (driver *TestServerDriver) ClientDisconnected(cc ClientContext) {
 	driver.clientMU.Lock()

handle_auth.go

@@ -57,17 +57,31 @@ func (c *clientHandler) handleUSER(param string) error {
 // Handle the "PASS" command
 func (c *clientHandler) handlePASS(param string) error {
 	var err error
+	var msg string
 	c.driver, err = c.server.driver.AuthUser(c, c.user, param)
 
+	dpa, ok := c.server.driver.(MainDriverExtensionPostAuthMessage)
+	if ok {
+		msg = dpa.PostAuthMessage(c, c.user, err)
+	}
+
 	switch {
-	case err == nil:
-		c.writeMessage(StatusUserLoggedIn, "Password ok, continue")
-	case err != nil:
-		c.writeMessage(StatusNotLoggedIn, fmt.Sprintf("Authentication problem: %v", err))
+	case err == nil && c.driver == nil:
+		c.writeMessage(StatusNotLoggedIn, "Unexpected exception (driver is nil)")
 		c.disconnect()
-	default:
-		c.writeMessage(StatusNotLoggedIn, "I can't deal with you (nil driver)")
+	case err != nil:
+		if msg == "" {
+			msg = fmt.Sprintf("Authentication error: %v", err)
+		}
+
+		c.writeMessage(StatusNotLoggedIn, msg)
 		c.disconnect()
+	case err == nil:
+		if msg == "" {
+			msg = "Password ok, continue"
+		}
+
+		c.writeMessage(StatusUserLoggedIn, msg)
 	}
 
 	return nil

handle_auth_test.go

@@ -90,6 +90,42 @@ func TestLoginFailure(t *testing.T) {
 	require.Error(t, err, "We should have failed to login")
 }
 
+func TestLoginCustom(t *testing.T) {
+	s := NewTestServerWithDriver(t, &TestServerDriver{Debug: true, customAuthMessage: true})
+	r := require.New(t)
+
+	conf := goftp.Config{
+		User:     authUser,
+		Password: authPass + "_wrong",
+	}
+
+	c, err := goftp.DialConfig(conf, s.Addr())
+	r.NoError(err, "Couldn't connect")
+
+	defer func() { panicOnError(c.Close()) }()
+
+	_, err = c.OpenRawConn()
+	r.Error(err, "We should have failed to login")
+}
+
+func TestLoginNil(t *testing.T) {
+	s := NewTestServer(t, true)
+	r := require.New(t)
+
+	conf := goftp.Config{
+		User:     "nil",
+		Password: "nil",
+	}
+
+	c, err := goftp.DialConfig(conf, s.Addr())
+	require.NoError(t, err, "Couldn't connect")
+
+	defer func() { panicOnError(c.Close()) }()
+
+	_, err = c.OpenRawConn()
+	r.Error(err)
+}
+
 func TestAuthTLS(t *testing.T) {
 	s := NewTestServerWithDriver(t, &TestServerDriver{
 		Debug: false,

handle_misc.go

@@ -11,7 +11,7 @@ import (
 
 var errUnknowHash = errors.New("unknown hash algorithm")
 
-func (c *clientHandler) handleAUTH(param string) error {
+func (c *clientHandler) handleAUTH(_ string) error {
 	if tlsConfig, err := c.server.driver.GetTLSConfig(); err == nil {
 		c.writeMessage(StatusAuthAccepted, "AUTH command ok. Expecting TLS Negotiation.")
 		c.conn = tls.Server(c.conn, tlsConfig)
@@ -274,7 +274,16 @@ func (c *clientHandler) handleTYPE(param string) error {
 
 func (c *clientHandler) handleQUIT(param string) error {
 	c.transferWg.Wait()
-	c.writeMessage(StatusClosingControlConn, "Goodbye")
+
+	var msg string
+
+	if quitter, ok := c.server.driver.(MainDriverExtensionQuitMessage); ok {
+		msg = quitter.QuitMessage()
+	} else {
+		msg = "Goodbye"
+	}
+
+	c.writeMessage(StatusClosingControlConn, msg)
 	c.disconnect()
 	c.reader = nil
 

handle_misc_test.go

@@ -275,6 +275,36 @@ func TestQuit(t *testing.T) {
 	require.Equal(t, StatusClosingControlConn, rc)
 }
 
+func TestQuitWithCustomMessage(_t *testing.T) {
+	s := NewTestServerWithDriver(_t, &TestServerDriver{
+		Debug:             true,
+		TLS:               true,
+		customQuitMessage: true,
+	})
+	t := require.New(_t)
+	conf := goftp.Config{
+		User:     authUser,
+		Password: authPass,
+		TLSConfig: &tls.Config{
+			//nolint:gosec
+			InsecureSkipVerify: true,
+		},
+		TLSMode: goftp.TLSExplicit,
+	}
+	c, err := goftp.DialConfig(conf, s.Addr())
+	t.NoError(err, "Couldn't connect")
+
+	defer func() { panicOnError(c.Close()) }()
+
+	raw, err := c.OpenRawConn()
+	t.NoError(err, "Couldn't open raw connection")
+
+	rc, msg, err := raw.SendCommand("QUIT")
+	t.NoError(err)
+	t.Equal(StatusClosingControlConn, rc)
+	t.Equal("Sayonara, bye bye!", msg)
+}
+
 func TestQuitWithTransferInProgress(t *testing.T) {
 	s := NewTestServerWithDriver(t, &TestServerDriver{
 		Debug: false,