Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move "Enter a password" cmd instructions to shared/cmd #5724

Merged
merged 28 commits into from
May 5, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
05dddc2
Move EnterPassword CMD to cmd package
0xKiwi May 3, 2020
81d4fb4
Add comment
0xKiwi May 3, 2020
3458b21
Merge refs/heads/master into move-pass-to-cmd
prylabs-bulldozer[bot] May 4, 2020
1743172
Merge refs/heads/master into move-pass-to-cmd
prylabs-bulldozer[bot] May 4, 2020
6bcecc0
Merge refs/heads/master into move-pass-to-cmd
prylabs-bulldozer[bot] May 4, 2020
25ed25f
Merge refs/heads/master into move-pass-to-cmd
prylabs-bulldozer[bot] May 4, 2020
c206139
Merge refs/heads/master into move-pass-to-cmd
prylabs-bulldozer[bot] May 4, 2020
91606f0
Merge refs/heads/master into move-pass-to-cmd
prylabs-bulldozer[bot] May 4, 2020
f1116cc
Merge refs/heads/master into move-pass-to-cmd
prylabs-bulldozer[bot] May 4, 2020
13826dc
Merge refs/heads/master into move-pass-to-cmd
prylabs-bulldozer[bot] May 5, 2020
30d1aee
Merge refs/heads/master into move-pass-to-cmd
prylabs-bulldozer[bot] May 5, 2020
d8685d7
Merge refs/heads/master into move-pass-to-cmd
prylabs-bulldozer[bot] May 5, 2020
331df8e
Merge refs/heads/master into move-pass-to-cmd
prylabs-bulldozer[bot] May 5, 2020
c4f140a
Merge refs/heads/master into move-pass-to-cmd
prylabs-bulldozer[bot] May 5, 2020
a64c600
Merge refs/heads/master into move-pass-to-cmd
prylabs-bulldozer[bot] May 5, 2020
12326f1
Merge refs/heads/master into move-pass-to-cmd
prylabs-bulldozer[bot] May 5, 2020
2356272
Merge refs/heads/master into move-pass-to-cmd
prylabs-bulldozer[bot] May 5, 2020
7eeb752
Merge refs/heads/master into move-pass-to-cmd
prylabs-bulldozer[bot] May 5, 2020
c38ea4d
Merge refs/heads/master into move-pass-to-cmd
prylabs-bulldozer[bot] May 5, 2020
d94ca0d
Merge refs/heads/master into move-pass-to-cmd
prylabs-bulldozer[bot] May 5, 2020
b501642
Add tracking issue and remove log.Fatal
0xKiwi May 5, 2020
16920a7
Merge branch 'move-pass-to-cmd' of github.com:prysmaticlabs/prysm int…
0xKiwi May 5, 2020
1c703b4
Add better error handling
0xKiwi May 5, 2020
605c37d
gaz
0xKiwi May 5, 2020
04bc4b5
Merge refs/heads/master into move-pass-to-cmd
prylabs-bulldozer[bot] May 5, 2020
24deb65
Comment fix
0xKiwi May 5, 2020
770cc41
Merge branch 'move-pass-to-cmd' of github.com:prysmaticlabs/prysm int…
0xKiwi May 5, 2020
e6c88c8
Update shared/cmd/helpers.go
terencechain May 5, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions shared/cmd/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ go_library(
"@com_github_sirupsen_logrus//:go_default_library",
"@in_gopkg_urfave_cli_v2//:go_default_library",
"@in_gopkg_urfave_cli_v2//altsrc:go_default_library",
"@org_golang_x_crypto//ssh/terminal:go_default_library",
],
)

Expand Down
16 changes: 16 additions & 0 deletions shared/cmd/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"strings"

"github.com/sirupsen/logrus"
"golang.org/x/crypto/ssh/terminal"
)

var log = logrus.WithField("prefix", "node")
Expand Down Expand Up @@ -42,3 +43,18 @@ func ConfirmAction(actionText string, deniedText string) (bool, error) {

return confirmed, nil
}

// EnterPassword queries the user for their password through the terminal, in order to make sure it is
// not passed in a visible way to the terminal.
func EnterPassword() (string, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functional comment

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also is a good unit test candidate

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 unit test would be good for this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Writing a test for terminal.ReadPassword is rather complicated, made a tracking issue at #5749

var passphrase string
log.Info("Enter a password:")
bytePassword, err := terminal.ReadPassword(int(os.Stdin.Fd()))
if err != nil {
log.Fatalf("Could not read account password: %v", err)
return passphrase, err
}
text := string(bytePassword)
passphrase = strings.Replace(text, "\n", "", -1)
return passphrase, nil
}
2 changes: 1 addition & 1 deletion validator/accounts/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ go_library(
],
deps = [
"//contracts/deposit-contract:go_default_library",
"//shared/cmd:go_default_library",
"//shared/keystore:go_default_library",
"//shared/params:go_default_library",
"@com_github_pkg_errors//:go_default_library",
"@com_github_sirupsen_logrus//:go_default_library",
"@org_golang_x_crypto//ssh/terminal:go_default_library",
],
)

Expand Down
15 changes: 6 additions & 9 deletions validator/accounts/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ import (

"github.com/pkg/errors"
contract "github.com/prysmaticlabs/prysm/contracts/deposit-contract"
"github.com/prysmaticlabs/prysm/shared/cmd"
"github.com/prysmaticlabs/prysm/shared/keystore"
"github.com/prysmaticlabs/prysm/shared/params"
"github.com/sirupsen/logrus"
"golang.org/x/crypto/ssh/terminal"
)

var log = logrus.WithField("prefix", "accounts")
Expand Down Expand Up @@ -136,16 +136,13 @@ func Exists(keystorePath string) (bool, error) {
// CreateValidatorAccount creates a validator account from the given cli context.
func CreateValidatorAccount(path string, passphrase string) (string, string, error) {
if passphrase == "" {
log.Info("Create a new validator account for eth2")
log.Info("Enter a password:")
bytePassword, err := terminal.ReadPassword(int(os.Stdin.Fd()))
log.Info("Creating a new validator account for eth2")
enteredPassphrase, err := cmd.EnterPassword()
if err != nil {
log.Fatalf("Could not read account password: %v", err)
return path, passphrase, err
log.Fatal(err)
return path, enteredPassphrase, err
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no point of returning the result here, when you have a fatal log above

Copy link
Contributor Author

@0xKiwi 0xKiwi May 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this is duplicated several times in the validator client, was gonna make another PR making sure it's either handled or returned, never both.

}
text := string(bytePassword)
passphrase = strings.Replace(text, "\n", "", -1)

passphrase = enteredPassphrase
}

if path == "" {
Expand Down