From 0cca9a485a2e79fd1b2d68db44b449a341c4743a Mon Sep 17 00:00:00 2001 From: Alberto Ricart Date: Wed, 11 Sep 2024 15:18:40 -0500 Subject: [PATCH 1/2] Added validation to cluster traffic - as downstream tools like nsc will need to perform some level of validation --- v2/account_claims.go | 36 +++++++++++++++++++++++++++++++++++- v2/account_claims_test.go | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/v2/account_claims.go b/v2/account_claims.go index e071a8c..00bbc8c 100644 --- a/v2/account_claims.go +++ b/v2/account_claims.go @@ -19,6 +19,7 @@ import ( "errors" "fmt" "sort" + "strings" "time" "github.com/nats-io/nkeys" @@ -230,6 +231,35 @@ func (ac *ExternalAuthorization) Validate(vr *ValidationResults) { } } +const ( + ClusterTrafficSystem = "system" + ClusterTrafficOwner = "owner" + ClusterTrafficOtherAccount = "account:" +) + +type ClusterTraffic string + +func (ct ClusterTraffic) Valid() error { + if ct == "" || ct == ClusterTrafficSystem || ct == ClusterTrafficOwner { + return nil + } + + if strings.HasPrefix(string(ct), ClusterTrafficOtherAccount) { + // so in JWT we would expect this to be an account ID + id := ct[len(ClusterTrafficOtherAccount):] + if !strings.HasPrefix(string(id), "A") { + return errors.New("cluster traffic should be an account public key") + } + _, err := nkeys.FromPublicKey(string(id)) + if err != nil { + return errors.New("cluster traffic is not a public account key") + } + } else { + return fmt.Errorf("unknown cluster traffic option: %q", ct) + } + return nil +} + // Account holds account specific claims data type Account struct { Imports Imports `json:"imports,omitempty"` @@ -241,7 +271,7 @@ type Account struct { Mappings Mapping `json:"mappings,omitempty"` Authorization ExternalAuthorization `json:"authorization,omitempty"` Trace *MsgTrace `json:"trace,omitempty"` - ClusterTraffic string `json:"cluster_traffic,omitempty"` + ClusterTraffic ClusterTraffic `json:"cluster_traffic,omitempty"` Info GenericFields } @@ -309,6 +339,10 @@ func (a *Account) Validate(acct *AccountClaims, vr *ValidationResults) { } a.SigningKeys.Validate(vr) a.Info.Validate(vr) + + if err := a.ClusterTraffic.Valid(); err != nil { + vr.AddError(err.Error()) + } } // AccountClaims defines the body of an account JWT diff --git a/v2/account_claims_test.go b/v2/account_claims_test.go index c3cfa5e..cb4dc67 100644 --- a/v2/account_claims_test.go +++ b/v2/account_claims_test.go @@ -990,3 +990,37 @@ func TestAccountClaimsTraceDestSampling(t *testing.T) { }) } } + +func TestClusterTraffic_Valid(t *testing.T) { + type clustertest struct { + input string + ok bool + } + + tests := []clustertest{ + {input: "", ok: true}, + {input: "system", ok: true}, + {input: "SYSTEM", ok: false}, + {input: "owner", ok: true}, + {input: "OWNER", ok: false}, + {input: "unknown", ok: false}, + {input: "account", ok: false}, + {input: "account:", ok: false}, + {input: "account:A", ok: false}, + {input: "account:B", ok: false}, + // seed - reject + {input: "account:SAAEVKMPCBXPP5JG5J4DWQQJTL6TJJE35UCTYON4E2AMPMHOVJPTUSWIZY", ok: false}, + {input: "account:ABDFLVEVLA2IOTEEP44IGMZE2SFRBNVCXH5DUGRQ36AUVB2I44TJTNIA", ok: true}, + } + + for _, test := range tests { + ct := ClusterTraffic(test.input) + err := ct.Valid() + if test.ok && err != nil { + t.Fatalf("unexpected err for input %q: %v", test.input, err) + } + if !test.ok && err == nil { + t.Fatalf("expected to fail input %q", test.input) + } + } +} From 97b12126d74ae3529a0513fdc2a9549931ae3958 Mon Sep 17 00:00:00 2001 From: Alberto Ricart Date: Fri, 13 Sep 2024 10:18:19 -0500 Subject: [PATCH 2/2] removed parsing of account for cluster traffic as that option will be delayed. --- v2/account_claims.go | 22 +++------------------- v2/account_claims_test.go | 6 ------ 2 files changed, 3 insertions(+), 25 deletions(-) diff --git a/v2/account_claims.go b/v2/account_claims.go index 00bbc8c..05850fc 100644 --- a/v2/account_claims.go +++ b/v2/account_claims.go @@ -19,7 +19,6 @@ import ( "errors" "fmt" "sort" - "strings" "time" "github.com/nats-io/nkeys" @@ -232,9 +231,8 @@ func (ac *ExternalAuthorization) Validate(vr *ValidationResults) { } const ( - ClusterTrafficSystem = "system" - ClusterTrafficOwner = "owner" - ClusterTrafficOtherAccount = "account:" + ClusterTrafficSystem = "system" + ClusterTrafficOwner = "owner" ) type ClusterTraffic string @@ -243,21 +241,7 @@ func (ct ClusterTraffic) Valid() error { if ct == "" || ct == ClusterTrafficSystem || ct == ClusterTrafficOwner { return nil } - - if strings.HasPrefix(string(ct), ClusterTrafficOtherAccount) { - // so in JWT we would expect this to be an account ID - id := ct[len(ClusterTrafficOtherAccount):] - if !strings.HasPrefix(string(id), "A") { - return errors.New("cluster traffic should be an account public key") - } - _, err := nkeys.FromPublicKey(string(id)) - if err != nil { - return errors.New("cluster traffic is not a public account key") - } - } else { - return fmt.Errorf("unknown cluster traffic option: %q", ct) - } - return nil + return fmt.Errorf("unknown cluster traffic option: %q", ct) } // Account holds account specific claims data diff --git a/v2/account_claims_test.go b/v2/account_claims_test.go index cb4dc67..5930313 100644 --- a/v2/account_claims_test.go +++ b/v2/account_claims_test.go @@ -1005,12 +1005,6 @@ func TestClusterTraffic_Valid(t *testing.T) { {input: "OWNER", ok: false}, {input: "unknown", ok: false}, {input: "account", ok: false}, - {input: "account:", ok: false}, - {input: "account:A", ok: false}, - {input: "account:B", ok: false}, - // seed - reject - {input: "account:SAAEVKMPCBXPP5JG5J4DWQQJTL6TJJE35UCTYON4E2AMPMHOVJPTUSWIZY", ok: false}, - {input: "account:ABDFLVEVLA2IOTEEP44IGMZE2SFRBNVCXH5DUGRQ36AUVB2I44TJTNIA", ok: true}, } for _, test := range tests {