Fix - non-logged-in needs a reply if auth needed

Refactor for testing - all janitors have sleep in a different function
from the work, so tests can trigger them

socketserver/server/commands.go

@@ -21,10 +21,10 @@ type Command string
 type CommandHandler func(*websocket.Conn, *ClientInfo, ClientMessage) (ClientMessage, error)
 
 var commandHandlers = map[Command]CommandHandler{
-	HelloCommand: C2SHello,
+	HelloCommand:   C2SHello,
-	"ping":       C2SPing,
+	"ping":         C2SPing,
-	"setuser":    C2SSetUser,
+	SetUserCommand: C2SSetUser,
-	"ready":      C2SReady,
+	ReadyCommand:   C2SReady,
 
 	"sub":   C2SSubscribe,
 	"unsub": C2SUnsubscribe,
@@ -143,7 +143,6 @@ func C2SSetUser(conn *websocket.Conn, client *ClientInfo, msg ClientMessage) (rm
 		go client.StartAuthorization(func(_ *ClientInfo, _ bool) {
 			client.MsgChannelKeepalive.Done()
 		})
-
 	}
 
 	return ResponseSuccess, nil
@@ -316,11 +315,11 @@ func C2SEmoticonUses(conn *websocket.Conn, client *ClientInfo, msg ClientMessage
 func aggregateDataSender() {
 	for {
 		time.Sleep(5 * time.Minute)
-		doSendAggregateData()
+		aggregateDataSender_do()
 	}
 }
 
-func doSendAggregateData() {
+func aggregateDataSender_do() {
 	followEventsLock.Lock()
 	follows := followEvents
 	followEvents = nil
@@ -538,11 +537,18 @@ func C2SHandleRemoteCommand(conn *websocket.Conn, client *ClientInfo, msg Client
 }
 
 const AuthorizationFailedErrorString = "Failed to verify your Twitch username."
+const AuthorizationNeededError = "You must be signed in to use that command."
 
 func doRemoteCommand(conn *websocket.Conn, msg ClientMessage, client *ClientInfo) {
 	resp, err := SendRemoteCommandCached(string(msg.Command), msg.origArguments, client.AuthInfo)
 
 	if err == ErrAuthorizationNeeded {
+		if client.TwitchUsername == "" {
+			// Not logged in
+			client.MessageChannel <- ClientMessage{MessageID: msg.MessageID, Command: ErrorCommand, Arguments: AuthorizationNeededError}
+			client.MsgChannelKeepalive.Done()
+			return
+		}
 		client.StartAuthorization(func(_ *ClientInfo, success bool) {
 			if success {
 				doRemoteCommand(conn, msg, client)

socketserver/server/handlecore.go

@@ -34,6 +34,12 @@ const ErrorCommand Command = "error"
 // Sending any other command will result in a CloseFirstMessageNotHello.
 const HelloCommand Command = "hello"
 
+// ReadyCommand is a C2S Command.
+// It indicates that the client is finished sending the initial 'sub' commands and the server should send the backlog.
+const ReadyCommand Command = "ready"
+
+const SetUserCommand Command = "set_user"
+
 // AuthorizeCommand is a S2C Command sent as part of Twitch username validation.
 const AuthorizeCommand Command = "do_authorize"
 

socketserver/server/irc.go

@@ -10,6 +10,7 @@ import (
 	"strings"
 	"sync"
 	"time"
+	"errors"
 )
 
 type AuthCallback func(client *ClientInfo, successful bool)
@@ -39,28 +40,31 @@ func AddPendingAuthorization(client *ClientInfo, challenge string, callback Auth
 func authorizationJanitor() {
 	for {
 		time.Sleep(5 * time.Minute)
-
+		authorizationJanitor_do()
-		func() {
-			cullTime := time.Now().Add(-30 * time.Minute)
-
-			PendingAuthLock.Lock()
-			defer PendingAuthLock.Unlock()
-
-			newPendingAuths := make([]PendingAuthorization, 0, len(PendingAuths))
-
-			for _, v := range PendingAuths {
-				if !cullTime.After(v.EnteredAt) {
-					newPendingAuths = append(newPendingAuths, v)
-				}
-			}
-
-			PendingAuths = newPendingAuths
-		}()
 	}
 }
 
+func authorizationJanitor_do() {
+	cullTime := time.Now().Add(-30 * time.Minute)
+
+	PendingAuthLock.Lock()
+	defer PendingAuthLock.Unlock()
+
+	newPendingAuths := make([]PendingAuthorization, 0, len(PendingAuths))
+
+	for _, v := range PendingAuths {
+		if !cullTime.After(v.EnteredAt) {
+			newPendingAuths = append(newPendingAuths, v)
+		} else {
+			v.Callback(v.Client, false)
+		}
+	}
+
+	PendingAuths = newPendingAuths
+}
+
 func (client *ClientInfo) StartAuthorization(callback AuthCallback) {
-	fmt.Println(DEBUG, "startig auth for user", client.TwitchUsername, client.RemoteAddr)
+	fmt.Println(DEBUG, "starting auth for user", client.TwitchUsername, client.RemoteAddr)
 	var nonce [32]byte
 	_, err := rand.Read(nonce[:])
 	if err != nil {
@@ -88,6 +92,8 @@ const AuthCommand = "AUTH"
 
 const DEBUG = "DEBUG"
 
+var errChallengeNotFound = errors.New("did not find a challenge solved by that message")
+
 func ircConnection() {
 
 	c := irc.SimpleClient("justinfan123")
@@ -113,46 +119,7 @@ func ircConnection() {
 		submittedUser := line.Nick
 		submittedChallenge := msgArray[1]
 
-		var auth PendingAuthorization
+		submitAuth(submittedUser, submittedChallenge)
-		var idx int = -1
-
-		PendingAuthLock.Lock()
-		for i, v := range PendingAuths {
-			if v.Client.TwitchUsername == submittedUser && v.Challenge == submittedChallenge {
-				auth = v
-				idx = i
-				break
-			}
-		}
-		if idx != -1 {
-			PendingAuths = append(PendingAuths[:idx], PendingAuths[idx+1:]...)
-		}
-		PendingAuthLock.Unlock()
-
-		if idx == -1 {
-			fmt.Println(DEBUG, "discarded msg - challenge not found", line.Raw)
-			return
-		}
-
-		// auth is valid, and removed from pending list
-
-		fmt.Println(DEBUG, "authorization success for user", auth.Client.TwitchUsername)
-		var usernameChanged bool
-		auth.Client.Mutex.Lock()
-		if auth.Client.TwitchUsername == submittedUser { // recheck condition
-			auth.Client.UsernameValidated = true
-		} else {
-			usernameChanged = true
-		}
-		auth.Client.Mutex.Unlock()
-
-		if auth.Callback != nil {
-			if !usernameChanged {
-				auth.Callback(auth.Client, true)
-			} else {
-				auth.Callback(auth.Client, false)
-			}
-		}
 	})
 
 	err := c.ConnectTo("irc.twitch.tv")
@@ -161,3 +128,45 @@ func ircConnection() {
 	}
 
 }
+
+func submitAuth(user, challenge string) error {
+	var auth PendingAuthorization
+	var idx int = -1
+
+	PendingAuthLock.Lock()
+	for i, v := range PendingAuths {
+		if v.Client.TwitchUsername == user && v.Challenge == challenge {
+			auth = v
+			idx = i
+			break
+		}
+	}
+	if idx != -1 {
+		PendingAuths = append(PendingAuths[:idx], PendingAuths[idx+1:]...)
+	}
+	PendingAuthLock.Unlock()
+
+	if idx == -1 {
+		return errChallengeNotFound // perhaps it was for another socket server
+	}
+
+	// auth is valid, and removed from pending list
+
+	fmt.Println(DEBUG, "authorization success for user", auth.Client.TwitchUsername)
+	var usernameChanged bool
+	auth.Client.Mutex.Lock()
+	if auth.Client.TwitchUsername == user { // recheck condition
+		auth.Client.UsernameValidated = true
+	} else {
+		usernameChanged = true
+	}
+	auth.Client.Mutex.Unlock()
+
+	if auth.Callback != nil {
+		if !usernameChanged {
+			auth.Callback(auth.Client, true)
+		} else {
+			auth.Callback(auth.Client, false)
+		}
+	}
+}

socketserver/server/subscriptions.go

@@ -149,21 +149,25 @@ const ReapingDelay = 1 * time.Minute
 func pubsubJanitor() {
 	for {
 		time.Sleep(ReapingDelay)
-		var cleanedUp = make([]string, 0, 6)
+		pubsubJanitor_do()
-		ChatSubscriptionLock.Lock()
+	}
-		for key, val := range ChatSubscriptionInfo {
+}
-			if val == nil || len(val.Members) == 0 {
-				delete(ChatSubscriptionInfo, key)
-				cleanedUp = append(cleanedUp, key)
-			}
-		}
-		ChatSubscriptionLock.Unlock()
 
-		if len(cleanedUp) != 0 {
+func pubsubJanitor_do() {
-			err := SendCleanupTopicsNotice(cleanedUp)
+	var cleanedUp = make([]string, 0, 6)
-			if err != nil {
+	ChatSubscriptionLock.Lock()
-				log.Println("error reporting cleaned subs:", err)
+	for key, val := range ChatSubscriptionInfo {
-			}
+		if val == nil || len(val.Members) == 0 {
+			delete(ChatSubscriptionInfo, key)
+			cleanedUp = append(cleanedUp, key)
+		}
+	}
+	ChatSubscriptionLock.Unlock()
+
+	if len(cleanedUp) != 0 {
+		err := SendCleanupTopicsNotice(cleanedUp)
+		if err != nil {
+			log.Println("error reporting cleaned subs:", err)
 		}
 	}
 }