Repository: nats-io/nsc
Pull request #61 (2018-12-20 19:07:40)

Normalize interactions requesting for signer keys
[CHANGE] signer functionality (-K) on interactive will always prompt for path/key on non-managed stores.
[CHANGE] signer params interactive will never prompt to generate a key, since signing keys should pre-exist
[CHANGE] signer params will show default or set value (-K) when prompting
[ADD] KeyStore functionality to return the path where a key is expected


Patch:
diff --git a/cmd/common.go b/cmd/common.go
index 8acefff..38ea6f1 100644
--- a/cmd/common.go
+++ b/cmd/common.go
@@ -298,22 +298,6 @@ func NKeyValidator(kind nkeys.PrefixByte) cli.Validator {
 	}
 }
 
-func EditKeyPath(kind nkeys.PrefixByte, label string, keypath *string) error {
-	ok, err := cli.PromptYN(fmt.Sprintf("generate an %s nkey", label))
-	if err != nil {
-		return err
-	}
-
-	if !ok {
-		v, err := cli.Prompt(fmt.Sprintf("path to the %s nkey", label), "", true, NKeyValidator(kind))
-		if err != nil {
-			return err
-		}
-		*keypath = v
-	}
-	return nil
-}
-
 func LoadFromURL(url string) ([]byte, error) {
 	c := &http.Client{Timeout: time.Second * 5}
 	r, err := c.Get(url)
diff --git a/cmd/defaults.go b/cmd/defaults.go
index f3f8925..8546690 100644
--- a/cmd/defaults.go
+++ b/cmd/defaults.go
@@ -54,10 +54,6 @@ func GetConfig() *ToolConfig {
 	return &config
 }
 
-func ResetConfigForTests() {
-	config = ToolConfig{}
-}
-
 func LoadOrInit(github string, toolHomeEnvName string) (*ToolConfig, error) {
 	var err error
 	if toolHomeEnvName == "" {
diff --git a/cmd/editaccount.go b/cmd/editaccount.go
index a52b662..76025bc 100644
--- a/cmd/editaccount.go
+++ b/cmd/editaccount.go
@@ -157,11 +157,11 @@ func (p *EditAccountParams) Load(ctx ActionCtx) error {
 
 func (p *EditAccountParams) PostInteractive(ctx ActionCtx) error {
 	var err error
-	if err = p.conns.Edit("max connections"); err != nil {
+	if err = p.conns.Edit("max connections (-1 unlimited)"); err != nil {
 		return err
 	}
 
-	if err = p.data.Edit("max data"); err != nil {
+	if err = p.data.Edit("max data (-1 unlimited)"); err != nil {
 		return err
 	}
 
diff --git a/cmd/signer.go b/cmd/signer.go
index b3da041..d525b6a 100644
--- a/cmd/signer.go
+++ b/cmd/signer.go
@@ -19,8 +19,11 @@ import (
 	"fmt"
 
 	"github.com/nats-io/nkeys"
+	"github.com/nats-io/nsc/cli"
 )
 
+// SignerParams is shared UI for a signer (-K flag). The key
+// for a signer is never generated and must be provided
 type SignerParams struct {
 	kind     nkeys.PrefixByte
 	signerKP nkeys.KeyPair
@@ -38,18 +41,29 @@ func (p *SignerParams) SetDefaults(kind nkeys.PrefixByte, allowManaged bool, ctx
 func (p *SignerParams) Edit(ctx ActionCtx) error {
 	var err error
 
-	p.signerKP, err = ctx.StoreCtx().ResolveKey(p.kind, KeyPathFlag)
+	sctx := ctx.StoreCtx()
+	p.signerKP, err = sctx.ResolveKey(p.kind, KeyPathFlag)
 	if err != nil {
 		return err
 	}
-	if p.signerKP == nil {
-		label := fmt.Sprintf("path to %s nkey or nkey", p.kind.String())
-		err = EditKeyPath(p.kind, label, &KeyPathFlag)
-		if err != nil {
-			return err
-		}
+
+	// skip showing a signer
+	if p.signerKP != nil && ctx.StoreCtx().Store.IsManaged() {
+		return nil
 	}
-	return nil
+	ks := sctx.KeyStore
+	switch p.kind {
+	case nkeys.PrefixByteOperator:
+		KeyPathFlag = ks.GetOperatorKeyPath(sctx.Operator.Name)
+	case nkeys.PrefixByteAccount:
+		KeyPathFlag = ks.GetAccountKeyPath(sctx.Account.Name)
+	case nkeys.PrefixByteCluster:
+		KeyPathFlag = ks.GetClusterKeyPath(sctx.Cluster.Name)
+	}
+
+	label := fmt.Sprintf("path to signer %s nkey or nkey", p.kind.String())
+	KeyPathFlag, err = cli.Prompt(label, KeyPathFlag, true, NKeyValidator(p.kind))
+	return err
 }
 
 func (p *SignerParams) Resolve(ctx ActionCtx) error {
diff --git a/cmd/store/keys.go b/cmd/store/keys.go
index 15b3aa0..4d6f31d 100644
--- a/cmd/store/keys.go
+++ b/cmd/store/keys.go
@@ -113,16 +113,16 @@ func (k *KeyStore) keypath(name string, kp nkeys.KeyPair, parent string) (string
 	}
 	switch kt {
 	case nkeys.PrefixByteOperator:
-		return filepath.Join(GetKeysDir(), k.Env, k.keyName(name)), nil
+		return k.GetOperatorKeyPath(name), nil
 	case nkeys.PrefixByteAccount:
-		return filepath.Join(GetKeysDir(), k.Env, Accounts, name, k.keyName(name)), nil
+		return k.GetAccountKeyPath(name), nil
 	case nkeys.PrefixByteUser:
 		if parent == "" {
 			return "", fmt.Errorf("user keys require an account parent")
 		}
 		return filepath.Join(GetKeysDir(), k.Env, Accounts, parent, Users, k.keyName(name)), nil
 	case nkeys.PrefixByteCluster:
-		return filepath.Join(GetKeysDir(), k.Env, Clusters, name, k.keyName(name)), nil
+		return k.GetClusterKeyPath(name), nil
 	case nkeys.PrefixByteServer:
 		if parent == "" {
 			return "", fmt.Errorf("servers keys require a cluster parent")
@@ -133,8 +133,20 @@ func (k *KeyStore) keypath(name string, kp nkeys.KeyPair, parent string) (string
 	}
 }
 
+func (k *KeyStore) GetOperatorKeyPath(name string) string {
+	return filepath.Join(GetKeysDir(), k.Env, k.keyName(name))
+}
+
+func (k *KeyStore) GetAccountKeyPath(name string) string {
+	return filepath.Join(GetKeysDir(), k.Env, Accounts, name, k.keyName(name))
+}
+
+func (k *KeyStore) GetClusterKeyPath(name string) string {
+	return filepath.Join(GetKeysDir(), k.Env, Clusters, name, k.keyName(name))
+}
+
 func (k *KeyStore) GetOperatorKey(name string) (nkeys.KeyPair, error) {
-	return k.Read(filepath.Join(GetKeysDir(), k.Env, k.keyName(name)))
+	return k.Read(k.GetOperatorKeyPath(name))
 }
 
 func (k *KeyStore) GetOperatorPublicKey(name string) (string, error) {
@@ -142,7 +154,7 @@ func (k *KeyStore) GetOperatorPublicKey(name string) (string, error) {
 }
 
 func (k *KeyStore) GetAccountKey(name string) (nkeys.KeyPair, error) {
-	return k.Read(filepath.Join(GetKeysDir(), k.Env, Accounts, name, k.keyName(name)))
+	return k.Read(k.GetAccountKeyPath(name))
 }
 
 func (k *KeyStore) GetAccountPublicKey(name string) (string, error) {
@@ -162,7 +174,7 @@ func (k *KeyStore) GetUserSeed(account string, name string) (string, error) {
 }
 
 func (k *KeyStore) GetClusterKey(name string) (nkeys.KeyPair, error) {
-	return k.Read(filepath.Join(GetKeysDir(), k.Env, Clusters, name, k.keyName(name)))
+	return k.Read(k.GetClusterKeyPath(name))
 }
 
 func (k *KeyStore) GetClusterPublicKey(name string) (string, error) {


Test patch:
diff --git a/cmd/addaccount_test.go b/cmd/addaccount_test.go
index 9888b28..022b93d 100644
--- a/cmd/addaccount_test.go
+++ b/cmd/addaccount_test.go
@@ -62,7 +62,7 @@ func Test_AddAccountInteractive(t *testing.T) {
 	ts := NewTestStore(t, "test")
 	defer ts.Done(t)
 
-	inputs := []interface{}{"A", true, "2018-01-01", "2050-01-01"}
+	inputs := []interface{}{"A", true, "2018-01-01", "2050-01-01", ts.OperatorKeyPath}
 
 	cmd := CreateAddAccountCmd()
 	HoistRootFlags(cmd)
diff --git a/cmd/addclustert_test.go b/cmd/addclustert_test.go
index 03a248b..0665d75 100644
--- a/cmd/addclustert_test.go
+++ b/cmd/addclustert_test.go
@@ -72,7 +72,7 @@ func Test_AddClusterInteractive(t *testing.T) {
 	ts := NewTestStore(t, "test")
 	defer ts.Done(t)
 
-	inputs := []interface{}{"a", true, "2018-01-01", "2050-01-01"}
+	inputs := []interface{}{"a", true, "2018-01-01", "2050-01-01", ts.OperatorKeyPath}
 
 	cmd := createAddClusterCmd()
 	HoistRootFlags(cmd)
diff --git a/cmd/addexport_test.go b/cmd/addexport_test.go
index d3e5a1b..d0bf0f8 100644
--- a/cmd/addexport_test.go
+++ b/cmd/addexport_test.go
@@ -125,13 +125,13 @@ func Test_AddExportAccountNameRequired(t *testing.T) {
 }
 
 func TestAddExportInteractive(t *testing.T) {
-	ts := NewTestStoreWithOperator(t, "test", nil)
+	ts := NewTestStore(t, "test")
 	defer ts.Done(t)
 
 	ts.AddAccount(t, "A")
 	ts.AddAccount(t, "B")
 
-	input := []interface{}{0, 0, "foo.>", "Foo Stream", false}
+	input := []interface{}{0, 0, "foo.>", "Foo Stream", false, ts.OperatorKeyPath}
 	cmd := createAddExportCmd()
 	HoistRootFlags(cmd)
 	_, _, err := ExecuteInteractiveCmd(cmd, input, "-i")
diff --git a/cmd/addimport_test.go b/cmd/addimport_test.go
index a5aa9a7..385139b 100644
--- a/cmd/addimport_test.go
+++ b/cmd/addimport_test.go
@@ -118,7 +118,7 @@ func Test_AddImportInteractive(t *testing.T) {
 
 	cmd := createAddImportCmd()
 	HoistRootFlags(cmd)
-	input := []interface{}{1, false, fp, "my import", "barfoo.>"}
+	input := []interface{}{1, false, fp, "my import", "barfoo.>", ts.OperatorKeyPath}
 	_, _, err = ExecuteInteractiveCmd(cmd, input, "-i")
 	require.NoError(t, err)
 
@@ -185,7 +185,7 @@ func Test_AddImport_PublicInteractive(t *testing.T) {
 
 	cmd := createAddImportCmd()
 	HoistRootFlags(cmd)
-	input := []interface{}{1, true, apub, "foobar.>", true, "test", "test.foobar.>"}
+	input := []interface{}{1, true, apub, "foobar.>", true, "test", "test.foobar.>", ts.OperatorKeyPath}
 	_, _, err = ExecuteInteractiveCmd(cmd, input, "-i")
 	require.NoError(t, err)
 
diff --git a/cmd/addserver_test.go b/cmd/addserver_test.go
index 9238213..6925fb3 100644
--- a/cmd/addserver_test.go
+++ b/cmd/addserver_test.go
@@ -72,7 +72,7 @@ func Test_AddServerInteractive(t *testing.T) {
 	_, _, err := ExecuteCmd(createAddClusterCmd(), "--name", "c")
 	require.NoError(t, err, "cluster creation")
 
-	inputs := []interface{}{"a", true, "2018-01-01", "2050-01-01"}
+	inputs := []interface{}{"a", true, "2018-01-01", "2050-01-01", ts.KeyStore.GetClusterKeyPath("c")}
 
 	cmd := createAddServerCmd()
 	HoistRootFlags(cmd)
diff --git a/cmd/adduser_test.go b/cmd/adduser_test.go
index 0e690a8..d90001c 100644
--- a/cmd/adduser_test.go
+++ b/cmd/adduser_test.go
@@ -72,7 +72,7 @@ func Test_AddUserInteractive(t *testing.T) {
 	_, _, err := ExecuteCmd(CreateAddAccountCmd(), "--name", "A")
 	require.NoError(t, err, "account creation")
 
-	inputs := []interface{}{"U", true, "2018-01-01", "2050-01-01"}
+	inputs := []interface{}{"U", true, "2018-01-01", "2050-01-01", ts.KeyStore.GetAccountKeyPath("A")}
 
 	cmd := CreateAddUserCmd()
 	HoistRootFlags(cmd)
diff --git a/cmd/defaults_test.go b/cmd/defaults_test.go
index f0b3372..1529d4b 100644
--- a/cmd/defaults_test.go
+++ b/cmd/defaults_test.go
@@ -29,7 +29,7 @@ func TestDefault_LoadOrInit(t *testing.T) {
 	dir := filepath.Join(d, "a")
 	require.NoError(t, os.Setenv("TEST_NAME", dir))
 
-	ResetConfigForTests()
+	ResetForTests()
 	tc, err := LoadOrInit("my/foo", "TEST_NAME")
 	require.NoError(t, err)
 
@@ -51,7 +51,7 @@ func TestDefault_LoadNewOnExisting(t *testing.T) {
 	fp := filepath.Join(ts.Dir, fmt.Sprintf("%s.json", GetToolName()))
 	require.NoError(t, WriteJson(fp, cc))
 
-	ResetConfigForTests()
+	ResetForTests()
 	tc, err := LoadOrInit("my/foo", "TEST_NAME")
 	require.NoError(t, err)
 	require.NotNil(t, tc)
diff --git a/cmd/deleteexport_test.go b/cmd/deleteexport_test.go
index 3d90098..85c09f2 100644
--- a/cmd/deleteexport_test.go
+++ b/cmd/deleteexport_test.go
@@ -60,7 +60,7 @@ func Test_DeleteExportInteractive(t *testing.T) {
 	ts.AddExport(t, "A", jwt.Stream, "baz", true)
 	ts.AddAccount(t, "B")
 
-	input := []interface{}{0, 0}
+	input := []interface{}{0, 0, ts.KeyStore.GetAccountKeyPath("A")}
 	cmd := createDeleteExportCmd()
 	HoistRootFlags(cmd)
 	_, _, err := ExecuteInteractiveCmd(cmd, input, "-i")
diff --git a/cmd/deleteimport_test.go b/cmd/deleteimport_test.go
index 6a5b98f..518d12c 100644
--- a/cmd/deleteimport_test.go
+++ b/cmd/deleteimport_test.go
@@ -71,7 +71,7 @@ func Test_DeleteImportInteractive(t *testing.T) {
 	ts.AddImport(t, "A", "foo", "B")
 	ts.AddImport(t, "A", "bar", "B")
 
-	input := []interface{}{1, false, 0}
+	input := []interface{}{1, false, 0, ts.OperatorKeyPath}
 	cmd := createDeleteImportCmd()
 	HoistRootFlags(cmd)
 	_, _, err := ExecuteInteractiveCmd(cmd, input)
diff --git a/cmd/generateactivation_test.go b/cmd/generateactivation_test.go
index 2023417..32c8664 100644
--- a/cmd/generateactivation_test.go
+++ b/cmd/generateactivation_test.go
@@ -146,7 +146,7 @@ func Test_InteractiveGenerate(t *testing.T) {
 	_, pub, _ := CreateAccountKey(t)
 
 	outpath := filepath.Join(ts.Dir, "token.jwt")
-	inputs := []interface{}{0, pub, "0", "0"}
+	inputs := []interface{}{0, pub, "0", "0", ts.KeyStore.GetAccountKeyPath("A")}
 	_, _, err := ExecuteInteractiveCmd(cmd, inputs, "-i", "--output-file", outpath)
 	require.NoError(t, err)
 
@@ -170,7 +170,7 @@ func Test_InteractiveExternalKeyGenerate(t *testing.T) {
 	err := ioutil.WriteFile(keyfile, []byte(pub), 0700)
 	require.NoError(t, err)
 
-	inputs := []interface{}{0, keyfile, "0", "0"}
+	inputs := []interface{}{0, keyfile, "0", "0", ts.KeyStore.GetAccountKeyPath("A")}
 	_, _, err = ExecuteInteractiveCmd(cmd, inputs, "-i", "--output-file", outpath)
 	require.NoError(t, err)
 
@@ -192,7 +192,7 @@ func Test_InteractiveMultipleAccountsGenerate(t *testing.T) {
 
 	_, pub, _ := CreateAccountKey(t)
 
-	inputs := []interface{}{0, 0, pub, "0", "0"}
+	inputs := []interface{}{0, 0, pub, "0", "0", ts.KeyStore.GetAccountKeyPath("A")}
 	_, _, err := ExecuteInteractiveCmd(cmd, inputs, "-i", "--output-file", outpath)
 	require.NoError(t, err)
 
diff --git a/cmd/util_test.go b/cmd/util_test.go
index d34ccb9..2216e51 100644
--- a/cmd/util_test.go
+++ b/cmd/util_test.go
@@ -40,8 +40,14 @@ type TestStore struct {
 	OperatorKeyPath string
 }
 
+// Some globals must be reset
+func ResetForTests() {
+	config = ToolConfig{}
+	KeyPathFlag = ""
+}
+
 func NewTestStoreWithOperator(t *testing.T, operatorName string, operator nkeys.KeyPair) *TestStore {
-	ResetConfigForTests()
+	ResetForTests()
 	var ts TestStore
 	WideFlag = true