fixes for PR comments, MaxControlLine update for tests

Signed-off-by: Derek Collison <derek@nats.io>

server/accounts.go

@@ -330,7 +330,7 @@ func (a *Account) pruneAutoExpireResponseMaps() {
 	}
 }
 
-// AddStreamImport will add in the stream import from a specific account with optional token.
+// AddStreamImportWithClaim will add in the stream import from a specific account with optional token.
 func (a *Account) AddStreamImportWithClaim(account *Account, from, prefix string, imClaim *jwt.Import) error {
 	if account == nil {
 		return ErrMissingAccount
@@ -411,7 +411,7 @@ func (a *Account) checkStreamImportAuthorizedNoLock(account *Account, subject st
 			return true
 		}
 		// Check if token required
-		if ea != nil && ea.tokenReq {
+		if ea.tokenReq {
 			return a.checkActivation(account, imClaim, true)
 		}
 		// If we have a matching account we are authorized
@@ -429,7 +429,7 @@ func (a *Account) checkStreamImportAuthorizedNoLock(account *Account, subject st
 				return true
 			}
 			// Check if token required
-			if ea != nil && ea.tokenReq {
+			if ea.tokenReq {
 				return a.checkActivation(account, imClaim, true)
 			}
 			_, ok := ea.approved[account.Name]
@@ -465,11 +465,12 @@ func (a *Account) activationExpired(subject string) {
 	}
 	// FIXME(dlc) - check services too?
 	si := a.imports.streams[subject]
-	a.mu.RUnlock()
-
 	if si == nil || si.invalid {
+		a.mu.RUnlock()
 		return
 	}
+	a.mu.RUnlock()
+
 	if si.acc.checkActivation(a, si.claim, false) {
 		// The token has been updated most likely and we are good to go.
 		return
@@ -669,9 +670,9 @@ func (s *Server) SetAccountResolver(ar AccountResolver) {
 	s.mu.Unlock()
 }
 
-// UpdateAccountClaims will update and existing account with new claims.
+// updateAccountClaims will update and existing account with new claims.
 // This will replace any exports or imports previously defined.
-func (s *Server) UpdateAccountClaims(a *Account, ac *jwt.AccountClaims) {
+func (s *Server) updateAccountClaims(a *Account, ac *jwt.AccountClaims) {
 	if a == nil {
 		return
 	}
@@ -756,7 +757,7 @@ func (s *Server) UpdateAccountClaims(a *Account, ac *jwt.AccountClaims) {
 // Helper to build an internal account structure from a jwt.AccountClaims.
 func (s *Server) buildInternalAccount(ac *jwt.AccountClaims) *Account {
 	acc := &Account{Name: ac.Subject, Issuer: ac.Issuer}
-	s.UpdateAccountClaims(acc, ac)
+	s.updateAccountClaims(acc, ac)
 	return acc
 }
 

server/client.go

@@ -1031,11 +1031,7 @@ func (c *client) authViolation() {
 		s.mu.Unlock()
 	}
 	if hasTrustedNkeys {
-		if c.opts.JWT != "" {
-			c.Errorf("%v", ErrAuthentication)
-		} else {
-			c.Errorf("%v", ErrAuthentication)
-		}
+		c.Errorf("%v", ErrAuthentication)
 	} else if hasNkeys {
 		c.Errorf("%s - Nkey %q",
 			ErrAuthentication.Error(),

server/const.go

@@ -34,8 +34,7 @@ const (
 var (
 	// gitCommit injected at build
 	gitCommit string
-	// trustedNkeys is a whitespace  separated array of
-	// trusted operator public nkeys.
+	// trustedNkeys is a whitespace separated array of trusted operator's public nkeys.
 	trustedNkeys string
 )
 

server/jwt_test.go

@@ -151,6 +151,7 @@ func TestJWTUserBadTrusted(t *testing.T) {
 	}
 }
 
+// Test that if a user tries to connect with an expired user JWT we do the right thing.
 func TestJWTUserExpired(t *testing.T) {
 	// Create a new user that we will make sure has expired.
 	nkp, _ := nkeys.CreateUser()
@@ -482,7 +483,7 @@ func TestJWTAccountRenew(t *testing.T) {
 	if acc == nil {
 		t.Fatalf("Expected to retrive the account")
 	}
-	s.UpdateAccountClaims(acc, nac)
+	s.updateAccountClaims(acc, nac)
 
 	// Now make sure we can connect.
 	c, cr, l = newClientForServer(s)
@@ -669,7 +670,7 @@ func TestJWTAccountBasicImportExport(t *testing.T) {
 	}
 	addAccountToMemResolver(s, string(barPub), barJWT)
 
-	s.UpdateAccountClaims(acc, barAC)
+	s.updateAccountClaims(acc, barAC)
 
 	// Our service import should have failed with a bad token.
 	if les := len(acc.imports.services); les != 0 {
@@ -694,7 +695,7 @@ func TestJWTAccountBasicImportExport(t *testing.T) {
 		t.Fatalf("Error generating account JWT: %v", err)
 	}
 	addAccountToMemResolver(s, string(barPub), barJWT)
-	s.UpdateAccountClaims(acc, barAC)
+	s.updateAccountClaims(acc, barAC)
 	// Our service import should have succeeded.
 	if les := len(acc.imports.services); les != 1 {
 		t.Fatalf("Expected imports services len of 1, got %d", les)
@@ -724,7 +725,7 @@ func TestJWTAccountBasicImportExport(t *testing.T) {
 		t.Fatalf("Error generating account JWT: %v", err)
 	}
 	addAccountToMemResolver(s, string(barPub), barJWT)
-	s.UpdateAccountClaims(acc, barAC)
+	s.updateAccountClaims(acc, barAC)
 	// Our service import should have succeeded. Should be the only one since we reset.
 	if les := len(acc.imports.services); les != 1 {
 		t.Fatalf("Expected imports services len of 1, got %d", les)
@@ -740,7 +741,7 @@ func TestJWTAccountBasicImportExport(t *testing.T) {
 		t.Fatalf("Error generating account JWT: %v", err)
 	}
 	addAccountToMemResolver(s, string(barPub), barJWT)
-	s.UpdateAccountClaims(acc, barAC)
+	s.updateAccountClaims(acc, barAC)
 	// Our stream import should have not succeeded.
 	if les := len(acc.imports.streams); les != 0 {
 		t.Fatalf("Expected imports services len of 0, got %d", les)
@@ -764,7 +765,7 @@ func TestJWTAccountBasicImportExport(t *testing.T) {
 		t.Fatalf("Error generating account JWT: %v", err)
 	}
 	addAccountToMemResolver(s, string(barPub), barJWT)
-	s.UpdateAccountClaims(acc, barAC)
+	s.updateAccountClaims(acc, barAC)
 	// Our stream import should have not succeeded.
 	if les := len(acc.imports.streams); les != 1 {
 		t.Fatalf("Expected imports services len of 1, got %d", les)
@@ -858,7 +859,7 @@ func TestJWTAccountImportExportUpdates(t *testing.T) {
 	}
 	addAccountToMemResolver(s, string(barPub), barJWT)
 	acc := s.LookupAccount(string(barPub))
-	s.UpdateAccountClaims(acc, barAC)
+	s.updateAccountClaims(acc, barAC)
 
 	checkShadow(0)
 
@@ -870,7 +871,7 @@ func TestJWTAccountImportExportUpdates(t *testing.T) {
 		t.Fatalf("Error generating account JWT: %v", err)
 	}
 	addAccountToMemResolver(s, string(barPub), barJWT)
-	s.UpdateAccountClaims(acc, barAC)
+	s.updateAccountClaims(acc, barAC)
 
 	checkShadow(1)
 
@@ -881,7 +882,7 @@ func TestJWTAccountImportExportUpdates(t *testing.T) {
 		t.Fatalf("Error generating account JWT: %v", err)
 	}
 	addAccountToMemResolver(s, string(fooPub), fooJWT)
-	s.UpdateAccountClaims(s.LookupAccount(string(fooPub)), fooAC)
+	s.updateAccountClaims(s.LookupAccount(string(fooPub)), fooAC)
 
 	checkShadow(0)
 
@@ -893,7 +894,7 @@ func TestJWTAccountImportExportUpdates(t *testing.T) {
 		t.Fatalf("Error generating account JWT: %v", err)
 	}
 	addAccountToMemResolver(s, string(fooPub), fooJWT)
-	s.UpdateAccountClaims(s.LookupAccount(string(fooPub)), fooAC)
+	s.updateAccountClaims(s.LookupAccount(string(fooPub)), fooAC)
 
 	checkShadow(0)
 
@@ -906,7 +907,7 @@ func TestJWTAccountImportExportUpdates(t *testing.T) {
 		t.Fatalf("Error generating account JWT: %v", err)
 	}
 	addAccountToMemResolver(s, string(fooPub), fooJWT)
-	s.UpdateAccountClaims(s.LookupAccount(string(fooPub)), fooAC)
+	s.updateAccountClaims(s.LookupAccount(string(fooPub)), fooAC)
 
 	checkShadow(1)
 }

server/server.go

@@ -192,7 +192,6 @@ func New(opts *Options) *Server {
 		configTime: now,
 	}
 
-	// ProcessTrustedNkeys
 	if !s.processTrustedNkeys() {
 		return nil
 	}
@@ -300,8 +299,8 @@ func (s *Server) processTrustedNkeys() bool {
 			if !nkeys.IsValidPublicOperatorKey(key) {
 				return false
 			}
-			s.trustedNkeys = s.opts.TrustedNkeys
 		}
+		s.trustedNkeys = s.opts.TrustedNkeys
 	}
 	return true
 }
@@ -326,13 +325,12 @@ func checkTrustedNkeyString(keys string) []string {
 // and will set the server field 'trustedNkeys'. Returns whether
 // it succeeded or not.
 func (s *Server) initStampedTrustedNkeys() bool {
-	tks := checkTrustedNkeyString(trustedNkeys)
-	if len(tks) == 0 {
+	// Check to see if we have an override in options, which will cause us to fail.
+	if len(s.opts.TrustedNkeys) > 0 {
 		return false
 	}
-	// Check to see if we have an override in options, which will
-	// cause us to fail also.
-	if len(s.opts.TrustedNkeys) > 0 {
+	tks := checkTrustedNkeyString(trustedNkeys)
+	if len(tks) == 0 {
 		return false
 	}
 	s.trustedNkeys = tks
@@ -509,13 +507,14 @@ func (s *Server) UpdateAccount(acc *Account) bool {
 	}
 	accClaims, err := s.verifyAccountClaims(claimJWT)
 	if err == nil && accClaims != nil {
-		s.UpdateAccountClaims(acc, accClaims)
+		s.updateAccountClaims(acc, accClaims)
 		return true
 	}
 	return false
 }
 
 // fetchRawAccountClaims will grab raw account claims iff we have a resolver.
+// Lock is held upon entry.
 func (s *Server) fetchRawAccountClaims(name string) (string, error) {
 	accResolver := s.accResolver
 	if accResolver == nil {

test/proto_test.go

@@ -26,6 +26,7 @@ const PROTO_TEST_PORT = 9922
 func runProtoServer() *server.Server {
 	opts := DefaultTestOptions
 	opts.Port = PROTO_TEST_PORT
+	opts.MaxControlLine = 256
 	return RunServer(&opts)
 }