Repository: nats-io/nsc
Pull request #541 (2023-01-25 16:26:15)

Fix: null pointer error in add user interactive mode
When an operator is configured to require a signing key (`StrictSigningKeyUsage`) but there are no signing keys configure on an account, using interactive mode to create a user results in a null pointer error: the resolved signing key slice is empty and interactive mode attempts to auto select index 0 which doesn't exist.

This PR will return a descriptive error to the user indicating that signing keys are required but no signing keys are available for use.


Patch:
diff --git a/cmd/adduser.go b/cmd/adduser.go
index 163b738..0db4865 100644
--- a/cmd/adduser.go
+++ b/cmd/adduser.go
@@ -196,6 +196,9 @@ func validUserSigners(ctx ActionCtx, accName string) ([]string, error) {
 			signers = append(signers, signingKey)
 		}
 	}
+	if opc.StrictSigningKeyUsage && len(ac.SigningKeys) == 0 {
+		return nil, errors.New("operator requires signing keys but account does not have any configured")
+	}
 	return signers, nil
 }
 
diff --git a/cmd/editauthorization.go b/cmd/editauthorization.go
index f0c0800..35d3cbc 100644
--- a/cmd/editauthorization.go
+++ b/cmd/editauthorization.go
@@ -1,5 +1,5 @@
 /*
- * Copyright 2022 The NATS Authors
+ * Copyright 2018-2023 The NATS Authors
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
@@ -118,8 +118,11 @@ func toPublicKey(s string, kind nkeys.PrefixByte) (string, error) {
 	if err != nil {
 		return "", err
 	}
+	if kp == nil {
+		return "", fmt.Errorf("%q is not a valid %s key", s, kind)
+	}
 	if err := nkeys.CompatibleKeyPair(kp, kind); err != nil {
-		return "", fmt.Errorf("%s is not a valid %s key", s, kind)
+		return "", fmt.Errorf("%q is not a valid %s key", s, kind)
 	}
 	return kp.PublicKey()
 }
diff --git a/cmd/select.go b/cmd/select.go
new file mode 100644
index 0000000..2df75b0
--- /dev/null
+++ b/cmd/select.go
@@ -0,0 +1,78 @@
+/*
+ * Copyright 2018-2022 The NATS Authors
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package cmd
+
+import (
+	"github.com/spf13/cobra"
+)
+
+func init() {
+	GetRootCmd().AddCommand(createSelecteCmd())
+}
+
+func createSelecteCmd() *cobra.Command {
+	cmd := &cobra.Command{
+		Use:   "select",
+		Short: "Set the current operator or account",
+	}
+	cmd.AddCommand(selectOperatorCmd())
+	cmd.AddCommand(selectAccountCmd())
+	return cmd
+}
+
+func selectOperatorCmd() *cobra.Command {
+	return &cobra.Command{
+		Use:   "operator",
+		Short: "set the operator",
+		Args: func(cmd *cobra.Command, args []string) error {
+			if err := cobra.ExactArgs(1)(cmd, args); err != nil {
+				return err
+			}
+
+			return nil
+		},
+		RunE: func(cmd *cobra.Command, args []string) error {
+			current := GetConfig()
+			if err := current.ContextConfig.Update("", args[0], ""); err != nil {
+				return err
+			}
+
+			return current.Save()
+		},
+	}
+}
+
+func selectAccountCmd() *cobra.Command {
+	return &cobra.Command{
+		Use:   "account",
+		Short: "set the account",
+		Args: func(cmd *cobra.Command, args []string) error {
+			if err := cobra.ExactArgs(1)(cmd, args); err != nil {
+				return err
+			}
+
+			return nil
+		},
+		RunE: func(cmd *cobra.Command, args []string) error {
+			current := GetConfig()
+			if err := current.ContextConfig.Update("", "", args[0]); err != nil {
+				return err
+			}
+
+			return current.Save()
+		},
+	}
+}


Test patch:
diff --git a/cmd/editauthorization_test.go b/cmd/editauthorization_test.go
index 2779e34..7029388 100644
--- a/cmd/editauthorization_test.go
+++ b/cmd/editauthorization_test.go
@@ -1,5 +1,5 @@
 /*
- * Copyright 2022 The NATS Authors
+ * Copyright 2018-2023 The NATS Authors
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
@@ -44,7 +44,7 @@ func Test_EditAuthorizationBadUser(t *testing.T) {
 	_, aPK, _ := CreateAccountKey(t)
 	_, _, err := ExecuteCmd(createEditAuthorizationCallout(), "--auth-user", aPK)
 	require.Error(t, err)
-	require.Equal(t, fmt.Sprintf("%s is not a valid user key", aPK), err.Error())
+	require.Equal(t, fmt.Sprintf("%q is not a valid user key", aPK), err.Error())
 }
 
 func Test_EditAuthorizationBadAccount(t *testing.T) {
@@ -56,7 +56,17 @@ func Test_EditAuthorizationBadAccount(t *testing.T) {
 	_, aPK, _ := CreateUserKey(t)
 	_, _, err := ExecuteCmd(createEditAuthorizationCallout(), "--allowed-account", aPK)
 	require.Error(t, err)
-	require.Equal(t, fmt.Sprintf("%s is not a valid account key", aPK), err.Error())
+	require.Equal(t, fmt.Sprintf("%q is not a valid account key", aPK), err.Error())
+}
+
+func Test_EditAuthorizationBadNonNkey(t *testing.T) {
+	ts := NewTestStore(t, "test")
+	defer ts.Done(t)
+
+	ts.AddAccount(t, "A")
+	_, _, err := ExecuteCmd(createEditAuthorizationCallout(), "--allowed-account", "hello")
+	require.Error(t, err)
+	require.Equal(t, fmt.Sprintf("%q is not a valid account key", "hello"), err.Error())
 }
 
 func Test_EditAuthorizationJustUser(t *testing.T) {
diff --git a/cmd/select_test.go b/cmd/select_test.go
new file mode 100644
index 0000000..387b66a
--- /dev/null
+++ b/cmd/select_test.go
@@ -0,0 +1,80 @@
+/*
+ * Copyright 2018-2022 The NATS Authors
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package cmd
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestSelectAccount(t *testing.T) {
+	require := require.New(t)
+	ts := NewTestStore(t, "test")
+	defer ts.Done(t)
+
+	_, _, err := ExecuteCmd(selectAccountCmd(), "A")
+	require.NotNil(err)
+
+	ts.AddAccount(t, "A")
+	ts.AddAccount(t, "B")
+
+	_, _, err = ExecuteCmd(selectAccountCmd(), "A")
+	require.Nil(err)
+	conf := GetConfig()
+	require.Equal(conf.Account, "A")
+
+	_, _, err = ExecuteCmd(selectAccountCmd(), "B")
+	require.Nil(err)
+	conf = GetConfig()
+	require.Equal(conf.Account, "B")
+
+	_, _, err = ExecuteCmd(selectAccountCmd(), "NO")
+	require.NotNil(err)
+	require.Contains(err.Error(), "\"NO\" not in accounts for operator")
+
+	conf = GetConfig()
+	require.Equal(conf.Account, "B")
+}
+
+func TestSelectOperator(t *testing.T) {
+	require := require.New(t)
+	ts := NewTestStore(t, "test")
+	defer ts.Done(t)
+
+	_, _, err := ExecuteCmd(selectOperatorCmd(), "test")
+	require.Nil(err)
+
+	ts.AddOperator(t, "test2")
+	ts.AddOperator(t, "test3")
+
+	_, _, err = ExecuteCmd(selectOperatorCmd(), "test2")
+	require.Nil(err)
+	conf := GetConfig()
+	require.Equal(conf.Operator, "test2")
+
+	_, _, err = ExecuteCmd(selectOperatorCmd(), "test3")
+	require.Nil(err)
+	conf = GetConfig()
+	require.Equal(conf.Operator, "test3")
+
+	_, _, err = ExecuteCmd(selectOperatorCmd(), "NO")
+	require.NotNil(err)
+	require.Contains(err.Error(), "operator \"NO\" not in")
+
+	conf = GetConfig()
+	require.Equal(conf.Operator, "test3")
+}