From 1fed2d910c674ca35310961e3e1a4ce629a902b1 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Wed, 10 Mar 2021 15:40:34 +0000 Subject: [PATCH] config: make config file system pluggable If you are using rclone a library you can decide to use the rclone config file system or not by calling configfile.LoadConfig(ctx) If you don't you will need to set `config.Data` to an implementation of `config.Storage`. Other changes - change interface of config.FileGet to remove unused default - remove MustValue from config.Storage interface - change GetValue to return string or bool like elsewhere in rclone - implement a default config file system which panics with helpful error - implement getWithDefault to replace the removed MustValue - don't embed goconfig.ConfigFile so we can change the methods --- backend/alias/alias_internal_test.go | 3 +- backend/cache/cache_internal_test.go | 6 +- backend/http/http_internal_test.go | 4 +- cmd/cmd.go | 4 + cmd/listremotes/listremotes.go | 2 +- cmd/mountlib/rc_test.go | 2 + cmd/serve/dlna/dlna_test.go | 4 +- cmd/serve/http/http_test.go | 4 +- cmd/serve/restic/restic_appendonly_test.go | 4 + fs/config/config.go | 53 ++-- fs/config/config_internal_test.go | 29 +++ fs/config/config_test.go | 269 ++++++++------------- fs/config/configfile/configfile.go | 92 ++++--- fs/config/crypt.go | 17 +- fs/config/crypt_test.go | 48 ++++ fs/config/default_storage.go | 61 +++++ fs/config/rc_test.go | 5 +- fs/rc/rcserver/rcserver_test.go | 5 +- fstest/fstest.go | 3 +- 19 files changed, 380 insertions(+), 235 deletions(-) create mode 100644 fs/config/config_internal_test.go create mode 100644 fs/config/crypt_test.go create mode 100644 fs/config/default_storage.go diff --git a/backend/alias/alias_internal_test.go b/backend/alias/alias_internal_test.go index 5839a9fff..8b4efa2ec 100644 --- a/backend/alias/alias_internal_test.go +++ b/backend/alias/alias_internal_test.go @@ -11,6 +11,7 @@ import ( _ "github.com/rclone/rclone/backend/local" // pull in test backend "github.com/rclone/rclone/fs" "github.com/rclone/rclone/fs/config" + "github.com/rclone/rclone/fs/config/configfile" "github.com/stretchr/testify/require" ) @@ -19,7 +20,7 @@ var ( ) func prepare(t *testing.T, root string) { - config.LoadConfig(context.Background()) + configfile.LoadConfig(context.Background()) // Configure the remote config.FileSet(remoteName, "type", "alias") diff --git a/backend/cache/cache_internal_test.go b/backend/cache/cache_internal_test.go index d5351899a..d15a39866 100644 --- a/backend/cache/cache_internal_test.go +++ b/backend/cache/cache_internal_test.go @@ -892,7 +892,7 @@ func (r *run) newCacheFs(t *testing.T, remote, id string, needRemote, purge bool m.Set("type", "cache") m.Set("remote", localRemote+":"+filepath.Join(os.TempDir(), localRemote)) } else { - remoteType := config.FileGet(remote, "type", "") + remoteType := config.FileGet(remote, "type") if remoteType == "" { t.Skipf("skipped due to invalid remote type for %v", remote) return nil, nil @@ -903,14 +903,14 @@ func (r *run) newCacheFs(t *testing.T, remote, id string, needRemote, purge bool m.Set("password", cryptPassword1) m.Set("password2", cryptPassword2) } - remoteRemote := config.FileGet(remote, "remote", "") + remoteRemote := config.FileGet(remote, "remote") if remoteRemote == "" { t.Skipf("skipped due to invalid remote wrapper for %v", remote) return nil, nil } remoteRemoteParts := strings.Split(remoteRemote, ":") remoteWrapping := remoteRemoteParts[0] - remoteType := config.FileGet(remoteWrapping, "type", "") + remoteType := config.FileGet(remoteWrapping, "type") if remoteType != "cache" { t.Skipf("skipped due to invalid remote type for %v: '%v'", remoteWrapping, remoteType) return nil, nil diff --git a/backend/http/http_internal_test.go b/backend/http/http_internal_test.go index 55d1bbc70..43e78444e 100644 --- a/backend/http/http_internal_test.go +++ b/backend/http/http_internal_test.go @@ -15,7 +15,7 @@ import ( "time" "github.com/rclone/rclone/fs" - "github.com/rclone/rclone/fs/config" + "github.com/rclone/rclone/fs/config/configfile" "github.com/rclone/rclone/fs/config/configmap" "github.com/rclone/rclone/fstest" "github.com/rclone/rclone/lib/rest" @@ -47,7 +47,7 @@ func prepareServer(t *testing.T) (configmap.Simple, func()) { ts := httptest.NewServer(handler) // Configure the remote - config.LoadConfig(context.Background()) + configfile.LoadConfig(context.Background()) // fs.Config.LogLevel = fs.LogLevelDebug // fs.Config.DumpHeaders = true // fs.Config.DumpBodies = true diff --git a/cmd/cmd.go b/cmd/cmd.go index 9ce1acd15..039da29e7 100644 --- a/cmd/cmd.go +++ b/cmd/cmd.go @@ -25,6 +25,7 @@ import ( "github.com/rclone/rclone/fs" "github.com/rclone/rclone/fs/accounting" "github.com/rclone/rclone/fs/cache" + "github.com/rclone/rclone/fs/config/configfile" "github.com/rclone/rclone/fs/config/configflags" "github.com/rclone/rclone/fs/config/flags" "github.com/rclone/rclone/fs/filter" @@ -382,6 +383,9 @@ func initConfig() { // Finish parsing any command line flags configflags.SetFlags(ci) + // Load the config + configfile.LoadConfig(ctx) + // Hide console window if ci.NoConsole { terminal.HideConsole() diff --git a/cmd/listremotes/listremotes.go b/cmd/listremotes/listremotes.go index e41bce10f..40b4ea12d 100644 --- a/cmd/listremotes/listremotes.go +++ b/cmd/listremotes/listremotes.go @@ -41,7 +41,7 @@ When uses with the -l flag it lists the types too. } for _, remote := range remotes { if listLong { - remoteType := config.FileGet(remote, "type", "UNKNOWN") + remoteType := config.FileGet(remote, "type") fmt.Printf("%-*s %s\n", maxlen+1, remote+":", remoteType) } else { fmt.Printf("%s:\n", remote) diff --git a/cmd/mountlib/rc_test.go b/cmd/mountlib/rc_test.go index de725d7b1..22c354bc5 100644 --- a/cmd/mountlib/rc_test.go +++ b/cmd/mountlib/rc_test.go @@ -13,6 +13,7 @@ import ( _ "github.com/rclone/rclone/cmd/cmount" _ "github.com/rclone/rclone/cmd/mount" _ "github.com/rclone/rclone/cmd/mount2" + "github.com/rclone/rclone/fs/config/configfile" "github.com/rclone/rclone/fs/rc" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -20,6 +21,7 @@ import ( func TestRc(t *testing.T) { ctx := context.Background() + configfile.LoadConfig(ctx) mount := rc.Calls.Get("mount/mount") assert.NotNil(t, mount) unmount := rc.Calls.Get("mount/unmount") diff --git a/cmd/serve/dlna/dlna_test.go b/cmd/serve/dlna/dlna_test.go index 883255e19..535c85f26 100644 --- a/cmd/serve/dlna/dlna_test.go +++ b/cmd/serve/dlna/dlna_test.go @@ -13,12 +13,12 @@ import ( "github.com/anacrolix/dms/soap" + "github.com/rclone/rclone/fs/config/configfile" "github.com/rclone/rclone/vfs" _ "github.com/rclone/rclone/backend/local" "github.com/rclone/rclone/cmd/serve/dlna/dlnaflags" "github.com/rclone/rclone/fs" - "github.com/rclone/rclone/fs/config" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -41,7 +41,7 @@ func startServer(t *testing.T, f fs.Fs) { } func TestInit(t *testing.T) { - config.LoadConfig(context.Background()) + configfile.LoadConfig(context.Background()) f, err := fs.NewFs(context.Background(), "testdata/files") l, _ := f.List(context.Background(), "") diff --git a/cmd/serve/http/http_test.go b/cmd/serve/http/http_test.go index ed76fa879..cec0bd36b 100644 --- a/cmd/serve/http/http_test.go +++ b/cmd/serve/http/http_test.go @@ -12,7 +12,7 @@ import ( _ "github.com/rclone/rclone/backend/local" "github.com/rclone/rclone/cmd/serve/httplib" "github.com/rclone/rclone/fs" - "github.com/rclone/rclone/fs/config" + "github.com/rclone/rclone/fs/config/configfile" "github.com/rclone/rclone/fs/filter" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -61,7 +61,7 @@ var ( func TestInit(t *testing.T) { ctx := context.Background() // Configure the remote - config.LoadConfig(context.Background()) + configfile.LoadConfig(context.Background()) // fs.Config.LogLevel = fs.LogLevelDebug // fs.Config.DumpHeaders = true // fs.Config.DumpBodies = true diff --git a/cmd/serve/restic/restic_appendonly_test.go b/cmd/serve/restic/restic_appendonly_test.go index 9087f941d..74b1ec1d3 100644 --- a/cmd/serve/restic/restic_appendonly_test.go +++ b/cmd/serve/restic/restic_appendonly_test.go @@ -1,6 +1,7 @@ package restic import ( + "context" "crypto/rand" "encoding/hex" "io" @@ -12,6 +13,7 @@ import ( "github.com/rclone/rclone/cmd" "github.com/rclone/rclone/cmd/serve/httplib/httpflags" + "github.com/rclone/rclone/fs/config/configfile" "github.com/stretchr/testify/require" ) @@ -63,6 +65,8 @@ func createOverwriteDeleteSeq(t testing.TB, path string) []TestRequest { // TestResticHandler runs tests on the restic handler code, especially in append-only mode. func TestResticHandler(t *testing.T) { + ctx := context.Background() + configfile.LoadConfig(ctx) buf := make([]byte, 32) _, err := io.ReadFull(rand.Reader, buf) require.NoError(t, err) diff --git a/fs/config/config.go b/fs/config/config.go index 3beea04e2..c68df35cc 100644 --- a/fs/config/config.go +++ b/fs/config/config.go @@ -64,7 +64,12 @@ const ( ConfigAuthNoBrowser = "config_auth_no_browser" ) -// Storage defines an interface for loading and saving the config file. +// Storage defines an interface for loading and saving config to +// persistent storage. Rclone provides a default implementation to +// load and save to a config file when this is imported +// +// import "github.com/rclone/rclone/fs/config/configfile" +// configfile.LoadConfig(ctx) type Storage interface { // GetSectionList returns a slice of strings with names for all the // sections @@ -80,11 +85,8 @@ type Storage interface { // GetKeyList returns the keys in this section GetKeyList(section string) []string - // GetValue returns the key in section or an error if not found - GetValue(section string, key string) (string, error) - - // MustValue returns the key in section returning defaultValue if not set - MustValue(section string, key string, defaultValue ...string) string + // GetValue returns the key in section with a found flag + GetValue(section string, key string) (value string, found bool) // SetValue sets the value under key in section SetValue(section string, key string, value string) @@ -104,8 +106,8 @@ type Storage interface { // Global var ( - // configFile is the global config data structure. Don't read it directly, use Data - Data Storage + // Data is the global config data structure + Data Storage = defaultStorage{} // CacheDir points to the cache directory. Users of this // should make a subdirectory and use MkdirAll() to create it @@ -286,6 +288,16 @@ func SetValueAndSave(name, key, value string) error { return nil } +// getWithDefault gets key out of section name returning defaultValue if not +// found. +func getWithDefault(name, key, defaultValue string) string { + value, found := Data.GetValue(name, key) + if !found { + return defaultValue + } + return value +} + // FileGetFresh reads the config key under section return the value or // an error if the config file was not found or that value couldn't be // read. @@ -293,7 +305,11 @@ func FileGetFresh(section, key string) (value string, err error) { if err := Data.Load(); err != nil { return "", err } - return Data.GetValue(section, key) + value, found := Data.GetValue(section, key) + if !found { + return "", errors.New("value not found") + } + return value, nil } // ShowRemotes shows an overview of the config file @@ -583,7 +599,7 @@ func matchProvider(providerConfig, provider string) bool { // ChooseOption asks the user to choose an option func ChooseOption(o *fs.Option, name string) string { - var subProvider = Data.MustValue(name, fs.ConfigProvider, "") + var subProvider = getWithDefault(name, fs.ConfigProvider, "") fmt.Println(o.Help) if o.IsPassword { actions := []string{"yYes type in my own password", "gGenerate random password"} @@ -832,7 +848,7 @@ func editOptions(ri *fs.RegInfo, name string, isNew bool) { if option.Advanced != advanced { continue } - subProvider := Data.MustValue(name, fs.ConfigProvider, "") + subProvider := getWithDefault(name, fs.ConfigProvider, "") if matchProvider(option.Provider, subProvider) && isVisible { if !isNew { fmt.Printf("Value %q = %q\n", option.Name, FileGet(name, option.Name)) @@ -902,7 +918,7 @@ func copyRemote(name string) string { newName := NewRemoteName() // Copy the keys for _, key := range Data.GetKeyList(name) { - value := Data.MustValue(name, key, "") + value := getWithDefault(name, key, "") Data.SetValue(newName, key, value) } return newName @@ -1026,21 +1042,20 @@ func Authorize(ctx context.Context, args []string, noAutoBrowser bool) { // FileGetFlag gets the config key under section returning the // the value and true if found and or ("", false) otherwise func FileGetFlag(section, key string) (string, bool) { - newValue, err := Data.GetValue(section, key) - return newValue, err == nil + return Data.GetValue(section, key) } -// FileGet gets the config key under section returning the -// default or empty string if not set. +// FileGet gets the config key under section returning the default if not set. // // It looks up defaults in the environment if they are present -func FileGet(section, key string, defaultVal ...string) string { +func FileGet(section, key string) string { + var defaultVal string envKey := fs.ConfigToEnv(section, key) newValue, found := os.LookupEnv(envKey) if found { - defaultVal = []string{newValue} + defaultVal = newValue } - return Data.MustValue(section, key, defaultVal...) + return getWithDefault(section, key, defaultVal) } // FileSet sets the key in section to value. It doesn't save diff --git a/fs/config/config_internal_test.go b/fs/config/config_internal_test.go new file mode 100644 index 000000000..98568d47a --- /dev/null +++ b/fs/config/config_internal_test.go @@ -0,0 +1,29 @@ +package config + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestMatchProvider(t *testing.T) { + for _, test := range []struct { + config string + provider string + want bool + }{ + {"", "", true}, + {"one", "one", true}, + {"one,two", "two", true}, + {"one,two,three", "two", true}, + {"one", "on", false}, + {"one,two,three", "tw", false}, + {"!one,two,three", "two", false}, + {"!one,two,three", "four", true}, + } { + what := fmt.Sprintf("%q,%q", test.config, test.provider) + got := matchProvider(test.config, test.provider) + assert.Equal(t, test.want, got, what) + } +} diff --git a/fs/config/config_test.go b/fs/config/config_test.go index 812dee11d..4afa4728d 100644 --- a/fs/config/config_test.go +++ b/fs/config/config_test.go @@ -1,4 +1,4 @@ -package config +package config_test import ( "bytes" @@ -9,6 +9,8 @@ import ( "testing" "github.com/rclone/rclone/fs" + "github.com/rclone/rclone/fs/config" + "github.com/rclone/rclone/fs/config/configfile" "github.com/rclone/rclone/fs/config/obscure" "github.com/rclone/rclone/fs/rc" "github.com/stretchr/testify/assert" @@ -18,7 +20,7 @@ import ( func testConfigFile(t *testing.T, configFileName string) func() { ctx := context.Background() ci := fs.GetConfig(ctx) - configKey = nil // reset password + config.ClearConfigPassword() _ = os.Unsetenv("_RCLONE_CONFIG_KEY_FILE") _ = os.Unsetenv("RCLONE_CONFIG_PASS") // create temp config file @@ -29,18 +31,17 @@ func testConfigFile(t *testing.T, configFileName string) func() { // temporarily adapt configuration oldOsStdout := os.Stdout - oldConfigPath := ConfigPath + oldConfigPath := config.ConfigPath oldConfig := *ci - oldConfigFile := Data - oldReadLine := ReadLine - oldPassword := Password + oldConfigFile := config.Data + oldReadLine := config.ReadLine + oldPassword := config.Password os.Stdout = nil - ConfigPath = path + config.ConfigPath = path ci = &fs.ConfigInfo{} - Data = nil - LoadConfig(ctx) - assert.Equal(t, []string{}, Data.GetSectionList()) + configfile.LoadConfig(ctx) + assert.Equal(t, []string{}, config.Data.GetSectionList()) // Fake a remote fs.Register(&fs.RegInfo{ @@ -65,11 +66,11 @@ func testConfigFile(t *testing.T, configFileName string) func() { assert.NoError(t, err) os.Stdout = oldOsStdout - ConfigPath = oldConfigPath - ReadLine = oldReadLine - Password = oldPassword + config.ConfigPath = oldConfigPath + config.ReadLine = oldReadLine + config.Password = oldPassword *ci = oldConfig - Data = oldConfigFile + config.Data = oldConfigFile _ = os.Unsetenv("_RCLONE_CONFIG_KEY_FILE") _ = os.Unsetenv("RCLONE_CONFIG_PASS") @@ -91,7 +92,7 @@ func TestCRUD(t *testing.T) { ctx := context.Background() // script for creating remote - ReadLine = makeReadLine([]string{ + config.ReadLine = makeReadLine([]string{ "config_test_remote", // type "true", // bool value "y", // type my own password @@ -99,29 +100,29 @@ func TestCRUD(t *testing.T) { "secret", // repeat "y", // looks good, save }) - NewRemote(ctx, "test") + config.NewRemote(ctx, "test") - assert.Equal(t, []string{"test"}, Data.GetSectionList()) - assert.Equal(t, "config_test_remote", FileGet("test", "type")) - assert.Equal(t, "true", FileGet("test", "bool")) - assert.Equal(t, "secret", obscure.MustReveal(FileGet("test", "pass"))) + assert.Equal(t, []string{"test"}, config.Data.GetSectionList()) + assert.Equal(t, "config_test_remote", config.FileGet("test", "type")) + assert.Equal(t, "true", config.FileGet("test", "bool")) + assert.Equal(t, "secret", obscure.MustReveal(config.FileGet("test", "pass"))) // normal rename, test → asdf - ReadLine = makeReadLine([]string{ + config.ReadLine = makeReadLine([]string{ "asdf", "asdf", "asdf", }) - RenameRemote("test") + config.RenameRemote("test") - assert.Equal(t, []string{"asdf"}, Data.GetSectionList()) - assert.Equal(t, "config_test_remote", FileGet("asdf", "type")) - assert.Equal(t, "true", FileGet("asdf", "bool")) - assert.Equal(t, "secret", obscure.MustReveal(FileGet("asdf", "pass"))) + assert.Equal(t, []string{"asdf"}, config.Data.GetSectionList()) + assert.Equal(t, "config_test_remote", config.FileGet("asdf", "type")) + assert.Equal(t, "true", config.FileGet("asdf", "bool")) + assert.Equal(t, "secret", obscure.MustReveal(config.FileGet("asdf", "pass"))) // delete remote - DeleteRemote("asdf") - assert.Equal(t, []string{}, Data.GetSectionList()) + config.DeleteRemote("asdf") + assert.Equal(t, []string{}, config.Data.GetSectionList()) } func TestChooseOption(t *testing.T) { @@ -129,7 +130,7 @@ func TestChooseOption(t *testing.T) { ctx := context.Background() // script for creating remote - ReadLine = makeReadLine([]string{ + config.ReadLine = makeReadLine([]string{ "config_test_remote", // type "false", // bool value "x", // bad choice @@ -138,26 +139,26 @@ func TestChooseOption(t *testing.T) { "y", // password OK "y", // looks good, save }) - Password = func(bits int) (string, error) { + config.Password = func(bits int) (string, error) { assert.Equal(t, 1024, bits) return "not very random password", nil } - NewRemote(ctx, "test") + config.NewRemote(ctx, "test") - assert.Equal(t, "false", FileGet("test", "bool")) - assert.Equal(t, "not very random password", obscure.MustReveal(FileGet("test", "pass"))) + assert.Equal(t, "false", config.FileGet("test", "bool")) + assert.Equal(t, "not very random password", obscure.MustReveal(config.FileGet("test", "pass"))) // script for creating remote - ReadLine = makeReadLine([]string{ + config.ReadLine = makeReadLine([]string{ "config_test_remote", // type "true", // bool value "n", // not required "y", // looks good, save }) - NewRemote(ctx, "test") + config.NewRemote(ctx, "test") - assert.Equal(t, "true", FileGet("test", "bool")) - assert.Equal(t, "", FileGet("test", "pass")) + assert.Equal(t, "true", config.FileGet("test", "bool")) + assert.Equal(t, "", config.FileGet("test", "pass")) } func TestNewRemoteName(t *testing.T) { @@ -165,22 +166,22 @@ func TestNewRemoteName(t *testing.T) { ctx := context.Background() // script for creating remote - ReadLine = makeReadLine([]string{ + config.ReadLine = makeReadLine([]string{ "config_test_remote", // type "true", // bool value "n", // not required "y", // looks good, save }) - NewRemote(ctx, "test") + config.NewRemote(ctx, "test") - ReadLine = makeReadLine([]string{ + config.ReadLine = makeReadLine([]string{ "test", // already exists "", // empty string not allowed "bad@characters", // bad characters "newname", // OK }) - assert.Equal(t, "newname", NewRemoteName()) + assert.Equal(t, "newname", config.NewRemoteName()) } func TestCreateUpdatePasswordRemote(t *testing.T) { @@ -193,44 +194,44 @@ func TestCreateUpdatePasswordRemote(t *testing.T) { break } t.Run(fmt.Sprintf("doObscure=%v,noObscure=%v", doObscure, noObscure), func(t *testing.T) { - require.NoError(t, CreateRemote(ctx, "test2", "config_test_remote", rc.Params{ + require.NoError(t, config.CreateRemote(ctx, "test2", "config_test_remote", rc.Params{ "bool": true, "pass": "potato", }, doObscure, noObscure)) - assert.Equal(t, []string{"test2"}, Data.GetSectionList()) - assert.Equal(t, "config_test_remote", FileGet("test2", "type")) - assert.Equal(t, "true", FileGet("test2", "bool")) - gotPw := FileGet("test2", "pass") + assert.Equal(t, []string{"test2"}, config.Data.GetSectionList()) + assert.Equal(t, "config_test_remote", config.FileGet("test2", "type")) + assert.Equal(t, "true", config.FileGet("test2", "bool")) + gotPw := config.FileGet("test2", "pass") if !noObscure { gotPw = obscure.MustReveal(gotPw) } assert.Equal(t, "potato", gotPw) wantPw := obscure.MustObscure("potato2") - require.NoError(t, UpdateRemote(ctx, "test2", rc.Params{ + require.NoError(t, config.UpdateRemote(ctx, "test2", rc.Params{ "bool": false, "pass": wantPw, "spare": "spare", }, doObscure, noObscure)) - assert.Equal(t, []string{"test2"}, Data.GetSectionList()) - assert.Equal(t, "config_test_remote", FileGet("test2", "type")) - assert.Equal(t, "false", FileGet("test2", "bool")) - gotPw = FileGet("test2", "pass") + assert.Equal(t, []string{"test2"}, config.Data.GetSectionList()) + assert.Equal(t, "config_test_remote", config.FileGet("test2", "type")) + assert.Equal(t, "false", config.FileGet("test2", "bool")) + gotPw = config.FileGet("test2", "pass") if doObscure { gotPw = obscure.MustReveal(gotPw) } assert.Equal(t, wantPw, gotPw) - require.NoError(t, PasswordRemote(ctx, "test2", rc.Params{ + require.NoError(t, config.PasswordRemote(ctx, "test2", rc.Params{ "pass": "potato3", })) - assert.Equal(t, []string{"test2"}, Data.GetSectionList()) - assert.Equal(t, "config_test_remote", FileGet("test2", "type")) - assert.Equal(t, "false", FileGet("test2", "bool")) - assert.Equal(t, "potato3", obscure.MustReveal(FileGet("test2", "pass"))) + assert.Equal(t, []string{"test2"}, config.Data.GetSectionList()) + assert.Equal(t, "config_test_remote", config.FileGet("test2", "type")) + assert.Equal(t, "false", config.FileGet("test2", "bool")) + assert.Equal(t, "potato3", obscure.MustReveal(config.FileGet("test2", "pass"))) }) } } @@ -254,44 +255,41 @@ func TestReveal(t *testing.T) { } func TestConfigLoad(t *testing.T) { - oldConfigPath := ConfigPath - ConfigPath = "./testdata/plain.conf" + oldConfigPath := config.ConfigPath + config.ConfigPath = "./testdata/plain.conf" defer func() { - ConfigPath = oldConfigPath + config.ConfigPath = oldConfigPath }() - configKey = nil // reset password - err := Data.Load() - if err != nil { - t.Fatal(err) - } - sections := Data.GetSectionList() + config.ClearConfigPassword() + configfile.LoadConfig(context.Background()) + sections := config.Data.GetSectionList() var expect = []string{"RCLONE_ENCRYPT_V0", "nounc", "unc"} assert.Equal(t, expect, sections) - keys := Data.GetKeyList("nounc") + keys := config.Data.GetKeyList("nounc") expect = []string{"type", "nounc"} assert.Equal(t, expect, keys) } func TestConfigLoadEncrypted(t *testing.T) { var err error - oldConfigPath := ConfigPath - ConfigPath = "./testdata/encrypted.conf" + oldConfigPath := config.ConfigPath + config.ConfigPath = "./testdata/encrypted.conf" defer func() { - ConfigPath = oldConfigPath - configKey = nil // reset password + config.ConfigPath = oldConfigPath + config.ClearConfigPassword() }() // Set correct password - err = setConfigPassword("asdf") + err = config.SetConfigPassword("asdf") require.NoError(t, err) - err = Data.Load() + err = config.Data.Load() require.NoError(t, err) - sections := Data.GetSectionList() + sections := config.Data.GetSectionList() var expect = []string{"nounc", "unc"} assert.Equal(t, expect, sections) - keys := Data.GetKeyList("nounc") + keys := config.Data.GetKeyList("nounc") expect = []string{"type", "nounc"} assert.Equal(t, expect, keys) } @@ -299,28 +297,28 @@ func TestConfigLoadEncrypted(t *testing.T) { func TestConfigLoadEncryptedWithValidPassCommand(t *testing.T) { ctx := context.Background() ci := fs.GetConfig(ctx) - oldConfigPath := ConfigPath + oldConfigPath := config.ConfigPath oldConfig := *ci - ConfigPath = "./testdata/encrypted.conf" + config.ConfigPath = "./testdata/encrypted.conf" // using ci.PasswordCommand, correct password ci.PasswordCommand = fs.SpaceSepList{"echo", "asdf"} defer func() { - ConfigPath = oldConfigPath - configKey = nil // reset password + config.ConfigPath = oldConfigPath + config.ClearConfigPassword() *ci = oldConfig ci.PasswordCommand = nil }() - configKey = nil // reset password + config.ClearConfigPassword() - err := Data.Load() + err := config.Data.Load() require.NoError(t, err) - sections := Data.GetSectionList() + sections := config.Data.GetSectionList() var expect = []string{"nounc", "unc"} assert.Equal(t, expect, sections) - keys := Data.GetKeyList("nounc") + keys := config.Data.GetKeyList("nounc") expect = []string{"type", "nounc"} assert.Equal(t, expect, keys) } @@ -328,21 +326,21 @@ func TestConfigLoadEncryptedWithValidPassCommand(t *testing.T) { func TestConfigLoadEncryptedWithInvalidPassCommand(t *testing.T) { ctx := context.Background() ci := fs.GetConfig(ctx) - oldConfigPath := ConfigPath + oldConfigPath := config.ConfigPath oldConfig := *ci - ConfigPath = "./testdata/encrypted.conf" + config.ConfigPath = "./testdata/encrypted.conf" // using ci.PasswordCommand, incorrect password ci.PasswordCommand = fs.SpaceSepList{"echo", "asdf-blurfl"} defer func() { - ConfigPath = oldConfigPath - configKey = nil // reset password + config.ConfigPath = oldConfigPath + config.ClearConfigPassword() *ci = oldConfig ci.PasswordCommand = nil }() - configKey = nil // reset password + config.ClearConfigPassword() - err := Data.Load() + err := config.Data.Load() require.Error(t, err) assert.Contains(t, err.Error(), "using --password-command derived password") } @@ -351,104 +349,43 @@ func TestConfigLoadEncryptedFailures(t *testing.T) { var err error // This file should be too short to be decoded. - oldConfigPath := ConfigPath - ConfigPath = "./testdata/enc-short.conf" - defer func() { ConfigPath = oldConfigPath }() - err = Data.Load() + oldConfigPath := config.ConfigPath + config.ConfigPath = "./testdata/enc-short.conf" + defer func() { config.ConfigPath = oldConfigPath }() + err = config.Data.Load() require.Error(t, err) // This file contains invalid base64 characters. - ConfigPath = "./testdata/enc-invalid.conf" - err = Data.Load() + config.ConfigPath = "./testdata/enc-invalid.conf" + err = config.Data.Load() require.Error(t, err) // This file contains invalid base64 characters. - ConfigPath = "./testdata/enc-too-new.conf" - err = Data.Load() + config.ConfigPath = "./testdata/enc-too-new.conf" + err = config.Data.Load() require.Error(t, err) // This file does not exist. - ConfigPath = "./testdata/filenotfound.conf" - err = Data.Load() - assert.Equal(t, ErrorConfigFileNotFound, err) -} - -func TestPassword(t *testing.T) { - defer func() { - configKey = nil // reset password - }() - var err error - // Empty password should give error - err = setConfigPassword(" \t ") - require.Error(t, err) - - // Test invalid utf8 sequence - err = setConfigPassword(string([]byte{0xff, 0xfe, 0xfd}) + "abc") - require.Error(t, err) - - // Simple check of wrong passwords - hashedKeyCompare(t, "mis", "match", false) - - // Check that passwords match after unicode normalization - hashedKeyCompare(t, "ff\u0041\u030A", "ffÅ", true) - - // Check that passwords preserves case - hashedKeyCompare(t, "abcdef", "ABCDEF", false) - -} - -func hashedKeyCompare(t *testing.T, a, b string, shouldMatch bool) { - err := setConfigPassword(a) - require.NoError(t, err) - k1 := configKey - - err = setConfigPassword(b) - require.NoError(t, err) - k2 := configKey - - if shouldMatch { - assert.Equal(t, k1, k2) - } else { - assert.NotEqual(t, k1, k2) - } -} - -func TestMatchProvider(t *testing.T) { - for _, test := range []struct { - config string - provider string - want bool - }{ - {"", "", true}, - {"one", "one", true}, - {"one,two", "two", true}, - {"one,two,three", "two", true}, - {"one", "on", false}, - {"one,two,three", "tw", false}, - {"!one,two,three", "two", false}, - {"!one,two,three", "four", true}, - } { - what := fmt.Sprintf("%q,%q", test.config, test.provider) - got := matchProvider(test.config, test.provider) - assert.Equal(t, test.want, got, what) - } + config.ConfigPath = "./testdata/filenotfound.conf" + err = config.Data.Load() + assert.Equal(t, config.ErrorConfigFileNotFound, err) } func TestFileRefresh(t *testing.T) { ctx := context.Background() defer testConfigFile(t, "refresh.conf")() - require.NoError(t, CreateRemote(ctx, "refresh_test", "config_test_remote", rc.Params{ + require.NoError(t, config.CreateRemote(ctx, "refresh_test", "config_test_remote", rc.Params{ "bool": true, }, false, false)) - b, err := ioutil.ReadFile(ConfigPath) + b, err := ioutil.ReadFile(config.ConfigPath) assert.NoError(t, err) b = bytes.Replace(b, []byte("refresh_test"), []byte("refreshed_test"), 1) - err = ioutil.WriteFile(ConfigPath, b, 0644) + err = ioutil.WriteFile(config.ConfigPath, b, 0644) assert.NoError(t, err) - assert.NotEqual(t, []string{"refreshed_test"}, Data.GetSectionList()) - err = FileRefresh() + assert.NotEqual(t, []string{"refreshed_test"}, config.Data.GetSectionList()) + err = config.FileRefresh() assert.NoError(t, err) - assert.Equal(t, []string{"refreshed_test"}, Data.GetSectionList()) + assert.Equal(t, []string{"refreshed_test"}, config.Data.GetSectionList()) } diff --git a/fs/config/configfile/configfile.go b/fs/config/configfile/configfile.go index 2cf03dc2c..8a87ecb0d 100644 --- a/fs/config/configfile/configfile.go +++ b/fs/config/configfile/configfile.go @@ -2,6 +2,7 @@ package configfile import ( "bytes" + "context" "io/ioutil" "os" "path/filepath" @@ -12,14 +13,27 @@ import ( "github.com/rclone/rclone/fs/config" ) -// GoConfig implements config file saving using a simple ini based -// format. -type GoConfig struct { - *goconfig.ConfigFile +// LoadConfig installs the config file handler and calls config.LoadConfig +func LoadConfig(ctx context.Context) { + config.Data = &Storage{} + config.LoadConfig(ctx) } -// Load the config from permanent storage -func (gc *GoConfig) Load() error { +// Storage implements config.Storage for saving and loading config +// data in a simple INI based file. +type Storage struct { + gc *goconfig.ConfigFile +} + +// Load the config from permanent storage, decrypting if necessary +func (s *Storage) Load() (err error) { + // Make sure we have a sensible default even when we error + defer func() { + if err != nil { + s.gc, _ = goconfig.LoadFromReader(bytes.NewReader([]byte{})) + } + }() + b, err := os.Open(config.ConfigPath) if err != nil { if os.IsNotExist(err) { @@ -34,22 +48,17 @@ func (gc *GoConfig) Load() error { return err } - if gc.ConfigFile == nil { - c, err := goconfig.LoadFromReader(cryptReader) - if err != nil { - return err - } - - gc.ConfigFile = c - } else { - return gc.ReloadData(cryptReader) + gc, err := goconfig.LoadFromReader(cryptReader) + if err != nil { + return err } + s.gc = gc return nil } -// Save the config to permanent storage -func (gc *GoConfig) Save() error { +// Save the config to permanent storage, encrypting if necessary +func (s *Storage) Save() error { dir, name := filepath.Split(config.ConfigPath) err := os.MkdirAll(dir, os.ModePerm) if err != nil { @@ -67,7 +76,7 @@ func (gc *GoConfig) Save() error { }() var buf bytes.Buffer - if err := goconfig.SaveConfigData(gc.ConfigFile, &buf); err != nil { + if err := goconfig.SaveConfigData(s.gc, &buf); err != nil { return errors.Errorf("Failed to save config file: %v", err) } @@ -111,9 +120,9 @@ func (gc *GoConfig) Save() error { } // Serialize the config into a string -func (gc *GoConfig) Serialize() (string, error) { +func (s *Storage) Serialize() (string, error) { var buf bytes.Buffer - if err := goconfig.SaveConfigData(gc.ConfigFile, &buf); err != nil { + if err := goconfig.SaveConfigData(s.gc, &buf); err != nil { return "", errors.Errorf("Failed to save config file: %v", err) } @@ -121,8 +130,8 @@ func (gc *GoConfig) Serialize() (string, error) { } // HasSection returns true if section exists in the config file -func (gc *GoConfig) HasSection(section string) bool { - _, err := gc.ConfigFile.GetSection(section) +func (s *Storage) HasSection(section string) bool { + _, err := s.gc.GetSection(section) if err != nil { return false } @@ -131,16 +140,39 @@ func (gc *GoConfig) HasSection(section string) bool { // DeleteSection removes the named section and all config from the // config file -func (gc *GoConfig) DeleteSection(section string) { - gc.ConfigFile.DeleteSection(section) +func (s *Storage) DeleteSection(section string) { + s.gc.DeleteSection(section) +} + +// GetSectionList returns a slice of strings with names for all the +// sections +func (s *Storage) GetSectionList() []string { + return s.gc.GetSectionList() +} + +// GetKeyList returns the keys in this section +func (s *Storage) GetKeyList(section string) []string { + return s.gc.GetKeyList(section) +} + +// GetValue returns the key in section with a found flag +func (s *Storage) GetValue(section string, key string) (value string, found bool) { + value, err := s.gc.GetValue(section, key) + if err != nil { + return "", false + } + return value, true } // SetValue sets the value under key in section -func (gc *GoConfig) SetValue(section string, key string, value string) { - gc.ConfigFile.SetValue(section, key, value) +func (s *Storage) SetValue(section string, key string, value string) { + s.gc.SetValue(section, key, value) } -// Check the interfaces are satisfied -var ( - _ config.File = (*GoConfig)(nil) -) +// DeleteKey removes the key under section +func (s *Storage) DeleteKey(section string, key string) bool { + return s.gc.DeleteKey(section, key) +} + +// Check the interface is satisfied +var _ config.Storage = (*Storage)(nil) diff --git a/fs/config/crypt.go b/fs/config/crypt.go index 3dabe70e9..1cf6cb1f8 100644 --- a/fs/config/crypt.go +++ b/fs/config/crypt.go @@ -101,7 +101,7 @@ func Decrypt(b io.ReadSeeker) (io.Reader, error) { return nil, errors.Wrap(err, "password command failed") } if pass := strings.Trim(stdout.String(), "\r\n"); pass != "" { - err := setConfigPassword(pass) + err := SetConfigPassword(pass) if err != nil { return nil, errors.Wrap(err, "incorrect password") } @@ -119,7 +119,7 @@ func Decrypt(b io.ReadSeeker) (io.Reader, error) { envpw := os.Getenv("RCLONE_CONFIG_PASS") if envpw != "" { - err := setConfigPassword(envpw) + err := SetConfigPassword(envpw) if err != nil { fs.Errorf(nil, "Using RCLONE_CONFIG_PASS returned: %v", err) } else { @@ -281,7 +281,7 @@ func getConfigPassword(q string) { } for { password := GetPassword(q) - err := setConfigPassword(password) + err := SetConfigPassword(password) if err == nil { return } @@ -289,10 +289,10 @@ func getConfigPassword(q string) { } } -// setConfigPassword will set the configKey to the hash of +// SetConfigPassword will set the configKey to the hash of // the password. If the length of the password is // zero after trimming+normalization, an error is returned. -func setConfigPassword(password string) error { +func SetConfigPassword(password string) error { password, err := checkPassword(password) if err != nil { return err @@ -338,11 +338,16 @@ func setConfigPassword(password string) error { return nil } +// ClearConfigPassword sets the current the password to empty +func ClearConfigPassword() { + configKey = nil +} + // changeConfigPassword will query the user twice // for a password. If the same password is entered // twice the key is updated. func changeConfigPassword() { - err := setConfigPassword(ChangePassword("NEW configuration")) + err := SetConfigPassword(ChangePassword("NEW configuration")) if err != nil { fmt.Printf("Failed to set config password: %v\n", err) return diff --git a/fs/config/crypt_test.go b/fs/config/crypt_test.go new file mode 100644 index 000000000..61c0ab5c8 --- /dev/null +++ b/fs/config/crypt_test.go @@ -0,0 +1,48 @@ +package config + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestPassword(t *testing.T) { + defer func() { + configKey = nil // reset password + }() + var err error + // Empty password should give error + err = SetConfigPassword(" \t ") + require.Error(t, err) + + // Test invalid utf8 sequence + err = SetConfigPassword(string([]byte{0xff, 0xfe, 0xfd}) + "abc") + require.Error(t, err) + + // Simple check of wrong passwords + hashedKeyCompare(t, "mis", "match", false) + + // Check that passwords match after unicode normalization + hashedKeyCompare(t, "ff\u0041\u030A", "ffÅ", true) + + // Check that passwords preserves case + hashedKeyCompare(t, "abcdef", "ABCDEF", false) + +} + +func hashedKeyCompare(t *testing.T, a, b string, shouldMatch bool) { + err := SetConfigPassword(a) + require.NoError(t, err) + k1 := configKey + + err = SetConfigPassword(b) + require.NoError(t, err) + k2 := configKey + + if shouldMatch { + assert.Equal(t, k1, k2) + } else { + assert.NotEqual(t, k1, k2) + } +} diff --git a/fs/config/default_storage.go b/fs/config/default_storage.go new file mode 100644 index 000000000..9998dd76a --- /dev/null +++ b/fs/config/default_storage.go @@ -0,0 +1,61 @@ +package config + +// Default config.Storage which panics with a useful error when used +type defaultStorage struct{} + +var noConfigStorage = "internal error: no config file system found. Did you call configfile.LoadConfig(ctx)?" + +// GetSectionList returns a slice of strings with names for all the +// sections +func (defaultStorage) GetSectionList() []string { + panic(noConfigStorage) +} + +// HasSection returns true if section exists in the config file +func (defaultStorage) HasSection(section string) bool { + panic(noConfigStorage) +} + +// DeleteSection removes the named section and all config from the +// config file +func (defaultStorage) DeleteSection(section string) { + panic(noConfigStorage) +} + +// GetKeyList returns the keys in this section +func (defaultStorage) GetKeyList(section string) []string { + panic(noConfigStorage) +} + +// GetValue returns the key in section with a found flag +func (defaultStorage) GetValue(section string, key string) (value string, found bool) { + panic(noConfigStorage) +} + +// SetValue sets the value under key in section +func (defaultStorage) SetValue(section string, key string, value string) { + panic(noConfigStorage) +} + +// DeleteKey removes the key under section +func (defaultStorage) DeleteKey(section string, key string) bool { + panic(noConfigStorage) +} + +// Load the config from permanent storage +func (defaultStorage) Load() error { + panic(noConfigStorage) +} + +// Save the config to permanent storage +func (defaultStorage) Save() error { + panic(noConfigStorage) +} + +// Serialize the config into a string +func (defaultStorage) Serialize() (string, error) { + panic(noConfigStorage) +} + +// Check the interface is satisfied +var _ Storage = defaultStorage{} diff --git a/fs/config/rc_test.go b/fs/config/rc_test.go index 5cc8f9b2b..89d3bb9e9 100644 --- a/fs/config/rc_test.go +++ b/fs/config/rc_test.go @@ -7,6 +7,7 @@ import ( _ "github.com/rclone/rclone/backend/local" "github.com/rclone/rclone/fs" "github.com/rclone/rclone/fs/config" + "github.com/rclone/rclone/fs/config/configfile" "github.com/rclone/rclone/fs/config/obscure" "github.com/rclone/rclone/fs/rc" "github.com/stretchr/testify/assert" @@ -16,6 +17,8 @@ import ( const testName = "configTestNameForRc" func TestRc(t *testing.T) { + ctx := context.Background() + configfile.LoadConfig(ctx) // Create the test remote call := rc.Calls.Get("config/create") assert.NotNil(t, call) @@ -26,7 +29,7 @@ func TestRc(t *testing.T) { "test_key": "sausage", }, } - out, err := call.Fn(context.Background(), in) + out, err := call.Fn(ctx, in) require.NoError(t, err) require.Nil(t, out) assert.Equal(t, "local", config.FileGet(testName, "type")) diff --git a/fs/rc/rcserver/rcserver_test.go b/fs/rc/rcserver/rcserver_test.go index 9ada37a92..b17f3dd0b 100644 --- a/fs/rc/rcserver/rcserver_test.go +++ b/fs/rc/rcserver/rcserver_test.go @@ -20,6 +20,7 @@ import ( _ "github.com/rclone/rclone/backend/local" "github.com/rclone/rclone/fs" "github.com/rclone/rclone/fs/accounting" + "github.com/rclone/rclone/fs/config/configfile" "github.com/rclone/rclone/fs/rc" ) @@ -101,9 +102,11 @@ type testRun struct { // Run a suite of tests func testServer(t *testing.T, tests []testRun, opt *rc.Options) { + ctx := context.Background() + configfile.LoadConfig(ctx) mux := http.NewServeMux() opt.HTTPOptions.Template = testTemplate - rcServer := newServer(context.Background(), opt, mux) + rcServer := newServer(ctx, opt, mux) for _, test := range tests { t.Run(test.Name, func(t *testing.T) { method := test.Method diff --git a/fstest/fstest.go b/fstest/fstest.go index b996d2259..b894eb004 100644 --- a/fstest/fstest.go +++ b/fstest/fstest.go @@ -25,6 +25,7 @@ import ( "github.com/rclone/rclone/fs" "github.com/rclone/rclone/fs/accounting" "github.com/rclone/rclone/fs/config" + "github.com/rclone/rclone/fs/config/configfile" "github.com/rclone/rclone/fs/hash" "github.com/rclone/rclone/fs/walk" "github.com/rclone/rclone/lib/random" @@ -70,7 +71,7 @@ func Initialise() { if envConfig := os.Getenv("RCLONE_CONFIG"); envConfig != "" { config.ConfigPath = envConfig } - config.LoadConfig(ctx) + configfile.LoadConfig(ctx) if *Verbose { ci.LogLevel = fs.LogLevelDebug }