Handle zero weights in cluster and non-cluster mappings, add new tests

v2/account_claims.go

@@ -16,6 +16,7 @@
 package jwt
 
 import (
+	"encoding/json"
 	"errors"
 	"fmt"
 	"sort"
@@ -136,14 +137,36 @@ func (o *OperatorLimits) Validate(vr *ValidationResults) {
 // WeightedMapping for publishes
 type WeightedMapping struct {
 	Subject Subject `json:"subject"`
-	Weight  uint8   `json:"weight,omitempty"`
+	Weight  uint8   `json:"weight"`
 	Cluster string  `json:"cluster,omitempty"`
 }
 
-func (m *WeightedMapping) GetWeight() uint8 {
-	if m.Weight == 0 {
-		return 100
+// UnmarshalJSON implements custom JSON unmarshalling for backward compatibility.
+// If weight field is missing (old JWTs), it defaults to 100 so existing JWTs that
+// omit the weight will continue to work properly
+func (m *WeightedMapping) UnmarshalJSON(data []byte) error {
+	temp := &struct {
+		Subject Subject `json:"subject"`
+		Weight  *uint8  `json:"weight"` // pointer to detect if field is present
+		Cluster string  `json:"cluster,omitempty"`
+	}{}
+	if err := json.Unmarshal(data, temp); err != nil {
+		return err
+	}
+	m.Subject = temp.Subject
+	m.Cluster = temp.Cluster
+	// if weight field is not present in JSON (old JWT), default to 100
+	if temp.Weight == nil {
+		m.Weight = 100
+	} else {
+		m.Weight = *temp.Weight
 	}
+	return nil
+}
+
+// GetWeight returns the weight value.
+// Deprecated: use Weight field directly.
+func (m *WeightedMapping) GetWeight() uint8 {
 	return m.Weight
 }
 
@@ -164,7 +187,7 @@ func (m *Mapping) Validate(vr *ValidationResults) {
 					vr.AddError("Mapping %q in cluster %q exceeds 100%% among all of it's weighted to mappings", ubFrom, e.Cluster)
 				}
 			} else {
-				total += e.GetWeight()
+				total += e.Weight
 			}
 		}
 		if total > 100 {

v2/account_claims_test.go

@@ -16,6 +16,7 @@
 package jwt
 
 import (
+	"encoding/json"
 	"fmt"
 	"strings"
 	"testing"
@@ -685,7 +686,7 @@ func TestAccountMapping(t *testing.T) { // don't block encoding!!!
 	vr = &ValidationResults{}
 	account.Mappings = Mapping{}
 	account.AddMapping("foo4",
-		WeightedMapping{Subject: "to1"}, // no weight means 100
+		WeightedMapping{Subject: "to1", Weight: 100},
 		WeightedMapping{Subject: "to2", Weight: 1})
 	account.Validate(vr)
 	if !vr.IsBlocking(false) {
@@ -743,6 +744,124 @@ func TestAccountClusterNoOver100Mapping(t *testing.T) { // don't block encoding!
 	}
 }
 
+func TestAccountClusterMappingWithZeroWeights(t *testing.T) {
+	akp := createAccountNKey(t)
+	apk := publicKey(akp, t)
+
+	account := NewAccountClaims(apk)
+	vr := &ValidationResults{}
+
+	// multiple mappings with weight 0 should be allowed and not count toward total
+	account.AddMapping("q",
+		WeightedMapping{Subject: "qq", Weight: 0, Cluster: "A"},   // 0 weight in cluster A
+		WeightedMapping{Subject: "bb", Weight: 100, Cluster: "A"}, // 100 weight in cluster A (total: 100)
+		WeightedMapping{Subject: "cc", Weight: 0, Cluster: "B"},   // 0 weight in cluster B
+		WeightedMapping{Subject: "dd", Weight: 0, Cluster: "B"},   // another 0 weight in cluster B
+		WeightedMapping{Subject: "ee", Weight: 50, Cluster: "B"},  // 50 weight in cluster B (total: 50)
+		WeightedMapping{Subject: "ff", Weight: 0},                 // 0 weight non-cluster
+		WeightedMapping{Subject: "gg", Weight: 50})                // 50 weight non-cluster (total: 50)
+	account.Validate(vr)
+	if !vr.IsEmpty() {
+		t.Fatal("Expected no errors")
+	}
+}
+
+func TestAccountMappingWith30And0Weights(t *testing.T) {
+	akp := createAccountNKey(t)
+	apk := publicKey(akp, t)
+
+	account := NewAccountClaims(apk)
+	vr := &ValidationResults{}
+
+	// weight 30 + weight 0 = 30, should be valid
+	account.AddMapping("q",
+		WeightedMapping{Subject: "qq", Weight: 30},
+		WeightedMapping{Subject: "bb", Weight: 0})
+	account.Validate(vr)
+	if !vr.IsEmpty() {
+		t.Fatal("Expected no errors")
+	}
+}
+
+func TestAccountMappingBackwardCompatibility(t *testing.T) {
+	// test that old JWTs without weight field get weight 100 on unmarshal
+	oldJWT := `{"subject":"hello", "to": "hi"}`
+
+	var m WeightedMapping
+	err := json.Unmarshal([]byte(oldJWT), &m)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// verify weight defaults to 100 for old JWTs without weight field
+	if m.Weight != 100 {
+		t.Fatalf("Expected weight 100 for old JWT without weight field, got: %d", m.Weight)
+	}
+
+	// test that new JWTs with weight 0 keep weight 0
+	newJWT := `{"subject":"hello", "to": "hi", "weight": 0}`
+	err = json.Unmarshal([]byte(newJWT), &m)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if m.Weight != 0 {
+		t.Fatalf("Expected weight 0 for new JWT with weight:0, got: %d", m.Weight)
+	}
+}
+
+func TestAccountMappingOldJWT(t *testing.T) {
+	// old JWT with mapping that doesn't have weight field
+	oldJWT := `eyJ0eXAiOiJKV1QiLCJhbGciOiJlZDI1NTE5LW5rZXkifQ.eyJqdGkiOiJCTFE3UllWSEs3QVZCN0gzRVgzRUpWRlNORTRPUEVUTkg1VlNZQkpVVTdUVTVCWkVDNjVRIiwiaWF0IjoxNzYzMjE2MDI5LCJpc3MiOiJPREo3Q0UyVklCWTdHM0dYVVZFTVFYR1ZRNlJSRlRPWFdaNEpOVzVBMklZTlNHNVk2VDRWNFNBUyIsIm5hbWUiOiJNIiwic3ViIjoiQUFYUVE3TFdQWUZGT1g1S0ZBNU02VjY3VVBOTzJKTUFKVTdKUVJRQkRDVjdRSUhGNllIUDU3Q0siLCJuYXRzIjp7ImxpbWl0cyI6eyJzdWJzIjotMSwiZGF0YSI6LTEsInBheWxvYWQiOi0xLCJpbXBvcnRzIjotMSwiZXhwb3J0cyI6LTEsIndpbGRjYXJkcyI6dHJ1ZSwiY29ubiI6LTEsImxlYWYiOi0xfSwiZGVmYXVsdF9wZXJtaXNzaW9ucyI6eyJwdWIiOnt9LCJzdWIiOnt9fSwibWFwcGluZ3MiOnsiYSI6W3sic3ViamVjdCI6ImIifV19LCJhdXRob3JpemF0aW9uIjp7fSwidHlwZSI6ImFjY291bnQiLCJ2ZXJzaW9uIjoyfX0.qFxSSQKqHxpl2qS21x1Yj8zqDufGLIp9Gncb-YBf3P-CYxB31Dtp5swSYOmsA8zEGYMdnynY7z_73LweHqedAg`
+
+	account, err := DecodeAccountClaims(oldJWT)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// verify mapping was loaded
+	if len(account.Mappings) != 1 {
+		t.Fatalf("Expected 1 mapping, got %d", len(account.Mappings))
+	}
+
+	// verify mapping "a" -> "b" exists and has weight 100 (default for old JWTs)
+	mappingsA, ok := account.Mappings["a"]
+	if !ok {
+		t.Fatal("Expected mapping 'a' to exist")
+	}
+	if len(mappingsA) != 1 {
+		t.Fatalf("Expected 1 mapping for 'a', got %d", len(mappingsA))
+	}
+	if mappingsA[0].Subject != "b" {
+		t.Fatalf("Expected subject 'b', got %s", mappingsA[0].Subject)
+	}
+	if mappingsA[0].Weight != 100 {
+		t.Fatalf("Expected weight 100 for old JWT mapping without weight field, got %d", mappingsA[0].Weight)
+	}
+
+	// validate should pass
+	vr := &ValidationResults{}
+	account.Validate(vr)
+	if !vr.IsEmpty() {
+		t.Fatalf("Expected no validation errors, got: %v", vr.Issues)
+	}
+}
+
+func TestSelfMapping(t *testing.T) {
+	akp := createAccountNKey(t)
+	apk := publicKey(akp, t)
+	account := NewAccountClaims(apk)
+	vr := &ValidationResults{}
+
+	// this is a self mapping - check there's no rejection of the sub
+	account.AddMapping("q",
+		WeightedMapping{Subject: "q", Weight: 0})
+	account.Validate(vr)
+	if !vr.IsEmpty() {
+		t.Fatal("Expected no errors")
+	}
+}
+
 func TestAccountExternalAuthorization(t *testing.T) {
 	akp := createAccountNKey(t)
 	apk := publicKey(akp, t)