[FIXED] LeafNode TLSMap and websocket auth override

We added authentication override block for websocket configuration in PR #1463 and #1465 which somehow introduced a drop in perf as reported by the bench tests. This PR refactors a bit to restore the performance numbers. This change also fixes the override behavior for websocket auth: - If websocket's NoAuthUser is configured, the websocket's auth block MUST define Users, and the user be present. - If there is any override (username/pwd,token,etc..) then the whole block config will be used when authenticating a websocket client, which means that if websocket NoAuthUser is empty we are not falling back to the regular client's NoAuthUser config. - TLSMap always override the regular client's config. That is, whatever TLSMap value specified in the websocket's tls{} block will be used. The TLSMap configuration was not used for LeafNodes. The behavior now will be: - If LeafNode's auth block contains users and TLSMap is true, the user is looked up based on the cert's info. If not found, authentication will fail. If found, it will be authenticated and bound to associated account. - If no user is specified in LeafNode's auth block and TLSMap is true, then the cert's info will be used against the global users map. Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
2026-04-16 19:14:41 -07:00 · 2020-06-11 17:06:54 -06:00
parent ddfbc33c95
commit d2a8282a0d
5 changed files with 292 additions and 125 deletions
--- a/server/auth.go
+++ b/server/auth.go
@@ -261,7 +261,7 @@ func (s *Server) buildNkeysAndUsersFromOptions(nko []*NkeyUser, uo []*User) (map
 	var nkeys map[string]*NkeyUser
 	var users map[string]*User

-	if len(nko) > 0 {
+	if nko != nil {
 		nkeys = make(map[string]*NkeyUser, len(nko))
 		for _, u := range nko {
 			copy := u.clone()
@@ -276,7 +276,7 @@ func (s *Server) buildNkeysAndUsersFromOptions(nko []*NkeyUser, uo []*User) (map
 			nkeys[u.Nkey] = copy
 		}
 	}
-	if len(uo) > 0 {
+	if uo != nil {
 		users = make(map[string]*User, len(uo))
 		for _, u := range uo {
 			copy := u.clone()
@@ -327,60 +327,6 @@ func (s *Server) isClientAuthorized(c *client) bool {
 	return s.processClientOrLeafAuthentication(c, opts)
 }

-type authOpts struct {
-	username   string
-	password   string
-	token      string
-	noAuthUser string
-	tlsMap     bool
-	users      map[string]*User
-	nkeys      map[string]*NkeyUser
-}
-
-func (s *Server) getAuthOpts(c *client, o *Options, auth *authOpts) bool {
-	// c.ws is immutable, but may need lock if we get race reports.
-	wsClient := c.ws != nil
-
-	authRequired := s.info.AuthRequired
-	// For websocket clients, if there is no top-level auth, then we
-	// check for websocket specifically.
-	if !authRequired && wsClient {
-		authRequired = s.websocket.authRequired
-	}
-	if !authRequired {
-		return false
-	}
-	auth.noAuthUser = o.NoAuthUser
-	auth.tlsMap = o.TLSMap
-	if wsClient {
-		wo := &o.Websocket
-		// If those are specified, override, regardless if there is
-		// auth configuration (like user/pwd, etc..) in websocket section.
-		if wo.NoAuthUser != _EMPTY_ {
-			auth.noAuthUser = wo.NoAuthUser
-		}
-		if wo.TLSMap {
-			auth.tlsMap = true
-		}
-		// Now check for websocket auth specific override
-		if s.websocket.authRequired {
-			auth.username = wo.Username
-			auth.password = wo.Password
-			auth.token = wo.Token
-			auth.users = s.websocket.users
-			auth.nkeys = s.websocket.nkeys
-			return true
-		}
-		// else fallback to regular auth config
-	}
-	auth.username = o.Username
-	auth.password = o.Password
-	auth.token = o.Authorization
-	auth.users = s.users
-	auth.nkeys = s.nkeys
-	return true
-}
-
 func (s *Server) processClientOrLeafAuthentication(c *client, opts *Options) bool {
 	var (
 		nkey *NkeyUser
@@ -389,16 +335,56 @@ func (s *Server) processClientOrLeafAuthentication(c *client, opts *Options) boo
 		user *User
 		ok   bool
 		err  error
-		auth authOpts
+		ao   bool // auth override
 	)
-
 	s.mu.Lock()
-	authRequired := s.getAuthOpts(c, opts, &auth)
+	authRequired := s.info.AuthRequired
+	// c.ws is immutable, but may need lock if we get race reports.
+	if !authRequired && c.ws != nil {
+		// If no auth required for regular clients, then check if
+		// we have an override for websocket clients.
+		authRequired = s.websocket.authOverride
+	}
 	if !authRequired {
 		// TODO(dlc) - If they send us credentials should we fail?
 		s.mu.Unlock()
 		return true
 	}
+	var (
+		username   string
+		password   string
+		token      string
+		noAuthUser string
+		users      map[string]*User
+		nkusers    map[string]*NkeyUser
+	)
+	tlsMap := opts.TLSMap
+	if c.ws != nil {
+		wo := &opts.Websocket
+		// Always override TLSMap.
+		tlsMap = wo.TLSMap
+		// The rest depends on if there was any auth override in
+		// the websocket's config.
+		if s.websocket.authOverride {
+			noAuthUser = wo.NoAuthUser
+			username = wo.Username
+			password = wo.Password
+			token = wo.Token
+			users = s.websocket.users
+			nkusers = s.websocket.nkeys
+			ao = true
+		}
+	} else if c.kind == LEAF {
+		tlsMap = opts.LeafNode.TLSMap
+	}
+	if !ao {
+		noAuthUser = opts.NoAuthUser
+		username = opts.Username
+		password = opts.Password
+		token = opts.Authorization
+		users = s.users
+		nkusers = s.nkeys
+	}

 	// Check if we have trustedKeys defined in the server. If so we require a user jwt.
 	if s.trustedKeys != nil {
@@ -424,21 +410,21 @@ func (s *Server) processClientOrLeafAuthentication(c *client, opts *Options) boo
 	}

 	// Check if we have nkeys or users for client.
-	hasNkeys := auth.nkeys != nil
-	hasUsers := auth.users != nil
+	hasNkeys := len(nkusers) > 0
+	hasUsers := len(users) > 0
 	if hasNkeys && c.opts.Nkey != "" {
-		nkey, ok = auth.nkeys[c.opts.Nkey]
+		nkey, ok = nkusers[c.opts.Nkey]
 		if !ok {
 			s.mu.Unlock()
 			return false
 		}
 	} else if hasUsers {
 		// Check if we are tls verify and are mapping users from the client_certificate
-		if auth.tlsMap {
+		if tlsMap {
 			var euser string
 			authorized := checkClientTLSCertSubject(c, func(u string) bool {
 				var ok bool
-				user, ok = auth.users[u]
+				user, ok = users[u]
 				if !ok {
 					c.Debugf("User in cert [%q], not found", u)
 					return false
@@ -451,20 +437,20 @@ func (s *Server) processClientOrLeafAuthentication(c *client, opts *Options) boo
 				return false
 			}
 			if c.opts.Username != "" {
-				s.Warnf("User found in connect proto, but user required from cert - %v", c)
+				s.Warnf("User %q found in connect proto, but user required from cert", c.opts.Username)
 			}
 			// Already checked that the client didn't send a user in connect
 			// but we set it here to be able to identify it in the logs.
 			c.opts.Username = euser
 		} else {
-			if c.kind == CLIENT && c.opts.Username == "" && auth.noAuthUser != "" {
-				if u, exists := auth.users[auth.noAuthUser]; exists {
+			if c.kind == CLIENT && c.opts.Username == "" && noAuthUser != "" {
+				if u, exists := users[noAuthUser]; exists {
 					c.opts.Username = u.Username
 					c.opts.Password = u.Password
 				}
 			}
 			if c.opts.Username != "" {
-				user, ok = auth.users[c.opts.Username]
+				user, ok = users[c.opts.Username]
 				if !ok {
 					s.mu.Unlock()
 					return false
@@ -586,13 +572,13 @@ func (s *Server) processClientOrLeafAuthentication(c *client, opts *Options) boo
 	}

 	if c.kind == CLIENT {
-		if auth.token != "" {
-			return comparePasswords(auth.token, c.opts.Token)
-		} else if auth.username != "" {
-			if auth.username != c.opts.Username {
+		if token != "" {
+			return comparePasswords(token, c.opts.Token)
+		} else if username != "" {
+			if username != c.opts.Username {
 				return false
 			}
-			return comparePasswords(auth.password, c.opts.Password)
+			return comparePasswords(password, c.opts.Password)
 		}
 	} else if c.kind == LEAF {
 		// There is no required username/password to connect and
@@ -784,6 +770,31 @@ func (s *Server) isLeafNodeAuthorized(c *client) bool {
 	if opts.LeafNode.Username != _EMPTY_ {
 		return isAuthorized(opts.LeafNode.Username, opts.LeafNode.Password, opts.LeafNode.Account)
 	} else if len(opts.LeafNode.Users) > 0 {
+		if opts.LeafNode.TLSMap {
+			var user *User
+			found := checkClientTLSCertSubject(c, func(u string) bool {
+				// This is expected to be a very small array.
+				for _, usr := range opts.LeafNode.Users {
+					if u == usr.Username {
+						user = usr
+						return true
+					}
+				}
+				c.Debugf("User in cert [%q], not found", u)
+				return false
+			})
+			if !found {
+				return false
+			}
+			if c.opts.Username != "" {
+				s.Warnf("User %q found in connect proto, but user required from cert", c.opts.Username)
+			}
+			c.opts.Username = user.Username
+			// This will authorize since are using an existing user,
+			// but it will also register with proper account.
+			return isAuthorized(user.Username, user.Password, user.Account.GetName())
+		}
+
 		// This is expected to be a very small array.
 		for _, u := range opts.LeafNode.Users {
 			if u.Username == c.opts.Username {
--- a/server/leafnode_test.go
+++ b/server/leafnode_test.go
@@ -1496,3 +1496,99 @@ func TestLeafNodeTmpClients(t *testing.T) {
 	checkLeafNodeConnected(t, b)
 	checkTmp(0)
 }
+
+func TestLeafNodeTLSVerifyAndMap(t *testing.T) {
+
+	accName := "MyAccount"
+	acc := NewAccount(accName)
+	certUserName := "CN=example.com,OU=NATS.io"
+	users := []*User{&User{Username: certUserName, Account: acc}}
+
+	for _, test := range []struct {
+		name        string
+		leafUsers   bool
+		provideCert bool
+	}{
+		{"no users override, provides cert", false, true},
+		{"no users override, does not provide cert", false, false},
+		{"users override, provides cert", true, true},
+		{"users override, does not provide cert", true, false},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			o := DefaultOptions()
+			o.Accounts = []*Account{acc}
+			o.LeafNode.Host = "127.0.0.1"
+			o.LeafNode.Port = -1
+			if test.leafUsers {
+				o.LeafNode.Users = users
+			} else {
+				o.Users = users
+			}
+			tc := &TLSConfigOpts{
+				CertFile: "../test/configs/certs/tlsauth/server.pem",
+				KeyFile:  "../test/configs/certs/tlsauth/server-key.pem",
+				CaFile:   "../test/configs/certs/tlsauth/ca.pem",
+				Verify:   true,
+			}
+			tlsc, err := GenTLSConfig(tc)
+			if err != nil {
+				t.Fatalf("Error creating tls config: %v", err)
+			}
+			o.LeafNode.TLSConfig = tlsc
+			o.LeafNode.TLSMap = true
+			s := RunServer(o)
+			defer s.Shutdown()
+
+			slo := DefaultOptions()
+			sltlsc := &tls.Config{}
+			if test.provideCert {
+				tc := &TLSConfigOpts{
+					CertFile: "../test/configs/certs/tlsauth/client.pem",
+					KeyFile:  "../test/configs/certs/tlsauth/client-key.pem",
+				}
+				var err error
+				sltlsc, err = GenTLSConfig(tc)
+				if err != nil {
+					t.Fatalf("Error generating tls config: %v", err)
+				}
+			}
+			sltlsc.InsecureSkipVerify = true
+			u, _ := url.Parse(fmt.Sprintf("nats://%s:%d", o.LeafNode.Host, o.LeafNode.Port))
+			slo.LeafNode.Remotes = []*RemoteLeafOpts{
+				{
+					TLSConfig: sltlsc,
+					URLs:      []*url.URL{u},
+				},
+			}
+			sl := RunServer(slo)
+			defer sl.Shutdown()
+
+			if !test.provideCert {
+				// Wait a bit and make sure we are not connecting
+				time.Sleep(100 * time.Millisecond)
+				checkLeafNodeConnectedCount(t, sl, 0)
+				return
+			}
+			checkLeafNodeConnected(t, sl)
+
+			var uname string
+			var accname string
+			s.mu.Lock()
+			for _, c := range s.leafs {
+				c.mu.Lock()
+				uname = c.opts.Username
+				if c.acc != nil {
+					accname = c.acc.GetName()
+				}
+				c.mu.Unlock()
+			}
+			s.mu.Unlock()
+			if uname != certUserName {
+				t.Fatalf("Expected username %q, got %q", certUserName, uname)
+			}
+			if accname != accName {
+				t.Fatalf("Expected account %q, got %v", accName, accname)
+			}
+		})
+	}
+}
--- a/server/server.go
+++ b/server/server.go
@@ -1901,7 +1901,7 @@ func (s *Server) createClient(conn net.Conn, ws *websocket) *client {
 	// then we use the websocket's specific boolean that will be set to true
 	// if there is any auth{} configured in websocket{}.
 	if ws != nil && !info.AuthRequired {
-		info.AuthRequired = s.websocket.authRequired
+		info.AuthRequired = s.websocket.authOverride
 	}
 	if s.nonceRequired() {
 		// Nonce handling
--- a/server/websocket.go
+++ b/server/websocket.go
@@ -103,7 +103,7 @@ type srvWebsocket struct {
 	connectURLsMap map[string]struct{}
 	users          map[string]*User
 	nkeys          map[string]*NkeyUser
-	authRequired   bool // indicate if there is auth override in websocket config
+	authOverride   bool // indicate if there is auth override in websocket config
 }

 type allowedOrigin struct {
@@ -735,6 +735,19 @@ func validateWebsocketOptions(o *Options) error {
 			return fmt.Errorf("unable to parse allowed origin: %v", err)
 		}
 	}
+	// If there is a NoAuthUser, we need to have Users defined and
+	// the user to be present.
+	if wo.NoAuthUser != _EMPTY_ {
+		if wo.Users == nil {
+			return fmt.Errorf("websocket no_auth_user %q configured, but users are not", wo.NoAuthUser)
+		}
+		for _, u := range wo.Users {
+			if u.Username == wo.NoAuthUser {
+				return nil
+			}
+		}
+		return fmt.Errorf("websocket no_auth_user %q not found in users configuration", wo.NoAuthUser)
+	}
 	return nil
 }

@@ -771,17 +784,18 @@ func (s *Server) wsSetOriginOptions(o *WebsocketOpts) {
 // store them in s.websocket.users/nkeys.
 // Also update a boolean that indicates if auth is required for
 // websocket clients.
+// Server lock is held on entry.
 func (s *Server) wsConfigAuth(opts *WebsocketOpts) {
 	ws := &s.websocket
 	if len(opts.Nkeys) > 0 || len(opts.Users) > 0 {
 		ws.nkeys, ws.users = s.buildNkeysAndUsersFromOptions(opts.Nkeys, opts.Users)
-		ws.authRequired = true
+		ws.authOverride = true
 	} else if opts.Username != "" || opts.Token != "" {
-		ws.authRequired = true
+		ws.authOverride = true
 	} else {
 		ws.users = nil
 		ws.nkeys = nil
-		ws.authRequired = false
+		ws.authOverride = false
 	}
 }

--- a/server/websocket_test.go
+++ b/server/websocket_test.go
@@ -1878,53 +1878,64 @@ func TestWSTLSVerifyClientCert(t *testing.T) {
 }

 func TestWSTLSVerifyAndMap(t *testing.T) {
-	o := testWSOptions()
+	accName := "MyAccount"
+	acc := NewAccount(accName)
 	certUserName := "CN=example.com,OU=NATS.io"
-	o.Users = []*User{&User{Username: certUserName}}
-	tc := &TLSConfigOpts{
-		CertFile: "../test/configs/certs/tlsauth/server.pem",
-		KeyFile:  "../test/configs/certs/tlsauth/server-key.pem",
-		CaFile:   "../test/configs/certs/tlsauth/ca.pem",
-		Verify:   true,
-	}
-	tlsc, err := GenTLSConfig(tc)
-	if err != nil {
-		t.Fatalf("Error creating tls config: %v", err)
-	}
-	o.Websocket.TLSConfig = tlsc
-	o.Websocket.TLSMap = true
-	s := RunServer(o)
-	defer s.Shutdown()
-
-	addr := fmt.Sprintf("%s:%d", o.Websocket.Host, o.Websocket.Port)
+	users := []*User{&User{Username: certUserName, Account: acc}}

 	for _, test := range []struct {
 		name        string
+		wsUsers     bool
 		provideCert bool
 	}{
-		{"client provides cert", true},
-		{"client does not provide cert", false},
+		{"no users override, client provides cert", false, true},
+		{"no users override, client does not provide cert", false, false},
+		{"sers override, client provides cert", true, true},
+		{"users override, client does not provide cert", true, false},
 	} {
 		t.Run(test.name, func(t *testing.T) {
+			o := testWSOptions()
+			o.Accounts = []*Account{acc}
+			if test.wsUsers {
+				o.Websocket.Users = users
+			} else {
+				o.Users = users
+			}
+			tc := &TLSConfigOpts{
+				CertFile: "../test/configs/certs/tlsauth/server.pem",
+				KeyFile:  "../test/configs/certs/tlsauth/server-key.pem",
+				CaFile:   "../test/configs/certs/tlsauth/ca.pem",
+				Verify:   true,
+			}
+			tlsc, err := GenTLSConfig(tc)
+			if err != nil {
+				t.Fatalf("Error creating tls config: %v", err)
+			}
+			o.Websocket.TLSConfig = tlsc
+			o.Websocket.TLSMap = true
+			s := RunServer(o)
+			defer s.Shutdown()
+
+			addr := fmt.Sprintf("%s:%d", o.Websocket.Host, o.Websocket.Port)
 			wsc, err := net.Dial("tcp", addr)
 			if err != nil {
 				t.Fatalf("Error creating ws connection: %v", err)
 			}
 			defer wsc.Close()
-			tlsc := &tls.Config{}
+			tlscc := &tls.Config{}
 			if test.provideCert {
 				tc := &TLSConfigOpts{
 					CertFile: "../test/configs/certs/tlsauth/client.pem",
 					KeyFile:  "../test/configs/certs/tlsauth/client-key.pem",
 				}
 				var err error
-				tlsc, err = GenTLSConfig(tc)
+				tlscc, err = GenTLSConfig(tc)
 				if err != nil {
 					t.Fatalf("Error generating tls config: %v", err)
 				}
 			}
-			tlsc.InsecureSkipVerify = true
-			wsc = tls.Client(wsc, tlsc)
+			tlscc.InsecureSkipVerify = true
+			wsc = tls.Client(wsc, tlscc)
 			if err := wsc.(*tls.Conn).Handshake(); err != nil {
 				t.Fatalf("Error during handshake: %v", err)
 			}
@@ -1971,12 +1982,22 @@ func TestWSTLSVerifyAndMap(t *testing.T) {
 				t.Fatalf("Expected PONG, got %s", msg)
 			}

+			var uname string
+			var accname string
 			c := s.getClient(info.CID)
-			c.mu.Lock()
-			un := c.opts.Username
-			c.mu.Unlock()
-			if un != certUserName {
-				t.Fatalf("Expected client's assigned username to be %q, got %q", certUserName, un)
+			if c != nil {
+				c.mu.Lock()
+				uname = c.opts.Username
+				if c.acc != nil {
+					accname = c.acc.GetName()
+				}
+				c.mu.Unlock()
+			}
+			if uname != certUserName {
+				t.Fatalf("Expected username %q, got %q", certUserName, uname)
+			}
+			if accname != accName {
+				t.Fatalf("Expected account %q, got %v", accName, accname)
 			}
 		})
 	}
@@ -2946,18 +2967,37 @@ func TestWSUsersAuth(t *testing.T) {
 	}
 }

+func TestWSNoAuthUserValidation(t *testing.T) {
+	// It is illegal to configure a websocket's NoAuthUser if websocket's
+	// auth config does not have a matching User.
+	// Create regular clients's User to make sure that we fail even if
+	// the websocket's NoAuthUser is found in opts.Users.
+	o := testWSOptions()
+	o.Users = []*User{&User{Username: "user", Password: "pwd"}}
+	o.Websocket.NoAuthUser = "user"
+	if _, err := NewServer(o); err == nil || !strings.Contains(err.Error(), "users are not") {
+		t.Fatalf("Expected error saying that users are not configured, got %v", err)
+	}
+
+	o.Websocket.Users = []*User{&User{Username: "wsuser", Password: "pwd"}}
+	o.Websocket.NoAuthUser = "notfound"
+	if _, err := NewServer(o); err == nil || !strings.Contains(err.Error(), "not found") {
+		t.Fatalf("Expected error saying no auth user not found, got %v", err)
+	}
+}
+
 func TestWSNoAuthUser(t *testing.T) {
 	for _, test := range []struct {
 		name          string
-		noAuthUser    string
-		wsNoAuthUser  string
-		user          string
-		acc           string
 		createWSUsers bool
+		useAuth       bool
+		expectedUser  string
+		expectedAcc   string
 	}{
-		{"use top-level no_auth_user", "user1", "", "user1", "normal", false},
-		{"use websocket no_auth_user no ws users", "user1", "user2", "user2", "normal", false},
-		{"use websocket no_auth_user with ws users", "user1", "wsuser1", "wsuser1", "websocket", true},
+		{"no override, no user provided", false, false, "noauth", "normal"},
+		{"no override, user povided", false, true, "user", "normal"},
+		{"override, no user provided", true, false, "wsnoauth", "websocket"},
+		{"override, user provided", true, true, "wsuser", "websocket"},
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			o := testWSOptions()
@@ -2965,16 +3005,16 @@ func TestWSNoAuthUser(t *testing.T) {
 			websocketAcc := NewAccount("websocket")
 			o.Accounts = []*Account{normalAcc, websocketAcc}
 			o.Users = []*User{
-				&User{Username: "user1", Password: "pwd1", Account: normalAcc},
-				&User{Username: "user2", Password: "pwd2", Account: normalAcc},
+				&User{Username: "noauth", Password: "pwd", Account: normalAcc},
+				&User{Username: "user", Password: "pwd", Account: normalAcc},
 			}
-			o.NoAuthUser = test.noAuthUser
-			o.Websocket.NoAuthUser = test.wsNoAuthUser
+			o.NoAuthUser = "noauth"
 			if test.createWSUsers {
 				o.Websocket.Users = []*User{
-					&User{Username: "wsuser1", Password: "pwd1", Account: websocketAcc},
-					&User{Username: "wsuser2", Password: "pwd2", Account: websocketAcc},
+					&User{Username: "wsnoauth", Password: "pwd", Account: websocketAcc},
+					&User{Username: "wsuser", Password: "pwd", Account: websocketAcc},
 				}
+				o.Websocket.NoAuthUser = "wsnoauth"
 			}
 			s := RunServer(o)
 			defer s.Shutdown()
@@ -2985,7 +3025,13 @@ func TestWSNoAuthUser(t *testing.T) {
 			var info serverInfo
 			json.Unmarshal([]byte(l[5:]), &info)

-			connectProto := "CONNECT {\"verbose\":false,\"protocol\":1}\r\nPING\r\n"
+			var connectProto string
+			if test.useAuth {
+				connectProto = fmt.Sprintf("CONNECT {\"verbose\":false,\"protocol\":1,\"user\":\"%s\",\"pass\":\"pwd\"}\r\nPING\r\n",
+					test.expectedUser)
+			} else {
+				connectProto = "CONNECT {\"verbose\":false,\"protocol\":1}\r\nPING\r\n"
+			}
 			wsmsg := testWSCreateClientMsg(wsBinaryMessage, 1, true, false, []byte(connectProto))
 			if _, err := wsc.Write(wsmsg); err != nil {
 				t.Fatalf("Error sending message: %v", err)
@@ -3000,11 +3046,11 @@ func TestWSNoAuthUser(t *testing.T) {
 			uname := c.opts.Username
 			aname := c.acc.GetName()
 			c.mu.Unlock()
-			if uname != test.user {
-				t.Fatalf("Expected selected user to be %q, got %q", test.user, uname)
+			if uname != test.expectedUser {
+				t.Fatalf("Expected selected user to be %q, got %q", test.expectedUser, uname)
 			}
-			if aname != test.acc {
-				t.Fatalf("Expected selected account to be %q, got %q", test.acc, aname)
+			if aname != test.expectedAcc {
+				t.Fatalf("Expected selected account to be %q, got %q", test.expectedAcc, aname)
 			}
 		})
 	}