[#1083] neofs-cli: use single flag for key and wallet

cmd/neofs-cli/modules/accounting.go

@@ -23,9 +23,7 @@ var accountingCmd = &cobra.Command{
 	PersistentPreRun: func(cmd *cobra.Command, args []string) {
 		flags := cmd.Flags()
 
-		_ = viper.BindPFlag(binaryKey, flags.Lookup(binaryKey))
 		_ = viper.BindPFlag(walletPath, flags.Lookup(walletPath))
-		_ = viper.BindPFlag(wif, flags.Lookup(wif))
 		_ = viper.BindPFlag(address, flags.Lookup(address))
 		_ = viper.BindPFlag(rpc, flags.Lookup(rpc))
 		_ = viper.BindPFlag(verbose, flags.Lookup(verbose))
@@ -68,9 +66,7 @@ var accountingBalanceCmd = &cobra.Command{
 func initAccountingBalanceCmd() {
 	ff := accountingBalanceCmd.Flags()
 
-	ff.StringP(binaryKey, binaryKeyShorthand, binaryKeyDefault, binaryKeyUsage)
 	ff.StringP(walletPath, walletPathShorthand, walletPathDefault, walletPathUsage)
-	ff.StringP(wif, wifShorthand, wifDefault, wifUsage)
 	ff.StringP(address, addressShorthand, addressDefault, addressUsage)
 	ff.StringP(rpc, rpcShorthand, rpcDefault, rpcUsage)
 	ff.BoolP(verbose, verboseShorthand, verboseDefault, verboseUsage)

cmd/neofs-cli/modules/control.go

@@ -25,9 +25,7 @@ var controlCmd = &cobra.Command{
 		ff := cmd.Flags()
 
 		_ = viper.BindPFlag(generateKey, ff.Lookup(generateKey))
-		_ = viper.BindPFlag(binaryKey, ff.Lookup(binaryKey))
 		_ = viper.BindPFlag(walletPath, ff.Lookup(walletPath))
-		_ = viper.BindPFlag(wif, ff.Lookup(wif))
 		_ = viper.BindPFlag(address, ff.Lookup(address))
 		_ = viper.BindPFlag(controlRPC, ff.Lookup(controlRPC))
 		_ = viper.BindPFlag(verbose, ff.Lookup(verbose))

cmd/neofs-cli/modules/key_test.go

@@ -0,0 +1,123 @@
+package cmd
+
+import (
+	"bytes"
+	"errors"
+	"io/ioutil"
+	"os"
+	"path"
+	"path/filepath"
+	"testing"
+
+	"github.com/nspcc-dev/neo-go/cli/input"
+	"github.com/nspcc-dev/neo-go/pkg/crypto/keys"
+	"github.com/nspcc-dev/neo-go/pkg/wallet"
+	"github.com/spf13/viper"
+	"github.com/stretchr/testify/require"
+	"golang.org/x/term"
+)
+
+func Test_getKey(t *testing.T) {
+	dir := t.TempDir()
+
+	wallPath := path.Join(dir, "wallet.json")
+	w, err := wallet.NewWallet(wallPath)
+	require.NoError(t, err)
+
+	acc1, err := wallet.NewAccount()
+	require.NoError(t, err)
+	require.NoError(t, acc1.Encrypt("pass", keys.NEP2ScryptParams()))
+	w.AddAccount(acc1)
+
+	acc2, err := wallet.NewAccount()
+	require.NoError(t, err)
+	require.NoError(t, acc2.Encrypt("pass", keys.NEP2ScryptParams()))
+	acc2.Default = true
+	w.AddAccount(acc2)
+	require.NoError(t, w.Save())
+	w.Close()
+
+	keyPath := path.Join(dir, "binary.key")
+	rawKey, err := keys.NewPrivateKey()
+	require.NoError(t, err)
+	require.NoError(t, ioutil.WriteFile(keyPath, rawKey.Bytes(), os.ModePerm))
+
+	wifKey, err := keys.NewPrivateKey()
+	require.NoError(t, err)
+
+	nep2Key, err := keys.NewPrivateKey()
+	require.NoError(t, err)
+	nep2, err := keys.NEP2Encrypt(nep2Key, "pass", keys.NEP2ScryptParams())
+	require.NoError(t, err)
+
+	in := bytes.NewBuffer(nil)
+	input.Terminal = term.NewTerminal(input.ReadWriter{
+		Reader: in,
+		Writer: ioutil.Discard,
+	}, "")
+
+	checkKeyError(t, filepath.Join(dir, "badfile"), errInvalidKey)
+
+	t.Run("wallet", func(t *testing.T) {
+		checkKeyError(t, wallPath, errInvalidPassword)
+
+		in.WriteString("invalid\r")
+		checkKeyError(t, wallPath, errInvalidPassword)
+
+		in.WriteString("pass\r")
+		checkKey(t, wallPath, acc2.PrivateKey()) // default account
+
+		viper.Set(address, acc1.Address)
+		in.WriteString("pass\r")
+		checkKey(t, wallPath, acc1.PrivateKey())
+
+		viper.Set(address, "not an address")
+		checkKeyError(t, wallPath, errInvalidAddress)
+
+		acc, err := wallet.NewAccount()
+		require.NoError(t, err)
+		viper.Set(address, acc.Address)
+		checkKeyError(t, wallPath, errInvalidAddress)
+	})
+
+	t.Run("WIF", func(t *testing.T) {
+		checkKey(t, wifKey.WIF(), wifKey)
+	})
+
+	t.Run("NEP-2", func(t *testing.T) {
+		checkKeyError(t, nep2, errInvalidPassword)
+
+		in.WriteString("invalid\r")
+		checkKeyError(t, nep2, errInvalidPassword)
+
+		in.WriteString("pass\r")
+		checkKey(t, nep2, nep2Key)
+	})
+
+	t.Run("raw key", func(t *testing.T) {
+		checkKey(t, keyPath, rawKey)
+	})
+
+	t.Run("generate", func(t *testing.T) {
+		viper.Set(generateKey, true)
+		actual, err := getKey()
+		require.NoError(t, err)
+		require.NotNil(t, actual)
+		for _, p := range []*keys.PrivateKey{nep2Key, rawKey, wifKey, acc1.PrivateKey(), acc2.PrivateKey()} {
+			require.NotEqual(t, p, actual, "expected new key to be generated")
+		}
+	})
+}
+
+func checkKeyError(t *testing.T, desc string, err error) {
+	viper.Set(walletPath, desc)
+	_, actualErr := getKey()
+	require.True(t, errors.Is(actualErr, err), "got: %v", actualErr)
+}
+
+func checkKey(t *testing.T, desc string, expected *keys.PrivateKey) {
+	viper.Set(walletPath, desc)
+	actual, err := getKey()
+	require.NoError(t, err)
+	require.Equal(t, &expected.PrivateKey, actual)
+}

cmd/neofs-cli/modules/root.go

@@ -44,20 +44,10 @@ const (
 	generateKeyDefault   = false
 	generateKeyUsage     = "generate new private key"
 
-	binaryKey          = "binary-key"
-	binaryKeyShorthand = ""
-	binaryKeyDefault   = ""
-	binaryKeyUsage     = "path to the raw private key file"
-
 	walletPath          = "wallet"
 	walletPathShorthand = "w"
 	walletPathDefault   = ""
-	walletPathUsage     = "path to the wallet"
-
-	wif          = "wif"
-	wifShorthand = ""
-	wifDefault   = ""
-	wifUsage     = "WIF or NEP-2"
+	walletPathUsage     = "WIF (NEP-2) string or path to the wallet or binary key"
 
 	address          = "address"
 	addressShorthand = ""
@@ -183,29 +173,34 @@ func getKey() (*ecdsa.PrivateKey, error) {
 		return &priv.PrivateKey, nil
 	}
 
-	if keyPath := viper.GetString(binaryKey); keyPath != "" {
-		return getKeyFromFile(keyPath)
+	// Ideally we want to touch file-system on the last step.
+	// However, asking for NEP-2 password seems to be confusing if we provide a wallet.
+	// Thus we try keys in the following order:
+	// 1. WIF
+	// 2. Raw binary key
+	// 3. Wallet file
+	// 4. NEP-2 encrypted WIF.
+	keyDesc := viper.GetString(walletPath)
+	priv, err := keys.NewPrivateKeyFromWIF(keyDesc)
+	if err == nil {
+		return &priv.PrivateKey, nil
 	}
 
-	if walletPath := viper.GetString(walletPath); walletPath != "" {
-		w, err := wallet.NewWalletFromFile(walletPath)
-		if err != nil {
-			return nil, fmt.Errorf("%w: %v", errInvalidKey, err)
-		}
-		return getKeyFromWallet(w, viper.GetString(address))
+	p, err := getKeyFromFile(keyDesc)
+	if err == nil {
+		return p, nil
 	}
 
-	wif := viper.GetString(wif)
-	if len(wif) == nep2Base58Length {
-		return getKeyFromNEP2(wif)
+	w, err := wallet.NewWalletFromFile(keyDesc)
+	if err == nil {
+		return getKeyFromWallet(w, viper.GetString(address))
 	}
 
-	priv, err := keys.NewPrivateKeyFromWIF(wif)
-	if err != nil {
-		return nil, fmt.Errorf("%w: %v", errInvalidKey, err)
+	if len(keyDesc) == nep2Base58Length {
+		return getKeyFromNEP2(keyDesc)
 	}
 
-	return &priv.PrivateKey, nil
+	return nil, errInvalidKey
 }
 
 func getKeyFromFile(keyPath string) (*ecdsa.PrivateKey, error) {
@@ -404,9 +399,7 @@ func initCommonFlags(cmd *cobra.Command) {
 	ff := cmd.Flags()
 
 	ff.BoolP(generateKey, generateKeyShorthand, generateKeyDefault, generateKeyUsage)
-	ff.StringP(binaryKey, binaryKeyShorthand, binaryKeyDefault, binaryKeyUsage)
 	ff.StringP(walletPath, walletPathShorthand, walletPathDefault, walletPathUsage)
-	ff.StringP(wif, wifShorthand, wifDefault, wifUsage)
 	ff.StringP(address, addressShorthand, addressDefault, addressUsage)
 	ff.StringP(rpc, rpcShorthand, rpcDefault, rpcUsage)
 	ff.BoolP(verbose, verboseShorthand, verboseDefault, verboseUsage)
@@ -417,9 +410,7 @@ func bindCommonFlags(cmd *cobra.Command) {
 	ff := cmd.Flags()
 
 	_ = viper.BindPFlag(generateKey, ff.Lookup(generateKey))
-	_ = viper.BindPFlag(binaryKey, ff.Lookup(binaryKey))
 	_ = viper.BindPFlag(walletPath, ff.Lookup(walletPath))
-	_ = viper.BindPFlag(wif, ff.Lookup(wif))
 	_ = viper.BindPFlag(address, ff.Lookup(address))
 	_ = viper.BindPFlag(rpc, ff.Lookup(rpc))
 	_ = viper.BindPFlag(verbose, ff.Lookup(verbose))

cmd/neofs-cli/modules/util.go

@@ -33,9 +33,7 @@ var (
 			flags := cmd.Flags()
 
 			_ = viper.BindPFlag(generateKey, flags.Lookup(generateKey))
-			_ = viper.BindPFlag(binaryKey, flags.Lookup(binaryKey))
 			_ = viper.BindPFlag(walletPath, flags.Lookup(walletPath))
-			_ = viper.BindPFlag(wif, flags.Lookup(wif))
 			_ = viper.BindPFlag(address, flags.Lookup(address))
 			_ = viper.BindPFlag(verbose, flags.Lookup(verbose))
 		},
@@ -194,9 +192,7 @@ func initCommonFlagsWithoutRPC(cmd *cobra.Command) {
 	flags := cmd.Flags()
 
 	flags.BoolP(generateKey, generateKeyShorthand, generateKeyDefault, generateKeyUsage)
-	flags.StringP(binaryKey, binaryKeyShorthand, binaryKeyDefault, binaryKeyUsage)
 	flags.StringP(walletPath, walletPathShorthand, walletPathDefault, walletPathUsage)
-	flags.StringP(wif, wifShorthand, wifDefault, wifUsage)
 	flags.StringP(address, addressShorthand, addressDefault, addressUsage)
 	flags.BoolP(verbose, verboseShorthand, verboseDefault, verboseUsage)
 }