diff --git a/internal/db/queries.go b/internal/db/queries.go index 655fa04..b5b8419 100644 --- a/internal/db/queries.go +++ b/internal/db/queries.go @@ -462,46 +462,207 @@ func (database *Database) JoinChannel( return nil } -// JoinChannelAsOperator adds a session to a channel with -// operator status. Used when a user creates a new channel. -func (database *Database) JoinChannelAsOperator( +// Errors that CreateOrJoinChannel refuses a join with. +var ( + ErrBannedFromChannel = errors.New("banned from channel") + ErrInviteOnlyChannel = errors.New("channel is invite-only") + ErrBadChannelKey = errors.New("wrong channel key") + ErrChannelIsFull = errors.New("channel is full") +) + +// CreateOrJoinChannel adds a session to the channel called +// name, creating the channel if it does not exist. The +// session is made operator if the channel has no members; +// otherwise checkJoinRestrictions may refuse it. A join +// uses up the session's invite to the channel. It returns +// the channel ID and whether the session was made operator. +// All of it is one transaction, and the database has one +// connection, so every other statement waits until the +// join is decided. +func (database *Database) CreateOrJoinChannel( ctx context.Context, - channelID, sessionID int64, -) error { - _, err := database.conn.ExecContext(ctx, + name string, + sessionID int64, + suppliedKey string, +) (int64, bool, error) { + transaction, err := database.conn.BeginTx(ctx, nil) + if err != nil { + return 0, false, fmt.Errorf("begin join: %w", err) + } + + defer func() { _ = transaction.Rollback() }() + + now := time.Now() + + _, err = transaction.ExecContext(ctx, + `INSERT OR IGNORE INTO channels + (name, created_at, updated_at) + VALUES (?, ?, ?)`, + name, now, now) + if err != nil { + return 0, false, fmt.Errorf("create channel: %w", err) + } + + var channelID, memberCount int64 + + err = transaction.QueryRowContext(ctx, + `SELECT id, + (SELECT COUNT(*) FROM channel_members + WHERE channel_id = channels.id) + FROM channels WHERE name = ?`, + name, + ).Scan(&channelID, &memberCount) + if err != nil { + return 0, false, fmt.Errorf("get channel: %w", err) + } + + isOperator := memberCount == 0 + if !isOperator { + err = checkJoinRestrictions( + ctx, transaction, channelID, sessionID, + suppliedKey, memberCount, + ) + if err != nil { + return 0, false, err + } + } + + _, err = transaction.ExecContext(ctx, `INSERT OR IGNORE INTO channel_members (channel_id, session_id, is_operator, joined_at) - VALUES (?, ?, 1, ?)`, - channelID, sessionID, time.Now()) + VALUES (?, ?, ?, ?)`, + channelID, sessionID, isOperator, now) if err != nil { - return fmt.Errorf( - "join channel as operator: %w", err, - ) + return 0, false, fmt.Errorf("join channel: %w", err) + } + + _, err = transaction.ExecContext(ctx, + `DELETE FROM channel_invites + WHERE channel_id = ? AND session_id = ?`, + channelID, sessionID) + if err != nil { + return 0, false, fmt.Errorf("clear invite: %w", err) + } + + err = transaction.Commit() + if err != nil { + return 0, false, fmt.Errorf("commit join: %w", err) + } + + return channelID, isOperator, nil +} + +// checkJoinRestrictions returns the error that refuses a +// session's join of a channel with memberCount members: +// ErrBannedFromChannel when a ban matches its hostmask, +// ErrInviteOnlyChannel when the channel is +i and the +// session has no invite, ErrBadChannelKey when suppliedKey +// is not the +k key, or ErrChannelIsFull when the channel +// has its +l number of members. It returns nil when none +// applies. +func checkJoinRestrictions( + ctx context.Context, + transaction *sql.Tx, + channelID, sessionID int64, + suppliedKey string, + memberCount int64, +) error { + banned, err := isSessionBannedInTx( + ctx, transaction, channelID, sessionID, + ) + if err != nil { + return err + } + + if banned { + return ErrBannedFromChannel + } + + var ( + inviteOnly, invited bool + key string + userLimit int64 + ) + + err = transaction.QueryRowContext(ctx, + `SELECT is_invite_only, channel_key, user_limit, + EXISTS (SELECT 1 FROM channel_invites + WHERE channel_id = channels.id + AND session_id = ?) + FROM channels WHERE id = ?`, + sessionID, channelID, + ).Scan(&inviteOnly, &key, &userLimit, &invited) + if err != nil { + return fmt.Errorf("get channel modes: %w", err) + } + + switch { + case inviteOnly && !invited: + return ErrInviteOnlyChannel + case key != "" && suppliedKey != key: + return ErrBadChannelKey + case userLimit > 0 && memberCount >= userLimit: + return ErrChannelIsFull } return nil } -// CountChannelMembers returns the number of members in -// a channel. -func (database *Database) CountChannelMembers( +// isSessionBannedInTx is IsSessionBanned, reading through +// transaction. +func isSessionBannedInTx( ctx context.Context, - channelID int64, -) (int64, error) { - var count int64 + transaction *sql.Tx, + channelID, sessionID int64, +) (bool, error) { + var nick, username, hostname string - err := database.conn.QueryRowContext(ctx, - `SELECT COUNT(*) FROM channel_members - WHERE channel_id = ?`, - channelID, - ).Scan(&count) + err := transaction.QueryRowContext(ctx, + `SELECT nick, username, hostname + FROM sessions WHERE id = ?`, + sessionID, + ).Scan(&nick, &username, &hostname) if err != nil { - return 0, fmt.Errorf( - "count channel members: %w", err, + return false, fmt.Errorf( + "get session hostmask: %w", err, ) } - return count, nil + hostmask := FormatHostmask(nick, username, hostname) + + rows, err := transaction.QueryContext(ctx, + `SELECT mask FROM channel_bans + WHERE channel_id = ?`, + channelID) + if err != nil { + return false, fmt.Errorf("list channel bans: %w", err) + } + + defer func() { _ = rows.Close() }() + + for rows.Next() { + var mask string + + err = rows.Scan(&mask) + if err != nil { + return false, fmt.Errorf( + "scan channel ban: %w", err, + ) + } + + if MatchBanMask(mask, hostmask) { + return true, nil + } + } + + err = rows.Err() + if err != nil { + return false, fmt.Errorf( + "iterate channel bans: %w", err, + ) + } + + return false, nil } // IsChannelOperator checks if a session has operator @@ -729,6 +890,49 @@ func (database *Database) DeleteChannelIfEmpty( return nil } +// PartChannelAndDeleteIfEmpty removes a session from a +// channel and deletes the channel if that left it with no +// members, in one transaction, so that no JOIN lands +// between the two. +func (database *Database) PartChannelAndDeleteIfEmpty( + ctx context.Context, + channelID, sessionID int64, +) error { + transaction, err := database.conn.BeginTx(ctx, nil) + if err != nil { + return fmt.Errorf("begin part: %w", err) + } + + defer func() { _ = transaction.Rollback() }() + + _, err = transaction.ExecContext(ctx, + `DELETE FROM channel_members + WHERE channel_id = ? AND session_id = ?`, + channelID, sessionID) + if err != nil { + return fmt.Errorf("part channel: %w", err) + } + + _, err = transaction.ExecContext(ctx, + `DELETE FROM channels WHERE id = ? + AND NOT EXISTS + (SELECT 1 FROM channel_members + WHERE channel_id = ?)`, + channelID, channelID) + if err != nil { + return fmt.Errorf( + "delete channel if empty: %w", err, + ) + } + + err = transaction.Commit() + if err != nil { + return fmt.Errorf("commit part: %w", err) + } + + return nil +} + // scanChannels scans rows into a ChannelInfo slice. func scanChannels( rows *sql.Rows, diff --git a/internal/service/service.go b/internal/service/service.go index 2977aa9..dabe74f 100644 --- a/internal/service/service.go +++ b/internal/service/service.go @@ -536,41 +536,13 @@ func (s *Service) JoinChannel( } } - chID, err := s.db.GetOrCreateChannel(ctx, channel) - if err != nil { - return nil, fmt.Errorf("get/create channel: %w", err) - } - - memberCount, countErr := s.db.CountChannelMembers( - ctx, chID, + chID, isCreator, err := s.db.CreateOrJoinChannel( + ctx, channel, sessionID, suppliedKey, ) - isCreator := countErr == nil && memberCount == 0 - - if !isCreator { - joinErr := checkJoinRestrictions( - ctx, s.db, chID, sessionID, - channel, suppliedKey, memberCount, - ) - if joinErr != nil { - return nil, joinErr - } - } - - if isCreator { - err = s.db.JoinChannelAsOperator( - ctx, chID, sessionID, - ) - } else { - err = s.db.JoinChannel(ctx, chID, sessionID) - } - if err != nil { - return nil, fmt.Errorf("join channel: %w", err) + return nil, joinError(channel, err) } - // Clear invite after successful join. - _ = s.db.ClearChannelInvite(ctx, chID, sessionID) - memberIDs, _ := s.db.GetChannelMemberIDs(ctx, chID) body, _ := json.Marshal([]string{channel}) //nolint:errchkjson @@ -622,8 +594,7 @@ func (s *Service) PartChannel( nil, body, nil, recipients, ) - s.db.PartChannel(ctx, chID, sessionID) //nolint:errcheck,gosec - s.db.DeleteChannelIfEmpty(ctx, chID) //nolint:errcheck,gosec + s.db.PartChannelAndDeleteIfEmpty(ctx, chID, sessionID) //nolint:errcheck,gosec return nil } @@ -748,8 +719,7 @@ func (s *Service) KickUser( params, body, nil, memberIDs, ) - s.db.PartChannel(ctx, chID, targetSID) //nolint:errcheck,gosec - s.db.DeleteChannelIfEmpty(ctx, chID) //nolint:errcheck,gosec + s.db.PartChannelAndDeleteIfEmpty(ctx, chID, targetSID) //nolint:errcheck,gosec return nil } @@ -1392,54 +1362,30 @@ func (s *Service) broadcastNickChange( } } -// checkJoinRestrictions validates Tier 2 join conditions: -// bans, invite-only, channel key, and user limit. -func checkJoinRestrictions( - ctx context.Context, - database *db.Database, - chID, sessionID int64, - channel, suppliedKey string, - memberCount int64, -) error { - isBanned, banErr := database.IsSessionBanned( - ctx, chID, sessionID, - ) - if banErr == nil && isBanned { +// joinError returns the IRC error for a JOIN of channel +// that db.CreateOrJoinChannel refused with err, or err +// wrapped if it is not a refusal. +func joinError(channel string, err error) error { + switch { + case errors.Is(err, db.ErrBannedFromChannel): return &IRCError{ Code: irc.ErrBannedFromChan, Params: []string{channel}, Message: "Cannot join channel (+b)", } - } - - isInviteOnly, ioErr := database.IsChannelInviteOnly( - ctx, chID, - ) - if ioErr == nil && isInviteOnly { - hasInvite, invErr := database.HasChannelInvite( - ctx, chID, sessionID, - ) - if invErr != nil || !hasInvite { - return &IRCError{ - Code: irc.ErrInviteOnlyChan, - Params: []string{channel}, - Message: "Cannot join channel (+i)", - } + case errors.Is(err, db.ErrInviteOnlyChannel): + return &IRCError{ + Code: irc.ErrInviteOnlyChan, + Params: []string{channel}, + Message: "Cannot join channel (+i)", } - } - - key, keyErr := database.GetChannelKey(ctx, chID) - if keyErr == nil && key != "" && suppliedKey != key { + case errors.Is(err, db.ErrBadChannelKey): return &IRCError{ Code: irc.ErrBadChannelKey, Params: []string{channel}, Message: "Cannot join channel (+k)", } - } - - limit, limErr := database.GetChannelUserLimit(ctx, chID) - if limErr == nil && limit > 0 && - memberCount >= int64(limit) { + case errors.Is(err, db.ErrChannelIsFull): return &IRCError{ Code: irc.ErrChannelIsFull, Params: []string{channel}, @@ -1447,7 +1393,7 @@ func checkJoinRestrictions( } } - return nil + return fmt.Errorf("join channel: %w", err) } // extractHashcashFromMeta parses the meta JSON and diff --git a/internal/service/service_test.go b/internal/service/service_test.go index 5081be3..6170f54 100644 --- a/internal/service/service_test.go +++ b/internal/service/service_test.go @@ -13,6 +13,7 @@ import ( "os" "slices" "strings" + "sync" "testing" "time" @@ -255,6 +256,402 @@ func TestPartChannel(t *testing.T) { } } +// concurrentJoinTrials is how many times each test of +// concurrent JOINs repeats its race. Before a JOIN ran as +// one transaction, ten trials lost each race every time. +const concurrentJoinTrials = 10 + +// runConcurrently runs each of funcs in its own goroutine, +// releasing them all at once, and returns their errors in +// the order of funcs. +func runConcurrently(funcs ...func() error) []error { + start := make(chan struct{}) + errs := make([]error, len(funcs)) + + var wg sync.WaitGroup + + for i, run := range funcs { + wg.Add(1) + + go func() { + defer wg.Done() + + <-start + + errs[i] = run() + }() + } + + close(start) + wg.Wait() + + return errs +} + +// joinConcurrently creates a session for each nick and has +// them all JOIN channel at once. It returns the result and +// error of each JOIN, in the order of nicks. +func joinConcurrently( + ctx context.Context, + t *testing.T, + env *testEnv, + channel string, + nicks ...string, +) ([]*service.JoinResult, []error) { + t.Helper() + + results := make([]*service.JoinResult, len(nicks)) + joins := make([]func() error, len(nicks)) + + for i, nick := range nicks { + sessionID := createSession(ctx, t, env.db, nick) + + joins[i] = func() error { + result, err := env.svc.JoinChannel( + ctx, sessionID, nick, channel, "", + ) + results[i] = result + + return err + } + } + + return results, runConcurrently(joins...) +} + +// TestConcurrentJoinsRespectUserLimit checks that of two +// concurrent JOINs of a +l 2 channel with one member, one +// gets in and the other gets ERR_CHANNELISFULL. +func TestConcurrentJoinsRespectUserLimit(t *testing.T) { + const userLimit = 2 + + env := newTestEnv(t) + ctx := t.Context() + + for trial := range concurrentJoinTrials { + channel := fmt.Sprintf("#full%d", trial) + opNick := fmt.Sprintf("op%d", trial) + opSID := createSession(ctx, t, env.db, opNick) + + created, err := env.svc.JoinChannel( + ctx, opSID, opNick, channel, "", + ) + if err != nil { + t.Fatalf("join %s: %v", opNick, err) + } + + err = env.db.SetChannelUserLimit( + ctx, created.ChannelID, userLimit, + ) + if err != nil { + t.Fatalf("set +l: %v", err) + } + + _, errs := joinConcurrently( + ctx, t, env, channel, + fmt.Sprintf("a%d", trial), fmt.Sprintf("b%d", trial), + ) + + refused := 0 + + for _, joinErr := range errs { + var ircErr *service.IRCError + + switch { + case joinErr == nil: + case errors.As(joinErr, &ircErr) && + ircErr.Code == irc.ErrChannelIsFull: + refused++ + default: + t.Errorf("trial %d: JOIN: %v", trial, joinErr) + } + } + + members, err := env.db.ChannelMembers(ctx, created.ChannelID) + if err != nil { + t.Fatalf("channel members: %v", err) + } + + if len(members) != userLimit || refused != 1 { + t.Errorf( + "trial %d: %d members and %d refused, want %d and 1", + trial, len(members), refused, userLimit, + ) + } + } +} + +// TestConcurrentFirstJoinsMakeOneOperator checks that of +// two concurrent JOINs that create a channel, exactly one +// makes its session the operator. +func TestConcurrentFirstJoinsMakeOneOperator(t *testing.T) { + env := newTestEnv(t) + ctx := t.Context() + + for trial := range concurrentJoinTrials { + results, errs := joinConcurrently( + ctx, t, env, fmt.Sprintf("#new%d", trial), + fmt.Sprintf("first%d", trial), + fmt.Sprintf("second%d", trial), + ) + + creators := 0 + + for i, err := range errs { + if err != nil { + t.Fatalf("trial %d: JOIN: %v", trial, err) + } + + if results[i].IsCreator { + creators++ + } + } + + members, err := env.db.ChannelMembers( + ctx, results[0].ChannelID, + ) + if err != nil { + t.Fatalf("channel members: %v", err) + } + + operators := 0 + + for _, member := range members { + if member.IsOperator { + operators++ + } + } + + if creators != 1 || operators != 1 { + t.Errorf( + "trial %d: %d creators and %d operators, want 1 each", + trial, creators, operators, + ) + } + } +} + +// TestJoinConcurrentWithLastPart checks that a JOIN +// concurrent with the PART of a channel's only member +// either joins a channel that still exists or gets +// ERR_NOSUCHCHANNEL, and never fails otherwise. +func TestJoinConcurrentWithLastPart(t *testing.T) { + env := newTestEnv(t) + ctx := t.Context() + + for trial := range concurrentJoinTrials { + channel := fmt.Sprintf("#last%d", trial) + leaverNick := fmt.Sprintf("leaver%d", trial) + leaverSID := createSession(ctx, t, env.db, leaverNick) + joinerNick := fmt.Sprintf("joiner%d", trial) + joinerSID := createSession(ctx, t, env.db, joinerNick) + + _, err := env.svc.JoinChannel( + ctx, leaverSID, leaverNick, channel, "", + ) + if err != nil { + t.Fatalf("join %s: %v", leaverNick, err) + } + + errs := runConcurrently( + func() error { + return env.svc.PartChannel( + ctx, leaverSID, leaverNick, channel, "", + ) + }, + func() error { + _, joinErr := env.svc.JoinChannel( + ctx, joinerSID, joinerNick, channel, "", + ) + + return joinErr + }, + ) + if errs[0] != nil { + t.Fatalf("trial %d: PART: %v", trial, errs[0]) + } + + var ircErr *service.IRCError + + switch { + case errs[1] == nil: + chID, lookupErr := env.db.GetChannelByName(ctx, channel) + isMember, _ := env.db.IsChannelMember(ctx, chID, joinerSID) + + if lookupErr != nil || !isMember { + t.Errorf( + "trial %d: JOIN succeeded, but %s is not on %s", + trial, joinerNick, channel, + ) + } + case errors.As(errs[1], &ircErr) && + ircErr.Code == irc.ErrNoSuchChannel: + default: + t.Errorf( + "trial %d: JOIN got %v, want success or 403", + trial, errs[1], + ) + } + } +} + +// TestJoinUsesUpInvite checks that the JOIN an invite let +// into a +i channel uses the invite up, so that a JOIN +// after leaving gets ERR_INVITEONLYCHAN. +func TestJoinUsesUpInvite(t *testing.T) { + const channel = "#club" + + env := newTestEnv(t) + ctx := t.Context() + + opSID := createSession(ctx, t, env.db, "op") + guestSID := createSession(ctx, t, env.db, "guest") + + created, err := env.svc.JoinChannel(ctx, opSID, "op", channel, "") + if err != nil { + t.Fatalf("join op: %v", err) + } + + err = env.db.SetChannelInviteOnly(ctx, created.ChannelID, true) + if err != nil { + t.Fatalf("set +i: %v", err) + } + + _, err = env.svc.InviteUser(ctx, opSID, "op", "guest", channel) + if err != nil { + t.Fatalf("invite guest: %v", err) + } + + _, err = env.svc.JoinChannel(ctx, guestSID, "guest", channel, "") + if err != nil { + t.Fatalf("invited join: %v", err) + } + + err = env.svc.PartChannel(ctx, guestSID, "guest", channel, "") + if err != nil { + t.Fatalf("part guest: %v", err) + } + + _, err = env.svc.JoinChannel(ctx, guestSID, "guest", channel, "") + + var ircErr *service.IRCError + if !errors.As(err, &ircErr) || ircErr.Code != irc.ErrInviteOnlyChan { + t.Fatalf("second join: want ERR_INVITEONLYCHAN, got %v", err) + } +} + +// TestJoinRefusedByHostmaskBan checks that a ban naming a +// session's nick, username and hostname refuses its JOIN +// with ERR_BANNEDFROMCHAN. +func TestJoinRefusedByHostmaskBan(t *testing.T) { + const channel = "#club" + + env := newTestEnv(t) + ctx := t.Context() + + opSID := createSession(ctx, t, env.db, "op") + + created, err := env.svc.JoinChannel(ctx, opSID, "op", channel, "") + if err != nil { + t.Fatalf("join op: %v", err) + } + + err = env.db.AddChannelBan( + ctx, created.ChannelID, "guest!user@host.example", "op", + ) + if err != nil { + t.Fatalf("add ban: %v", err) + } + + guestSID, _, _, err := env.db.CreateSession( + ctx, "guest", "user", "host.example", "127.0.0.1", + ) + if err != nil { + t.Fatalf("create session guest: %v", err) + } + + _, err = env.svc.JoinChannel(ctx, guestSID, "guest", channel, "") + + var ircErr *service.IRCError + if !errors.As(err, &ircErr) || ircErr.Code != irc.ErrBannedFromChan { + t.Fatalf("want ERR_BANNEDFROMCHAN, got %v", err) + } +} + +// TestPartDeletesChannelOnlyWhenEmpty checks that a PART +// keeps a channel other members are still on and deletes +// the channel when its last member parts. +func TestPartDeletesChannelOnlyWhenEmpty(t *testing.T) { + const channel = "#room" + + env := newTestEnv(t) + ctx := t.Context() + + aliceSID := createSession(ctx, t, env.db, "alice") + bobSID := createSession(ctx, t, env.db, "bob") + + _, 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.svc.PartChannel(ctx, aliceSID, "alice", channel, "") + if err != nil { + t.Fatalf("part alice: %v", err) + } + + chID, err := env.db.GetChannelByName(ctx, channel) + if err != nil { + t.Fatalf("channel gone while bob is on it: %v", err) + } + + isMember, _ := env.db.IsChannelMember(ctx, chID, bobSID) + if !isMember { + t.Fatal("bob is not on the channel after alice parted") + } + + err = env.svc.PartChannel(ctx, bobSID, "bob", channel, "") + if err != nil { + t.Fatalf("part bob: %v", err) + } + + _, err = env.db.GetChannelByName(ctx, channel) + if err == nil { + t.Fatal("channel still exists after its last member parted") + } +} + +// TestKickOfLastMemberDeletesChannel checks that a KICK +// that leaves a channel with no members deletes it. +func TestKickOfLastMemberDeletesChannel(t *testing.T) { + const channel = "#room" + + env := newTestEnv(t) + ctx := t.Context() + + sid := createSession(ctx, t, env.db, "alice") + + _, err := env.svc.JoinChannel(ctx, sid, "alice", channel, "") + if err != nil { + t.Fatalf("join alice: %v", err) + } + + err = env.svc.KickUser(ctx, sid, "alice", channel, "alice", "") + if err != nil { + t.Fatalf("kick alice: %v", err) + } + + _, err = env.db.GetChannelByName(ctx, channel) + if err == nil { + t.Fatal("channel still exists after its last member was kicked") + } +} + func TestSendChannelMessage(t *testing.T) { env := newTestEnv(t) ctx := t.Context()