Merge pull request #848 from docker/passphrase-caching

Refactor passphrase to cache passphrases individually

passphrase/passphrase.go

@@ -50,13 +50,10 @@ func PromptRetriever() notary.PassRetriever {
 }
 
 type boundRetriever struct {
-	in                             io.Reader
-	out                            io.Writer
-	aliasMap                       map[string]string
-	userEnteredTargetsSnapshotPass bool
-	targetsSnapshotPass            string
-	userEnteredRootPass            bool
-	rootPass                       string
+	in              io.Reader
+	out             io.Writer
+	aliasMap        map[string]string
+	passphraseCache map[string]string
 }
 
 func (br *boundRetriever) getPassphrase(keyName, alias string, createNew bool, numAttempts int) (string, bool, error) {
@@ -65,11 +62,8 @@ func (br *boundRetriever) getPassphrase(keyName, alias string, createNew bool, n
 			fmt.Fprintln(br.out, tufRootKeyGenerationWarning)
 		}
 
-		if br.userEnteredTargetsSnapshotPass && (alias == tufSnapshotAlias || alias == tufTargetsAlias) {
-			return br.targetsSnapshotPass, false, nil
-		}
-		if br.userEnteredRootPass && (alias == "root") {
-			return br.rootPass, false, nil
+		if pass, ok := br.passphraseCache[alias]; ok {
+			return pass, false, nil
 		}
 	} else if !createNew { // per `if`, numAttempts > 0 if we're at this `else`
 		if numAttempts > 3 {
@@ -173,14 +167,7 @@ func (br *boundRetriever) verifyAndConfirmPassword(stdin *bufio.Reader, retPass,
 }
 
 func (br *boundRetriever) cachePassword(alias, retPass string) {
-	if alias == tufSnapshotAlias || alias == tufTargetsAlias {
-		br.userEnteredTargetsSnapshotPass = true
-		br.targetsSnapshotPass = retPass
-	}
-	if alias == tufRootAlias {
-		br.userEnteredRootPass = true
-		br.rootPass = retPass
-	}
+	br.passphraseCache[alias] = retPass
 }
 
 // PromptRetrieverWithInOut returns a new Retriever which will provide a
@@ -190,13 +177,10 @@ func (br *boundRetriever) cachePassword(alias, retPass string) {
 // is nil, a sensible default will be used.
 func PromptRetrieverWithInOut(in io.Reader, out io.Writer, aliasMap map[string]string) notary.PassRetriever {
 	bound := &boundRetriever{
-		in:                             in,
-		out:                            out,
-		aliasMap:                       aliasMap,
-		userEnteredTargetsSnapshotPass: false,
-		targetsSnapshotPass:            "",
-		userEnteredRootPass:            false,
-		rootPass:                       "",
+		in:              in,
+		out:             out,
+		aliasMap:        aliasMap,
+		passphraseCache: make(map[string]string),
 	}
 
 	return bound.getPassphrase

passphrase/passphrase_test.go

@@ -69,28 +69,47 @@ func TestGetPassphraseForCreatingDelegationKey(t *testing.T) {
 	require.Equal(t, expectedText, lines)
 }
 
-// PromptRetrieverWithInOut, if asked for root, targets, delegation, and
-// snapshot passphrases in that order will only prompt for root, targets, and
-// delegation passphrases because it caches the targets password and uses it
-// for snapshot.
-func TestGetRootTargetsDelegation(t *testing.T) {
+// PromptRetrieverWithInOut, if asked for root, targets, snapshot, and delegation
+// passphrases in that order will cache each of the keys except for the delegation key
+func TestRolePromptingAndCaching(t *testing.T) {
 	var in bytes.Buffer
 	var out bytes.Buffer
 
 	retriever := PromptRetrieverWithInOut(&in, &out, nil)
 
 	assertAskOnceForKey(t, &in, &out, retriever, "rootpassword", data.CanonicalRootRole)
 	assertAskOnceForKey(t, &in, &out, retriever, "targetspassword", data.CanonicalTargetsRole)
+	assertAskOnceForKey(t, &in, &out, retriever, "snapshotpassword", data.CanonicalSnapshotRole)
 	assertAskOnceForKey(t, &in, &out, retriever, "delegationpass", "targets/delegation")
 
-	// now ask for snapshot password, but it should already be cached, it
-	// won't ask and  no input necessary.
-	pass, giveUp, err := retriever("repo/0123456789abcdef", data.CanonicalSnapshotRole, false, 0)
+	// ask for root password, but it should already be cached
+	pass, giveUp, err := retriever("repo/0123456789abcdef", data.CanonicalRootRole, false, 0)
+	require.NoError(t, err)
+	require.False(t, giveUp)
+	require.Equal(t, "rootpassword", pass)
+
+	// ask for targets password, but it should already be cached
+	pass, giveUp, err = retriever("repo/0123456789abcdef", data.CanonicalTargetsRole, false, 0)
 	require.NoError(t, err)
 	require.False(t, giveUp)
 	require.Equal(t, "targetspassword", pass)
 
+	// ask for snapshot password, but it should already be cached
+	pass, giveUp, err = retriever("repo/0123456789abcdef", data.CanonicalSnapshotRole, false, 0)
+	require.NoError(t, err)
+	require.False(t, giveUp)
+	require.Equal(t, "snapshotpassword", pass)
+
+	// ask for targets/delegation password, but it should already be cached
+	pass, giveUp, err = retriever("repo/0123456789abcdef", "targets/delegation", false, 0)
+	require.NoError(t, err)
+	require.False(t, giveUp)
+	require.Equal(t, "delegationpass", pass)
+
+	// ask for different delegation password, which should not be cached
+	_, _, err = retriever("repo/0123456789abcdef", "targets/delegation/new", false, 0)
+	require.Error(t, err)
 	text, err := ioutil.ReadAll(&out)
 	require.NoError(t, err)
-	require.Empty(t, text)
+	require.Contains(t, string(text), "Enter passphrase for targets/delegation/new key with ID 0123456 (repo):")
 }