config: make config create/update encrypt passwords where necessary

Before this change when using "rclone config create" it wasn't
possible to add passwords in one go, it was necessary to call "rclone
config password" to add the passwords afterwards as "rclone config
create" didn't obscure passwords.

After this change "rclone config create" and "rclone config update"
will obscure passwords as necessary as will the corresponding API
calls config/create and config/update.

This makes "rclone config password" and its API config/password
obsolete, however they will be left for backwards compatibility.

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 <key> <value>.
 
-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 <key> <value>.
 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)

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()

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 {

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.`,