Publish ClientConn only after login and guard Account with the state mutex

NewClientConn added the connection to the ClientManager before Account
and Version were assigned, so any goroutine iterating ClientMgr.List()
during the login handshake could dereference a nil Account (e.g.
HandleSetUser reading c.Account.Login) and panic. Account was also read
and written across goroutines with no synchronization: HandleSetUser
wrote c.Account.Access on another client's connection while that
client's own transaction loop read it in Authorize.

The connection is now added to the manager in handleNewConnection only
once Account, Version, UserName, and Flags are initialized, so a
published client is always fully formed. Failed logins never publish
the connection at all; Disconnect's manager delete is a no-op for them.

Account joins the mutex-guarded session state with accessors in the
established style: SetAccount/GetAccount, SetAccountAccess for the one
post-login mutation, AccessBytes for building transaction fields, and
Authorize now takes the read lock. All cross-goroutine call sites go
through the accessors.

HandleSetUser previously recomputed the admin flag from the client's
stale access level and only converged on the following edit; the access
update now happens before the recompute.

1 files changed, 49 insertions, 6 deletions

diff --git a/hotline/client_conn.go b/hotline/client_conn.go
index 3419e0a..fe4dc72 100644
--- a/hotline/client_conn.go
+++ b/hotline/client_conn.go
@@ -32,13 +32,13 @@ type ClientConn struct {
 	ID         ClientID
 	Version    []byte // TODO: make fixed size of 2
 
-	Account *Account
-	Server  *Server // TODO: consider adding methods to interact with server
+	Server *Server // TODO: consider adding methods to interact with server
 
 	// The following fields hold mutable session state guarded by mu.  They are read and written
 	// by multiple goroutines (the client's own transaction loop, other clients' handlers, and the
 	// server keepalive loop), so production code must use the accessor methods below.  Direct
 	// field access is only safe before the connection is shared, e.g. in tests.
+	Account   *Account
 	Flags     UserFlags
 	UserName  []byte
 	Icon      []byte // TODO: make fixed size of 2
@@ -197,6 +197,45 @@ func (cc *ClientConn) GetAutoReply() []byte {
 	return cc.AutoReply
 }
 
+// SetAccount attaches the authenticated account to the client.
+func (cc *ClientConn) SetAccount(account *Account) {
+	cc.mu.Lock()
+	defer cc.mu.Unlock()
+
+	cc.Account = account
+}
+
+// GetAccount returns the client's account, or nil before login completes.  Every account field
+// except Access is immutable after login, so callers may read them from the returned struct;
+// Access must be read via Authorize or AccessBytes because SetAccountAccess mutates it.
+func (cc *ClientConn) GetAccount() *Account {
+	cc.mu.RLock()
+	defer cc.mu.RUnlock()
+
+	return cc.Account
+}
+
+// SetAccountAccess replaces the access bitmap on the client's account.
+func (cc *ClientConn) SetAccountAccess(access AccessBitmap) {
+	cc.mu.Lock()
+	defer cc.mu.Unlock()
+
+	cc.Account.Access = access
+}
+
+// AccessBytes returns a copy of the account's access bitmap, suitable for use as a transaction
+// field.  It returns nil before login completes.
+func (cc *ClientConn) AccessBytes() []byte {
+	cc.mu.RLock()
+	defer cc.mu.RUnlock()
+
+	if cc.Account == nil {
+		return nil
+	}
+	access := cc.Account.Access
+	return access[:]
+}
+
 // incrementIdleTime adds interval seconds to the client's idle time.  It returns true if this
 // crossed the idle threshold and marked the client as away, in which case the caller should
 // notify other clients of the change.
@@ -227,8 +266,8 @@ func (cc *ClientConn) clearIdleAndAway() bool {
 }
 
 func (cc *ClientConn) FileRoot() string {
-	if cc.Account.FileRoot != "" {
-		return cc.Account.FileRoot
+	if account := cc.GetAccount(); account != nil && account.FileRoot != "" {
+		return account.FileRoot
 	}
 	return cc.Server.Config.FileRoot
 }
@@ -331,6 +370,9 @@ func (cc *ClientConn) Authenticate(login string, password []byte) bool {
 
 // Authorize checks if the user account has the specified permission
 func (cc *ClientConn) Authorize(access int) bool {
+	cc.mu.RLock()
+	defer cc.mu.RUnlock()
+
 	if cc.Account == nil {
 		return false
 	}
@@ -424,11 +466,12 @@ func formatDownloadList(fts []FileTransfer) (s string) {
 }
 
 func (cc *ClientConn) String() string {
+	account := cc.GetAccount()
 	template := fmt.Sprintf(
 		userInfoTemplate,
 		cc.GetUserName(),
-		cc.Account.Name,
-		cc.Account.Login,
+		account.Name,
+		account.Login,
 		cc.RemoteAddr,
 		formatDownloadList(cc.ClientFileTransferMgr.Get(FileDownload)),
 		formatDownloadList(cc.ClientFileTransferMgr.Get(FolderDownload)),