Repository: nats-io/nsc
Pull request #668 (2024-09-12 20:24:30)

Tags are now case-sensitive, describe account lists tags associated with the account
[CHANGE] Tags (--tag and --rm-tag) are now case-sensitive
[FIX] describe accounts was not printing tags associated with the account

Fixes https://github.com/nats-io/nats-server/issues/5681

aricart: > Looks ok, what about the potential breaking impact?

by itself nothing - the tags will be all lowercase on anything loaded. However, sloppy input on the cli will possibly create unexpected results.
aricart: After trying a couple of options settled on the change that JWT API uses are always case-sensitive as that can be telegraphed through the release notes. But for `nsc`, added the flag `--strict-tags`, which allows non-lowercased values for tag operations. 

This will catch sloppy usages of the flag where values are not lowercase and request that the `--strict-tags` option be specified. The other check is that the JWT will now report errors when trying to remove tags that are not present. 

Patch:
diff --git a/cmd/describer.go b/cmd/describer.go
index be913ad..c9f865f 100644
--- a/cmd/describer.go
+++ b/cmd/describer.go
@@ -443,10 +443,7 @@ func AddStandardClaimInfo(table *tablewriter.Table, claims jwt.Claims) {
 		}
 		tags = ac.Tags
 	}
-	if acc, ok := claims.(*jwt.ActivationClaims); ok {
-		if acc.IssuerAccount != "" {
-			issuer = acc.IssuerAccount
-		}
+	if acc, ok := claims.(*jwt.AccountClaims); ok {
 		tags = acc.Tags
 	}
 	if uc, ok := claims.(*jwt.UserClaims); ok {
diff --git a/cmd/editaccount.go b/cmd/editaccount.go
index 08ecf7a..e881447 100644
--- a/cmd/editaccount.go
+++ b/cmd/editaccount.go
@@ -57,6 +57,7 @@ func createEditAccount() *cobra.Command {
 			return RunAction(cmd, args, params)
 		},
 	}
+	cmd.Flags().BoolVarP(&params.strictTags, "strict-tags", "", false, "allow tags to be case-sensitive, default false")
 	cmd.Flags().StringSliceVarP(&params.tags, "tag", "", nil, "add tags for user - comma separated list or option can be specified multiple times")
 	cmd.Flags().StringSliceVarP(&params.rmTags, "rm-tag", "", nil, "remove tag - comma separated list or option can be specified multiple times")
 	params.conns = -1
diff --git a/cmd/editoperator.go b/cmd/editoperator.go
index ab36974..f9d5fc0 100644
--- a/cmd/editoperator.go
+++ b/cmd/editoperator.go
@@ -40,6 +40,7 @@ func createEditOperatorCmd() *cobra.Command {
 	}
 	params.signingKeys.BindFlags("sk", "", nkeys.PrefixByteOperator, cmd)
 	cmd.Flags().StringSliceVarP(&params.rmSigningKeys, "rm-sk", "", nil, "remove signing key - comma separated list or option can be specified multiple times")
+	cmd.Flags().BoolVarP(&params.strictTags, "strict-tags", "", false, "allow tags to be case-sensitive, default false")
 	cmd.Flags().StringSliceVarP(&params.tags, "tag", "", nil, "add tags for user - comma separated list or option can be specified multiple times")
 	cmd.Flags().StringSliceVarP(&params.rmTags, "rm-tag", "", nil, "remove tag - comma separated list or option can be specified multiple times")
 	cmd.Flags().StringVarP(&params.asu, "account-jwt-server-url", "u", "", "set account jwt server url for nsc sync (only http/https or nats service (nats/tls/ws/wss) urls supported if updating with nsc)")
diff --git a/cmd/edituser.go b/cmd/edituser.go
index 0338590..7b2eaac 100644
--- a/cmd/edituser.go
+++ b/cmd/edituser.go
@@ -71,6 +71,7 @@ nsc edit user --name <n> --rm-response-perms
 			return RunAction(cmd, args, &params)
 		},
 	}
