Repository: nats-io/nsc
Pull request #665 (2024-09-11 07:16:36)

[FIX] edit signing-key was unable to remove a connection type when in lowercase
This change ensures that connection types are uppercased, and autofixes any type that is lowercase in an existing scoped signing-key.

In reference to #664 

Please review carefully, this is my first contribution to `nsc`.

maxarndt: @aricart thanks for the review! I rebased my branch. You can kick-off the pipelines again.

Patch:
diff --git a/cmd/editscopedsk.go b/cmd/editscopedsk.go
index 5ebb3b6..f7e0362 100644
--- a/cmd/editscopedsk.go
+++ b/cmd/editscopedsk.go
@@ -17,6 +17,7 @@ package cmd
 
 import (
 	"fmt"
+	"strings"
 
 	"github.com/nats-io/jwt/v2"
 	"github.com/nats-io/nkeys"
@@ -70,6 +71,11 @@ func (p *EditScopedSkParams) SetDefaults(ctx ActionCtx) error {
 	}
 	p.SignerParams.SetDefaults(nkeys.PrefixByteOperator, true, ctx)
 
+	// allow the user to enter inputs in lc
+	for i, v := range p.connTypes {
+		p.connTypes[i] = strings.ToUpper(v)
+	}
+
 	return nil
 }
 
@@ -153,6 +159,12 @@ func (p *EditScopedSkParams) Load(ctx ActionCtx) error {
 	if s == nil {
 		s = &jwt.UserScope{}
 	}
+
+	// if the signing key has an allowed connection type in lowercase fix it
+	for i, v := range s.(*jwt.UserScope).Template.AllowedConnectionTypes {
+		s.(*jwt.UserScope).Template.AllowedConnectionTypes[i] = strings.ToUpper(v)
+	}
+
 	return p.UserPermissionLimits.Load(ctx, s.(*jwt.UserScope).Template)
 }
 
