diff --git a/CHANGELOG.md b/CHANGELOG.md index 525ead0c8969..ec00e06109e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [\#9427](https://github.com/cosmos/cosmos-sdk/pull/9427) Move simapp `FundAccount` and `FundModuleAccount` to `x/bank/testutil` * (client/tx) [\#9421](https://github.com/cosmos/cosmos-sdk/pull/9421/) `BuildUnsignedTx`, `BuildSimTx`, `PrintUnsignedStdTx` functions are moved to the Tx Factory as methods. +* (client/keys) [\#9407](https://github.com/cosmos/cosmos-sdk/pull/9601) Added `keys rename` CLI command and `Keyring.Rename` interface method to rename a key in the keyring. * (x/slashing) [\#9458](https://github.com/cosmos/cosmos-sdk/pull/9458) Coins burned from slashing is now returned from Slash function and included in Slash event. * [\#9246](https://github.com/cosmos/cosmos-sdk/pull/9246) The `New` method for the network package now returns an error. * (codec) [\#9521](https://github.com/cosmos/cosmos-sdk/pull/9521) Removed deprecated `clientCtx.JSONCodec` from `client.Context`. diff --git a/client/keys/rename.go b/client/keys/rename.go new file mode 100644 index 000000000000..2c5f11263de0 --- /dev/null +++ b/client/keys/rename.go @@ -0,0 +1,67 @@ +package keys + +import ( + "bufio" + "fmt" + + "github.com/cosmos/cosmos-sdk/client" + "github.com/cosmos/cosmos-sdk/client/input" + "github.com/cosmos/cosmos-sdk/crypto/keyring" + "github.com/spf13/cobra" +) + +// RenameKeyCommand renames a key from the key store. +func RenameKeyCommand() *cobra.Command { + cmd := &cobra.Command{ + Use: "rename ", + Short: "Rename an existing key", + Long: `Rename a key from the Keybase backend. + +Note that renaming offline or ledger keys will rename +only the public key references stored locally, i.e. +private keys stored in a ledger device cannot be renamed with the CLI. +`, + Args: cobra.ExactArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + buf := bufio.NewReader(cmd.InOrStdin()) + clientCtx, err := client.GetClientQueryContext(cmd) + if err != nil { + return err + } + + oldName, newName := args[0], args[1] + + info, err := clientCtx.Keyring.Key(oldName) + if err != nil { + return err + } + + // confirm rename, unless -y is passed + if skip, _ := cmd.Flags().GetBool(flagYes); !skip { + prompt := fmt.Sprintf("Key reference will be renamed from %s to %s. Continue?", args[0], args[1]) + if yes, err := input.GetConfirmation(prompt, buf, cmd.ErrOrStderr()); err != nil { + return err + } else if !yes { + return nil + } + } + + if err := clientCtx.Keyring.Rename(oldName, newName); err != nil { + return err + } + + if info.GetType() == keyring.TypeLedger || info.GetType() == keyring.TypeOffline { + cmd.PrintErrln("Public key reference renamed") + return nil + } + + cmd.PrintErrln(fmt.Sprintf("Key was successfully renamed from %s to %s", oldName, newName)) + + return nil + }, + } + + cmd.Flags().BoolP(flagYes, "y", false, "Skip confirmation prompt when renaming offline or ledger key references") + + return cmd +} diff --git a/client/keys/rename_test.go b/client/keys/rename_test.go new file mode 100644 index 000000000000..37d15ecba25e --- /dev/null +++ b/client/keys/rename_test.go @@ -0,0 +1,98 @@ +package keys + +import ( + "context" + "fmt" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/cosmos/cosmos-sdk/client" + "github.com/cosmos/cosmos-sdk/client/flags" + "github.com/cosmos/cosmos-sdk/crypto/hd" + "github.com/cosmos/cosmos-sdk/crypto/keyring" + "github.com/cosmos/cosmos-sdk/testutil" + sdk "github.com/cosmos/cosmos-sdk/types" +) + +func Test_runRenameCmd(t *testing.T) { + // temp keybase + kbHome := t.TempDir() + cmd := RenameKeyCommand() + cmd.Flags().AddFlagSet(Commands(kbHome).PersistentFlags()) + mockIn := testutil.ApplyMockIODiscardOutErr(cmd) + + yesF, _ := cmd.Flags().GetBool(flagYes) + require.False(t, yesF) + + fakeKeyName1 := "runRenameCmd_Key1" + fakeKeyName2 := "runRenameCmd_Key2" + + path := sdk.GetConfig().GetFullBIP44Path() + + kb, err := keyring.New(sdk.KeyringServiceName(), keyring.BackendTest, kbHome, mockIn) + require.NoError(t, err) + + // put fakeKeyName1 in keyring + _, err = kb.NewAccount(fakeKeyName1, testutil.TestMnemonic, "", path, hd.Secp256k1) + require.NoError(t, err) + + clientCtx := client.Context{}. + WithKeyringDir(kbHome). + WithKeyring(kb) + + ctx := context.WithValue(context.Background(), client.ClientContextKey, &clientCtx) + + // rename a key 'blah' which doesnt exist + cmd.SetArgs([]string{"blah", "blaah", fmt.Sprintf("--%s=%s", flags.FlagHome, kbHome)}) + err = cmd.ExecuteContext(ctx) + require.Error(t, err) + require.EqualError(t, err, "blah.info: key not found") + + // User confirmation missing + cmd.SetArgs([]string{ + fakeKeyName1, + "nokey", + fmt.Sprintf("--%s=%s", flags.FlagHome, kbHome), + fmt.Sprintf("--%s=%s", flags.FlagKeyringBackend, keyring.BackendTest), + }) + err = cmd.Execute() + require.Error(t, err) + require.Equal(t, "EOF", err.Error()) + + oldKey, err := kb.Key(fakeKeyName1) + require.NoError(t, err) + + // add a confirmation + cmd.SetArgs([]string{ + fakeKeyName1, + fakeKeyName2, + fmt.Sprintf("--%s=%s", flags.FlagHome, kbHome), + fmt.Sprintf("--%s=true", flagYes), + fmt.Sprintf("--%s=%s", flags.FlagKeyringBackend, keyring.BackendTest), + }) + require.NoError(t, cmd.Execute()) + + // key1 is gone + _, err = kb.Key(fakeKeyName1) + require.Error(t, err) + + // key2 exists now + renamedKey, err := kb.Key(fakeKeyName2) + require.NoError(t, err) + + require.Equal(t, oldKey.GetPubKey(), renamedKey.GetPubKey()) + require.Equal(t, oldKey.GetType(), renamedKey.GetType()) + require.Equal(t, oldKey.GetAddress(), renamedKey.GetAddress()) + require.Equal(t, oldKey.GetAlgo(), renamedKey.GetAlgo()) + + // try to rename key1 but it doesnt exist anymore so error + cmd.SetArgs([]string{ + fakeKeyName1, + fakeKeyName2, + fmt.Sprintf("--%s=%s", flags.FlagHome, kbHome), + fmt.Sprintf("--%s=true", flagYes), + fmt.Sprintf("--%s=%s", flags.FlagKeyringBackend, keyring.BackendTest), + }) + require.Error(t, cmd.Execute()) +} diff --git a/client/keys/root.go b/client/keys/root.go index 938a0d53a14c..e8b1ef156987 100644 --- a/client/keys/root.go +++ b/client/keys/root.go @@ -45,6 +45,7 @@ The pass backend requires GnuPG: https://gnupg.org/ ListKeysCmd(), ShowKeysCmd(), DeleteKeyCommand(), + RenameKeyCommand(), ParseKeyStringCommand(), MigrateCommand(), ) diff --git a/client/keys/root_test.go b/client/keys/root_test.go index b6c2f5f88f48..f66ae9265de6 100644 --- a/client/keys/root_test.go +++ b/client/keys/root_test.go @@ -11,5 +11,5 @@ func TestCommands(t *testing.T) { assert.NotNil(t, rootCommands) // Commands are registered - assert.Equal(t, 9, len(rootCommands.Commands())) + assert.Equal(t, 10, len(rootCommands.Commands())) } diff --git a/crypto/keyring/keyring.go b/crypto/keyring/keyring.go index eb5762497688..717f589fa789 100644 --- a/crypto/keyring/keyring.go +++ b/crypto/keyring/keyring.go @@ -41,6 +41,9 @@ const ( keyringFileDirName = "keyring-file" keyringTestDirName = "keyring-test" passKeyringPrefix = "keyring-%s" + + // temporary pass phrase for exporting a key during a key rename + passPhrase = "temp" ) var ( @@ -64,6 +67,9 @@ type Keyring interface { Delete(uid string) error DeleteByAddress(address sdk.Address) error + // Rename an existing key from the Keyring + Rename(from string, to string) error + // NewMnemonic generates a new mnemonic, derives a hierarchical deterministic key from it, and // persists the key to storage. Returns the generated mnemonic and the key Info. // It returns an error if it fails to generate a key for the given algo type, or if @@ -288,8 +294,10 @@ func (ks keystore) ExportPrivKeyArmorByAddress(address sdk.Address, encryptPassp } func (ks keystore) ImportPrivKey(uid, armor, passphrase string) error { - if _, err := ks.Key(uid); err == nil { - return fmt.Errorf("cannot overwrite key: %s", uid) + if k, err := ks.Key(uid); err == nil { + if uid == k.GetName() { + return fmt.Errorf("cannot overwrite key: %s", uid) + } } privKey, algo, err := crypto.UnarmorDecryptPrivKey(armor, passphrase) @@ -426,6 +434,30 @@ func (ks keystore) DeleteByAddress(address sdk.Address) error { return nil } +func (ks keystore) Rename(oldName, newName string) error { + _, err := ks.Key(newName) + if err == nil { + return fmt.Errorf("rename failed: %s already exists in the keyring", newName) + } + + armor, err := ks.ExportPrivKeyArmor(oldName, passPhrase) + if err != nil { + return err + } + + err = ks.ImportPrivKey(newName, armor, passPhrase) + if err != nil { + return err + } + + err = ks.Delete(oldName) + if err != nil { + return err + } + + return nil +} + func (ks keystore) Delete(uid string) error { info, err := ks.Key(uid) if err != nil { @@ -783,16 +815,22 @@ func (ks keystore) writeInfo(info Info) error { } // existsInDb returns true if key is in DB. Error is returned only when we have error -// different thant ErrKeyNotFound +// different than ErrKeyNotFound func (ks keystore) existsInDb(info Info) (bool, error) { - if _, err := ks.db.Get(addrHexKeyAsString(info.GetAddress())); err == nil { - return true, nil // address lookup succeeds - info exists + if item, err := ks.db.Get(addrHexKeyAsString(info.GetAddress())); err == nil { + if item.Key == info.GetName() { + return true, nil // address lookup succeeds - info exists + } + return false, nil } else if err != keyring.ErrKeyNotFound { return false, err // received unexpected error - returns error } - if _, err := ks.db.Get(infoKey(info.GetName())); err == nil { - return true, nil // uid lookup succeeds - info exists + if item, err := ks.db.Get(infoKey(info.GetName())); err == nil { + if item.Key == info.GetName() { + return true, nil // uid lookup succeeds - info exists + } + return false, nil } else if err != keyring.ErrKeyNotFound { return false, err // received unexpected error - returns } diff --git a/crypto/keyring/keyring_test.go b/crypto/keyring/keyring_test.go index 2f8e928e7a7e..c2a0df70a7d9 100644 --- a/crypto/keyring/keyring_test.go +++ b/crypto/keyring/keyring_test.go @@ -1153,6 +1153,72 @@ func TestBackendConfigConstructors(t *testing.T) { require.Equal(t, "keyring-test", backend.PassPrefix) } +func TestRenameKey(t *testing.T) { + testCases := []struct { + name string + run func(Keyring) + }{ + { + name: "rename a key", + run: func(kr Keyring) { + oldKeyUID, newKeyUID := "old", "new" + oldKeyInfo := newKeyInfo(t, kr, oldKeyUID) + err := kr.Rename(oldKeyUID, newKeyUID) // rename from "old" to "new" + require.NoError(t, err) + newInfo, err := kr.Key(newKeyUID) // new key should be in keyring + require.NoError(t, err) + requireEqualRenamedKey(t, newInfo, oldKeyInfo) // oldinfo and newinfo should be the same + oldKeyInfo, err = kr.Key(oldKeyUID) // old key should be gone from keyring + require.Error(t, err) + }, + }, + { + name: "cant rename a key that doesnt exist", + run: func(kr Keyring) { + err := kr.Rename("bogus", "bogus2") + require.Error(t, err) + }, + }, + { + name: "cant rename a key to an already existing key name", + run: func(kr Keyring) { + key1, key2 := "existingKey", "existingKey2" // create 2 keys + newKeyInfo(t, kr, key1) + newKeyInfo(t, kr, key2) + err := kr.Rename(key2, key1) + require.Equal(t, fmt.Errorf("rename failed: %s already exists in the keyring", key1), err) + assertKeysExist(t, kr, key1, key2) // keys should still exist after failed rename + }, + }, + { + name: "cant rename key to itself", + run: func(kr Keyring) { + keyName := "keyName" + newKeyInfo(t, kr, keyName) + err := kr.Rename(keyName, keyName) + require.Equal(t, fmt.Errorf("rename failed: %s already exists in the keyring", keyName), err) + assertKeysExist(t, kr, keyName) + }, + }, + } + + for _, tc := range testCases { + tc := tc + kr := newKeyring(t, "testKeyring") + t.Run(tc.name, func(t *testing.T) { + tc.run(kr) + }) + } +} + +func requireEqualRenamedKey(t *testing.T, newKey, oldKey Info) { + require.NotEqual(t, newKey.GetName(), oldKey.GetName()) + require.Equal(t, newKey.GetAddress(), oldKey.GetAddress()) + require.Equal(t, newKey.GetPubKey(), oldKey.GetPubKey()) + require.Equal(t, newKey.GetAlgo(), oldKey.GetAlgo()) + require.Equal(t, newKey.GetType(), oldKey.GetType()) +} + func requireEqualInfo(t *testing.T, key Info, mnemonic Info) { require.Equal(t, key.GetName(), mnemonic.GetName()) require.Equal(t, key.GetAddress(), mnemonic.GetAddress()) @@ -1161,4 +1227,23 @@ func requireEqualInfo(t *testing.T, key Info, mnemonic Info) { require.Equal(t, key.GetType(), mnemonic.GetType()) } +func newKeyring(t *testing.T, name string) Keyring { + kr, err := New(name, "test", t.TempDir(), nil) + require.NoError(t, err) + return kr +} + +func newKeyInfo(t *testing.T, kr Keyring, name string) Info { + keyInfo, _, err := kr.NewMnemonic(name, English, sdk.FullFundraiserPath, DefaultBIP39Passphrase, hd.Secp256k1) + require.NoError(t, err) + return keyInfo +} + +func assertKeysExist(t *testing.T, kr Keyring, names ...string) { + for _, n := range names { + _, err := kr.Key(n) + require.NoError(t, err) + } +} + func accAddr(info Info) sdk.AccAddress { return info.GetAddress() }