Make an IRC listener connection follow its session's nick (closes #184)

When the session's nick was changed from another client, such as the
web client over the HTTP API, the IRC listener connection kept the old
nick and sent its messages, joins and later NICK changes under it. The
relay now gives the connection the new nick when it delivers a NICK
change whose old nick is the connection's own: nicks are unique, so its
own session made that change. As the relay goroutine can now change the
nick, the command handlers read it under the connection's lock, through
currentNick, as the relay already did.

Model: opus-5-5

internal/ircserver/commands.go

@@ -81,7 +81,7 @@ func (c *Conn) handleNick(
 	}
 
 	oldMask := c.hostmask()
-	oldNick := c.nick
+	oldNick := c.currentNick()
 
 	err := c.svc.ChangeNick(
 		ctx, c.sessionID, c.clientID, oldNick, newNick,
@@ -129,7 +129,7 @@ func (c *Conn) handlePrivmsg(
 
 	if strings.HasPrefix(target, "#") {
 		_, err := c.svc.SendChannelMessage(
-			ctx, c.sessionID, c.clientID, c.nick,
+			ctx, c.sessionID, c.clientID, c.currentNick(),
 			msg.Command, target, body, nil,
 		)
 		if err != nil {
@@ -137,7 +137,7 @@ func (c *Conn) handlePrivmsg(
 		}
 	} else {
 		result, err := c.svc.SendDirectMessage(
-			ctx, c.sessionID, c.clientID, c.nick,
+			ctx, c.sessionID, c.clientID, c.currentNick(),
 			msg.Command, target, body, nil,
 		)
 		if err != nil {
@@ -199,7 +199,7 @@ func (c *Conn) joinChannel(
 	channel, key string,
 ) {
 	result, err := c.svc.JoinChannel(
-		ctx, c.sessionID, c.clientID, c.nick, channel, key,
+		ctx, c.sessionID, c.clientID, c.currentNick(), channel, key,
 	)
 	if err != nil {
 		c.sendIRCError(err)
@@ -337,7 +337,7 @@ func (c *Conn) partChannel(
 	channel, reason string,
 ) {
 	err := c.svc.PartChannel(
-		ctx, c.sessionID, c.clientID, c.nick, channel, reason,
+		ctx, c.sessionID, c.clientID, c.currentNick(), channel, reason,
 	)
 	if err != nil {
 		c.sendIRCError(err)
@@ -401,7 +401,7 @@ func (c *Conn) handleTopic(
 	newTopic := msg.Params[1]
 
 	err := c.svc.SetTopic(
-		ctx, c.sessionID, c.clientID, c.nick, channel, newTopic,
+		ctx, c.sessionID, c.clientID, c.currentNick(), channel, newTopic,
 	)
 	if err != nil {
 		c.sendIRCError(err)
@@ -708,7 +708,7 @@ func (c *Conn) applyChannelModes(
 		c.send(FormatMessage(c.hostmask(), "MODE", params...))
 
 		c.svc.BroadcastMode(
-			ctx, c.clientID, c.nick, channel, chID, modeReply,
+			ctx, c.clientID, c.currentNick(), channel, chID, modeReply,
 		)
 	}
 }
@@ -1185,7 +1185,7 @@ func (c *Conn) handleKick(
 	}
 
 	err := c.svc.KickUser(
-		ctx, c.sessionID, c.clientID, c.nick,
+		ctx, c.sessionID, c.clientID, c.currentNick(),
 		channel, targetNick, reason,
 	)
 	if err != nil {
@@ -1225,7 +1225,7 @@ func (c *Conn) handlePassPostReg(
 		return
 	}
 
-	c.sendFromServer("NOTICE", c.nick,
+	c.sendFromServer("NOTICE", c.currentNick(),
 		"Password set. You can reconnect using "+
 			"PASS <password> with your nick.",
 	)
@@ -1249,7 +1249,7 @@ func (c *Conn) handleInvite(
 	channel := msg.Params[1]
 
 	targetID, err := c.svc.InviteUser(
-		ctx, c.sessionID, c.nick, targetNick, channel,
+		ctx, c.sessionID, c.currentNick(), targetNick, channel,
 	)
 	if err != nil {
 		c.sendIRCError(err)
@@ -1266,7 +1266,7 @@ func (c *Conn) handleInvite(
 	body, _ := json.Marshal([]string{channel}) //nolint:errchkjson
 
 	_, _, _ = c.svc.FanOut( //nolint:dogsled // fire-and-forget broadcast
-		ctx, "INVITE", c.nick, targetNick, targetID, 0,
+		ctx, "INVITE", c.currentNick(), targetNick, targetID, 0,
 		nil, body, nil, []int64{targetID}, 0,
 	)
 }

internal/ircserver/conn.go

@@ -345,7 +345,7 @@ func (c *Conn) sendNumeric(
 	code irc.IRCMessageType,
 	params ...string,
 ) {
-	nick := c.nick
+	nick := c.currentNick()
 	if nick == "" {
 		nick = "*"
 	}
@@ -369,9 +369,11 @@ func (c *Conn) sendFromServer(
 // hostmask returns the client's full hostmask
 // (nick!user@host).
 func (c *Conn) hostmask() string {
+	nick := c.currentNick()
+
 	user := c.username
 	if user == "" {
-		user = c.nick
+		user = nick
 	}
 
 	host := c.hostname
@@ -379,7 +381,7 @@ func (c *Conn) hostmask() string {
 		host = c.remoteIP
 	}
 
-	return c.nick + "!" + user + "@" + host
+	return nick + "!" + user + "@" + host
 }
 
 // handleMessage dispatches a parsed IRC message using
@@ -657,7 +659,7 @@ func (c *Conn) checkPasswordLength(pw string) bool {
 		return true
 	}
 
-	c.sendFromServer("NOTICE", c.nick,
+	c.sendFromServer("NOTICE", c.currentNick(),
 		"Password must be 8 to 72 bytes",
 	)
 

internal/ircserver/relay.go

@@ -230,11 +230,22 @@ func (c *Conn) deliverPart(msg *db.IRCMessage, text string) {
 	}
 }
 
-// deliverNickChange sends a NICK change notification.
+// deliverNickChange sends a NICK change notification. When
+// the old nick is the connection's own, the change was made
+// from another client of its session, as nicks are unique,
+// and the connection takes the new nick too.
 func (c *Conn) deliverNickChange(
 	msg *db.IRCMessage,
 	newNick string,
 ) {
+	c.mu.Lock()
+
+	if msg.From == c.nick {
+		c.nick = newNick
+	}
+
+	c.mu.Unlock()
+
 	prefix := msg.From + "!" + msg.From + "@*"
 
 	c.send(FormatMessage(prefix, "NICK", newNick))

internal/ircserver/relay_test.go

@@ -511,13 +511,10 @@ func TestRelayOwnSessionOnce(t *testing.T) {
 	}
 }
 
-// ownSessionCopies connects alice to the IRC listener and has
-// her join #chan, which bob is on too, then adds an HTTP API
-// client to her session with the password her connection set,
-// as POST /api/v1/login does. It makes the command line from
-// her connection, or with api from the HTTP API client when api
-// is not nil, and returns how many copies of it her connection
-// and her HTTP API client got.
+// ownSessionCopies sets up alice with aliceWithHTTPClient. It
+// makes the command line from her connection, or with api from
+// the HTTP API client when api is not nil, and returns how many
+// copies of it her connection and her HTTP API client got.
 func ownSessionCopies(
 	t *testing.T,
 	line string,
@@ -525,27 +522,9 @@ func ownSessionCopies(
 ) (int, int) {
 	t.Helper()
 
-	const password = "alicepass"
-
 	ctx := t.Context()
 	env := newTestEnv(t)
-
-	alice := env.dial(t)
-	alice.send("PASS " + password)
-	alice.register("alice")
-	// Registration stores the password before the JOIN runs.
-	alice.joinAndDrain("#chan")
-
-	bob := env.dial(t)
-	bob.register("bob")
-	bob.joinAndDrain("#chan")
-
-	sessionID, httpClientID, _, err := env.database.LoginUser(
-		ctx, "alice", password, "127.0.0.1", "localhost",
-	)
-	if err != nil {
-		t.Fatalf("log in as alice: %v", err)
-	}
+	alice, _, sessionID, httpClientID := aliceWithHTTPClient(t, env)
 
 	var lines []string
 
@@ -555,7 +534,7 @@ func ownSessionCopies(
 		// Her echo comes before the reply to her next command.
 		lines = alice.sendAndExpect("PING :done", "PONG")
 	} else {
-		err = api(ctx, env.srv.Service(), sessionID)
+		err := api(ctx, env.srv.Service(), sessionID)
 		if err != nil {
 			t.Fatalf("%s over the HTTP API: %v", line, err)
 		}
@@ -601,3 +580,78 @@ func ownSessionCopies(
 
 	return ircCopies, httpCopies
 }
+
+// aliceWithHTTPClient connects alice to the IRC listener and
+// has her join #chan, which bob is on too, then adds an HTTP API
+// client to her session with the password her connection set,
+// as POST /api/v1/login does. It returns her connection, bob's
+// connection, her session and her HTTP API client.
+func aliceWithHTTPClient(
+	t *testing.T, env *testEnv,
+) (*testClient, *testClient, int64, int64) {
+	t.Helper()
+
+	const password = "alicepass"
+
+	alice := env.dial(t)
+	alice.send("PASS " + password)
+	alice.register("alice")
+	// Registration stores the password before the JOIN runs.
+	alice.joinAndDrain("#chan")
+
+	bob := env.dial(t)
+	bob.register("bob")
+	bob.joinAndDrain("#chan")
+
+	sessionID, httpClientID, _, err := env.database.LoginUser(
+		t.Context(), "alice", password, "127.0.0.1", "localhost",
+	)
+	if err != nil {
+		t.Fatalf("log in as alice: %v", err)
+	}
+
+	return alice, bob, sessionID, httpClientID
+}
+
+// TestRelayNickChangeFromHTTPAPI checks that once alice's nick
+// is changed from her HTTP API client, her IRC listener
+// connection uses the new nick: what she then sends from it
+// reaches bob under the new nick.
+func TestRelayNickChangeFromHTTPAPI(t *testing.T) {
+	t.Parallel()
+
+	env := newTestEnv(t)
+	alice, bob, sessionID, _ := aliceWithHTTPClient(t, env)
+
+	bob.joinAndDrain("#other")
+
+	// As the HTTP API does.
+	err := env.srv.Service().ChangeNick(
+		t.Context(), sessionID, 0, "alice", "alice2",
+	)
+	if err != nil {
+		t.Fatalf("change nick over the HTTP API: %v", err)
+	}
+
+	// Her connection has taken the new nick once it has sent her
+	// the change.
+	const change = ":alice!alice@* NICK alice2"
+
+	alice.readUntil(func(l string) bool { return l == change })
+	bob.readUntil(func(l string) bool { return l == change })
+
+	for _, step := range []struct {
+		line, want string
+	}{
+		{"PRIVMSG #chan :hi there", ":alice2!alice2@* PRIVMSG #chan :hi there"},
+		{"JOIN #other", ":alice2!alice2@* JOIN #other"},
+		{"NICK alice3", ":alice2!alice2@* NICK alice3"},
+	} {
+		alice.send(step.line)
+
+		got := bob.readLine()
+		if got != step.want {
+			t.Errorf("%s: got %q, want %q", step.line, got, step.want)
+		}
+	}
+}