diff --git a/cmd/config/config.go b/cmd/config/config.go index f6c0c6234..a209a6a0d 100644 --- a/cmd/config/config.go +++ b/cmd/config/config.go @@ -98,6 +98,9 @@ Note that if the config process would normally ask a question the default is taken. Each time that happens rclone will print a message saying how to affect the value taken. +If any of the parameters passed is a password field, then rclone will +automatically obscure them before putting them in the config file. + So for example if you wanted to configure a Google Drive remote but using remote authorization you would do this: @@ -125,10 +128,14 @@ var configUpdateCommand = &cobra.Command{ Update an existing remote's options. The options should be passed in in pairs of . -For example to update the env_auth field of a remote of name myremote you would do: +For example to update the env_auth field of a remote of name myremote +you would do: rclone config update myremote swift env_auth true +If any of the parameters passed is a password field, then rclone will +automatically obscure them before putting them in the config file. + If the remote uses oauth the token will be updated, if you don't require this add an extra parameter thus: @@ -168,6 +175,9 @@ should be passed in in pairs of . For example to set password of a remote of name myremote you would do: rclone config password myremote fieldname mypassword + +This command is obsolete now that "config update" and "config create" +both support obscuring passwords directly. `, RunE: func(command *cobra.Command, args []string) error { cmd.CheckArgs(3, 256, command, args) diff --git a/fs/config/config.go b/fs/config/config.go index 71b91160f..c48a2f885 100644 --- a/fs/config/config.go +++ b/fs/config/config.go @@ -935,9 +935,38 @@ func suppressConfirm() func() { // keyValues should be key, value pairs. func UpdateRemote(name string, keyValues rc.Params) error { defer suppressConfirm()() + + // Work out which options need to be obscured + needsObscure := map[string]struct{}{} + if fsType := FileGet(name, "type"); fsType != "" { + if ri, err := fs.Find(fsType); err != nil { + fs.Debugf(nil, "Couldn't find fs for type %q", fsType) + } else { + for _, opt := range ri.Options { + if opt.IsPassword { + needsObscure[opt.Name] = struct{}{} + } + } + } + } else { + fs.Debugf(nil, "UpdateRemote: Couldn't find fs type") + } + // Set the config for k, v := range keyValues { - getConfigData().SetValue(name, k, fmt.Sprint(v)) + vStr := fmt.Sprint(v) + // Obscure parameter if necessary + if _, ok := needsObscure[k]; ok { + _, err := obscure.Reveal(vStr) + if err != nil { + // If error => not already obscured, so obscure it + vStr, err = obscure.Obscure(vStr) + if err != nil { + return errors.Wrap(err, "UpdateRemote: obscure failed") + } + } + } + getConfigData().SetValue(name, k, vStr) } RemoteConfig(name) SaveConfig() diff --git a/fs/config/config_test.go b/fs/config/config_test.go index 902a04c17..efd654bba 100644 --- a/fs/config/config_test.go +++ b/fs/config/config_test.go @@ -8,20 +8,17 @@ import ( "github.com/ncw/rclone/fs" "github.com/ncw/rclone/fs/config/obscure" + "github.com/ncw/rclone/fs/rc" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func TestCRUD(t *testing.T) { +func testConfigFile(t *testing.T, configFileName string) func() { configKey = nil // reset password // create temp config file - tempFile, err := ioutil.TempFile("", "crud.conf") + tempFile, err := ioutil.TempFile("", configFileName) assert.NoError(t, err) path := tempFile.Name() - defer func() { - err := os.Remove(path) - assert.NoError(t, err) - }() assert.NoError(t, tempFile.Close()) // temporarily adapt configuration @@ -34,25 +31,52 @@ func TestCRUD(t *testing.T) { ConfigPath = path fs.Config = &fs.ConfigInfo{} configFile = nil - defer func() { - os.Stdout = oldOsStdout - ConfigPath = oldConfigPath - ReadLine = oldReadLine - fs.Config = oldConfig - configFile = oldConfigFile - }() LoadConfig() assert.Equal(t, []string{}, getConfigData().GetSectionList()) // Fake a remote - fs.Register(&fs.RegInfo{Name: "config_test_remote"}) + fs.Register(&fs.RegInfo{ + Name: "config_test_remote", + Options: fs.Options{ + { + Name: "bool", + Default: false, + IsPassword: false, + }, + { + Name: "pass", + Default: "", + IsPassword: true, + }, + }, + }) - // add new remote + // Undo the above + return func() { + err := os.Remove(path) + assert.NoError(t, err) + + os.Stdout = oldOsStdout + ConfigPath = oldConfigPath + ReadLine = oldReadLine + fs.Config = oldConfig + configFile = oldConfigFile + } +} + +func TestCRUD(t *testing.T) { + defer testConfigFile(t, "crud.conf")() + + // expect script for creating remote i := 0 ReadLine = func() string { answers := []string{ "config_test_remote", // type + "true", // bool value + "y", // type my own password + "secret", // password + "secret", // repeat "y", // looks good, save } i = i + 1 @@ -60,27 +84,68 @@ func TestCRUD(t *testing.T) { } NewRemote("test") - assert.Equal(t, []string{"test"}, configFile.GetSectionList()) - // Reload the config file to workaround this bug - // https://github.com/Unknwon/goconfig/issues/39 - configFile, err = loadConfigFile() - require.NoError(t, err) + assert.Equal(t, []string{"test"}, configFile.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"))) // normal rename, test → asdf ReadLine = func() string { return "asdf" } RenameRemote("test") + assert.Equal(t, []string{"asdf"}, configFile.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"))) // no-op rename, asdf → asdf RenameRemote("asdf") + assert.Equal(t, []string{"asdf"}, configFile.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"))) // delete remote DeleteRemote("asdf") assert.Equal(t, []string{}, configFile.GetSectionList()) } +func TestCreateUpatePasswordRemote(t *testing.T) { + defer testConfigFile(t, "update.conf")() + + require.NoError(t, CreateRemote("test2", "config_test_remote", rc.Params{ + "bool": true, + "pass": "potato", + })) + + assert.Equal(t, []string{"test2"}, configFile.GetSectionList()) + assert.Equal(t, "config_test_remote", FileGet("test2", "type")) + assert.Equal(t, "true", FileGet("test2", "bool")) + assert.Equal(t, "potato", obscure.MustReveal(FileGet("test2", "pass"))) + + require.NoError(t, UpdateRemote("test2", rc.Params{ + "bool": false, + "pass": obscure.MustObscure("potato2"), + "spare": "spare", + })) + + assert.Equal(t, []string{"test2"}, configFile.GetSectionList()) + assert.Equal(t, "config_test_remote", FileGet("test2", "type")) + assert.Equal(t, "false", FileGet("test2", "bool")) + assert.Equal(t, "potato2", obscure.MustReveal(FileGet("test2", "pass"))) + + require.NoError(t, PasswordRemote("test2", rc.Params{ + "pass": "potato3", + })) + + assert.Equal(t, []string{"test2"}, configFile.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"))) +} + // Test some error cases func TestReveal(t *testing.T) { for _, test := range []struct { diff --git a/fs/config/rc.go b/fs/config/rc.go index 99856e627..68e3b2b4c 100644 --- a/fs/config/rc.go +++ b/fs/config/rc.go @@ -118,7 +118,6 @@ func init() { Help: `This takes the following parameters - name - name of remote -- type - type of new remote ` + extraHelp + ` See the [config ` + name + ` command](/commands/rclone_config_` + name + `/) command for more information on the above.`,