config: add --obscure and --no-obscure flags to config create/update

Before this change there was some ambiguity about whether passwords
were obscured on not passing them into config create or config update.

This change adds the --obscure and --no-obscure flags to make the
intent clear.

It also updates the remote control and the tests.

Fixes #3728

cmd/config/config.go

@@ -14,6 +14,7 @@ import (
 	"github.com/rclone/rclone/fs/config/flags"
 	"github.com/rclone/rclone/fs/rc"
 	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
 )
 
 func init() {
@@ -92,6 +93,26 @@ var configProvidersCommand = &cobra.Command{
 	},
 }
 
+var (
+	configObscure   bool
+	configNoObscure bool
+)
+
+const configPasswordHelp = `
+If any of the parameters passed is a password field, then rclone will
+automatically obscure them if they aren't already obscured before
+putting them in the config file.
+
+**NB** If the password parameter is 22 characters or longer and
+consists only of base64 characters then rclone can get confused about
+whether the password is already obscured or not and put unobscured
+passwords into the config file. If you want to be 100% certain that
+the passwords get obscured then use the "--obscure" flag, or if you
+are 100% certain you are already passing obscured passwords then use
+"--no-obscure".  You can also set osbscured passwords using the
+"rclone config password" command.
+`
+
 var configCreateCommand = &cobra.Command{
 	Use:   "create <name> <type> [<key> <value>]*",
 	Short: `Create a new remote with name, type and options.`,
@@ -107,10 +128,7 @@ you would do:
 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.
-
+` + configPasswordHelp + `
-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:
 
@@ -122,7 +140,7 @@ using remote authorization you would do this:
 		if err != nil {
 			return err
 		}
-		err = config.CreateRemote(args[0], args[1], in)
+		err = config.CreateRemote(args[0], args[1], in, configObscure, configNoObscure)
 		if err != nil {
 			return err
 		}
@@ -131,6 +149,13 @@ using remote authorization you would do this:
 	},
 }
 
