Refuse passwords over 72 bytes and confirm only stored ones (closes #144)

bcrypt cannot hash a password longer than 72 bytes. PASS over the HTTP
API answered one with a 500, and the IRC listener confirmed it without
storing it.

Both transports now refuse a password outside 8 to 72 bytes with the
reply a short one already got: 461 over the HTTP API, a NOTICE over the
IRC listener. setPassword returns its error, and "Password set" is sent
only when the password was stored. A PASS given before registration is
checked after the welcome burst and gets the same NOTICE.

Model: opus-5-5

README.md

@@ -1405,8 +1405,7 @@ Include the hashcash stamp in the `meta` field:
 **Server validation:** The server checks that the stamp is well-formed, meets
 the required difficulty, is bound to the correct channel and message body, has a
 recent date, and has not been previously used. Spent stamps are cached for 1
-year to prevent replay attacks. The stamp is checked after membership, bans and
+year to prevent replay attacks.
-`+m`, so a message refused for one of those does not spend it.
 
 **Error responses:** If the channel requires hashcash and the stamp is missing,
 invalid, or replayed, the server returns `ERR_CANNOTSENDTOCHAN (404)` with a
@@ -3088,10 +3087,6 @@ limits that punish shared networks.
     - Stamp has not been used before (replay prevention; the spent stamps are
       kept in memory, so a restart forgets them)
 
-    The stamp is spent only when the session is created. A request refused with
-    400 or 409 leaves it unspent, and the client can send the same stamp again
-    with another nick or username.
-
 ### Stamp Format
 
 Standard hashcash format:
@@ -3164,13 +3159,12 @@ creating one session pays once and keeps their session.
 ### Why Hashcash and Not Rate Limits?
 
 - **Little state to track**: No IP tables, no token buckets, no sliding windows.
-  The server hashes the stamp and remembers the stamps already spent.
+  The server verifies a single hash and remembers the stamps already spent.
 - **Works through NATs and proxies**: Doesn't punish shared IPs (university
   campuses, corporate networks, Tor exits). Every client computes their own
   proof independently.
 - **Cost falls on the requester**: The server's verification cost is constant
-  regardless of difficulty: at most two SHA-256 hashes of the stamp per session
+  (one SHA-256 hash) regardless of difficulty. Only the client does more work.
-  request, one to check it and one to spend it. Only the client does more work.
 - **Fits the "no accounts" philosophy**: Proof-of-work is the cost of entry. No
   registration, no email, no phone number, no CAPTCHA. Just compute.
 - **Language-agnostic**: SHA-256 is available in every programming language. The

internal/cli/api/client_test.go

@@ -207,7 +207,7 @@ func TestCreateSessionSendsNickAndStamp(t *testing.T) {
 			return
 		}
 
