Repository: nats-io/nsc (go)

Issue:
Bad diagnostic for adding user without using signing-key when operator is strict_signing_key_usage
### What version were you using?

nsc 2.8.4.

### What environment was the server running in?

Linux/amd64 (Ubuntu 20.04 LTS)

### Is this defect reproducible?

Yes.

1. Have an operator with the `strict_signing_key_usage` set to true.  Optionally, forget that this is the case.
2. Create a new account.
3. Try to create a user in the account.
4. See an error about "any of the following" with nothing following

### Given the capability you are leveraging, describe your expectation?

A diagnostic which points me in the right direction, along the lines of:

```
error: operator 'foobar' has strict_signing_key_usage set, refusing to add user to account 'baz' without an account signing key
```

### Given the expectation, what is the defect you are observing?

```console
$ nsc add user --name pt-second-pdp-2023-12-06
Error: unable to resolve any of the following signing keys in the keystore:
[error code 1] $
```

Pull request:
Fixes related to --require-signing-key option in operator
[FIX] added a guard to edit operator --require-signing-key to ensure that operator has at least one signing key before setting it

[FIX] added a guard when removing signing keys from the operator that require signing key is not set [FIX] added better messages in cases where a signing key is required because of --require-signing-key option when adding users

Fixes #628

Patch:
diff --git a/cmd/adduser.go b/cmd/adduser.go
index b70a47a..d4d4904 100644
--- a/cmd/adduser.go
+++ b/cmd/adduser.go
@@ -222,6 +222,20 @@ func (p *AddUserParams) Validate(ctx ActionCtx) error {
 		p.userName = GetRandomName(0)
 	}
 
+	s := ctx.StoreCtx().Store
+	claim, err := s.ReadAccountClaim(p.AccountContextParams.Name)
+	if err != nil {
+		return fmt.Errorf("reading account %q failed: %v", p.AccountContextParams.Name, err)
+	}
+
+	o, err := s.ReadOperatorClaim()
+	if err != nil {
+		return err
+	}
+	if o.StrictSigningKeyUsage && len(claim.SigningKeys) == 0 {
+		return errors.New("unable to issue users when operator requires signing keys and the account has none")
+	}
+
 	if err = p.AccountContextParams.Validate(ctx); err != nil {
 		return err
 	}
@@ -253,12 +267,11 @@ func (p *AddUserParams) Validate(ctx ActionCtx) error {
 		}
 	}
 
