security: enforce channel membership check in handleTopic (#75)
All checks were successful
check / check (push) Successful in 1m48s
All checks were successful
check / check (push) Successful in 1m48s
## Summary `handleTopic` in `internal/handlers/api.go` did NOT check that the user was a member of the channel before allowing them to set a topic. Any authenticated user could set the topic on any channel they hadn't joined. ## Changes - **`internal/handlers/api.go`**: Added `IsChannelMember` check after resolving the channel ID and before calling `executeTopic`, mirroring the existing pattern in `handleChannelMsg`. Non-members now receive `ERR_NOTONCHANNEL` (442). - **`internal/handlers/api_test.go`**: Added `TestTopicNonMember` — creates a channel with one user, then verifies a second user who hasn't joined receives numeric 442 when attempting to set the topic. ## Testing - All existing tests pass - New `TestTopicNonMember` test validates the fix - `docker build .` passes clean (formatting, linting, tests, build) closes #33 Co-authored-by: user <user@Mac.lan guest wan> Reviewed-on: #75 Co-authored-by: clawbot <clawbot@noreply.example.org> Co-committed-by: clawbot <clawbot@noreply.example.org>
This commit was merged in pull request #75.
This commit is contained in:
@@ -1641,6 +1641,32 @@ func (hdlr *Handlers) handleTopic(
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
isMember, err := hdlr.params.Database.IsChannelMember(
|
||||||
|
request.Context(), chID, sessionID,
|
||||||
|
)
|
||||||
|
if err != nil {
|
||||||
|
hdlr.log.Error(
|
||||||
|
"check membership failed", "error", err,
|
||||||
|
)
|
||||||
|
hdlr.respondError(
|
||||||
|
writer, request,
|
||||||
|
"internal error",
|
||||||
|
http.StatusInternalServerError,
|
||||||
|
)
|
||||||
|
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
if !isMember {
|
||||||
|
hdlr.respondIRCError(
|
||||||
|
writer, request, clientID, sessionID,
|
||||||
|
irc.ErrNotOnChannel, nick, []string{channel},
|
||||||
|
"You're not on that channel",
|
||||||
|
)
|
||||||
|
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
hdlr.executeTopic(
|
hdlr.executeTopic(
|
||||||
writer, request,
|
writer, request,
|
||||||
sessionID, clientID, nick,
|
sessionID, clientID, nick,
|
||||||
|
|||||||
@@ -1140,6 +1140,42 @@ func TestTopicMissingBody(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestTopicNonMember(t *testing.T) {
|
||||||
|
tserver := newTestServer(t)
|
||||||
|
aliceToken := tserver.createSession("alice_topic")
|
||||||
|
bobToken := tserver.createSession("bob_topic")
|
||||||
|
|
||||||
|
// Only alice joins the channel.
|
||||||
|
tserver.sendCommand(aliceToken, map[string]any{
|
||||||
|
commandKey: joinCmd, toKey: "#topicpriv",
|
||||||
|
})
|
||||||
|
|
||||||
|
// Drain bob's initial messages.
|
||||||
|
_, lastID := tserver.pollMessages(bobToken, 0)
|
||||||
|
|
||||||
|
// Bob tries to set topic without joining.
|
||||||
|
status, _ := tserver.sendCommand(
|
||||||
|
bobToken,
|
||||||
|
map[string]any{
|
||||||
|
commandKey: "TOPIC",
|
||||||
|
toKey: "#topicpriv",
|
||||||
|
bodyKey: []string{"Hijacked topic"},
|
||||||
|
},
|
||||||
|
)
|
||||||
|
if status != http.StatusOK {
|
||||||
|
t.Fatalf("expected 200, got %d", status)
|
||||||
|
}
|
||||||
|
|
||||||
|
msgs, _ := tserver.pollMessages(bobToken, lastID)
|
||||||
|
|
||||||
|
if !findNumeric(msgs, "442") {
|
||||||
|
t.Fatalf(
|
||||||
|
"expected ERR_NOTONCHANNEL (442), got %v",
|
||||||
|
msgs,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestPing(t *testing.T) {
|
func TestPing(t *testing.T) {
|
||||||
tserver := newTestServer(t)
|
tserver := newTestServer(t)
|
||||||
token := tserver.createSession("ping_user")
|
token := tserver.createSession("ping_user")
|
||||||
|
|||||||
Reference in New Issue
Block a user