+func init() {
+	for _, cmdFlags := range []*pflag.FlagSet{configCreateCommand.Flags(), configUpdateCommand.Flags()} {
+		flags.BoolVarP(cmdFlags, &configObscure, "obscure", "", false, "Force any passwords to be obscured.")
+		flags.BoolVarP(cmdFlags, &configNoObscure, "no-obscure", "", false, "Force any passwords not to be obscured.")
+	}
+}
+
 var configUpdateCommand = &cobra.Command{
 	Use:   "update <name> [<key> <value>]+",
 	Short: `Update options in an existing remote.`,
@@ -142,10 +167,7 @@ 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
-
+` + configPasswordHelp + `
-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:
 
@@ -157,7 +179,7 @@ require this add an extra parameter thus:
 		if err != nil {
 			return err
 		}
-		err = config.UpdateRemote(args[0], in)
+		err = config.UpdateRemote(args[0], in, configObscure, configNoObscure)
 		if err != nil {
 			return err
 		}

fs/config/config.go

@@ -1030,7 +1030,10 @@ func suppressConfirm() func() {
 
 // UpdateRemote adds the keyValues passed in to the remote of name.
 // keyValues should be key, value pairs.
-func UpdateRemote(name string, keyValues rc.Params) error {
+func UpdateRemote(name string, keyValues rc.Params, doObscure, noObscure bool) error {
+	if doObscure && noObscure {
+		return errors.New("can't use --obscure and --no-obscure together")
+	}
 	err := fspath.CheckConfigName(name)
 	if err != nil {
 		return err
@@ -1039,18 +1042,20 @@ func UpdateRemote(name string, keyValues rc.Params) error {
 
 	// Work out which options need to be obscured
 	needsObscure := map[string]struct{}{}
-	if fsType := FileGet(name, "type"); fsType != "" {
+	if !noObscure {
-		if ri, err := fs.Find(fsType); err != nil {
+		if fsType := FileGet(name, "type"); fsType != "" {
-			fs.Debugf(nil, "Couldn't find fs for type %q", fsType)
+			if ri, err := fs.Find(fsType); err != nil {
-		} else {
+				fs.Debugf(nil, "Couldn't find fs for type %q", fsType)
-			for _, opt := range ri.Options {
+			} else {
-				if opt.IsPassword {
+				for _, opt := range ri.Options {
-					needsObscure[opt.Name] = struct{}{}
+					if opt.IsPassword {
+						needsObscure[opt.Name] = struct{}{}
+					}
 				}
 			}
+		} else {
+			fs.Debugf(nil, "UpdateRemote: Couldn't find fs type")
 		}
-	} else {
-		fs.Debugf(nil, "UpdateRemote: Couldn't find fs type")
 	}
 
 	// Set the config
@@ -1059,8 +1064,9 @@ func UpdateRemote(name string, keyValues rc.Params) error {
 		// Obscure parameter if necessary
 		if _, ok := needsObscure[k]; ok {
 			_, err := obscure.Reveal(vStr)
-			if err != nil {
+			if err != nil || doObscure {
 				// If error => not already obscured, so obscure it
+				// or we are forced to obscure
 				vStr, err = obscure.Obscure(vStr)
 				if err != nil {
 					return errors.Wrap(err, "UpdateRemote: obscure failed")
@@ -1077,7 +1083,7 @@ func UpdateRemote(name string, keyValues rc.Params) error {
 // CreateRemote creates a new remote with name, provider and a list of
 // parameters which are key, value pairs.  If update is set then it
 // adds the new keys rather than replacing all of them.
-func CreateRemote(name string, provider string, keyValues rc.Params) error {
+func CreateRemote(name string, provider string, keyValues rc.Params, doObscure, noObscure bool) error {
 	err := fspath.CheckConfigName(name)
 	if err != nil {
 		return err
@@ -1087,7 +1093,7 @@ func CreateRemote(name string, provider string, keyValues rc.Params) error {
 	// Set the type
 	getConfigData().SetValue(name, "type", provider)
 	// Set the remaining values
-	return UpdateRemote(name, keyValues)
+	return UpdateRemote(name, keyValues, doObscure, noObscure)
 }
 
 // PasswordRemote adds the keyValues passed in to the remote of name.
@@ -1101,7 +1107,7 @@ func PasswordRemote(name string, keyValues rc.Params) error {
 	for k, v := range keyValues {
 		keyValues[k] = obscure.MustObscure(fmt.Sprint(v))
 	}
-	return UpdateRemote(name, keyValues)
+	return UpdateRemote(name, keyValues, false, true)
 }
 
 // JSONListProviders prints all the providers and options in JSON format

fs/config/config_test.go

@@ -180,35 +180,54 @@ func TestNewRemoteName(t *testing.T) {
 func TestCreateUpatePasswordRemote(t *testing.T) {
 	defer testConfigFile(t, "update.conf")()
 
-	require.NoError(t, CreateRemote("test2", "config_test_remote", rc.Params{
+	for _, doObscure := range []bool{false, true} {
-		"bool": true,
+		for _, noObscure := range []bool{false, true} {
-		"pass": "potato",
+			if doObscure && noObscure {
-	}))
+				break
+			}
+			t.Run(fmt.Sprintf("doObscure=%v,noObscure=%v", doObscure, noObscure), func(t *testing.T) {
+				require.NoError(t, CreateRemote("test2", "config_test_remote", rc.Params{
+					"bool": true,
+					"pass": "potato",
+				}, doObscure, noObscure))
 
-	assert.Equal(t, []string{"test2"}, configFile.GetSectionList())
+				assert.Equal(t, []string{"test2"}, configFile.GetSectionList())
-	assert.Equal(t, "config_test_remote", FileGet("test2", "type"))
+				assert.Equal(t, "config_test_remote", FileGet("test2", "type"))
-	assert.Equal(t, "true", FileGet("test2", "bool"))
+				assert.Equal(t, "true", FileGet("test2", "bool"))
-	assert.Equal(t, "potato", obscure.MustReveal(FileGet("test2", "pass")))
+				gotPw := FileGet("test2", "pass")
+				if !noObscure {
+					gotPw = obscure.MustReveal(gotPw)
+				}
+				assert.Equal(t, "potato", gotPw)
 
-	require.NoError(t, UpdateRemote("test2", rc.Params{
+				wantPw := obscure.MustObscure("potato2")
-		"bool":  false,
+				require.NoError(t, UpdateRemote("test2", rc.Params{
-		"pass":  obscure.MustObscure("potato2"),
+					"bool":  false,
-		"spare": "spare",
+					"pass":  wantPw,
-	}))
+					"spare": "spare",
+				}, doObscure, noObscure))
 
-	assert.Equal(t, []string{"test2"}, configFile.GetSectionList())
+				assert.Equal(t, []string{"test2"}, configFile.GetSectionList())
-	assert.Equal(t, "config_test_remote", FileGet("test2", "type"))
+				assert.Equal(t, "config_test_remote", FileGet("test2", "type"))
-	assert.Equal(t, "false", FileGet("test2", "bool"))
+				assert.Equal(t, "false", FileGet("test2", "bool"))
-	assert.Equal(t, "potato2", obscure.MustReveal(FileGet("test2", "pass")))
+				gotPw = FileGet("test2", "pass")
+				if doObscure {
+					gotPw = obscure.MustReveal(gotPw)
+				}
+				assert.Equal(t, wantPw, gotPw)
 
-	require.NoError(t, PasswordRemote("test2", rc.Params{
+				require.NoError(t, PasswordRemote("test2", rc.Params{
-		"pass": "potato3",
+					"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")))
+			})
+		}
+	}
 
-	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
@@ -409,7 +428,7 @@ func TestFileRefresh(t *testing.T) {
 	defer testConfigFile(t, "refresh.conf")()
 	require.NoError(t, CreateRemote("refresh_test", "config_test_remote", rc.Params{
 		"bool": true,
-	}))
+	}, false, false))
 	b, err := ioutil.ReadFile(ConfigPath)
 	assert.NoError(t, err)
 

fs/config/rc.go

@@ -111,6 +111,10 @@ func init() {
 		if name == "create" {
 			extraHelp = "- type - type of the new remote\n"
 		}
+		if name == "create" || name == "update" {
+			extraHelp += "- obscure - optional bool - forces obscuring of passwords\n"
+			extraHelp += "- noObscure - optional bool - forces passwords not to be obscured\n"
+		}
 		rc.Add(rc.Call{
 			Path:         "config/" + name,
 			AuthRequired: true,
@@ -140,15 +144,17 @@ func rcConfig(ctx context.Context, in rc.Params, what string) (out rc.Params, er
 	if err != nil {
 		return nil, err
 	}
+	doObscure, _ := in.GetBool("obscure")
+	noObscure, _ := in.GetBool("noObscure")
 	switch what {
 	case "create":
 		remoteType, err := in.GetString("type")
 		if err != nil {
 			return nil, err
 		}
-		return nil, CreateRemote(name, remoteType, parameters)
+		return nil, CreateRemote(name, remoteType, parameters, doObscure, noObscure)
 	case "update":
-		return nil, UpdateRemote(name, parameters)
+		return nil, UpdateRemote(name, parameters, doObscure, noObscure)
 	case "password":
 		return nil, PasswordRemote(name, parameters)
 	}

fs/config/rc_test.go

@@ -101,11 +101,12 @@ func TestRc(t *testing.T) {
 	t.Run("Password", func(t *testing.T) {
 		call := rc.Calls.Get("config/password")
 		assert.NotNil(t, call)
+		pw2 := obscure.MustObscure("password")
 		in := rc.Params{
 			"name": testName,
 			"parameters": rc.Params{
 				"test_key":  "rutabaga",
-				"test_key2": "cabbage",
+				"test_key2": pw2, // check we encode an already encoded password
 			},
 		}
 		out, err := call.Fn(context.Background(), in)
@@ -114,7 +115,7 @@ func TestRc(t *testing.T) {
 
 		assert.Equal(t, "local", config.FileGet(testName, "type"))
 		assert.Equal(t, "rutabaga", obscure.MustReveal(config.FileGet(testName, "test_key")))
-		assert.Equal(t, "cabbage", obscure.MustReveal(config.FileGet(testName, "test_key2")))
+		assert.Equal(t, pw2, obscure.MustReveal(config.FileGet(testName, "test_key2")))
 	})
 
 	// Delete the test remote