-		err = validator.Claim(payload.Hashcash, stampBits)
+		err = validator.Validate(payload.Hashcash, stampBits)
 		if err != nil {
 			writer.WriteHeader(http.StatusPaymentRequired)
 

internal/handlers/api.go

@@ -199,7 +199,7 @@ func (hdlr *Handlers) handleCreateSession(
 		return
 	}
 
-	if !hdlr.checkHashcash(
+	if !hdlr.validateHashcash(
 		writer, request, payload.Hashcash,
 	) {
 		return
@@ -232,15 +232,14 @@ func (hdlr *Handlers) handleCreateSession(
 	}
 
 	hdlr.executeCreateSession(
-		writer, request,
+		writer, request, payload.Nick, username,
-		payload.Nick, username, payload.Hashcash,
 	)
 }
 
 func (hdlr *Handlers) executeCreateSession(
 	writer http.ResponseWriter,
 	request *http.Request,
-	nick, username, stamp string,
+	nick, username string,
 ) {
 	remoteIP := clientIP(request)
 
@@ -248,17 +247,12 @@ func (hdlr *Handlers) executeCreateSession(
 		request.Context(), remoteIP,
 	)
 
-	if !hdlr.claimHashcash(writer, request, stamp) {
-		return
-	}
-
 	sessionID, clientID, token, err :=
 		hdlr.params.Database.CreateSession(
 			request.Context(),
 			nick, username, hostname, remoteIP,
 		)
 	if err != nil {
-		hdlr.hashcashVal.Release(stamp)
 		hdlr.handleCreateSessionError(
 			writer, request, err,
 		)
@@ -279,10 +273,10 @@ func (hdlr *Handlers) executeCreateSession(
 	}, http.StatusCreated)
 }
 
-// checkHashcash checks a hashcash stamp if required,
+// validateHashcash validates a hashcash stamp if required.
-// without spending it. Returns false if the check failed
+// Returns false if validation failed and a response was
-// and a response was already sent.
+// already sent.
-func (hdlr *Handlers) checkHashcash(
+func (hdlr *Handlers) validateHashcash(
 	writer http.ResponseWriter,
 	request *http.Request,
 	stamp string,
@@ -301,31 +295,7 @@ func (hdlr *Handlers) checkHashcash(
 		return false
 	}
 
-	err := hdlr.hashcashVal.Check(
+	err := hdlr.hashcashVal.Validate(
-		stamp, hdlr.params.Config.HashcashBits,
-	)
-	if err != nil {
-		hdlr.respondError(
-			writer, request,
-			"invalid hashcash stamp: "+err.Error(),
-			http.StatusPaymentRequired,
-		)
-
-		return false
-	}
-
-	return true
-}
-
-// claimHashcash spends a stamp that checkHashcash accepted.
-// Returns false if Claim refused the stamp, normally because
-// it was already spent, and a response was already sent.
-func (hdlr *Handlers) claimHashcash(
-	writer http.ResponseWriter,
-	request *http.Request,
-	stamp string,
-) bool {
-	err := hdlr.hashcashVal.Claim(
 		stamp, hdlr.params.Config.HashcashBits,
 	)
 	if err != nil {

internal/handlers/api_test.go

@@ -29,7 +29,6 @@ import (
 	"golang.org/x/crypto/bcrypt"
 	"modernc.org/sqlite"
 	"sneak.berlin/go/neoirc/internal/broker"
-	api "sneak.berlin/go/neoirc/internal/cli/api"
 	"sneak.berlin/go/neoirc/internal/config"
 	"sneak.berlin/go/neoirc/internal/db"
 	"sneak.berlin/go/neoirc/internal/globals"
@@ -2906,73 +2905,84 @@ func TestPassCommand(t *testing.T) {
 	}
 }
 
-// The shortest and longest password PASS accepts; bcrypt
+func TestPassCommandShortPassword(t *testing.T) {
-// cannot hash more than 72 bytes.
+	tserver := newTestServer(t)
-const (
+	token := tserver.createSession("shortpw")
-	minPasswordBytes = 8
-	maxPasswordBytes = 72
-)
 
-// TestPassCommandLength checks that PASS stores a password of
+	// Drain initial messages.
-// 8 to 72 bytes and refuses any other with 461. The limit
+	_, lastID := tserver.pollMessages(token, 0)
-// counts bytes: "é" is two bytes in UTF-8, so 72 of them are
+
-// refused.
+	// Try short password — should fail.
-func TestPassCommandLength(t *testing.T) {
+	status, _ := tserver.sendCommand(
-	cases := []struct {
+		token,
-		name     string
+		map[string]any{
-		password string
+			commandKey: irc.CmdPass,
-		stored   bool
+			bodyKey:    []string{"short"},
-	}{
+		},
-		{"empty", "", false},
+	)
-		{"7 bytes", strings.Repeat("a", minPasswordBytes-1), false},
+	if status != http.StatusOK {
-		{"8 bytes", strings.Repeat("a", minPasswordBytes), true},
+		t.Fatalf("expected 200, got %d", status)
-		{"72 bytes", strings.Repeat("a", maxPasswordBytes), true},
-		{"73 bytes", strings.Repeat("a", maxPasswordBytes+1), false},
-		{"72 characters", strings.Repeat("é", maxPasswordBytes), false},
 	}
 
-	for _, tc := range cases {
+	msgs, _ := tserver.pollMessages(token, lastID)
-		t.Run(tc.name, func(t *testing.T) {
-			tserver := newTestServer(t)
-			token := tserver.createSession("pwlength")
 
-			_, lastID := tserver.pollMessages(token, 0)
+	if !findNumeric(msgs, "461") {
+		t.Fatalf(
+			"expected ERR_NEEDMOREPARAMS (461), got %v",
+			msgs,
+		)
+	}
+}
 
-			status, result := tserver.sendCommand(
+// maxPasswordBytes is the longest password bcrypt can hash.
-				token,
+const maxPasswordBytes = 72
-				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 {
+// TestPassCommandLongPassword checks that a password over
-				msgs, _ := tserver.pollMessages(token, lastID)
+// maxPasswordBytes is refused like a short one, and that one
-				if !findNumeric(msgs, "461") {
+// of exactly maxPasswordBytes is stored.
-					t.Fatalf(
+func TestPassCommandLongPassword(t *testing.T) {
-						"expected ERR_NEEDMOREPARAMS (461), got %v",
+	tserver := newTestServer(t)
-						msgs,
+	token := tserver.createSession("longpw")
-					)
-				}
 
-				return
+	// Drain initial messages.
-			}
+	_, lastID := tserver.pollMessages(token, 0)
 
-			if result[statusKey] != "ok" {
+	status, _ := tserver.sendCommand(
-				t.Fatalf("expected ok, got %v", result)
+		token,
-			}
+		map[string]any{
+			commandKey: irc.CmdPass,
+			bodyKey: []string{
+				strings.Repeat("a", maxPasswordBytes+1),
+			},
+		},
+	)
+	if status != http.StatusOK {
+		t.Fatalf("expected 200, got %d", status)
+	}
 
-			postJSONExpectStatus(
+	msgs, _ := tserver.pollMessages(token, lastID)
-				t, tserver, "/api/v1/login",
+
-				map[string]string{
+	if !findNumeric(msgs, "461") {
-					nickKey: "pwlength", passwordKey: tc.password,
+		t.Fatalf(
-				},
+			"expected ERR_NEEDMOREPARAMS (461), got %v",
-				http.StatusOK,
+			msgs,
-			)
+		)
-		})
+	}
+
+	status, result := tserver.sendCommand(
+		token,
+		map[string]any{
+			commandKey: irc.CmdPass,
+			bodyKey: []string{
+				strings.Repeat("a", maxPasswordBytes),
+			},
+		},
+	)
+	if status != http.StatusOK || result[statusKey] != "ok" {
+		t.Fatalf(
+			"expected 200 and ok, got %d: %v",
+			status, result,
+		)
 	}
 }
 
@@ -4115,121 +4125,6 @@ func TestChannelHashcashMissingBitsArg(t *testing.T) {
 	}
 }
 
-// postSessionWithStamp posts a session request for nick and
-// username carrying stamp and returns the response status.
-// An empty username defaults to the nick.
-func postSessionWithStamp(
-	t *testing.T,
-	tserver *testServer,
-	nick, username, stamp string,
-) int {
-	t.Helper()
-
-	body, err := json.Marshal(map[string]string{
-		nickKey: nick, "username": username, "pow_token": stamp,
-	})
-	if err != nil {
-		t.Fatalf("marshal: %v", err)
-	}
-
-	resp, err := doRequest(
-		t, http.MethodPost, tserver.url(apiSession),
-		bytes.NewReader(body),
-	)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	_ = resp.Body.Close()
-
-	return resp.StatusCode
-}
-
-// sessionCount returns the number of sessions in tserver's
-// database.
-func sessionCount(t *testing.T, tserver *testServer) int64 {
-	t.Helper()
-
-	count, err := tserver.database.GetUserCount(t.Context())
-	if err != nil {
-		t.Fatalf("count sessions: %v", err)
-	}
-
-	return count
-}
-
-// TestRefusedSessionLeavesStampUnspent checks that a
-// session request is refused with 402 for an invalid stamp
-// before its nick is looked at, that a request refused with
-// 400 or 409 does not spend its stamp, so the same stamp
-// then creates a session, that the stamp stays spent after
-// that, that a spent stamp with an invalid nick is refused
-// for its nick, and that no refused request creates a
-// session.
-func TestRefusedSessionLeavesStampUnspent(t *testing.T) {
-	const (
-		bits    = 2
-		badNick = "bad nick"
-	)
-
-	tserver := newTestServerWith(t, bits, "", "")
-
-	// A server with no SERVER_NAME takes stamps for "neoirc".
-	status := postSessionWithStamp(
-		t, tserver, "taken", "", api.MintHashcash(bits, "neoirc"),
-	)
-	if status != http.StatusCreated {
-		t.Fatalf("create taken: want 201, got %d", status)
-	}
-
-	stamp := api.MintHashcash(bits, "neoirc")
-	otherServerStamp := api.MintHashcash(bits, "elsewhere")
-
-	steps := []struct {
-		nick, username, stamp string
-		want                  int
-	}{
-		{badNick, "", otherServerStamp, http.StatusPaymentRequired},
-		{badNick, "", stamp, http.StatusBadRequest},
-		{"fresh", "bad username", stamp, http.StatusBadRequest},
-		{"taken", "", stamp, http.StatusConflict},
-		{"fresh", "", stamp, http.StatusCreated},
-		{"another", "", stamp, http.StatusPaymentRequired},
-		// Refused as already spent, the stamp is not given back.
-		{"later", "", stamp, http.StatusPaymentRequired},
-		// Whether the stamp is spent is looked at after the nick.
-		{badNick, "", stamp, http.StatusBadRequest},
-		// The refused request for "another" left the nick free.
-		{"another", "", api.MintHashcash(bits, "neoirc"), http.StatusCreated},
-	}
-
-	sessions := sessionCount(t, tserver)
-
-	for _, step := range steps {
-		status = postSessionWithStamp(
-			t, tserver, step.nick, step.username, step.stamp,
-		)
-		if status != step.want {
-			t.Fatalf(
-				"nick %q, username %q: want %d, got %d",
-				step.nick, step.username, step.want, status,
-			)
-		}
-
-		if status == http.StatusCreated {
-			sessions++
-		}
-
-		count := sessionCount(t, tserver)
-		if count != sessions {
-			t.Fatalf(
-				"nick %q, username %q: want %d sessions, got %d",
-				step.nick, step.username, sessions, count,
-			)
-		}
-	}
-}
-
 func TestNamesShowsHostmask(t *testing.T) {
 	tserver := newTestServer(t)
 

internal/hashcash/hashcash.go

@@ -77,13 +77,9 @@ func NewValidator(resource string) *Validator {
 	return validator
 }
 
-// Check returns nil if requiredBits is not positive, or if
+// Validate checks a hashcash stamp. It returns nil if the
-// the stamp is a well-formed, recently dated proof of work
+// stamp is valid and has not been seen before.
-// for this resource with at least requiredBits. It does not
+func (v *Validator) Validate(
-// look at or change the set of spent stamps, so a request
-// can check its stamp first and Claim it after its other
-// checks.
-func (v *Validator) Check(
 	stamp string,
 	requiredBits int,
 ) error {
@@ -91,49 +87,9 @@ func (v *Validator) Check(
 		return nil
 	}
 
-	_, err := v.check(stamp, requiredBits)
-
-	return err
-}
-
-// Claim checks the stamp as Check does and, when
-// requiredBits is positive, records it as spent until it
-// expires. It returns an error if the stamp is already
-// spent. Release gives a claimed stamp back.
-func (v *Validator) Claim(
-	stamp string,
-	requiredBits int,
-) error {
-	if requiredBits <= 0 {
-		return nil
-	}
-
-	stampTime, err := v.check(stamp, requiredBits)
-	if err != nil {
-		return err
-	}
-
-	return v.checkAndRecordStamp(stamp, stampTime)
-}
-
-// Release forgets a claimed stamp so that it can be claimed
-// again, for a request that failed after its Claim.
-// Releasing a stamp that is not claimed does nothing.
-func (v *Validator) Release(stamp string) {
-	v.mu.Lock()
-	defer v.mu.Unlock()
-
-	delete(v.spent, stamp)
-}
-
-// check returns the stamp's date if Check accepts it.
-func (v *Validator) check(
-	stamp string,
-	requiredBits int,
-) (time.Time, error) {
 	parts := strings.Split(stamp, ":")
 	if len(parts) != stampFields {
-		return time.Time{}, fmt.Errorf(
+		return fmt.Errorf(
 			"%w: expected %d, got %d",
 			errInvalidFields, stampFields, len(parts),
 		)
@@ -148,25 +104,25 @@ func (v *Validator) check(
 		version, bitsStr, resource, requiredBits,
 	)
 	if err != nil {
-		return time.Time{}, err
+		return err
 	}
 
 	stampTime, err := parseStampDate(dateStr)
 	if err != nil {
-		return time.Time{}, err
+		return err
 	}
 
 	err = validateTime(stampTime)
 	if err != nil {
-		return time.Time{}, err
+		return err
 	}
 
 	err = validateProof(stamp, requiredBits)
 	if err != nil {
-		return time.Time{}, err
+		return err
 	}
 
-	return stampTime, nil
+	return v.checkAndRecordStamp(stamp, stampTime)
 }
 
 func (v *Validator) validateHeader(

internal/hashcash/hashcash_test.go

@@ -85,7 +85,7 @@ func TestMintAndValidate(t *testing.T) {
 		t, testBits, "test-resource", todayDate(),
 	)
 
-	err := validator.Claim(stamp, testBits)
+	err := validator.Validate(stamp, testBits)
 	if err != nil {
 		t.Fatalf("valid stamp rejected: %v", err)
 	}
@@ -99,74 +99,17 @@ func TestReplayDetection(t *testing.T) {
 		t, testBits, "test-resource", todayDate(),
 	)
 
-	err := validator.Claim(stamp, testBits)
+	err := validator.Validate(stamp, testBits)
 	if err != nil {
 		t.Fatalf("first use failed: %v", err)
 	}
 
-	err = validator.Claim(stamp, testBits)
+	err = validator.Validate(stamp, testBits)
 	if err == nil {
 		t.Fatal("replay not detected")
 	}
 }
 
-// TestCheckDoesNotSpendStamp checks that a stamp Check
-// accepts can still be claimed, and only once, and that
-// Check still accepts it once it is spent.
-func TestCheckDoesNotSpendStamp(t *testing.T) {
-	t.Parallel()
-
-	validator := hashcash.NewValidator("test-resource")
-	stamp := mintStampWithDate(
-		t, testBits, "test-resource", todayDate(),
-	)
-
-	for range 2 {
-		err := validator.Check(stamp, testBits)
-		if err != nil {
-			t.Fatalf("valid stamp rejected: %v", err)
-		}
-	}
-
-	err := validator.Claim(stamp, testBits)
-	if err != nil {
-		t.Fatalf("claim after check failed: %v", err)
-	}
-
-	err = validator.Claim(stamp, testBits)
-	if err == nil {
-		t.Fatal("replay not detected")
-	}
-
-	err = validator.Check(stamp, testBits)
-	if err != nil {
-		t.Fatalf("spent stamp rejected by Check: %v", err)
-	}
-}
-
-// TestReleasedStampCanBeClaimedAgain checks that Release
-// gives a claimed stamp back.
-func TestReleasedStampCanBeClaimedAgain(t *testing.T) {
-	t.Parallel()
-
-	validator := hashcash.NewValidator("test-resource")
-	stamp := mintStampWithDate(
-		t, testBits, "test-resource", todayDate(),
-	)
-
-	err := validator.Claim(stamp, testBits)
-	if err != nil {
-		t.Fatalf("first claim failed: %v", err)
-	}
-
-	validator.Release(stamp)
-
-	err = validator.Claim(stamp, testBits)
-	if err != nil {
-		t.Fatalf("claim after release failed: %v", err)
-	}
-}
-
 func TestResourceMismatch(t *testing.T) {
 	t.Parallel()
 
@@ -175,7 +118,7 @@ func TestResourceMismatch(t *testing.T) {
 		t, testBits, "wrong-resource", todayDate(),
 	)
 
-	err := validator.Claim(stamp, testBits)
+	err := validator.Validate(stamp, testBits)
 	if err == nil {
 		t.Fatal("expected resource mismatch error")
 	}
@@ -186,7 +129,7 @@ func TestInvalidStampFormat(t *testing.T) {
 
 	validator := hashcash.NewValidator("test-resource")
 
-	err := validator.Claim(
+	err := validator.Validate(
 		"not:a:valid:stamp", testBits,
 	)
 	if err == nil {
@@ -203,7 +146,7 @@ func TestBadVersion(t *testing.T) {
 		testBits, todayDate(), "test-resource",
 	)
 
-	err := validator.Claim(stamp, testBits)
+	err := validator.Validate(stamp, testBits)
 	if err == nil {
 		t.Fatal("expected bad version error")
 	}
@@ -219,7 +162,7 @@ func TestInsufficientDifficulty(t *testing.T) {
 		todayDate(), "test-resource",
 	)
 
-	err := validator.Claim(stamp, testBits)
+	err := validator.Validate(stamp, testBits)
 	if err == nil {
 		t.Fatal("expected insufficient bits error")
 	}
@@ -235,7 +178,7 @@ func TestExpiredStamp(t *testing.T) {
 		t, testBits, "test-resource", oldDate,
 	)
 
-	err := validator.Claim(stamp, testBits)
+	err := validator.Validate(stamp, testBits)
 	if err == nil {
 		t.Fatal("expected expired stamp error")
 	}
@@ -246,14 +189,9 @@ func TestZeroBitsSkipsValidation(t *testing.T) {
 
 	validator := hashcash.NewValidator("test-resource")
 
-	err := validator.Check("garbage", 0)
+	err := validator.Validate("garbage", 0)
 	if err != nil {
-		t.Fatalf("zero bits should skip the check: %v", err)
+		t.Fatalf("zero bits should skip: %v", err)
-	}
-
-	err = validator.Claim("garbage", 0)
-	if err != nil {
-		t.Fatalf("zero bits should skip the claim: %v", err)
 	}
 }
 
@@ -266,7 +204,7 @@ func TestLongDateFormat(t *testing.T) {
 		t, testBits, "test-resource", longDate,
 	)
 
-	err := validator.Claim(stamp, testBits)
+	err := validator.Validate(stamp, testBits)
 	if err != nil {
 		t.Fatalf("long date stamp rejected: %v", err)
 	}
@@ -281,7 +219,7 @@ func TestBadDateFormat(t *testing.T) {
 		testBits, "test-resource",
 	)
 
-	err := validator.Claim(stamp, testBits)
+	err := validator.Validate(stamp, testBits)
 	if err == nil {
 		t.Fatal("expected bad date error")
 	}
@@ -297,7 +235,7 @@ func TestMultipleUniqueStamps(t *testing.T) {
 			t, testBits, "test-resource", todayDate(),
 		)
 
-		err := validator.Claim(stamp, testBits)
+		err := validator.Validate(stamp, testBits)
 		if err != nil {
 			t.Fatalf("unique stamp rejected: %v", err)
 		}
@@ -313,7 +251,7 @@ func TestHigherBitsStillValid(t *testing.T) {
 		t, 4, "test-resource", todayDate(),
 	)
 
-	err := validator.Claim(stamp, testBits)
+	err := validator.Validate(stamp, testBits)
 	if err != nil {
 		t.Fatalf(
 			"higher-difficulty stamp rejected: %v",

internal/ircserver/conn.go

@@ -63,7 +63,6 @@ type Conn struct {
 	registered bool
 	gotNick    bool
 	gotUser    bool
-	gotPass    bool
 	passWord   string
 
 	lastQueueID int64
@@ -438,7 +437,6 @@ func (c *Conn) handlePreRegistration(
 		}
 
 		c.passWord = msg.Params[0]
-		c.gotPass = true
 	case irc.CmdNick:
 		if len(msg.Params) < 1 {
 			c.sendNumeric(
@@ -558,12 +556,10 @@ func (c *Conn) completeRegistration(ctx context.Context) {
 	c.deliverLusers(ctx)
 	c.deliverMOTD()
 
-	// If PASS was given before registration, store its
+	// If PASS was provided before registration, set the
-	// password now, after the welcome burst, so that a
+	// session password now. A failure to store it is only
-	// NOTICE refusing it reaches a registered client. A
+	// logged, as it is for PASS after registration.
-	// failure to store it is only logged, as it is for PASS
+	if c.passWord != "" && c.checkPasswordLength(c.passWord) {
-	// after registration.
-	if c.gotPass && c.checkPasswordLength(c.passWord) {
 		_ = c.setPassword(ctx, c.passWord)
 	}
 

internal/ircserver/server_test.go

@@ -1158,91 +1158,47 @@ func TestHandlePassPostRegistration(t *testing.T) {
 	)
 }
 
-// The shortest and longest password PASS accepts; bcrypt
+// maxPasswordBytes is the longest password bcrypt can hash.
-// cannot hash more than 72 bytes.
+const maxPasswordBytes = 72
-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
+// assertCanLogin fails the test if nick cannot log in with
-// the server should store it.
+// password.
-type passwordCase struct {
+func assertCanLogin(t *testing.T, env *testEnv, nick, password string) {
-	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",
 	)
-
+	if err != nil || token == "" {
-	loggedIn := err == nil && token != ""
+		t.Errorf("log in as %s: token %q, %v", nick, token, err)
-	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
+// TestPassPostRegistrationLength checks that a PASS over
-// registration stores and confirms a password of 8 to 72
+// maxPasswordBytes after registration is refused with a
-// bytes, and refuses any other with a NOTICE.
+// NOTICE, and that one of exactly maxPasswordBytes is stored
+// and confirmed.
 func TestPassPostRegistrationLength(t *testing.T) {
 	t.Parallel()
 
 	env := newTestEnv(t)
+	client := env.dial(t)
 
-	for i, tc := range passwordCases() {
+	client.register("longpass")
-		t.Run(tc.name, func(t *testing.T) {
-			t.Parallel()
 
-			nick := fmt.Sprintf("postpass%d", i)
+	client.sendAndExpect(
-			client := env.dial(t)
+		"PASS :"+strings.Repeat("a", maxPasswordBytes+1),
+		passwordLengthNotice,
+	)
 
-			client.register(nick)
+	longest := strings.Repeat("a", maxPasswordBytes)
-			client.send("PASS :" + tc.password)
+	client.sendAndExpect("PASS :"+longest, "Password set")
 
-			// The server answers commands in order, so its
+	assertCanLogin(t, env, "longpass", longest)
-			// 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
@@ -1277,46 +1233,56 @@ func TestPassPostRegistrationNotStored(t *testing.T) {
 }
 
 // TestPassBeforeRegistrationLength checks that a PASS before
-// registration stores a password of 8 to 72 bytes, and that
+// registration that is too short or too long is refused with
-// any other is refused with a NOTICE once registration
+// a NOTICE once registration completes.
-// completes.
 func TestPassBeforeRegistrationLength(t *testing.T) {
 	t.Parallel()
 
+	cases := []struct {
+		nick, password string
+	}{
+		{"preshort", "short"},
+		{"prelong", strings.Repeat("a", maxPasswordBytes+1)},
+	}
+
 	env := newTestEnv(t)
 
-	for i, tc := range passwordCases() {
+	for _, tc := range cases {
-		t.Run(tc.name, func(t *testing.T) {
+		t.Run(tc.nick, func(t *testing.T) {
 			t.Parallel()
 
-			nick := fmt.Sprintf("prepass%d", i)
 			client := env.dial(t)
 
 			client.send("PASS :" + tc.password)
-			client.register(nick)
+			client.register(tc.nick)
 
-			// The password is checked after the welcome
+			client.readUntil(func(line string) bool {
-			// burst, which register reads; the PONG comes
+				return strings.Contains(line, passwordLengthNotice)
-			// 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)
 		})
 	}
 }
 
+// TestPassBeforeRegistrationStored checks that a PASS of
+// exactly maxPasswordBytes before registration is stored.
+func TestPassBeforeRegistrationStored(t *testing.T) {
+	t.Parallel()
+
+	env := newTestEnv(t)
+	client := env.dial(t)
+
+	longest := strings.Repeat("a", maxPasswordBytes)
+
+	client.send("PASS :" + longest)
+	client.register("prestored")
+
+	// The password is stored after the welcome burst; the
+	// PONG comes after that.
+	client.sendAndExpect("PING :after", "PONG")
+
+	assertCanLogin(t, env, "prestored", longest)
+}
+
 func TestPreRegistrationNotRegistered(t *testing.T) {
 	t.Parallel()
 

internal/service/service.go

@@ -387,14 +387,13 @@ func excludeSession(
 	return out
 }
 
-// SendChannelMessage checks membership, bans and
+// SendChannelMessage validates the hashcash stamp of a
-// moderation, then the hashcash stamp of a PRIVMSG to a +H
+// PRIVMSG to a +H channel, membership and moderation, then
-// channel, then fans out a message to all channel members
+// fans out a message to all channel members except the
-// except the sender. The stamp is checked after the other
+// sender. Returns the database row ID, message UUID, and
-// checks so that a message they refuse does not spend it.
+// any error. The dbID lets callers enqueue the same
-// Returns the database row ID, message UUID, and any error.
+// message to the sender when echo is needed (HTTP
-// The dbID lets callers enqueue the same message to the
+// transport).
-// sender when echo is needed (HTTP transport).
 func (s *Service) SendChannelMessage(
 	ctx context.Context,
 	sessionID int64,
@@ -410,6 +409,15 @@ func (s *Service) SendChannelMessage(
 		return 0, "", noSuchChannel
 	}
 
+	if command == irc.CmdPrivmsg {
+		err = s.checkChannelHashcash(
+			ctx, chID, channel, body, meta,
+		)
+		if err != nil {
+			return 0, "", err
+		}
+	}
+
 	isMember, _ := s.db.IsChannelMember(
 		ctx, chID, sessionID,
 	)
@@ -451,15 +459,6 @@ func (s *Service) SendChannelMessage(
 		}
 	}
 
-	if command == irc.CmdPrivmsg {
-		err = s.checkChannelHashcash(
-			ctx, chID, channel, body, meta,
-		)
-		if err != nil {
-			return 0, "", err
-		}
-	}
-
 	memberIDs, _ := s.db.GetChannelMemberIDs(ctx, chID)
 	recipients := excludeSession(memberIDs, sessionID)
 

internal/service/service_test.go

@@ -24,7 +24,6 @@ import (
 	"sneak.berlin/go/neoirc/internal/config"
 	"sneak.berlin/go/neoirc/internal/db"
 	"sneak.berlin/go/neoirc/internal/globals"
-	"sneak.berlin/go/neoirc/internal/hashcash"
 	"sneak.berlin/go/neoirc/internal/logger"
 	"sneak.berlin/go/neoirc/internal/service"
 	"sneak.berlin/go/neoirc/pkg/irc"
@@ -1057,88 +1056,6 @@ func TestSendChannelMessage_Moderated(t *testing.T) {
 	}
 }
 
-// TestRefusedChannelMessageLeavesStampUnspent checks that a
-// PRIVMSG to a +H channel refused because the sender is not
-// a member, is banned, or has no voice on a +m channel does
-// not spend its hashcash stamp: the operator can then send
-// the same message with it.
-func TestRefusedChannelMessageLeavesStampUnspent(t *testing.T) {
-	const (
-		channel = "#paid"
-		bits    = 2
-	)
-
-	refusals := []struct {
-		name string
-		// refuse stops bob from sending to the channel.
-		refuse func(ctx context.Context, env *testEnv, chID, bobSID int64) error
-	}{
-		{"not a member", func(ctx context.Context, env *testEnv, _, bobSID int64) error {
-			return env.svc.PartChannel(ctx, bobSID, "bob", channel, "")
-		}},
-		{"banned", func(ctx context.Context, env *testEnv, chID, _ int64) error {
-			return env.db.AddChannelBan(ctx, chID, "bob!*@*", "alice")
-		}},
-		{"moderated", func(ctx context.Context, env *testEnv, chID, _ int64) error {
-			return env.svc.SetChannelFlag(ctx, chID, 'm', true)
-		}},
-	}
-
-	body, _ := json.Marshal([]string{"hello"}) //nolint:errchkjson
-
-	for _, refusal := range refusals {
-		t.Run(refusal.name, func(t *testing.T) {
-			env := newTestEnv(t)
-			ctx := t.Context()
-
-			aliceSID := createSession(ctx, t, env.db, "alice")
-			bobSID := createSession(ctx, t, env.db, "bob")
-
-			joined, err := env.svc.JoinChannel(ctx, aliceSID, "alice", channel, "")
-			if err != nil {
-				t.Fatalf("join alice: %v", err)
-			}
-
-			_, err = env.svc.JoinChannel(ctx, bobSID, "bob", channel, "")
-			if err != nil {
-				t.Fatalf("join bob: %v", err)
-			}
-
-			err = env.db.SetChannelHashcashBits(ctx, joined.ChannelID, bits)
-			if err != nil {
-				t.Fatalf("set +H: %v", err)
-			}
-
-			err = refusal.refuse(ctx, env, joined.ChannelID, bobSID)
-			if err != nil {
-				t.Fatalf("refuse bob: %v", err)
-			}
-
-			stamp := hashcash.MintChannelStamp(
-				bits, channel, hashcash.BodyHash(body),
-			)
-			meta, _ := json.Marshal(map[string]string{"hashcash": stamp})
-
-			_, _, err = env.svc.SendChannelMessage(
-				ctx, bobSID, "bob", irc.CmdPrivmsg, channel, body, meta,
-			)
-
-			var ircErr *service.IRCError
-			if !errors.As(err, &ircErr) ||
-				ircErr.Code != irc.ErrCannotSendToChan {
-				t.Fatalf("bob: want ERR_CANNOTSENDTOCHAN, got %v", err)
-			}
-
-			_, _, err = env.svc.SendChannelMessage(
-				ctx, aliceSID, "alice", irc.CmdPrivmsg, channel, body, meta,
-			)
-			if err != nil {
-				t.Fatalf("alice with the same stamp: %v", err)
-			}
-		})
-	}
-}
-
 func TestQueryUserMode(t *testing.T) {
 	env := newTestEnv(t)
 	ctx := t.Context()