From 31bb80fe267fdf62bbcce6ef6f0e63e2f3c6ea86 Mon Sep 17 00:00:00 2001 From: Max Arndt <8988283+maxarndt@users.noreply.github.com> Date: Wed, 11 Sep 2024 21:35:09 +0200 Subject: [PATCH] [FIX] edit signing-key was unable to remove a connection type when in lowercase (#665) fix comments --- cmd/editscopedsk.go | 16 +++++- cmd/editscopedsk_test.go | 121 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 1 deletion(-) diff --git a/cmd/editscopedsk.go b/cmd/editscopedsk.go index 5ebb3b6a..f7e03622 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/cmd/editscopedsk_test.go b/cmd/editscopedsk_test.go index 61eff253..727ec2fb 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) +}