Register the broker wait before the first poll (closes #157)

GET /api/v1/messages read the client output queue and only then
registered with the broker. Notify wakes only the channels registered
at that moment, so a message queued between the empty read and the
registration waited out the whole long-poll timeout. The handler now
registers first and removes its channel on every exit path; the IRC
relay loop registers before each drain in the same way.

The handlers read the queue through a function field that defaults to
Database.PollMessages, so tests can act just after a read: one checks
that a message queued there comes back at once, others that no wait
channel stays registered however a request ends.

Model: opus-5-5

README.md

@@ -180,7 +180,7 @@ For users who want to access the same session from multiple devices:
 - **Set password via IRC PASS command**: the authenticated client sends
   `POST /api/v1/messages` with `{"command":"PASS","body":["mypassword"]}`. The
   server hashes the password with bcrypt and stores it on the session. Password
-  must be at least 8 characters.
+  must be 8 to 72 bytes.
 - **Login from another client**: `POST /api/v1/login` with nick and password →
   server verifies the password, creates a new client for the existing session,
   and sets an auth cookie. Channel memberships are shared; the new client gets
@@ -835,10 +835,11 @@ Set a password on the current session, enabling multi-client login via
 
 **Behavior:**
 
-- `body[0]` is the password. Must be at least 8 characters.
+- `body[0]` is the password. Must be 8 to 72 bytes; bcrypt cannot hash a longer
+  one.
 - On success, the server responds with `{"status": "ok"}`.
-- If the password is too short or missing, the server sends ERR_NEEDMOREPARAMS
+- If the password is too short, too long or missing, the server sends
-  (461) via the message queue.
+  ERR_NEEDMOREPARAMS (461) via the message queue.
 - Calling PASS again overwrites the previous password.
 - Once a password is set, `POST /api/v1/login` can be used with the nick and
   password to create additional clients on the same session.
@@ -2678,9 +2679,11 @@ IRC_LISTEN_ADDR: ""
 
 - **Wire format**: CR-LF delimited lines, max 512 bytes per line
 - **Connection registration**: Clients must send `NICK` and `USER` to register.
-  An optional `PASS` before registration sets the session password (minimum 8
+  An optional `PASS` before registration sets the session password (8 to 72
-  characters). Registering creates a session without a hashcash stamp; closing
+  bytes); one outside that range is refused with a `NOTICE` once registration
-  the connection ends the session, as `QUIT` does.
+  completes, as `PASS` after registration is. Registering creates a session
+  without a hashcash stamp; closing the connection ends the session, as `QUIT`
+  does.
 - **PING**: a connection that sends nothing for 90 seconds is sent a `PING`, and
   is closed if it sends nothing in the 30 seconds after that; any line, such as
   the client's `PONG`, starts both waits again.

internal/handlers/api.go

@@ -855,7 +855,8 @@ func (hdlr *Handlers) HandleGetMessages() http.HandlerFunc {
 
 // longPoll waits up to timeout seconds for the broker to
 // signal waitCh, then answers with the messages queued for
-// clientID after afterID.
+// clientID after afterID. It answers nothing if the client
+// goes away first.
 func (hdlr *Handlers) longPoll(
 	writer http.ResponseWriter,
 	request *http.Request,

internal/handlers/api_test.go

@@ -2561,7 +2561,7 @@ func TestLongPollTimeout(t *testing.T) {
 // a message queued, and the broker notified, after the long
 // poll's first read of the queue found nothing but before the
 // poll waits, is returned at once and not when the poll times
-// out.
+// out, and that the poll then holds no broker wait channel.
 func TestLongPollReturnsMessageQueuedAfterFirstRead(t *testing.T) {
 	const (
 		nick   = "lp_late"
@@ -2632,6 +2632,260 @@ func TestLongPollReturnsMessageQueuedAfterFirstRead(t *testing.T) {
 		t.Fatalf("long poll: got %v, want the PRIVMSG from %s",
 			msgs, sender)
 	}
+
+	tserver.checkWaitChannelRemoved(sessionID)
+}
+
+// errReadFailed is what a test's replacement for
+// Database.PollMessages returns to make a read of the queue
+// fail.
+var errReadFailed = errors.New("read failed")
+
+// TestGetMessagesRemovesWaitChannel checks that a request to
+// GET /api/v1/messages leaves no broker wait channel behind,
+// whether it is answered at once, after its timeout, or with
+// an error because a read of the queue failed.
+func TestGetMessagesRemovesWaitChannel(t *testing.T) {
+	const (
+		// longTimeout, in seconds, is the timeout of the
+		// requests that must be answered without waiting.
+		longTimeout = 10
+
+		// shortTimeout, in seconds, is the timeout of the
+		// requests that wait it out.
+		shortTimeout = 1
+	)
+
+	// queued is whether a message is queued before the request,
+	// whose timeout is in seconds. failingRead is as for
+	// getMessagesFailingRead. wantReads is how many times the
+	// handler reads the queue: once if it answers at once,
+	// twice if it waits first.
+	tests := []struct {
+		name                  string
+		queued                bool
+		timeout, failingRead  int
+		wantReads, wantStatus int
+	}{
+		{
+			name: "nothing queued, timeout 0", timeout: 0,
+			wantReads: 1, wantStatus: http.StatusOK,
+		},
+		{
+			name: "message queued", queued: true, timeout: longTimeout,
+			wantReads: 1, wantStatus: http.StatusOK,
+		},
+		{
+			name: "timed out", timeout: shortTimeout,
+			wantReads: 2, wantStatus: http.StatusOK,
+		},
+		{
+			name: "first read failed", timeout: longTimeout, failingRead: 1,
+			wantReads: 1, wantStatus: http.StatusInternalServerError,
+		},
+		{
+			name: "read after the wait failed", timeout: shortTimeout,
+			failingRead: 2, wantReads: 2,
+			wantStatus: http.StatusInternalServerError,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			const nick = "wc_poller"
+
+			tserver := newTestServer(t)
+			token := tserver.createSession(nick)
+
+			_, lastID := tserver.pollMessages(token, 0)
+
+			sessionID, err := tserver.database.GetSessionByNick(
+				t.Context(), nick,
+			)
+			if err != nil {
+				t.Fatalf("get session of %s: %v", nick, err)
+			}
+
+			if tc.queued {
+				err = tserver.queueMessage(
+					t.Context(), sessionID, "wc_sender", nick,
+				)
+				if err != nil {
+					t.Fatalf("queue message: %v", err)
+				}
+			}
+
+			status, reads := tserver.getMessagesFailingRead(
+				token, lastID, tc.timeout, tc.failingRead,
+			)
+			if status != tc.wantStatus {
+				t.Errorf("status %d, want %d", status, tc.wantStatus)
+			}
+
+			if reads != tc.wantReads {
+				t.Errorf("the handler read the queue %d times, want %d",
+					reads, tc.wantReads)
+			}
+
+			tserver.checkWaitChannelRemoved(sessionID)
+		})
+	}
+}
+
+// getMessagesFailingRead sends GET /api/v1/messages for the
+// client token with timeout and after afterID. The handler's
+// read of the queue numbered failingRead, counting from 1,
+// fails; with 0, none does. It returns the response status
+// and how many times the handler read the queue.
+func (tserver *testServer) getMessagesFailingRead(
+	token string,
+	afterID int64,
+	timeout, failingRead int,
+) (int, int) {
+	tserver.t.Helper()
+
+	reads := 0
+
+	tserver.handlers.SetPollMessages(func(
+		ctx context.Context,
+		clientID, afterQueueID int64,
+		limit int,
+	) ([]db.IRCMessage, int64, error) {
+		reads++
+
+		if reads == failingRead {
+			return nil, 0, errReadFailed
+		}
+
+		return tserver.database.PollMessages(
+			ctx, clientID, afterQueueID, limit,
+		)
+	})
+
+	resp, err := doRequestAuth(
+		tserver.t, http.MethodGet,
+		tserver.url(fmt.Sprintf(
+			"%s?timeout=%d&after=%d", apiMessages, timeout, afterID,
+		)),
+		token, nil,
+	)
+	if err != nil {
+		tserver.t.Fatalf("get messages: %v", err)
+	}
+
+	_ = resp.Body.Close()
+
+	return resp.StatusCode, reads
+}
+
+// TestLongPollRemovesWaitChannelWhenClientGivesUp checks that
+// a long poll whose client gives up while it waits leaves no
+// broker wait channel behind.
+func TestLongPollRemovesWaitChannelWhenClientGivesUp(t *testing.T) {
+	const (
+		nick = "wc_leaver"
+
+		// pollTimeout is the long poll's timeout, in seconds.
+		pollTimeout = 10
+
+		// readDeadline is how long the handler has to read
+		// the queue.
+		readDeadline = 5 * time.Second
+	)
+
+	tserver := newTestServer(t)
+	token := tserver.createSession(nick)
+
+	_, lastID := tserver.pollMessages(token, 0)
+
+	sessionID, err := tserver.database.GetSessionByNick(t.Context(), nick)
+	if err != nil {
+		t.Fatalf("get session of %s: %v", nick, err)
+	}
+
+	read := make(chan struct{})
+
+	var once sync.Once
+
+	tserver.handlers.SetPollMessages(func(
+		ctx context.Context,
+		clientID, afterQueueID int64,
+		limit int,
+	) ([]db.IRCMessage, int64, error) {
+		msgs, lastQID, pollErr := tserver.database.PollMessages(
+			ctx, clientID, afterQueueID, limit,
+		)
+
+		once.Do(func() { close(read) })
+
+		return msgs, lastQID, pollErr
+	})
+
+	ctx, giveUp := context.WithCancel(t.Context())
+	defer giveUp()
+
+	request, err := http.NewRequestWithContext(
+		ctx, http.MethodGet,
+		tserver.url(fmt.Sprintf(
+			"%s?timeout=%d&after=%d", apiMessages, pollTimeout, lastID,
+		)),
+		nil,
+	)
+	if err != nil {
+		t.Fatalf("new request: %v", err)
+	}
+
+	request.AddCookie(&http.Cookie{ //nolint:exhaustruct,gosec // only name+value
+		Name:  authCookieName,
+		Value: token,
+	})
+
+	done := make(chan struct{})
+
+	go func() {
+		defer close(done)
+
+		resp, doErr := http.DefaultClient.Do(request)
+		if doErr == nil {
+			_ = resp.Body.Close()
+		}
+	}()
+
+	select {
+	case <-read:
+	case <-time.After(readDeadline):
+		t.Fatal("the long poll did not read the queue")
+	}
+
+	giveUp()
+	<-done
+
+	tserver.checkWaitChannelRemoved(sessionID)
+}
+
+// checkWaitChannelRemoved fails the test unless the broker
+// holds no wait channel for the session sessionID. A client
+// can have its response before the handler has returned, and
+// a handler notices a client that gives up only later, so it
+// waits up to removeDeadline for that.
+func (tserver *testServer) checkWaitChannelRemoved(sessionID int64) {
+	tserver.t.Helper()
+
+	const (
+		removeDeadline = 5 * time.Second
+		checkStep      = 10 * time.Millisecond
+	)
+
+	deadline := time.Now().Add(removeDeadline)
+
+	for tserver.broker.ListenerCount(sessionID) != 0 {
+		if time.Now().After(deadline) {
+			tserver.t.Fatalf("got %d broker wait channels, want 0",
+				tserver.broker.ListenerCount(sessionID))
+		}
+
+		time.Sleep(checkStep)
+	}
 }
 
 // queueMessage stores a PRIVMSG from sender to nick, queues
@@ -3013,32 +3267,73 @@ func TestPassCommand(t *testing.T) {
 	}
 }
 
-func TestPassCommandShortPassword(t *testing.T) {
+// The shortest and longest password PASS accepts; bcrypt
-	tserver := newTestServer(t)
+// cannot hash more than 72 bytes.
-	token := tserver.createSession("shortpw")
+const (
+	minPasswordBytes = 8
+	maxPasswordBytes = 72
+)
 
-	// Drain initial messages.
+// TestPassCommandLength checks that PASS stores a password of
-	_, lastID := tserver.pollMessages(token, 0)
+// 8 to 72 bytes and refuses any other with 461. The limit
-
+// counts bytes: "é" is two bytes in UTF-8, so 72 of them are
-	// Try short password — should fail.
+// refused.
-	status, _ := tserver.sendCommand(
+func TestPassCommandLength(t *testing.T) {
-		token,
+	cases := []struct {
-		map[string]any{
+		name     string
-			commandKey: irc.CmdPass,
+		password string
-			bodyKey:    []string{"short"},
+		stored   bool
-		},
+	}{
-	)
+		{"empty", "", false},
-	if status != http.StatusOK {
+		{"7 bytes", strings.Repeat("a", minPasswordBytes-1), false},
-		t.Fatalf("expected 200, got %d", status)
+		{"8 bytes", strings.Repeat("a", minPasswordBytes), true},
+		{"72 bytes", strings.Repeat("a", maxPasswordBytes), true},
+		{"73 bytes", strings.Repeat("a", maxPasswordBytes+1), false},
+		{"72 characters", strings.Repeat("é", maxPasswordBytes), false},
 	}
 
-	msgs, _ := tserver.pollMessages(token, lastID)
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			tserver := newTestServer(t)
+			token := tserver.createSession("pwlength")
 
-	if !findNumeric(msgs, "461") {
+			_, lastID := tserver.pollMessages(token, 0)
-		t.Fatalf(
+
-			"expected ERR_NEEDMOREPARAMS (461), got %v",
+			status, result := tserver.sendCommand(
-			msgs,
+				token,
-		)
+				map[string]any{
+					commandKey: irc.CmdPass,
+					bodyKey:    []string{tc.password},
+				},
+			)
+			if status != http.StatusOK {
+				t.Fatalf("expected 200, got %d: %v", status, result)
+			}
+
+			if !tc.stored {
+				msgs, _ := tserver.pollMessages(token, lastID)
+				if !findNumeric(msgs, "461") {
+					t.Fatalf(
+						"expected ERR_NEEDMOREPARAMS (461), got %v",
+						msgs,
+					)
+				}
+
+				return
+			}
+
+			if result[statusKey] != "ok" {
+				t.Fatalf("expected ok, got %v", result)
+			}
+
+			postJSONExpectStatus(
+				t, tserver, "/api/v1/login",
+				map[string]string{
+					nickKey: "pwlength", passwordKey: tc.password,
+				},
+				http.StatusOK,
+			)
+		})
 	}
 }
 

internal/handlers/auth.go

@@ -8,7 +8,12 @@ import (
 	"sneak.berlin/go/neoirc/pkg/irc"
 )
 
-const minPasswordLength = 8
+// A session password is 8 to 72 bytes; bcrypt cannot hash
+// a longer one.
+const (
+	minPasswordLength = 8
+	maxPasswordLength = 72
+)
 
 // HandleLogin authenticates a user with nick and password.
 func (hdlr *Handlers) HandleLogin() http.HandlerFunc {
@@ -149,12 +154,13 @@ func (hdlr *Handlers) handlePass(
 
 	password := lines[0]
 
-	if len(password) < minPasswordLength {
+	if len(password) < minPasswordLength ||
+		len(password) > maxPasswordLength {
 		hdlr.respondIRCError(
 			writer, request, clientID, sessionID,
 			irc.ErrNeedMoreParams, nick,
 			[]string{irc.CmdPass},
-			"Password must be at least 8 characters",
+			"Password must be 8 to 72 bytes",
 		)
 
 		return

internal/handlers/handlers.go

@@ -67,7 +67,8 @@ type Handlers struct {
 
 	// pollMessages is Database.PollMessages, with which
 	// HandleGetMessages reads the client output queue. Tests
-	// replace it to queue a message just after a read.
+	// replace it to act just after a read, or to make a read
+	// fail.
 	pollMessages func(
 		ctx context.Context,
 		clientID, afterQueueID int64,

internal/ircserver/commands.go

@@ -1216,15 +1216,14 @@ func (c *Conn) handlePassPostReg(
 	}
 
 	password := msg.Params[0]
-	if len(password) < minPasswordLen {
+	if !c.checkPasswordLength(password) {
-		c.sendFromServer("NOTICE", c.nick,
-			"Password must be at least 8 characters",
-		)
-
 		return
 	}
 
-	c.setPassword(ctx, password)
+	err := c.setPassword(ctx, password)
+	if err != nil {
+		return
+	}
 
 	c.sendFromServer("NOTICE", c.nick,
 		"Password set. You can reconnect using "+

internal/ircserver/conn.go

@@ -27,6 +27,7 @@ const (
 	pingInterval    = 90 * time.Second
 	pongDeadline    = 30 * time.Second
 	minPasswordLen  = 8
+	maxPasswordLen  = 72 // bcrypt cannot hash a longer one
 	maxHashcashBits = 40
 )
 
@@ -62,6 +63,7 @@ type Conn struct {
 	registered bool
 	gotNick    bool
 	gotUser    bool
+	gotPass    bool
 	passWord   string
 
 	lastQueueID int64
@@ -436,6 +438,7 @@ func (c *Conn) handlePreRegistration(
 		}
 
 		c.passWord = msg.Params[0]
+		c.gotPass = true
 	case irc.CmdNick:
 		if len(msg.Params) < 1 {
 			c.sendNumeric(
@@ -550,17 +553,20 @@ func (c *Conn) completeRegistration(ctx context.Context) {
 	// So that KILL, from either transport, can close it.
 	c.svc.RegisterWireConn(sessionID, c)
 
-	// If PASS was provided before registration, set the
-	// session password.
-	if c.passWord != "" && len(c.passWord) >= minPasswordLen {
-		c.setPassword(ctx, c.passWord)
-	}
-
 	// Send welcome burst.
 	c.deliverWelcome()
 	c.deliverLusers(ctx)
 	c.deliverMOTD()
 
+	// If PASS was given before registration, store its
+	// password now, after the welcome burst, so that a
+	// NOTICE refusing it reaches a registered client. A
+	// failure to store it is only logged, as it is for PASS
+	// after registration.
+	if c.gotPass && c.checkPasswordLength(c.passWord) {
+		_ = c.setPassword(ctx, c.passWord)
+	}
+
 	// Start the message relay goroutine.
 	go c.relayMessages(ctx)
 }
@@ -643,13 +649,33 @@ func (c *Conn) deliverMOTD() {
 	)
 }
 
-// setPassword sets a bcrypt password on the session.
+// checkPasswordLength reports whether pw is 8 to 72 bytes
-func (c *Conn) setPassword(ctx context.Context, pw string) {
+// long, and sends the client a NOTICE saying so when it is
+// not.
+func (c *Conn) checkPasswordLength(pw string) bool {
+	if len(pw) >= minPasswordLen && len(pw) <= maxPasswordLen {
+		return true
+	}
+
+	c.sendFromServer("NOTICE", c.nick,
+		"Password must be 8 to 72 bytes",
+	)
+
+	return false
+}
+
+// setPassword sets a bcrypt password on the session, and
+// logs and returns the error when it was not stored.
+func (c *Conn) setPassword(ctx context.Context, pw string) error {
 	// Use the database's auth module to hash and store.
 	err := c.database.SetPassword(ctx, c.sessionID, pw)
 	if err != nil {
 		c.log.Error(
 			"failed to set password", "error", err,
 		)
+
+		return err
 	}
+
+	return nil
 }

internal/ircserver/relay_test.go

@@ -47,14 +47,13 @@ func (env *testEnv) queueMessage(
 	env.brk.Notify(sessionID)
 }
 
-// TestRelayKeepsOneWaitChannel checks that an idle IRC
+// TestRelayKeepsOneWaitChannel checks that an IRC listener
-// listener connection holds at most one broker wait channel
+// connection holds at most one broker wait channel across
-// across several relay ticks, and none once it closes.
+// several relay ticks, both while idle and after the relay
+// has delivered it a message, and none once it closes.
 func TestRelayKeepsOneWaitChannel(t *testing.T) {
 	t.Parallel()
 
-	const idleTicks = 5
-
 	env := newTestEnv(t)
 
 	client := env.dial(t)
@@ -65,15 +64,17 @@ func TestRelayKeepsOneWaitChannel(t *testing.T) {
 		t.Fatalf("get session of idle: %v", err)
 	}
 
-	for tick := range idleTicks {
+	env.checkOneWaitChannel(t, sessionID)
-		time.Sleep(ircserver.PollInterval)
 
-		got := env.brk.ListenerCount(sessionID)
+	env.queueMessage(
-		if got > 1 {
+		t, "idle", "PRIVMSG", "carol", "idle", []string{"hello"},
-			t.Fatalf("tick %d: got %d wait channels, want at most 1",
+	)
-				tick+1, got)
+
-		}
+	client.readUntil(func(l string) bool {
-	}
+		return strings.Contains(l, "hello")
+	})
+
+	env.checkOneWaitChannel(t, sessionID)
 
 	err = client.conn.Close()
 	if err != nil {
@@ -92,6 +93,28 @@ func TestRelayKeepsOneWaitChannel(t *testing.T) {
 	}
 }
 
+// checkOneWaitChannel fails the test if the session
+// sessionID holds more than one broker wait channel at any of
+// several relay ticks.
+func (env *testEnv) checkOneWaitChannel(
+	t *testing.T,
+	sessionID int64,
+) {
+	t.Helper()
+
+	const ticks = 5
+
+	for tick := range ticks {
+		time.Sleep(ircserver.PollInterval)
+
+		got := env.brk.ListenerCount(sessionID)
+		if got > 1 {
+			t.Fatalf("tick %d: got %d wait channels, want at most 1",
+				tick+1, got)
+		}
+	}
+}
+
 // TestRelayMultiLineBody checks that each line of a PRIVMSG
 // or NOTICE body sent over the HTTP API reaches an IRC
 // listener user as its own line.

internal/ircserver/server_test.go

@@ -1158,6 +1158,165 @@ func TestHandlePassPostRegistration(t *testing.T) {
 	)
 }
 
+// The shortest and longest password PASS accepts; bcrypt
+// cannot hash more than 72 bytes.
+const (
+	minPasswordBytes = 8
+	maxPasswordBytes = 72
+)
+
+// passwordLengthNotice is the NOTICE text for a password that
+// is too short or too long.
+const passwordLengthNotice = "Password must be 8 to 72 bytes"
+
+// passwordCase is a password a PASS test sends, and whether
+// the server should store it.
+type passwordCase struct {
+	name     string
+	password string
+	stored   bool
+}
+
+// passwordCases returns the passwords the PASS length tests
+// send: one of 8 to 72 bytes is stored and any other is
+// refused. The limit counts bytes: "é" is two bytes in UTF-8,
+// so 72 of them are refused.
+func passwordCases() []passwordCase {
+	return []passwordCase{
+		{"empty", "", false},
+		{"7 bytes", strings.Repeat("a", minPasswordBytes-1), false},
+		{"8 bytes", strings.Repeat("a", minPasswordBytes), true},
+		{"72 bytes", strings.Repeat("a", maxPasswordBytes), true},
+		{"73 bytes", strings.Repeat("a", maxPasswordBytes+1), false},
+		{"72 characters", strings.Repeat("é", maxPasswordBytes), false},
+	}
+}
+
+// assertStored fails the test unless nick can log in with
+// password exactly when stored is true.
+func assertStored(
+	t *testing.T, env *testEnv, nick, password string, stored bool,
+) {
+	t.Helper()
+
+	_, _, token, err := env.database.LoginUser(
+		t.Context(), nick, password, "127.0.0.1", "localhost",
+	)
+
+	loggedIn := err == nil && token != ""
+	if loggedIn != stored {
+		t.Errorf(
+			"log in as %s: logged in %v, want %v (error: %v)",
+			nick, loggedIn, stored, err,
+		)
+	}
+}
+
+// TestPassPostRegistrationLength checks that PASS after
+// registration stores and confirms a password of 8 to 72
+// bytes, and refuses any other with a NOTICE.
+func TestPassPostRegistrationLength(t *testing.T) {
+	t.Parallel()
+
+	env := newTestEnv(t)
+
+	for i, tc := range passwordCases() {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			nick := fmt.Sprintf("postpass%d", i)
+			client := env.dial(t)
+
+			client.register(nick)
+			client.send("PASS :" + tc.password)
+
+			// The server answers commands in order, so its
+			// reply to PASS comes before the PONG.
+			lines := client.sendAndExpect("PING :after", "PONG")
+
+			reply := passwordLengthNotice
+			if tc.stored {
+				reply = "Password set"
+			}
+
+			assertContains(t, lines, reply, "reply to PASS")
+			assertStored(t, env, nick, tc.password, tc.stored)
+		})
+	}
+}
+
+// TestPassPostRegistrationNotStored checks that PASS is not
+// confirmed when the password could not be stored.
+func TestPassPostRegistrationNotStored(t *testing.T) {
+	t.Parallel()
+
+	env := newTestEnv(t)
+	client := env.dial(t)
+
+	client.register("failpass")
+
+	_, err := env.database.GetDB().ExecContext(t.Context(),
+		`CREATE TRIGGER refuse_password
+		 BEFORE UPDATE OF password_hash ON sessions
+		 BEGIN SELECT RAISE(ABORT, 'refused'); END`,
+	)
+	if err != nil {
+		t.Fatalf("create trigger: %v", err)
+	}
+
+	client.send("PASS :mypassword123")
+
+	// The server answers commands in order, so anything it
+	// sent about PASS comes before the PONG.
+	lines := client.sendAndExpect("PING :after", "PONG")
+
+	assertNotContains(
+		t, lines, "Password set",
+		"confirmation of a password that was not stored",
+	)
+}
+
+// TestPassBeforeRegistrationLength checks that a PASS before
+// registration stores a password of 8 to 72 bytes, and that
+// any other is refused with a NOTICE once registration
+// completes.
+func TestPassBeforeRegistrationLength(t *testing.T) {
+	t.Parallel()
+
+	env := newTestEnv(t)
+
+	for i, tc := range passwordCases() {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			nick := fmt.Sprintf("prepass%d", i)
+			client := env.dial(t)
+
+			client.send("PASS :" + tc.password)
+			client.register(nick)
+
+			// The password is checked after the welcome
+			// burst, which register reads; the PONG comes
+			// after that.
+			lines := client.sendAndExpect("PING :after", "PONG")
+
+			if tc.stored {
+				assertNotContains(
+					t, lines, passwordLengthNotice,
+					"refusal of a valid password",
+				)
+			} else {
+				assertContains(
+					t, lines, passwordLengthNotice,
+					"refusal after registration",
+				)
+			}
+
+			assertStored(t, env, nick, tc.password, tc.stored)
+		})
+	}
+}
+
 func TestPreRegistrationNotRegistered(t *testing.T) {
 	t.Parallel()