@@ -161,7 +173,9 @@ func (p *EditScopedSkParams) PostInteractive(ctx ActionCtx) error {
 }
 
 func (p *EditScopedSkParams) Validate(ctx ActionCtx) error {
-	p.UserPermissionLimits.Validate(ctx)
+	if err := p.UserPermissionLimits.Validate(ctx); err != nil {
+		return err
+	}
 
 	if err := p.SignerParams.ResolveWithPriority(ctx, p.claim.Issuer); err != nil {
 		return err
diff --git a/go.mod b/go.mod
index 0ce8927..74eb257 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.5.8
+	github.com/nats-io/jwt/v2 v2.6.0
 	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
@@ -19,7 +19,7 @@ require (
 	github.com/spf13/pflag v1.0.5
 	github.com/stretchr/testify v1.9.0
 	github.com/xlab/tablewriter v0.0.0-20160610135559-80b567a11ad5
-	golang.org/x/text v0.17.0
+	golang.org/x/text v0.18.0
 )
 
 require (
@@ -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.26.0 // indirect
-	golang.org/x/net v0.28.0 // indirect
-	golang.org/x/oauth2 v0.22.0 // indirect
-	golang.org/x/sys v0.24.0 // indirect
-	golang.org/x/term v0.23.0 // indirect
+	golang.org/x/crypto v0.27.0 // indirect
+	golang.org/x/net v0.29.0 // indirect
+	golang.org/x/oauth2 v0.23.0 // indirect
+	golang.org/x/sys v0.25.0 // indirect
+	golang.org/x/term v0.24.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 4f1c7ad..804d24d 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.5.8 h1:uvdSzwWiEGWGXf+0Q+70qv6AQdvcvxrv9hPM0RiPamE=
-github.com/nats-io/jwt/v2 v2.5.8/go.mod h1:ZdWS1nZa6WMZfFwwgpEaqBV8EPGVgOTDHN/wTbz0Y5A=
+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/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=
@@ -110,8 +110,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.26.0 h1:RrRspgV4mU+YwB4FYnuBoKsUapNIL5cohGAmSH3azsw=
-golang.org/x/crypto v0.26.0/go.mod h1:GY7jblb9wI+FOo5y8/S2oY4zWP07AkOJ4+jxCqdqn54=
+golang.org/x/crypto v0.27.0 h1:GXm2NjJrPaiv/h1tb2UH8QfgC/hOf/+z0p6PT8o1w7A=
+golang.org/x/crypto v0.27.0/go.mod h1:1Xngt8kV6Dvbssa53Ziq6Eqn0HqbZi5Z6R0ZpwQzt70=
 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=
@@ -120,12 +120,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.28.0 h1:a9JDOJc5GMUJ0+UDqmLT86WiEy7iWyIhz8gz8E4e5hE=
-golang.org/x/net v0.28.0/go.mod h1:yqtgsTWOOnlGLG9GFRrK3++bGOUEkNBoHZc8MEDWPNg=
+golang.org/x/net v0.29.0 h1:5ORfpBpCs4HzDYoodCDBbwHzdR5UrLBZ3sOnUJmFoHo=
+golang.org/x/net v0.29.0/go.mod h1:gLkgy8jTGERgjzMic6DS9+SP0ajcu6Xu3Orq/SpETg0=
 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.22.0 h1:BzDx2FehcG7jJwgWLELCdmLuxk2i+x9UDpSiss2u0ZA=
-golang.org/x/oauth2 v0.22.0/go.mod h1:XYTD2NtWslqkgxebSiOHnXEap4TF09sJSc7H1sXbhtI=
+golang.org/x/oauth2 v0.23.0 h1:PbgcYx2W7i4LvjJWEbf0ngHV6qJYr86PkAV3bXdLEbs=
+golang.org/x/oauth2 v0.23.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=
@@ -142,20 +142,20 @@ golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBc
 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.21.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
-golang.org/x/sys v0.24.0 h1:Twjiwq9dn6R1fQcyiK+wQyHWfaz/BJB+YIpzU/Cv3Xg=
-golang.org/x/sys v0.24.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
+golang.org/x/sys v0.25.0 h1:r+8e+loiHxRqhXVl6ML1nO3l1+oFoWbnlu2Ehimmi34=
+golang.org/x/sys v0.25.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.23.0 h1:F6D4vR+EHoL9/sWAWgAR1H2DcHr4PareCbAaCo1RpuU=
-golang.org/x/term v0.23.0/go.mod h1:DgV24QBUrK6jhZXl+20l6UWznPlwAHm1Q1mGHtydmSk=
+golang.org/x/term v0.24.0 h1:Mh5cbb+Zk2hqqXNO7S1iTjEphVL+jb8ZWaqh/g+JWkM=
+golang.org/x/term v0.24.0/go.mod h1:lOBK/LVxemqiMij05LGJ0tzNr8xlmwBRJ81PX6wVLH8=
 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.17.0 h1:XtiM5bkSOt+ewxlOE/aE/AKEHibwj/6gvWMl9Rsh0Qc=
-golang.org/x/text v0.17.0/go.mod h1:BuEKDfySbSR4drPmRPG/7iBdf8hvFMuRexcpahXilzY=
+golang.org/x/text v0.18.0 h1:XvMDiNzPAl0jr17s6W9lcaIhGUfUORdGCNsuLmPG224=
+golang.org/x/text v0.18.0/go.mod h1:BuEKDfySbSR4drPmRPG/7iBdf8hvFMuRexcpahXilzY=
 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/editscopedsk_test.go b/cmd/editscopedsk_test.go
index 61eff25..727ec2f 100644
--- a/cmd/editscopedsk_test.go
+++ b/cmd/editscopedsk_test.go
@@ -17,6 +17,7 @@ package cmd
 
 import (
 	"os"
+	"strings"
 	"testing"
 
 	"github.com/nats-io/jwt/v2"
@@ -221,3 +222,123 @@ func Test_EditScopedSkByRole(t *testing.T) {
 	require.Equal(t, us.Role, "foo")
 	require.Len(t, us.Template.Sub.Allow, 1)
 }
+
+func Test_EditScopedSkConnType(t *testing.T) {
+	ts := NewTestStore(t, "edit scope")
+	defer ts.Done(t)
+
+	_, err := ts.Store.ReadOperatorClaim()
+	require.NoError(t, err)
+
+	ts.AddAccount(t, "A")
+
+	// add the scope with a generate
+	_, _, err = ExecuteCmd(createEditSkopedSkCmd(), "--sk", "generate", "--role", "foo")
+	require.NoError(t, err)
+
+	// try to add invalid conn type
+	_, _, err = ExecuteCmd(createEditSkopedSkCmd(), "--sk", "foo", "--conn-type", "bar")
+	require.Error(t, err)
+
+	// add lower case conn type - this is prevented now, but worked in the past
+	ac, err := ts.Store.ReadAccountClaim("A")
+	require.NoError(t, err)
+	scope, ok := ac.SigningKeys.GetScope(ac.SigningKeys.Keys()[0])
+	require.True(t, ok)
+	scope.(*jwt.UserScope).Template.AllowedConnectionTypes.Add(strings.ToLower(jwt.ConnectionTypeStandard))
+	ac.SigningKeys.AddScopedSigner(scope)
+	token, err := ac.Encode(ts.OperatorKey)
+	require.NoError(t, err)
+	ts.Store.StoreClaim([]byte(token))
+	// test if lower case conn type was added correctly to the sk
+	ac, err = ts.Store.ReadAccountClaim("A")
+	require.NoError(t, err)
+	require.Len(t, ac.SigningKeys.Keys(), 1)
+	scope, ok = ac.SigningKeys.GetScope(ac.SigningKeys.Keys()[0])
+	require.True(t, ok)
+	us, ok := scope.(*jwt.UserScope)
+	require.True(t, ok)
+	require.NotNil(t, us)
+	require.Len(t, us.Template.AllowedConnectionTypes, 1)
+	require.Equal(t, strings.ToLower(jwt.ConnectionTypeStandard), us.Template.AllowedConnectionTypes[0])
+
+	// add lower case conn type - should be transformed upper case
+	_, _, err = ExecuteCmd(createEditSkopedSkCmd(), "--sk", "foo", "--conn-type", strings.ToLower(jwt.ConnectionTypeMqtt))
+	require.NoError(t, err)
+	ac, err = ts.Store.ReadAccountClaim("A")
+	require.NoError(t, err)
+	require.Len(t, ac.SigningKeys.Keys(), 1)
+	scope, ok = ac.SigningKeys.GetScope(ac.SigningKeys.Keys()[0])
+	require.True(t, ok)
+	us, ok = scope.(*jwt.UserScope)
+	require.True(t, ok)
+	require.NotNil(t, us)
+	require.Len(t, us.Template.AllowedConnectionTypes, 2)
+	require.Equal(t, jwt.ConnectionTypeMqtt, us.Template.AllowedConnectionTypes[1])
+
+	// test if the set above fixed the lower case conn type added before
+	require.Equal(t, jwt.ConnectionTypeStandard, us.Template.AllowedConnectionTypes[0])
+}
+
+func Test_EditScopedSkRmConnType(t *testing.T) {
+	ts := NewTestStore(t, "edit scope")
+	defer ts.Done(t)
+
+	_, err := ts.Store.ReadOperatorClaim()
+	require.NoError(t, err)
+
+	ts.AddAccount(t, "A")
+
+	// add the scope with a generate
+	_, _, err = ExecuteCmd(createEditSkopedSkCmd(), "--sk", "generate", "--role", "foo")
+	require.NoError(t, err)
+
+	// add lower case conn types - this is prevented now, but worked in the past
+	ac, err := ts.Store.ReadAccountClaim("A")
+	require.NoError(t, err)
+	scope, ok := ac.SigningKeys.GetScope(ac.SigningKeys.Keys()[0])
+	require.True(t, ok)
+	scope.(*jwt.UserScope).Template.AllowedConnectionTypes.Add(strings.ToLower(jwt.ConnectionTypeStandard))
+	scope.(*jwt.UserScope).Template.AllowedConnectionTypes.Add(strings.ToLower(jwt.ConnectionTypeWebsocket))
+	ac.SigningKeys.AddScopedSigner(scope)
+	token, err := ac.Encode(ts.OperatorKey)
+	require.NoError(t, err)
+	ts.Store.StoreClaim([]byte(token))
+	// test if lower case conn type was added correctly to the sk
+	ac, err = ts.Store.ReadAccountClaim("A")
+	require.NoError(t, err)
+	require.Len(t, ac.SigningKeys.Keys(), 1)
+	scope, ok = ac.SigningKeys.GetScope(ac.SigningKeys.Keys()[0])
+	require.True(t, ok)
+	us, ok := scope.(*jwt.UserScope)
+	require.True(t, ok)
+	require.NotNil(t, us)
+	require.Len(t, us.Template.AllowedConnectionTypes, 2)
+	require.Equal(t, strings.ToLower(jwt.ConnectionTypeStandard), us.Template.AllowedConnectionTypes[0])
+	require.Equal(t, strings.ToLower(jwt.ConnectionTypeWebsocket), us.Template.AllowedConnectionTypes[1])
+
+	// remove first conn type via lower cased input
+	_, _, err = ExecuteCmd(createEditSkopedSkCmd(), "--sk", "foo", "--rm-conn-type", strings.ToLower(jwt.ConnectionTypeStandard))
+	require.NoError(t, err)
+	ac, err = ts.Store.ReadAccountClaim("A")
+	require.NoError(t, err)
+	require.Len(t, ac.SigningKeys.Keys(), 1)
+	scope, ok = ac.SigningKeys.GetScope(ac.SigningKeys.Keys()[0])
+	require.True(t, ok)
+	us, ok = scope.(*jwt.UserScope)
+	require.True(t, ok)
+	require.NotNil(t, us)
+	require.Len(t, us.Template.AllowedConnectionTypes, 1)
+	// remove second conn type via upper cased input
+	_, _, err = ExecuteCmd(createEditSkopedSkCmd(), "--sk", "foo", "--rm-conn-type", jwt.ConnectionTypeWebsocket)
+	require.NoError(t, err)
+	ac, err = ts.Store.ReadAccountClaim("A")
+	require.NoError(t, err)
+	require.Len(t, ac.SigningKeys.Keys(), 1)
+	scope, ok = ac.SigningKeys.GetScope(ac.SigningKeys.Keys()[0])
+	require.True(t, ok)
+	us, ok = scope.(*jwt.UserScope)
+	require.True(t, ok)
+	require.NotNil(t, us)
+	require.Len(t, us.Template.AllowedConnectionTypes, 0)
+}