From 00c6642fad9c04965601fe75d23b370406acdaa9 Mon Sep 17 00:00:00 2001 From: Fred Date: Fri, 17 Sep 2021 22:48:19 +0100 Subject: [PATCH] seafile: fix 2fa state machine --- backend/seafile/seafile.go | 13 ++- backend/seafile/seafile_internal_test.go | 122 +++++++++++++++++++++-- 2 files changed, 119 insertions(+), 16 deletions(-) diff --git a/backend/seafile/seafile.go b/backend/seafile/seafile.go index e6f744ba3..70f3730bd 100644 --- a/backend/seafile/seafile.go +++ b/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") diff --git a/backend/seafile/seafile_internal_test.go b/backend/seafile/seafile_internal_test.go index 784e8c845..53c7a3042 100644 --- a/backend/seafile/seafile_internal_test.go +++ b/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) + }) + } +}