-	s := ctx.StoreCtx().Store
-	if claim, err := s.ReadAccountClaim(p.AccountContextParams.Name); err != nil {
-		return fmt.Errorf("reading account %q failed: %v", p.AccountContextParams.Name, err)
-	} else if claim.Limits.DisallowBearer && p.bearer {
+	if claim.Limits.DisallowBearer && p.bearer {
 		return fmt.Errorf("account %q forbids the use of bearer token", p.AccountContextParams.Name)
-	} else if s.Has(store.Accounts, p.AccountContextParams.Name, store.Users, store.JwtName(p.userName)) {
+	}
+
+	if s.Has(store.Accounts, p.AccountContextParams.Name, store.Users, store.JwtName(p.userName)) {
 		return fmt.Errorf("the user %q already exists", p.userName)
 	}
 
diff --git a/cmd/editoperator.go b/cmd/editoperator.go
index 548067f..ab36974 100644
--- a/cmd/editoperator.go
+++ b/cmd/editoperator.go
@@ -16,6 +16,7 @@
 package cmd
 
 import (
+	"errors"
 	"fmt"
 	"strings"
 
@@ -111,7 +112,7 @@ func (p *EditOperatorParams) Load(ctx ActionCtx) error {
 		p.sysAcc = oc.SystemAccount
 	}
 
-	if !p.reqSk {
+	if !ctx.CurrentCmd().Flags().Changed("require-signing-keys") {
 		p.reqSk = oc.StrictSigningKeyUsage
 	}
 
@@ -254,6 +255,7 @@ func (p *EditOperatorParams) Validate(ctx ActionCtx) error {
 			}
 		}
 	}
+
 	if err = p.signingKeys.Valid(); err != nil {
 		return err
 	}
@@ -287,6 +289,12 @@ func (p *EditOperatorParams) Run(ctx ActionCtx) (store.Status, error) {
 		p.claim.StrictSigningKeyUsage = p.reqSk
 		r.AddOK("strict signing key usage set to: %t", p.reqSk)
 	}
+
+	if p.claim.StrictSigningKeyUsage && len(p.claim.SigningKeys) == 0 {
+		fmt.Printf("%t\n", p.reqSk)
+		return nil, errors.New("operator requires at least one signing key")
+	}
+
 	flags := ctx.CurrentCmd().Flags()
 	p.claim.AccountServerURL = p.asu
 	if flags.Changed("account-jwt-server-url") {
diff --git a/go.mod b/go.mod
index 9eed493..033b4f7 100644
--- a/go.mod
+++ b/go.mod
@@ -9,9 +9,9 @@ require (
 	github.com/mitchellh/go-homedir v1.1.0
 	github.com/nats-io/cliprompts/v2 v2.0.0-20231014115920-801ca035562a
 	github.com/nats-io/jsm.go v0.1.1
-	github.com/nats-io/jwt/v2 v2.5.6
+	github.com/nats-io/jwt/v2 v2.5.7
 	github.com/nats-io/nats-server/v2 v2.10.12
-	github.com/nats-io/nats.go v1.34.1
+	github.com/nats-io/nats.go v1.35.0
 	github.com/nats-io/nkeys v0.4.7
 	github.com/nats-io/nuid v1.0.1
 	github.com/rhysd/go-github-selfupdate v1.2.3
@@ -19,14 +19,14 @@ require (
 	github.com/spf13/pflag v1.0.5
 	github.com/stretchr/testify v1.7.1
 	github.com/xlab/tablewriter v0.0.0-20160610135559-80b567a11ad5
-	golang.org/x/text v0.14.0
+	golang.org/x/text v0.15.0
 )
 
 require (
 	github.com/AlecAivazis/survey/v2 v2.3.7 // indirect
 	github.com/cpuguy83/go-md2man/v2 v2.0.4 // indirect
 	github.com/davecgh/go-spew v1.1.1 // indirect
-	github.com/fatih/color v1.16.0 // indirect
+	github.com/fatih/color v1.17.0 // indirect
 	github.com/google/go-github/v30 v30.1.0 // indirect
 	github.com/google/go-querystring v1.1.0 // indirect
 	github.com/inconshreveable/go-update v0.0.0-20160112193335-8152e7eb6ccf // indirect
@@ -43,11 +43,11 @@ require (
 	github.com/russross/blackfriday/v2 v2.1.0 // indirect
 	github.com/tcnksm/go-gitconfig v0.1.2 // indirect
 	github.com/ulikunitz/xz v0.5.12 // indirect
-	golang.org/x/crypto v0.22.0 // indirect
-	golang.org/x/net v0.24.0 // indirect
-	golang.org/x/oauth2 v0.19.0 // indirect
-	golang.org/x/sys v0.19.0 // indirect
-	golang.org/x/term v0.19.0 // indirect
+	golang.org/x/crypto v0.23.0 // indirect
+	golang.org/x/net v0.25.0 // indirect
+	golang.org/x/oauth2 v0.20.0 // indirect
+	golang.org/x/sys v0.20.0 // indirect
+	golang.org/x/term v0.20.0 // indirect
 	golang.org/x/time v0.5.0 // indirect
 	gopkg.in/yaml.v2 v2.4.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
diff --git a/go.sum b/go.sum
index 0262ae8..515fb02 100644
--- a/go.sum
+++ b/go.sum
@@ -18,8 +18,8 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkpeCY=
 github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto=
-github.com/fatih/color v1.16.0 h1:zmkK9Ngbjj+K0yRhTVONQh1p/HknKYSlNT+vZCzyokM=
-github.com/fatih/color v1.16.0/go.mod h1:fL2Sau1YI5c0pdGEVCbKQbLXB6edEj1ZgiY4NijnWvE=
+github.com/fatih/color v1.17.0 h1:GlRw1BRJxkpqUCBKzKOw098ed57fEsKeNjpTe3cSjK4=
+github.com/fatih/color v1.17.0/go.mod h1:YZ7TlrGPkiz6ku9fK3TLD/pl3CpsiFyu8N92HLgmosI=
 github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo=
 github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
 github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
@@ -70,12 +70,12 @@ github.com/nats-io/cliprompts/v2 v2.0.0-20231014115920-801ca035562a h1:28qvB6peS
 github.com/nats-io/cliprompts/v2 v2.0.0-20231014115920-801ca035562a/go.mod h1:oweZn7AeaVJYKlNHfCIhznJVsdySLSng55vfuINE/d0=
 github.com/nats-io/jsm.go v0.1.1 h1:6vjllz276SdC+3Fb3XI71p9B6toxkCruuB1K6unQEr0=
 github.com/nats-io/jsm.go v0.1.1/go.mod h1:cFz5wR1pW0zLFotntS4HA7V8Wm+sf8zpF+iQJHbsS6M=
-github.com/nats-io/jwt/v2 v2.5.6 h1:Cp618+z4q042sWqHiSoIHFT08OZtAskui0hTmRfmGGQ=
-github.com/nats-io/jwt/v2 v2.5.6/go.mod h1:ZdWS1nZa6WMZfFwwgpEaqBV8EPGVgOTDHN/wTbz0Y5A=
+github.com/nats-io/jwt/v2 v2.5.7 h1:j5lH1fUXCnJnY8SsQeB/a/z9Azgu2bYIDvtPVNdxe2c=
+github.com/nats-io/jwt/v2 v2.5.7/go.mod h1:ZdWS1nZa6WMZfFwwgpEaqBV8EPGVgOTDHN/wTbz0Y5A=
 github.com/nats-io/nats-server/v2 v2.10.12 h1:G6u+RDrHkw4bkwn7I911O5jqys7jJVRY6MwgndyUsnE=
 github.com/nats-io/nats-server/v2 v2.10.12/go.mod h1:H1n6zXtYLFCgXcf/SF8QNTSIFuS8tyZQMN9NguUHdEs=
-github.com/nats-io/nats.go v1.34.1 h1:syWey5xaNHZgicYBemv0nohUPPmaLteiBEUT6Q5+F/4=
-github.com/nats-io/nats.go v1.34.1/go.mod h1:Ubdu4Nh9exXdSz0RVWRFBbRfrbSxOYd26oF0wkWclB8=
+github.com/nats-io/nats.go v1.35.0 h1:XFNqNM7v5B+MQMKqVGAyHwYhyKb48jrenXNxIU20ULk=
+github.com/nats-io/nats.go v1.35.0/go.mod h1:Ubdu4Nh9exXdSz0RVWRFBbRfrbSxOYd26oF0wkWclB8=
 github.com/nats-io/nkeys v0.4.7 h1:RwNJbbIdYCoClSDNY7QVKZlyb/wfT6ugvFCiKy6vDvI=
 github.com/nats-io/nkeys v0.4.7/go.mod h1:kqXRgRDPlGy7nGaEDMuYzmiJCIAAWDK0IMBtDmGD0nc=
 github.com/nats-io/nuid v1.0.1 h1:5iA8DT8V7q8WK2EScv2padNa/rTESc1KdnPw4TC2paw=
@@ -111,8 +111,8 @@ golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACk
 golang.org/x/crypto v0.0.0-20190530122614-20be4c3c3ed5/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I=
 golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
-golang.org/x/crypto v0.22.0 h1:g1v0xeRhjcugydODzvb3mEM9SQ0HGp9s/nh3COQ/C30=
-golang.org/x/crypto v0.22.0/go.mod h1:vr6Su+7cTlO45qkww3VDJlzDn0ctJvRgYbC2NvXHt+M=
+golang.org/x/crypto v0.23.0 h1:dIJU/v2J8Mdglj/8rJ6UUOM3Zc9zLZxVZwwxMooUSAI=
+golang.org/x/crypto v0.23.0/go.mod h1:CKFgDieR+mRhux2Lsu27y0fO304Db0wZe70UKqHu0v8=
 golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4=
 golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
@@ -121,12 +121,12 @@ golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn
 golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
 golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
 golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c=
-golang.org/x/net v0.24.0 h1:1PcaxkF854Fu3+lvBIx5SYn9wRlBzzcnHZSiaFFAb0w=
-golang.org/x/net v0.24.0/go.mod h1:2Q7sJY5mzlzWjKtYUEXSlBWCdyaioyXzRB2RtU8KVE8=
+golang.org/x/net v0.25.0 h1:d/OCCoBEUq33pjydKrGQhw7IlUPI2Oylr+8qLx49kac=
+golang.org/x/net v0.25.0/go.mod h1:JkAGAh7GEvH74S6FOH42FLoXpXbE/aqXSrIQjXgsiwM=
 golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
 golang.org/x/oauth2 v0.0.0-20181106182150-f42d05182288/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
-golang.org/x/oauth2 v0.19.0 h1:9+E/EZBCbTLNrbN35fHv/a/d/mOBatymz1zbtQrXpIg=
-golang.org/x/oauth2 v0.19.0/go.mod h1:vYi7skDa1x015PmRRYZ7+s1cWyPgrPiSYRe4rnsexc8=
+golang.org/x/oauth2 v0.20.0 h1:4mQdhULixXKP1rwYBW0vAijoXnkTG0BLCDRzfe1idMo=
+golang.org/x/oauth2 v0.20.0/go.mod h1:XYTD2NtWslqkgxebSiOHnXEap4TF09sJSc7H1sXbhtI=
 golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
@@ -143,20 +143,20 @@ golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBc
 golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.19.0 h1:q5f1RH2jigJ1MoAWp2KTp3gm5zAGFUTarQZ5U386+4o=
-golang.org/x/sys v0.19.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
+golang.org/x/sys v0.20.0 h1:Od9JTbYCk261bKm4M/mw7AklTlFYIa0bIp9BgSm1S8Y=
+golang.org/x/sys v0.20.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
 golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw=
 golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
 golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
-golang.org/x/term v0.19.0 h1:+ThwsDv+tYfnJFhF4L8jITxu1tdTWRTZpdsWgEgjL6Q=
-golang.org/x/term v0.19.0/go.mod h1:2CuTdWZ7KHSQwUzKva0cbMg6q2DMI3Mmxp+gKJbskEk=
+golang.org/x/term v0.20.0 h1:VnkxpohqXaOBYJtBmEppKUG6mXpi+4O6purfc2+sMhw=
+golang.org/x/term v0.20.0/go.mod h1:8UkIAJTvZgivsXaD6/pH6U9ecQzZ45awqEOzuCvwpFY=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
 golang.org/x/text v0.3.5/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
 golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
 golang.org/x/text v0.4.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8=
-golang.org/x/text v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ=
-golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU=
+golang.org/x/text v0.15.0 h1:h1V/4gjBv8v9cjcR6+AR5+/cIYK5N/WAgiv4xlsEtAk=
+golang.org/x/text v0.15.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU=
 golang.org/x/time v0.5.0 h1:o7cqy6amK/52YcAKIPlM3a+Fpj35zvRj2TP+e1xFSfk=
 golang.org/x/time v0.5.0/go.mod h1:3BpzKBy/shNhVucY/MWOyx10tF3SFh9QdLuxbVysPQM=
 golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=


Test patch:
diff --git a/cmd/adduser_test.go b/cmd/adduser_test.go
index 108d0ac..8d3313c 100644
--- a/cmd/adduser_test.go
+++ b/cmd/adduser_test.go
@@ -601,3 +601,23 @@ func Test_AddUserBadName(t *testing.T) {
 	require.Error(t, err)
 	require.Contains(t, err.Error(), "name cannot contain '/' or '\\'")
 }
+
+func Test_AddUserRequireSk(t *testing.T) {
+	ts := NewTestStore(t, "0")
+	defer ts.Done(t)
+
+	_, _, err := ExecuteCmd(createEditOperatorCmd(), "--require-signing-keys", "--sk", "generate")
+	require.NoError(t, err)
+
+	ts.AddAccount(t, "A")
+
+	_, _, err = ExecuteCmd(CreateAddUserCmd(), "U")
+	require.Error(t, err)
+	require.Equal(t, "unable to issue users when operator requires signing keys and the account has none", err.Error())
+
+	_, _, err = ExecuteCmd(createEditAccount(), "--sk", "generate")
+	require.NoError(t, err)
+
+	_, _, err = ExecuteCmd(CreateAddUserCmd(), "U")
+	require.NoError(t, err)
+}
diff --git a/cmd/editoperator_test.go b/cmd/editoperator_test.go
index bac99f3..9cfb35f 100644
--- a/cmd/editoperator_test.go
+++ b/cmd/editoperator_test.go
@@ -397,3 +397,26 @@ func Test_ExpireShouldNotBeUnset(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, expires, oc.Expires)
 }
+
+func Test_CannotSetRequireSKWithoutSK(t *testing.T) {
+	ts := NewTestStore(t, "0")
+	defer ts.Done(t)
+
+	_, _, err := ExecuteCmd(createEditOperatorCmd(), "--require-signing-keys")
+	require.Error(t, err)
+	require.Equal(t, "operator requires at least one signing key", err.Error())
+
+	_, _, err = ExecuteCmd(createEditOperatorCmd(), "--require-signing-keys", "--sk", "generate")
+	require.NoError(t, err)
+
+	oc, err := ts.Store.ReadOperatorClaim()
+	require.NoError(t, err)
+
+	_, _, err = ExecuteCmd(createEditOperatorCmd(), "--require-signing-keys=false", "--rm-sk", oc.SigningKeys[0])
+	require.NoError(t, err)
+
+	oc, err = ts.Store.ReadOperatorClaim()
+	require.NoError(t, err)
+	require.False(t, oc.StrictSigningKeyUsage)
+	require.Empty(t, oc.SigningKeys)
+}