From 9fb43b2c463d65e2f040242ec771d544abdcec41 Mon Sep 17 00:00:00 2001
From: Nicola Murino <nicola.murino@gmail.com>
Date: Wed, 24 Nov 2021 11:27:32 +0100
Subject: [PATCH] docs: clarify how multi-step auth works with external
 authentication

Fixes #617
---
 .github/workflows/development.yml |  4 +-
 .github/workflows/release.yml     |  2 +-
 docs/external-auth.md             |  2 +
 sftpd/sftpd_test.go               | 68 +++++++++++++++++++++++++++++++
 4 files changed, 73 insertions(+), 3 deletions(-)

diff --git a/.github/workflows/development.yml b/.github/workflows/development.yml
index 56a52fe4..cd93e590 100644
--- a/.github/workflows/development.yml
+++ b/.github/workflows/development.yml
@@ -340,7 +340,7 @@ jobs:
             apt-get install -q -y curl gcc git
             if [ ${{ matrix.go }} == 'latest' ]
             then
-              GO_VERSION=$(curl https://golang.org/VERSION?m=text)
+              GO_VERSION=$(curl -L https://go.dev/VERSION?m=text)
             else
               GO_VERSION=${{ matrix.go }}
             fi
@@ -349,7 +349,7 @@ jobs:
             then
               GO_DOWNLOAD_ARCH=armv6l
             fi
-            curl --retry 5 --retry-delay 2 --connect-timeout 10 -o go.tar.gz -L https://golang.org/dl/${GO_VERSION}.linux-${GO_DOWNLOAD_ARCH}.tar.gz
+            curl --retry 5 --retry-delay 2 --connect-timeout 10 -o go.tar.gz -L https://go.dev/dl/${GO_VERSION}.linux-${GO_DOWNLOAD_ARCH}.tar.gz
             tar -C /usr/local -xzf go.tar.gz
           run: |
             export PATH=$PATH:/usr/local/go/bin
diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
index 5c946c96..9b24b0df 100644
--- a/.github/workflows/release.yml
+++ b/.github/workflows/release.yml
@@ -293,7 +293,7 @@ jobs:
             then
               GO_DOWNLOAD_ARCH=armv6l
             fi
-            curl --retry 5 --retry-delay 2 --connect-timeout 10 -o go.tar.gz -L https://golang.org/dl/go${{ steps.get_version.outputs.GO_VERSION }}.linux-${GO_DOWNLOAD_ARCH}.tar.gz
+            curl --retry 5 --retry-delay 2 --connect-timeout 10 -o go.tar.gz -L https://go.dev/dl/go${{ steps.get_version.outputs.GO_VERSION }}.linux-${GO_DOWNLOAD_ARCH}.tar.gz
             tar -C /usr/local -xzf go.tar.gz
           run: |
             export PATH=$PATH:/usr/local/go/bin
diff --git a/docs/external-auth.md b/docs/external-auth.md
index 530962ca..7097eb50 100644
--- a/docs/external-auth.md
+++ b/docs/external-auth.md
@@ -39,6 +39,8 @@ If authentication succeeds the HTTP response code must be 200 and the response b
 
 If the authentication fails the HTTP response code must be != 200 or the returned SFTPGo user must have an empty username.
 
+If the hook returns a user who is only allowed to authenticate using public key + password (multi step authentication), your hook will be invoked for each authentication step, so it must validate the public key and password separately. SFTPGo will take care that the client uses the allowed sequence.
+
 Actions defined for users added/updated will not be executed in this case and an already logged in user with the same username will not be disconnected.
 
 The program hook must finish within 30 seconds, the HTTP hook timeout will use the global configuration for HTTP clients.
diff --git a/sftpd/sftpd_test.go b/sftpd/sftpd_test.go
index 4a40ab1f..d223f419 100644
--- a/sftpd/sftpd_test.go
+++ b/sftpd/sftpd_test.go
@@ -2802,6 +2802,74 @@ func TestLoginExternalAuthPwdAndPubKey(t *testing.T) {
 	assert.NoError(t, err)
 }
 
+func TestExternalAuthMultiStepLoginKeyAndPwd(t *testing.T) {
+	if runtime.GOOS == osWindows {
+		t.Skip("this test is not available on Windows")
+	}
+	u := getTestUser(true)
+	u.Password = defaultPassword
+	u.Filters.DeniedLoginMethods = append(u.Filters.DeniedLoginMethods, []string{
+		dataprovider.SSHLoginMethodKeyAndKeyboardInt,
+		dataprovider.SSHLoginMethodPublicKey,
+		dataprovider.LoginMethodPassword,
+		dataprovider.SSHLoginMethodKeyboardInteractive,
+	}...)
+
+	err := dataprovider.Close()
+	assert.NoError(t, err)
+	err = config.LoadConfig(configDir, "")
+	assert.NoError(t, err)
+	providerConf := config.GetProviderConf()
+	err = os.WriteFile(extAuthPath, getExtAuthScriptContent(u, false, false, ""), os.ModePerm)
+	assert.NoError(t, err)
+	providerConf.ExternalAuthHook = extAuthPath
+	providerConf.ExternalAuthScope = 0
+	err = dataprovider.Initialize(providerConf, configDir, true)
+	assert.NoError(t, err)
+
+	signer, err := ssh.ParsePrivateKey([]byte(testPrivateKey))
+	assert.NoError(t, err)
+	authMethods := []ssh.AuthMethod{
+		ssh.PublicKeys(signer),
+		ssh.Password(defaultPassword),
+	}
+	conn, client, err := getCustomAuthSftpClient(u, authMethods, "")
+	if assert.NoError(t, err) {
+		defer conn.Close()
+		defer client.Close()
+		assert.NoError(t, checkBasicSFTP(client))
+	}
+	// wrong sequence should fail
+	authMethods = []ssh.AuthMethod{
+		ssh.Password(defaultPassword),
+		ssh.PublicKeys(signer),
+	}
+	_, _, err = getCustomAuthSftpClient(u, authMethods, "")
+	assert.Error(t, err)
+
+	// public key only auth must fail
+	_, _, err = getSftpClient(u, true)
+	assert.Error(t, err)
+	// password only auth must fail
+	_, _, err = getSftpClient(u, false)
+	assert.Error(t, err)
+
+	_, err = httpdtest.RemoveUser(u, http.StatusOK)
+	assert.NoError(t, err)
+	err = os.RemoveAll(u.GetHomeDir())
+	assert.NoError(t, err)
+
+	err = dataprovider.Close()
+	assert.NoError(t, err)
+	err = config.LoadConfig(configDir, "")
+	assert.NoError(t, err)
+	providerConf = config.GetProviderConf()
+	err = dataprovider.Initialize(providerConf, configDir, true)
+	assert.NoError(t, err)
+	err = os.Remove(extAuthPath)
+	assert.NoError(t, err)
+}
+
 func TestExternalAuthEmptyResponse(t *testing.T) {
 	if runtime.GOOS == osWindows {
 		t.Skip("this test is not available on Windows")