From fc4c563e292f7bbdd8ceaf2c2cc549503e0b2ee2 Mon Sep 17 00:00:00 2001
From: ValarDragon <dojha12@gmail.com>
Date: Tue, 10 Jul 2018 19:56:09 -0700
Subject: [PATCH] keys: Keybase.Update no longer asks for newpass if oldpass is
 incorrect

Achieved by refactoring the parameter newpass as follows:

* (newpass string) -> (getNewpass func() (string, error))

Closes #1629
---
 CHANGELOG.md                |  8 ++++++++
 client/keys/update.go       | 24 +++++++++++++-----------
 crypto/keys/keybase.go      |  9 +++++++--
 crypto/keys/keybase_test.go | 10 ++++++----
 crypto/keys/types.go        |  2 +-
 5 files changed, 35 insertions(+), 18 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 44fa7290ef..bfb5523d52 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,13 @@
 # Changelog
 
+## Pending
+
+BREAKING CHANGES
+- [keys] Keybase.Update function now takes in a function to get the newpass, rather than the password itself
+
+BUG FIXES
+- [keys] \#1629 - updating password no longer asks for a new password when the first entered password was incorrect
+
 ## 0.20.0
 
 *July 10th, 2018*
diff --git a/client/keys/update.go b/client/keys/update.go
index 0308b89bdb..78a81bf0e6 100644
--- a/client/keys/update.go
+++ b/client/keys/update.go
@@ -26,23 +26,23 @@ func runUpdateCmd(cmd *cobra.Command, args []string) error {
 	name := args[0]
 
 	buf := client.BufferStdin()
+	kb, err := GetKeyBase()
+	if err != nil {
+		return err
+	}
 	oldpass, err := client.GetPassword(
 		"Enter the current passphrase:", buf)
 	if err != nil {
 		return err
 	}
-	newpass, err := client.GetCheckPassword(
-		"Enter the new passphrase:",
-		"Repeat the new passphrase:", buf)
-	if err != nil {
-		return err
+
+	getNewpass := func() (string, error) {
+		return client.GetCheckPassword(
+			"Enter the new passphrase:",
+			"Repeat the new passphrase:", buf)
 	}
 
-	kb, err := GetKeyBase()
-	if err != nil {
-		return err
-	}
-	err = kb.Update(name, oldpass, newpass)
+	err = kb.Update(name, oldpass, getNewpass)
 	if err != nil {
 		return err
 	}
@@ -81,8 +81,10 @@ func UpdateKeyRequestHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	getNewpass := func() (string, error) { return m.NewPassword, nil }
+
 	// TODO check if account exists and if password is correct
-	err = kb.Update(name, m.OldPassword, m.NewPassword)
+	err = kb.Update(name, m.OldPassword, getNewpass)
 	if err != nil {
 		w.WriteHeader(401)
 		w.Write([]byte(err.Error()))
diff --git a/crypto/keys/keybase.go b/crypto/keys/keybase.go
index 7cf43eea67..80de47d3df 100644
--- a/crypto/keys/keybase.go
+++ b/crypto/keys/keybase.go
@@ -330,8 +330,9 @@ func (kb dbKeybase) Delete(name, passphrase string) error {
 // encrypted.
 //
 // oldpass must be the current passphrase used for encryption,
-// newpass will be the only valid passphrase from this time forward.
-func (kb dbKeybase) Update(name, oldpass, newpass string) error {
+// getNewpass is a function to get the passphrase to permanently replace
+// the current passphrase
+func (kb dbKeybase) Update(name, oldpass string, getNewpass func() (string, error)) error {
 	info, err := kb.Get(name)
 	if err != nil {
 		return err
@@ -343,6 +344,10 @@ func (kb dbKeybase) Update(name, oldpass, newpass string) error {
 		if err != nil {
 			return err
 		}
+		newpass, err := getNewpass()
+		if err != nil {
+			return err
+		}
 		kb.writeLocalKey(key, name, newpass)
 		return nil
 	default:
diff --git a/crypto/keys/keybase_test.go b/crypto/keys/keybase_test.go
index c910849865..1ca4317922 100644
--- a/crypto/keys/keybase_test.go
+++ b/crypto/keys/keybase_test.go
@@ -167,9 +167,10 @@ func TestSignVerify(t *testing.T) {
 }
 
 func assertPassword(t *testing.T, cstore Keybase, name, pass, badpass string) {
-	err := cstore.Update(name, badpass, pass)
+	getNewpass := func() (string, error) { return pass, nil }
+	err := cstore.Update(name, badpass, getNewpass)
 	require.NotNil(t, err)
-	err = cstore.Update(name, pass, pass)
+	err = cstore.Update(name, pass, getNewpass)
 	require.Nil(t, err, "%+v", err)
 }
 
@@ -265,12 +266,13 @@ func TestAdvancedKeyManagement(t *testing.T) {
 	assertPassword(t, cstore, n1, p1, p2)
 
 	// update password requires the existing password
-	err = cstore.Update(n1, "jkkgkg", p2)
+	getNewpass := func() (string, error) { return p2, nil }
+	err = cstore.Update(n1, "jkkgkg", getNewpass)
 	require.NotNil(t, err)
 	assertPassword(t, cstore, n1, p1, p2)
 
 	// then it changes the password when correct
-	err = cstore.Update(n1, p1, p2)
+	err = cstore.Update(n1, p1, getNewpass)
 	require.NoError(t, err)
 	// p2 is now the proper one!
 	assertPassword(t, cstore, n1, p2, p1)
diff --git a/crypto/keys/types.go b/crypto/keys/types.go
index 0e77725c31..74cedf9199 100644
--- a/crypto/keys/types.go
+++ b/crypto/keys/types.go
@@ -34,7 +34,7 @@ type Keybase interface {
 	CreateOffline(name string, pubkey crypto.PubKey) (info Info, err error)
 
 	// The following operations will *only* work on locally-stored keys
-	Update(name, oldpass, newpass string) error
+	Update(name, oldpass string, getNewpass func() (string, error)) error
 	Import(name string, armor string) (err error)
 	ImportPubKey(name string, armor string) (err error)
 	Export(name string) (armor string, err error)