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

Currently have static priority of what key is used irregardless of
whether a flag was provided via CLI or in config. This makes it
impossible to override some of the config settings. While we could try
to check if the key is provided by CLI by binding CLI flag under to
viper under a different name the same problem would occur for config/environment
variables. Fixing all of this with current set of keys is too complicate.
In this commit we revert changes from #610 and use a single flag for all types of keys.

Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
This commit is contained in:
Evgenii Stratonikov 2022-01-19 13:56:10 +03:00 committed by Alex Vanin
parent ac82899e85
commit 759421ebbf
5 changed files with 145 additions and 41 deletions

View file

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

View file

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

View file

@ -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)
}

View file

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

View file

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