+	cmd.Flags().BoolVarP(&params.strictTags, "strict-tags", "", false, "allow tags to be case-sensitive, default false")
 	cmd.Flags().StringSliceVarP(&params.tags, "tag", "", nil, "add tags for user - comma separated list or option can be specified multiple times")
 	cmd.Flags().StringSliceVarP(&params.rmTags, "rm-tag", "", nil, "remove tag - comma separated list or option can be specified multiple times")
 	cmd.Flags().StringVarP(&params.name, "name", "n", "", "user name")
diff --git a/cmd/genericclaimparams.go b/cmd/genericclaimparams.go
index d9ed387..4cd7b74 100644
--- a/cmd/genericclaimparams.go
+++ b/cmd/genericclaimparams.go
@@ -30,8 +30,9 @@ import (
 // GenericClaimsParams - TimeParams and tags
 type GenericClaimsParams struct {
 	TimeParams
-	tags   []string
-	rmTags []string
+	tags       []string
+	rmTags     []string
+	strictTags bool
 }
 
 func (sp *GenericClaimsParams) Edit(current []string) error {
@@ -100,6 +101,21 @@ func (sp *GenericClaimsParams) Valid() error {
 	if err := sp.TimeParams.Validate(); err != nil {
 		return err
 	}
+	if !sp.strictTags {
+		for _, t := range sp.tags {
+			tt := strings.ToLower(t)
+			if t != tt {
+				return fmt.Errorf("--tag %q is not lowercased, specify option --strict-tags to honor non-lowercased values", t)
+			}
+		}
+		for _, t := range sp.rmTags {
+			tt := strings.ToLower(t)
+			if t != tt {
+				return fmt.Errorf("--rm-tag %q is not lowercased, specify option --strict-tags", t)
+			}
+		}
+	}
+
 	return nil
 }
 
@@ -145,15 +161,19 @@ func (sp *GenericClaimsParams) Run(ctx ActionCtx, claim jwt.Claims, r *store.Rep
 	}
 
 	tags.Add(sp.tags...)
-	tags.Remove(sp.rmTags...)
+	if err := tags.Remove(sp.rmTags...); err != nil {
+		return err
+	}
 	sort.Strings(*tags)
 
 	if r != nil {
 		for _, t := range sp.tags {
-			r.AddOK("added tag %q", strings.ToLower(t))
+			t = strings.TrimSpace(t)
+			r.AddOK("added tag %q", t)
 		}
 		for _, t := range sp.rmTags {
-			r.AddOK("removed tag %q", strings.ToLower(t))
+			t = strings.TrimSpace(t)
+			r.AddOK("removed tag %q", t)
 		}
 	}
 	return nil
diff --git a/go.mod b/go.mod
index 74eb257..4909bc7 100644
--- a/go.mod
+++ b/go.mod
@@ -9,7 +9,7 @@ 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.2
-	github.com/nats-io/jwt/v2 v2.6.0
+	github.com/nats-io/jwt/v2 v2.6.1-0.20240917143920-d3fa85bd725f
 	github.com/nats-io/nats-server/v2 v2.10.18
 	github.com/nats-io/nats.go v1.37.0
 	github.com/nats-io/nkeys v0.4.7
diff --git a/go.sum b/go.sum
index 804d24d..85363af 100644
--- a/go.sum
+++ b/go.sum
@@ -69,8 +69,8 @@ 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.2 h1:T4Fq88a03sPAPWYwrOLQ85oanYsC2Bs6517rUiWBMpQ=
 github.com/nats-io/jsm.go v0.1.2/go.mod h1:tnubE70CAKi5TNfQiq6XHFqWTuSIe1H7X4sDwfq6ZK8=
-github.com/nats-io/jwt/v2 v2.6.0 h1:yXoBTdEotZw3NujMT+Nnu1UPNlFWdKQ3d0JJF/+pJag=
-github.com/nats-io/jwt/v2 v2.6.0/go.mod h1:ZdWS1nZa6WMZfFwwgpEaqBV8EPGVgOTDHN/wTbz0Y5A=
+github.com/nats-io/jwt/v2 v2.6.1-0.20240917143920-d3fa85bd725f h1:izYA+fDsvJ71/nlCRmaDFPEhVpmumWx0aSNE00b+00I=
+github.com/nats-io/jwt/v2 v2.6.1-0.20240917143920-d3fa85bd725f/go.mod h1:ZdWS1nZa6WMZfFwwgpEaqBV8EPGVgOTDHN/wTbz0Y5A=
 github.com/nats-io/nats-server/v2 v2.10.18 h1:tRdZmBuWKVAFYtayqlBB2BuCHNGAQPvoQIXOKwU3WSM=
 github.com/nats-io/nats-server/v2 v2.10.18/go.mod h1:97Qyg7YydD8blKlR8yBsUlPlWyZKjA7Bp5cl3MUE9K8=
 github.com/nats-io/nats.go v1.37.0 h1:07rauXbVnnJvv1gfIyghFEo6lUcYRY0WXc3x7x0vUxE=


Test patch:
diff --git a/cmd/editaccount_test.go b/cmd/editaccount_test.go
index 5bb8464..057e036 100644
--- a/cmd/editaccount_test.go
+++ b/cmd/editaccount_test.go
@@ -37,7 +37,7 @@ func Test_EditAccount(t *testing.T) {
 		{createEditAccount(), []string{"edit", "account"}, nil, []string{"specify an edit option"}, true},
 		{createEditAccount(), []string{"edit", "account", "--info-url", "http://foo/bar"}, nil, []string{"changed info url to"}, false},
 		{createEditAccount(), []string{"edit", "account", "--description", "my account is about this"}, nil, []string{"changed description to"}, false},
-		{createEditAccount(), []string{"edit", "account", "--tag", "A", "--name", "A"}, nil, []string{"edited account \"A\""}, false},
+		{createEditAccount(), []string{"edit", "account", "--tag", "a", "--name", "A"}, nil, []string{"edited account \"A\""}, false},
 	}
 
 	tests.Run(t, "root", "edit")
@@ -60,7 +60,7 @@ func Test_EditAccount_Tag(t *testing.T) {
 	defer ts.Done(t)
 
 	ts.AddAccount(t, "A")
-	_, _, err := ExecuteCmd(createEditAccount(), "--tag", "A,B,C")
+	_, _, err := ExecuteCmd(createEditAccount(), "--tag", "a,b,c")
 	require.NoError(t, err)
 
 	ac, err := ts.Store.ReadAccountClaim("A")
@@ -75,17 +75,17 @@ func Test_EditAccount_RmTag(t *testing.T) {
 	defer ts.Done(t)
 
 	ts.AddAccount(t, "A")
-	_, _, err := ExecuteCmd(createEditAccount(), "--tag", "A,B,C")
+	_, _, err := ExecuteCmd(createEditAccount(), "--tag", "A,B,C", "--strict-tags")
 	require.NoError(t, err)
 
-	_, _, err = ExecuteCmd(createEditAccount(), "--rm-tag", "A,B")
+	_, _, err = ExecuteCmd(createEditAccount(), "--rm-tag", "A,B", "--strict-tags")
 	require.NoError(t, err)
 
 	ac, err := ts.Store.ReadAccountClaim("A")
 	require.NoError(t, err)
 
 	require.Len(t, ac.Tags, 1)
-	require.ElementsMatch(t, ac.Tags, []string{"c"})
+	require.ElementsMatch(t, ac.Tags, []string{"C"})
 }
 
 func Test_EditAccount_Times(t *testing.T) {
@@ -380,17 +380,17 @@ func Test_EditSysAccount(t *testing.T) {
 	for idx, n := range jsOptions {
 		flag := fmt.Sprintf("--%s", n)
 		if idx > 0 {
-			_, _, err = ExecuteCmd(createEditAccount(), "SYS", "--tag", "A", flag, "1")
+			_, _, err = ExecuteCmd(createEditAccount(), "SYS", "--tag", "a", flag, "1")
 			require.Error(t, err)
 			require.Contains(t, err.Error(), flag)
 		} else {
-			_, _, err = ExecuteCmd(createEditAccount(), "SYS", "--tag", "A", flag)
+			_, _, err = ExecuteCmd(createEditAccount(), "SYS", "--tag", "a", flag)
 			require.Error(t, err)
 			require.Contains(t, err.Error(), flag)
 		}
 	}
 	// defaults are removed automatically
-	_, _, err = ExecuteCmd(createEditAccount(), "SYS", "--tag", "A")
+	_, _, err = ExecuteCmd(createEditAccount(), "SYS", "--tag", "a")
 	require.NoError(t, err)
 }
 
@@ -524,3 +524,28 @@ func Test_EnableTierNoOtherFlag(t *testing.T) {
 	require.Error(t, err)
 	require.Equal(t, "rm-js-tier is exclusive of all other js options", err.Error())
 }
+
+func TestEditAccountStrictTags(t *testing.T) {
+	ts := NewTestStore(t, "O")
+	defer ts.Done(t)
+
+	ts.AddAccount(t, "A")
+
+	_, _, err := ExecuteCmd(createEditAccount(), "--tag", "a")
+	require.NoError(t, err)
+
+	_, _, err = ExecuteCmd(createEditAccount(), "--rm-tag", "A")
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "--rm-tag \"A\" is not lowercased")
+
+	_, _, err = ExecuteCmd(createEditAccount(), "--rm-tag", "A", "--strict-tags")
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "unable to remove tag: \"A\" - not found")
+
+	_, _, err = ExecuteCmd(createEditAccount(), "--tag", "A", "--strict-tags")
+	require.NoError(t, err)
+
+	uc, err := ts.Store.ReadAccountClaim("A")
+	require.NoError(t, err)
+	require.True(t, uc.Tags.Equals(&jwt.TagList{"A", "a"}))
+}
diff --git a/cmd/editoperator_test.go b/cmd/editoperator_test.go
index 9cfb35f..0ceb3bd 100644
--- a/cmd/editoperator_test.go
+++ b/cmd/editoperator_test.go
@@ -43,7 +43,7 @@ func Test_EditOperator(t *testing.T) {
 		{createEditOperatorCmd(), []string{"edit", "operator", "--sk"}, nil, []string{"flag needs an argument"}, true},
 		{createEditOperatorCmd(), []string{"edit", "operator", "--sk", "SAADOZRUTPZS6LIXS6CSSSW5GXY3DNMQMSDTVWHQNHQTIBPGNSADSMBPEU"}, nil, []string{"invalid operator signing key"}, true},
 		{createEditOperatorCmd(), []string{"edit", "operator", "--sk", "OBMWGGURAFWMH3AFDX65TVIH4ZYSL7UKZ3LOH2ZRWIAU7PGZ3IJNR6W5"}, nil, []string{"edited operator"}, false},
-		{createEditOperatorCmd(), []string{"edit", "operator", "--tag", "O", "--start", "2019-04-13", "--expiry", "2050-01-01"}, nil, []string{"edited operator"}, false},
+		{createEditOperatorCmd(), []string{"edit", "operator", "--tag", "o", "--start", "2019-04-13", "--expiry", "2050-01-01"}, nil, []string{"edited operator"}, false},
 		{createEditOperatorCmd(), []string{"edit", "operator", "--require-signing-keys"}, nil, []string{"needs to be issued with a signing key first"}, true},
 	}
 
@@ -420,3 +420,29 @@ func Test_CannotSetRequireSKWithoutSK(t *testing.T) {
 	require.False(t, oc.StrictSigningKeyUsage)
 	require.Empty(t, oc.SigningKeys)
 }
+
+func TestEditOperatorStrictTags(t *testing.T) {
+	ts := NewTestStore(t, "O")
+	defer ts.Done(t)
+
+	_, _, err := ExecuteCmd(createEditOperatorCmd(), "--tag", "A")
+	require.Error(t, err)
+
+	_, _, err = ExecuteCmd(createEditOperatorCmd(), "--tag", "a")
+	require.NoError(t, err)
+
+	_, _, err = ExecuteCmd(createEditOperatorCmd(), "--rm-tag", "A")
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "--rm-tag \"A\" is not lowercased")
+
+	_, _, err = ExecuteCmd(createEditOperatorCmd(), "--rm-tag", "A", "--strict-tags")
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "unable to remove tag: \"A\" - not found")
+
+	_, _, err = ExecuteCmd(createEditOperatorCmd(), "--tag", "A", "--strict-tags")
+	require.NoError(t, err)
+
+	oc, err := ts.Store.ReadOperatorClaim()
+	require.NoError(t, err)
+	require.True(t, oc.Tags.Equals(&jwt.TagList{"A", "a"}))
+}
diff --git a/cmd/edituser_test.go b/cmd/edituser_test.go
index 7a4b706..b86157b 100644
--- a/cmd/edituser_test.go
+++ b/cmd/edituser_test.go
@@ -16,11 +16,12 @@
 package cmd
 
 import (
-	"github.com/nats-io/nsc/v2/cmd/store"
 	"strings"
 	"testing"
 	"time"
 
+	"github.com/nats-io/nsc/v2/cmd/store"
+
 	"github.com/nats-io/nkeys"
 
 	cli "github.com/nats-io/cliprompts/v2"
@@ -38,12 +39,13 @@ func Test_EditUser(t *testing.T) {
 
 	tests := CmdTests{
 		{createEditUserCmd(), []string{"edit", "user"}, nil, []string{"specify an edit option"}, true},
-		{createEditUserCmd(), []string{"edit", "user", "--tag", "A", "--account", "A"}, nil, []string{"edited user \"a\""}, false},
+		{createEditUserCmd(), []string{"edit", "user", "--tag", "A", "--account", "A"}, nil, []string{"--tag \"A\" is not lowercased"}, true},
+		{createEditUserCmd(), []string{"edit", "user", "--tag", "a", "--account", "A"}, nil, []string{"edited user \"a\""}, false},
 		{createEditUserCmd(), []string{"edit", "user", "--conn-type", "MQTT", "--rm-conn-type", "LEAFNODE", "--account", "A"}, nil, []string{"added connection type MQTT", "added connection type MQTT"}, false},
 		{createEditUserCmd(), []string{"edit", "user", "--conn-type", "LEAFNODE_WS", "--account", "A"}, nil, []string{"added connection type LEAFNODE_WS"}, false},
 		{createEditUserCmd(), []string{"edit", "user", "--conn-type", "MQTT_WS", "--account", "A"}, nil, []string{"added connection type MQTT_WS"}, false},
-		{createEditUserCmd(), []string{"edit", "user", "--tag", "B", "--account", "B"}, nil, []string{"user name is required"}, true},
-		{createEditUserCmd(), []string{"edit", "user", "--tag", "B", "--account", "B", "--name", "bb"}, nil, []string{"edited user \"bb\""}, false},
+		{createEditUserCmd(), []string{"edit", "user", "--tag", "b", "--account", "B"}, nil, []string{"user name is required"}, true},
+		{createEditUserCmd(), []string{"edit", "user", "--tag", "b", "--account", "B", "--name", "bb"}, nil, []string{"edited user \"bb\""}, false},
 	}
 
 	tests.Run(t, "root", "edit")
@@ -107,7 +109,7 @@ func Test_EditUser_Tag(t *testing.T) {
 	defer ts.Done(t)
 
 	ts.AddUser(t, "A", "a")
-	_, _, err := ExecuteCmd(createEditUserCmd(), "--tag", "A,B,C")
+	_, _, err := ExecuteCmd(createEditUserCmd(), "--tag", "A,B,C", "--strict-tags")
 	require.NoError(t, err)
 
 	cc, err := ts.Store.ReadUserClaim("A", "a")
@@ -115,9 +117,9 @@ func Test_EditUser_Tag(t *testing.T) {
 	require.NotNil(t, cc)
 
 	require.Len(t, cc.Tags, 3)
-	require.ElementsMatch(t, cc.Tags, []string{"a", "b", "c"})
+	require.ElementsMatch(t, cc.Tags, []string{"A", "B", "C"})
 
-	_, _, err = ExecuteCmd(createEditUserCmd(), "--rm-tag", "A,B")
+	_, _, err = ExecuteCmd(createEditUserCmd(), "--rm-tag", "A,B", "--strict-tags")
 	require.NoError(t, err)
 
 	cc, err = ts.Store.ReadUserClaim("A", "a")
@@ -125,7 +127,7 @@ func Test_EditUser_Tag(t *testing.T) {
 	require.NotNil(t, cc)
 
 	require.Len(t, cc.Tags, 1)
-	require.ElementsMatch(t, cc.Tags, []string{"c"})
+	require.ElementsMatch(t, cc.Tags, []string{"C"})
 
 }
 
@@ -595,3 +597,29 @@ func Test_EditUserConnectionDeleteCase(t *testing.T) {
 	require.NoError(t, err)
 	require.Len(t, claim.AllowedConnectionTypes, 0)
 }
+
+func TestEditUserStrictTags(t *testing.T) {
+	ts := NewTestStore(t, "O")
+	defer ts.Done(t)
+
+	ts.AddAccount(t, "A")
+	ts.AddUser(t, "A", "U")
+
+	_, _, err := ExecuteCmd(createEditUserCmd(), "--tag", "a")
+	require.NoError(t, err)
+
+	_, _, err = ExecuteCmd(createEditUserCmd(), "--rm-tag", "A")
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "--rm-tag \"A\" is not lowercased")
+
+	_, _, err = ExecuteCmd(createEditUserCmd(), "--rm-tag", "A", "--strict-tags")
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "unable to remove tag: \"A\" - not found")
+
+	_, _, err = ExecuteCmd(createEditUserCmd(), "--tag", "A", "--strict-tags")
+	require.NoError(t, err)
+
+	uc, err := ts.Store.ReadUserClaim("A", "U")
+	require.NoError(t, err)
+	require.True(t, uc.Tags.Equals(&jwt.TagList{"A", "a"}))
+}
diff --git a/cmd/push_test.go b/cmd/push_test.go
index 07c3fab..8dbf503 100644
--- a/cmd/push_test.go
+++ b/cmd/push_test.go
@@ -40,7 +40,7 @@ func Test_SyncOK(t *testing.T) {
 	ts.AddAccount(t, "A")
 
 	// edit the jwt
-	_, _, err = ExecuteCmd(createEditAccount(), "--tag", "A")
+	_, _, err = ExecuteCmd(createEditAccount(), "--tag", "A", "--strict-tags")
 	require.NoError(t, err)
 
 	// sync the store
@@ -50,7 +50,8 @@ func Test_SyncOK(t *testing.T) {
 	// verify the tag was stored
 	ac, err := ts.Store.ReadAccountClaim("A")
 	require.NoError(t, err)
-	require.Contains(t, ac.Tags, "a")
+	require.Contains(t, ac.Tags, "A")
+	require.True(t, ac.Tags.Contains("A"))
 }
 
 func Test_SyncNoURL(t *testing.T) {
@@ -120,7 +121,7 @@ func Test_SyncManualServer(t *testing.T) {
 	ts.AddAccount(t, "A")
 
 	// edit the jwt
-	_, _, err = ExecuteCmd(createEditAccount(), "--tag", "A")
+	_, _, err = ExecuteCmd(createEditAccount(), "--tag", "a")
 	require.NoError(t, err)
 
 	// sync the store