From d0410a121505f02dc5cd57555accb076d5ee654b Mon Sep 17 00:00:00 2001 From: Halimao <1065621723@qq.com> Date: Thu, 14 Dec 2023 10:35:00 +0800 Subject: [PATCH 1/8] refactor: use "input.GetPassword" for bip39 passphrase --- client/keys/add.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/keys/add.go b/client/keys/add.go index a30a2d865a20..9574866f4ddd 100644 --- a/client/keys/add.go +++ b/client/keys/add.go @@ -313,7 +313,7 @@ func runAddCmd(ctx client.Context, cmd *cobra.Command, args []string, inBuf *buf // override bip39 passphrase if interactive { - bip39Passphrase, err = input.GetString( + bip39Passphrase, err = input.GetPassword( "Enter your bip39 passphrase. This is combined with the mnemonic to derive the seed. "+ "Most users should just hit enter to use the default, \"\"", inBuf) if err != nil { @@ -322,7 +322,7 @@ func runAddCmd(ctx client.Context, cmd *cobra.Command, args []string, inBuf *buf // if they use one, make them re-enter it if len(bip39Passphrase) != 0 { - p2, err := input.GetString("Repeat the passphrase:", inBuf) + p2, err := input.GetPassword("Repeat the passphrase:", inBuf) if err != nil { return err } From 857446df4a4c6ea6d23e25d37d9dfa6200fd8e38 Mon Sep 17 00:00:00 2001 From: Halimao <1065621723@qq.com> Date: Thu, 14 Dec 2023 10:44:27 +0800 Subject: [PATCH 2/8] add newline for password input prompt --- client/keys/add.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/keys/add.go b/client/keys/add.go index 9574866f4ddd..df53a1b5f3de 100644 --- a/client/keys/add.go +++ b/client/keys/add.go @@ -315,14 +315,14 @@ func runAddCmd(ctx client.Context, cmd *cobra.Command, args []string, inBuf *buf if interactive { bip39Passphrase, err = input.GetPassword( "Enter your bip39 passphrase. This is combined with the mnemonic to derive the seed. "+ - "Most users should just hit enter to use the default, \"\"", inBuf) + "Most users should just hit enter to use the default, \"\"\n", inBuf) if err != nil { return err } // if they use one, make them re-enter it if len(bip39Passphrase) != 0 { - p2, err := input.GetPassword("Repeat the passphrase:", inBuf) + p2, err := input.GetPassword("Repeat the passphrase:\n", inBuf) if err != nil { return err } From 5871d65c5d20093c61caa7ef4a3312c4126f7161 Mon Sep 17 00:00:00 2001 From: Halimao <1065621723@qq.com> Date: Thu, 14 Dec 2023 10:52:40 +0800 Subject: [PATCH 3/8] fix when passphrase input is empty --- client/input/input.go | 15 +++++++++++++++ client/keys/add.go | 4 ++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/client/input/input.go b/client/input/input.go index 31daa99be32f..58b2d206dfbd 100644 --- a/client/input/input.go +++ b/client/input/input.go @@ -77,6 +77,21 @@ func GetString(prompt string, buf *bufio.Reader) (string, error) { return strings.TrimSpace(out), nil } +// GetSecretString returns the secret string output of a given reader. +func GetSecretString(prompt string, buf *bufio.Reader) (secret string, err error) { + if inputIsTty() { + secret, err = speakeasy.FAsk(os.Stderr, prompt) + } else { + secret, err = readLineFromBuf(buf) + } + + if err != nil { + return "", err + } + + return secret, nil +} + // inputIsTty returns true iff we have an interactive prompt, // where we can disable echo and request to repeat the password. // If false, we can optimize for piped input from another command diff --git a/client/keys/add.go b/client/keys/add.go index df53a1b5f3de..13dd35ec4764 100644 --- a/client/keys/add.go +++ b/client/keys/add.go @@ -313,7 +313,7 @@ func runAddCmd(ctx client.Context, cmd *cobra.Command, args []string, inBuf *buf // override bip39 passphrase if interactive { - bip39Passphrase, err = input.GetPassword( + bip39Passphrase, err = input.GetSecretString( "Enter your bip39 passphrase. This is combined with the mnemonic to derive the seed. "+ "Most users should just hit enter to use the default, \"\"\n", inBuf) if err != nil { @@ -322,7 +322,7 @@ func runAddCmd(ctx client.Context, cmd *cobra.Command, args []string, inBuf *buf // if they use one, make them re-enter it if len(bip39Passphrase) != 0 { - p2, err := input.GetPassword("Repeat the passphrase:\n", inBuf) + p2, err := input.GetSecretString("Repeat the passphrase:\n", inBuf) if err != nil { return err } From 754b8f68b88443850c9af84cd59c54a24e3095b2 Mon Sep 17 00:00:00 2001 From: Halimao <1065621723@qq.com> Date: Thu, 14 Dec 2023 11:11:46 +0800 Subject: [PATCH 4/8] refactor: limit passphrase length to at least 8 --- client/keys/add.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/client/keys/add.go b/client/keys/add.go index 13dd35ec4764..4f2c353a0552 100644 --- a/client/keys/add.go +++ b/client/keys/add.go @@ -322,7 +322,12 @@ func runAddCmd(ctx client.Context, cmd *cobra.Command, args []string, inBuf *buf // if they use one, make them re-enter it if len(bip39Passphrase) != 0 { - p2, err := input.GetSecretString("Repeat the passphrase:\n", inBuf) + if len(bip39Passphrase) < input.MinPassLength { + // Return the given password to the upstream client so it can handle a + // non-STDIN failure gracefully. + return fmt.Errorf("password must be at least %d characters", input.MinPassLength) + } + p2, err := input.GetPassword("Repeat the passphrase:\n", inBuf) if err != nil { return err } From b9163ba3c4fa1d95cc5f82dd390f925e0b8fcecc Mon Sep 17 00:00:00 2001 From: Halimao <1065621723@qq.com> Date: Thu, 14 Dec 2023 11:15:45 +0800 Subject: [PATCH 5/8] password 2 passphrase --- client/keys/add.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/keys/add.go b/client/keys/add.go index 4f2c353a0552..e11ec0b1b4c4 100644 --- a/client/keys/add.go +++ b/client/keys/add.go @@ -323,9 +323,9 @@ func runAddCmd(ctx client.Context, cmd *cobra.Command, args []string, inBuf *buf // if they use one, make them re-enter it if len(bip39Passphrase) != 0 { if len(bip39Passphrase) < input.MinPassLength { - // Return the given password to the upstream client so it can handle a + // Return the given passphrase to the upstream client so it can handle a // non-STDIN failure gracefully. - return fmt.Errorf("password must be at least %d characters", input.MinPassLength) + return fmt.Errorf("passphrase must be at least %d characters", input.MinPassLength) } p2, err := input.GetPassword("Repeat the passphrase:\n", inBuf) if err != nil { From 210650542dfda8defdaf412356998a6b5a488f34 Mon Sep 17 00:00:00 2001 From: Halimao <1065621723@qq.com> Date: Thu, 14 Dec 2023 11:24:30 +0800 Subject: [PATCH 6/8] add test --- client/keys/add_test.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/client/keys/add_test.go b/client/keys/add_test.go index 53ce3af2798e..09f0c81b03e2 100644 --- a/client/keys/add_test.go +++ b/client/keys/add_test.go @@ -12,6 +12,7 @@ import ( "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/flags" + "github.com/cosmos/cosmos-sdk/client/input" addresscodec "github.com/cosmos/cosmos-sdk/codec/address" "github.com/cosmos/cosmos-sdk/crypto/hd" "github.com/cosmos/cosmos-sdk/crypto/keyring" @@ -116,7 +117,18 @@ func Test_runAddCmdBasic(t *testing.T) { fmt.Sprintf("--%s=false", flagRecover), }) - const password = "password1!" + const ( + password = "password1!" + invalidPassword = "1234" + ) + + // set password default interactive key generation successfully + mockIn.Reset("\n\n") + require.NoError(t, cmd.ExecuteContext(ctx)) + + // set invalid password interactive key generation fail + mockIn.Reset("\n" + invalidPassword + "\n") + require.EqualError(t, cmd.ExecuteContext(ctx), fmt.Sprintf("passphrase must be at least %d characters", input.MinPassLength)) // set password and complete interactive key generation successfully mockIn.Reset("\n" + password + "\n" + password + "\n") From a1db514dcb5ec3744da6b1028c7bc7f34932de26 Mon Sep 17 00:00:00 2001 From: Halimao <1065621723@qq.com> Date: Fri, 15 Dec 2023 08:52:31 +0800 Subject: [PATCH 7/8] add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e11c1e31e5e7..b5b90ecd2ba4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements +* (client/keys) [#18743](https://github.com/cosmos/cosmos-sdk/pull/18743) Improve ` keys add -i` by hiding inputting of bip39 passphrase. * (client/keys) [#18703](https://github.com/cosmos/cosmos-sdk/pull/18703) Improve ` keys add` and ` keys show` by checking whether there are duplicate keys in the multisig case. * (x/gov) [#18707](https://github.com/cosmos/cosmos-sdk/pull/18707) Improve genesis validation. * (x/bank) [#18636](https://github.com/cosmos/cosmos-sdk/pull/18636) `SendCoinsFromModuleToAccount`, `SendCoinsFromModuleToModule`, `SendCoinsFromAccountToModule`, `DelegateCoinsFromAccountToModule`, `UndelegateCoinsFromModuleToAccount`, `MintCoins` and `BurnCoins` methods now returns an error instead of panicking if any module accounts does not exist or unauthorized. From f40017bc3450282cdfa68150678dbccd23b741ed Mon Sep 17 00:00:00 2001 From: Halimao <1065621723@qq.com> Date: Fri, 15 Dec 2023 17:13:42 +0800 Subject: [PATCH 8/8] revert limit for length --- client/keys/add.go | 7 +------ client/keys/add_test.go | 10 +--------- 2 files changed, 2 insertions(+), 15 deletions(-) diff --git a/client/keys/add.go b/client/keys/add.go index e11ec0b1b4c4..13dd35ec4764 100644 --- a/client/keys/add.go +++ b/client/keys/add.go @@ -322,12 +322,7 @@ func runAddCmd(ctx client.Context, cmd *cobra.Command, args []string, inBuf *buf // if they use one, make them re-enter it if len(bip39Passphrase) != 0 { - if len(bip39Passphrase) < input.MinPassLength { - // Return the given passphrase to the upstream client so it can handle a - // non-STDIN failure gracefully. - return fmt.Errorf("passphrase must be at least %d characters", input.MinPassLength) - } - p2, err := input.GetPassword("Repeat the passphrase:\n", inBuf) + p2, err := input.GetSecretString("Repeat the passphrase:\n", inBuf) if err != nil { return err } diff --git a/client/keys/add_test.go b/client/keys/add_test.go index 09f0c81b03e2..6dff014b2263 100644 --- a/client/keys/add_test.go +++ b/client/keys/add_test.go @@ -12,7 +12,6 @@ import ( "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/flags" - "github.com/cosmos/cosmos-sdk/client/input" addresscodec "github.com/cosmos/cosmos-sdk/codec/address" "github.com/cosmos/cosmos-sdk/crypto/hd" "github.com/cosmos/cosmos-sdk/crypto/keyring" @@ -117,19 +116,12 @@ func Test_runAddCmdBasic(t *testing.T) { fmt.Sprintf("--%s=false", flagRecover), }) - const ( - password = "password1!" - invalidPassword = "1234" - ) + const password = "password1!" // set password default interactive key generation successfully mockIn.Reset("\n\n") require.NoError(t, cmd.ExecuteContext(ctx)) - // set invalid password interactive key generation fail - mockIn.Reset("\n" + invalidPassword + "\n") - require.EqualError(t, cmd.ExecuteContext(ctx), fmt.Sprintf("passphrase must be at least %d characters", input.MinPassLength)) - // set password and complete interactive key generation successfully mockIn.Reset("\n" + password + "\n" + password + "\n") require.NoError(t, cmd.ExecuteContext(ctx))