added additional scrutiny to DidSign in the case of signing keys (#190)

- [FIX] fixed the DidSign for account claims to return true only if the signing key and the issuer account constraints met - this api is only used by nsc
- [TEST] fixed test that used improper user JWT
- [TEST] fixed tests that expected failure, but were out of order with a following test that now triggers the error

.github/workflows/go-test.yaml

@@ -1,4 +1,4 @@
-name: nsc testing
+name: jwt testing
 on: [push, pull_request]
 
 jobs:

account_claims.go

@@ -1,5 +1,5 @@
 /*
- * Copyright 2018-2020 The NATS Authors
+ * Copyright 2018-2023 The NATS Authors
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
@@ -178,13 +178,20 @@ func (a *AccountClaims) Claims() *ClaimsData {
 }
 
 // DidSign checks the claims against the account's public key and its signing keys
-func (a *AccountClaims) DidSign(op Claims) bool {
-	if op != nil {
-		issuer := op.Claims().Issuer
+func (a *AccountClaims) DidSign(c Claims) bool {
+	if c != nil {
+		issuer := c.Claims().Issuer
 		if issuer == a.Subject {
 			return true
 		}
-		return a.SigningKeys.Contains(issuer)
+		uc, ok := c.(*UserClaims)
+		if ok && uc.IssuerAccount == a.Subject {
+			return a.SigningKeys.Contains(issuer)
+		}
+		at, ok := c.(*ActivationClaims)
+		if ok && at.IssuerAccount == a.Subject {
+			return a.SigningKeys.Contains(issuer)
+		}
 	}
 	return false
 }

account_claims_test.go

@@ -1,5 +1,5 @@
 /*
- * Copyright 2018 The NATS Authors
+ * Copyright 2018-2023 The NATS Authors
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
@@ -428,6 +428,7 @@ func TestAccountSignedBy(t *testing.T) {
 
 	// claim signed by alternate key
 	uc := NewUserClaims(upk)
+	uc.IssuerAccount = apk1
 	utoken, err := uc.Encode(akp2)
 	if err != nil {
 		t.Fatal(err)
@@ -559,3 +560,40 @@ func TestUserRevocationAll(t *testing.T) {
 		t.Error("foo should have not been revoked")
 	}
 }
+
+func TestAccountClaims_DidSign(t *testing.T) {
+	akp := createAccountNKey(t)
+	apk := publicKey(akp, t)
+	skp := createAccountNKey(t)
+	spk := publicKey(skp, t)
+
+	ac := NewAccountClaims(apk)
+	ac.SigningKeys.Add(spk)
+
+	upk := publicKey(createUserNKey(t), t)
+	uc := NewUserClaims(upk)
+	tok, err := uc.Encode(akp)
+	if err != nil {
+		t.Fatal("error encoding")
+	}
+	uc, err = DecodeUserClaims(tok)
+	if err != nil {
+		t.Fatal("error decoding")
+	}
+	if !ac.DidSign(uc) {
+		t.Fatal("expected account to have been issued")
+	}
+	uc = NewUserClaims(upk)
+	uc.IssuerAccount = publicKey(createAccountNKey(t), t)
+	tok, err = uc.Encode(skp)
+	if err != nil {
+		t.Fatalf("encode failed %v", err)
+	}
+	uc, err = DecodeUserClaims(tok)
+	if err != nil {
+		t.Fatalf("decode failed %v", err)
+	}
+	if ac.DidSign(uc) {
+		t.Fatal("this is not issued by account A")
+	}
+}

activation_claims_test.go

@@ -393,15 +393,15 @@ func TestActivationClaimAccountIDValidation(t *testing.T) {
 		t.Fatal("expected activation account id to be different")
 	}
 
+	if !account.DidSign(ac) {
+		t.Fatal("expected account to have signed activation")
+	}
+
 	ac.IssuerAccount = publicKey(createUserNKey(t), t)
 	ac.Validate(&vr)
 	if len(vr.Issues) != 1 {
 		t.Fatal("expected validation error")
 	}
-
-	if !account.DidSign(ac) {
-		t.Fatal("expected account to have signed activation")
-	}
 }
 
 func TestCleanSubject(t *testing.T) {

v2/account_claims.go

@@ -1,5 +1,5 @@
 /*
- * Copyright 2018-2020 The NATS Authors
+ * Copyright 2018-2023 The NATS Authors
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
@@ -355,13 +355,20 @@ func (a *AccountClaims) Claims() *ClaimsData {
 }
 
 // DidSign checks the claims against the account's public key and its signing keys
-func (a *AccountClaims) DidSign(uc Claims) bool {
-	if uc != nil {
-		issuer := uc.Claims().Issuer
+func (a *AccountClaims) DidSign(c Claims) bool {
+	if c != nil {
+		issuer := c.Claims().Issuer
 		if issuer == a.Subject {
 			return true
 		}
-		return a.SigningKeys.Contains(issuer)
+		uc, ok := c.(*UserClaims)
+		if ok && uc.IssuerAccount == a.Subject {
+			return a.SigningKeys.Contains(issuer)
+		}
+		at, ok := c.(*ActivationClaims)
+		if ok && at.IssuerAccount == a.Subject {
+			return a.SigningKeys.Contains(issuer)
+		}
 	}
 	return false
 }

v2/account_claims_test.go

@@ -1,5 +1,5 @@
 /*
- * Copyright 2018-2020 The NATS Authors
+ * Copyright 2018-2023 The NATS Authors
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
@@ -472,6 +472,7 @@ func TestAccountSignedBy(t *testing.T) {
 
 	// claim signed by alternate key
 	uc := NewUserClaims(upk)
+	uc.IssuerAccount = apk1
 	utoken, err := uc.Encode(akp2)
 	if err != nil {
 		t.Fatal(err)
@@ -761,3 +762,40 @@ func TestAccountExternalAuthorizationRequiresOneUser(t *testing.T) {
 		vr.Errors()[0].Error(),
 		t)
 }
+
+func TestAccountClaims_DidSign(t *testing.T) {
+	akp := createAccountNKey(t)
+	apk := publicKey(akp, t)
+	skp := createAccountNKey(t)
+	spk := publicKey(skp, t)
+
+	ac := NewAccountClaims(apk)
+	ac.SigningKeys.Add(spk)
+
+	upk := publicKey(createUserNKey(t), t)
+	uc := NewUserClaims(upk)
+	tok, err := uc.Encode(akp)
+	if err != nil {
+		t.Fatal("error encoding")
+	}
+	uc, err = DecodeUserClaims(tok)
+	if err != nil {
+		t.Fatal("error decoding")
+	}
+	if !ac.DidSign(uc) {
+		t.Fatal("expected account to have been issued")
+	}
+	uc = NewUserClaims(upk)
+	uc.IssuerAccount = publicKey(createAccountNKey(t), t)
+	tok, err = uc.Encode(skp)
+	if err != nil {
+		t.Fatalf("encode failed %v", err)
+	}
+	uc, err = DecodeUserClaims(tok)
+	if err != nil {
+		t.Fatalf("decode failed %v", err)
+	}
+	if ac.DidSign(uc) {
+		t.Fatal("this is not issued by account A")
+	}
+}

v2/activation_claims_test.go

@@ -331,16 +331,15 @@ func TestActivationClaimAccountIDValidation(t *testing.T) {
 	if ac.IssuerAccount != issuerAccountPK {
 		t.Fatal("expected activation account id to be different")
 	}
+	if !account.DidSign(ac) {
+		t.Fatal("expected account to have signed activation")
+	}
 
 	ac.IssuerAccount = publicKey(createUserNKey(t), t)
 	ac.Validate(&vr)
 	if len(vr.Issues) != 1 {
 		t.Fatal("expected validation error")
 	}
-
-	if !account.DidSign(ac) {
-		t.Fatal("expected account to have signed activation")
-	}
 }
 
 func TestCleanSubject(t *testing.T) {