seafile: fix 2fa state machine

backend/seafile/seafile.go

@@ -325,17 +325,20 @@ func Config(ctx context.Context, name string, m configmap.Mapper, config fs.Conf
 
 	switch config.State {
 	case "":
-		// Just make sure we do have a password
+		// Empty state means it's the first call to the Config function
 		if password == "" {
-			return fs.ConfigPassword("", "config_password", "Two-factor authentication: please enter your password (it won't be saved in the configuration)")
+			return fs.ConfigPassword("password", "config_password", "Two-factor authentication: please enter your password (it won't be saved in the configuration)")
 		}
-		return fs.ConfigGoto("password")
+		// password was successfully loaded from the config
+		return fs.ConfigGoto("2fa")
 	case "password":
+		// password should be coming from the previous state (entered by the user)
 		password = config.Result
 		if password == "" {
-			return fs.ConfigError("password", "Password can't be blank")
+			return fs.ConfigError("", "Password can't be blank")
 		}
-		m.Set(configPassword, obscure.MustObscure(config.Result))
+		// save it into the configuration file and keep going
+		m.Set(configPassword, obscure.MustObscure(password))
 		return fs.ConfigGoto("2fa")
 	case "2fa":
 		return fs.ConfigInput("2fa_do", "config_2fa", "Two-factor authentication: please enter your 2FA code")

backend/seafile/seafile_internal_test.go

@@ -1,10 +1,15 @@
 package seafile
 
 import (
+	"context"
 	"path"
 	"testing"
 
+	"github.com/rclone/rclone/fs"
+	"github.com/rclone/rclone/fs/config/configmap"
+	"github.com/rclone/rclone/fs/config/obscure"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 type pathData struct {
@@ -19,77 +24,77 @@ type pathData struct {
 // from a mix of configuration data and path command line argument
 func TestSplitPath(t *testing.T) {
 	testData := []pathData{
-		pathData{
+		{
 			configLibrary:   "",
 			configRoot:      "",
 			argumentPath:    "",
 			expectedLibrary: "",
 			expectedPath:    "",
 		},
-		pathData{
+		{
 			configLibrary:   "",
 			configRoot:      "",
 			argumentPath:    "Library",
 			expectedLibrary: "Library",
 			expectedPath:    "",
 		},
-		pathData{
+		{
 			configLibrary:   "",
 			configRoot:      "",
 			argumentPath:    path.Join("Library", "path", "to", "file"),
 			expectedLibrary: "Library",
 			expectedPath:    path.Join("path", "to", "file"),
 		},
-		pathData{
+		{
 			configLibrary:   "Library",
 			configRoot:      "",
 			argumentPath:    "",
 			expectedLibrary: "Library",
 			expectedPath:    "",
 		},
-		pathData{
+		{
 			configLibrary:   "Library",
 			configRoot:      "",
 			argumentPath:    "path",
 			expectedLibrary: "Library",
 			expectedPath:    "path",
 		},
-		pathData{
+		{
 			configLibrary:   "Library",
 			configRoot:      "",
 			argumentPath:    path.Join("path", "to", "file"),
 			expectedLibrary: "Library",
 			expectedPath:    path.Join("path", "to", "file"),
 		},
-		pathData{
+		{
 			configLibrary:   "Library",
 			configRoot:      "root",
 			argumentPath:    "",
 			expectedLibrary: "Library",
 			expectedPath:    "root",
 		},
-		pathData{
+		{
 			configLibrary:   "Library",
 			configRoot:      path.Join("root", "path"),
 			argumentPath:    "",
 			expectedLibrary: "Library",
 			expectedPath:    path.Join("root", "path"),
 		},
-		pathData{
+		{
 			configLibrary:   "Library",
 			configRoot:      "root",
 			argumentPath:    "path",
 			expectedLibrary: "Library",
 			expectedPath:    path.Join("root", "path"),
 		},
-		pathData{
+		{
 			configLibrary:   "Library",
 			configRoot:      "root",
 			argumentPath:    path.Join("path", "to", "file"),
 			expectedLibrary: "Library",
 			expectedPath:    path.Join("root", "path", "to", "file"),
 		},
-		pathData{
+		{
 			configLibrary:   "Library",
 			configRoot:      path.Join("root", "path"),
 			argumentPath:    path.Join("subpath", "to", "file"),
@@ -121,3 +126,98 @@ func TestSplitPathIntoSlice(t *testing.T) {
 		assert.Equal(t, expected, output)
 	}
 }
+
+func Test2FAStateMachine(t *testing.T) {
+	fixtures := []struct {
+		name               string
+		mapper             configmap.Mapper
+		input              fs.ConfigIn
+		expectState        string
+		expectErrorMessage string
+		expectResult       string
+		expectFail         bool
+	}{
+		{
+			name:       "no url",
+			mapper:     configmap.Simple{},
+			input:      fs.ConfigIn{State: ""},
+			expectFail: true,
+		},
+		{
+			name:       "2fa not set",
+			mapper:     configmap.Simple{"url": "http://localhost/"},
+			input:      fs.ConfigIn{State: ""},
+			expectFail: true,
+		},
+		{
+			name:       "unknown state",
+			mapper:     configmap.Simple{"url": "http://localhost/", "2fa": "true", "user": "username"},
+			input:      fs.ConfigIn{State: "unknown"},
+			expectFail: true,
+		},
+		{
+			name:        "no password in config",
+			mapper:      configmap.Simple{"url": "http://localhost/", "2fa": "true", "user": "username"},
+			input:       fs.ConfigIn{State: ""},
+			expectState: "password",
+		},
+		{
+			name:        "config ready for 2fa token",
+			mapper:      configmap.Simple{"url": "http://localhost/", "2fa": "true", "user": "username", "pass": obscure.MustObscure("password")},
+			input:       fs.ConfigIn{State: ""},
+			expectState: "2fa",
+		},
+		{
+			name:               "password not entered",
+			mapper:             configmap.Simple{"url": "http://localhost/", "2fa": "true", "user": "username"},
+			input:              fs.ConfigIn{State: "password"},
+			expectState:        "",
+			expectErrorMessage: "Password can't be blank",
+		},
+		{
+			name:        "password entered",
+			mapper:      configmap.Simple{"url": "http://localhost/", "2fa": "true", "user": "username"},
+			input:       fs.ConfigIn{State: "password", Result: "password"},
+			expectState: "2fa",
+		},
+		{
+			name:        "ask for a 2fa code",
+			mapper:      configmap.Simple{"url": "http://localhost/", "2fa": "true", "user": "username"},
+			input:       fs.ConfigIn{State: "2fa"},
+			expectState: "2fa_do",
+		},
+		{
+			name:               "no 2fa code entered",
+			mapper:             configmap.Simple{"url": "http://localhost/", "2fa": "true", "user": "username"},
+			input:              fs.ConfigIn{State: "2fa_do"},
+			expectState:        "2fa", // ask for a code again
+			expectErrorMessage: "2FA codes can't be blank",
+		},
+		{
+			name:        "2fa error and retry",
+			mapper:      configmap.Simple{"url": "http://localhost/", "2fa": "true", "user": "username"},
+			input:       fs.ConfigIn{State: "2fa_error", Result: "true"},
+			expectState: "2fa", // ask for a code again
+		},
+		{
+			name:       "2fa error and fail",
+			mapper:     configmap.Simple{"url": "http://localhost/", "2fa": "true", "user": "username"},
+			input:      fs.ConfigIn{State: "2fa_error"},
+			expectFail: true,
+		},
+	}
+
+	for _, fixture := range fixtures {
+		t.Run(fixture.name, func(t *testing.T) {
+			output, err := Config(context.Background(), "test", fixture.mapper, fixture.input)
+			if fixture.expectFail {
+				require.Error(t, err)
+				t.Log(err)
+				return
+			}
+			assert.Equal(t, fixture.expectState, output.State)
+			assert.Equal(t, fixture.expectErrorMessage, output.Error)
+			assert.Equal(t, fixture.expectResult, output.Result)
+		})
+	}
+}