fix: resolve 4 Critical + 2 High + 1 Medium server protocol violations

- Critical #1: Add READ_MESSAGES permission checks to channel_focus,
  GET /channels, GET /messages, and GET /search
- Critical #2: Send type "auth_error" instead of "error" with AUTH_ERROR
  code, preventing infinite client reconnect loops
- Critical #3: Replace role_id (number) with role (string name) in
  member_join, auth_ok, and ready payloads via JOIN on roles table
- Critical #4: Always include attachments field (empty array) in
  chat_message broadcasts to prevent client crash
- High #2: Add /api/v1/health endpoint alongside /health
- Medium #1: Handle ping WS messages with pong response
This commit is contained in:
jevb
2026-03-16 16:54:56 +01:00
parent 01387dc033
commit 54221e8c07
6 changed files with 115 additions and 25 deletions
+55 -2
View File
@@ -6,6 +6,7 @@ import (
"github.com/go-chi/chi/v5"
"github.com/owncord/server/db"
"github.com/owncord/server/permissions"
)
const (
@@ -24,9 +25,28 @@ func MountChannelRoutes(r chi.Router, database *db.DB) {
r.With(AuthMiddleware(database)).Get("/api/v1/search", handleSearch(database))
}
// hasChannelPermREST checks whether the role has the given permission on the channel,
// accounting for Administrator bypass and channel overrides.
func hasChannelPermREST(database *db.DB, role *db.Role, channelID, perm int64) bool {
if role == nil {
return false
}
if permissions.HasAdmin(role.Permissions) {
return true
}
allow, deny, err := database.GetChannelPermissions(channelID, role.ID)
if err != nil {
return false
}
effective := permissions.EffectivePerms(role.Permissions, allow, deny)
return effective&perm == perm
}
// handleListChannels returns all channels the authenticated user can see.
func handleListChannels(database *db.DB) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
role, _ := r.Context().Value(RoleKey).(*db.Role)
channels, err := database.ListChannels()
if err != nil {
writeJSON(w, http.StatusInternalServerError, errorResponse{
@@ -35,7 +55,18 @@ func handleListChannels(database *db.DB) http.HandlerFunc {
})
return
}
writeJSON(w, http.StatusOK, channels)
// Filter channels by READ_MESSAGES permission.
var visible []db.Channel
for _, ch := range channels {
if hasChannelPermREST(database, role, ch.ID, permissions.ReadMessages) {
visible = append(visible, ch)
}
}
if visible == nil {
visible = []db.Channel{}
}
writeJSON(w, http.StatusOK, visible)
}
}
@@ -64,6 +95,16 @@ func handleGetMessages(database *db.DB) http.HandlerFunc {
return
}
// Permission check: user must have READ_MESSAGES on this channel.
role, _ := r.Context().Value(RoleKey).(*db.Role)
if !hasChannelPermREST(database, role, channelID, permissions.ReadMessages) {
writeJSON(w, http.StatusForbidden, errorResponse{
Error: "FORBIDDEN",
Message: "no permission to view this channel",
})
return
}
// Parse query params.
before := int64(0)
if raw := r.URL.Query().Get("before"); raw != "" {
@@ -169,10 +210,22 @@ func handleSearch(database *db.DB) http.HandlerFunc {
return
}
// Post-filter results by READ_MESSAGES permission on each channel.
role, _ := r.Context().Value(RoleKey).(*db.Role)
var filtered []db.MessageSearchResult
for _, res := range results {
if hasChannelPermREST(database, role, res.ChannelID, permissions.ReadMessages) {
filtered = append(filtered, res)
}
}
if filtered == nil {
filtered = []db.MessageSearchResult{}
}
type response struct {
Results []db.MessageSearchResult `json:"results"`
}
writeJSON(w, http.StatusOK, response{Results: results})
writeJSON(w, http.StatusOK, response{Results: filtered})
}
}
+1
View File
@@ -38,6 +38,7 @@ func NewRouter(cfg *config.Config, database *db.DB, ver string) http.Handler {
// Versioned API routes.
r.Route("/api/v1", func(r chi.Router) {
r.Get("/health", handleHealth(ver))
r.Get("/info", handleInfo(cfg, ver))
})
+7 -3
View File
@@ -293,13 +293,17 @@ type MemberSummary struct {
Username string `json:"username"`
Avatar *string `json:"avatar"`
Status string `json:"status"`
RoleID int64 `json:"role_id"`
Role string `json:"role"`
}
// ListMembers returns all non-banned users as lightweight summaries.
func (d *DB) ListMembers() ([]MemberSummary, error) {
rows, err := d.sqlDB.Query(
`SELECT id, username, avatar, status, role_id FROM users WHERE banned = 0 ORDER BY username ASC`,
`SELECT u.id, u.username, u.avatar, u.status, r.name
FROM users u
JOIN roles r ON u.role_id = r.id
WHERE u.banned = 0
ORDER BY u.username ASC`,
)
if err != nil {
return nil, fmt.Errorf("ListMembers: %w", err)
@@ -309,7 +313,7 @@ func (d *DB) ListMembers() ([]MemberSummary, error) {
var members []MemberSummary
for rows.Next() {
var m MemberSummary
if err := rows.Scan(&m.ID, &m.Username, &m.Avatar, &m.Status, &m.RoleID); err != nil {
if err := rows.Scan(&m.ID, &m.Username, &m.Avatar, &m.Status, &m.Role); err != nil {
return nil, fmt.Errorf("ListMembers scan: %w", err)
}
members = append(members, m)
+10 -1
View File
@@ -105,6 +105,8 @@ func (h *Hub) handleMessage(c *Client, raw []byte) {
h.handleVoiceICE(c, env.Payload)
case "soundboard_play":
h.handleSoundboard(c, env.Payload)
case "ping":
c.sendMsg(buildJSON(map[string]any{"type": "pong"}))
default:
slog.Warn("ws handleMessage unknown type", "type", env.Type, "user_id", c.userID)
c.sendMsg(buildErrorMsg("UNKNOWN_TYPE", fmt.Sprintf("unknown message type: %s", env.Type)))
@@ -198,7 +200,7 @@ func (h *Hub) handleChatSend(c *Client, reqID string, payload json.RawMessage) {
c.sendMsg(buildChatSendOK(reqID, msgID, msg.Timestamp))
// Broadcast to channel.
broadcast := buildChatMessage(msgID, channelID, c.userID, username, avatar, content, msg.Timestamp, p.ReplyTo)
broadcast := buildChatMessage(msgID, channelID, c.userID, username, avatar, content, msg.Timestamp, p.ReplyTo, nil)
h.BroadcastToChannel(channelID, broadcast)
}
@@ -430,6 +432,13 @@ func (h *Hub) handleChannelFocus(c *Client, payload json.RawMessage) {
if err != nil || chID <= 0 {
return
}
// Permission check: user must have READ_MESSAGES on the target channel.
if !h.hasChannelPerm(c, chID, permissions.ReadMessages) {
c.sendMsg(buildErrorMsg("FORBIDDEN", "no permission to view this channel"))
return
}
c.mu.Lock()
c.channelID = chID
c.mu.Unlock()
+26 -10
View File
@@ -35,6 +35,17 @@ func buildErrorMsg(code, message string) []byte {
})
}
// buildAuthError produces an auth_error envelope per PROTOCOL.md.
// The client treats this type as non-recoverable and stops reconnecting.
func buildAuthError(message string) []byte {
return buildJSON(map[string]any{
"type": "auth_error",
"payload": map[string]string{
"message": message,
},
})
}
// buildPresenceMsg constructs a presence broadcast payload.
func buildPresenceMsg(userID int64, status string) []byte {
return buildJSON(map[string]any{
@@ -47,7 +58,7 @@ func buildPresenceMsg(userID int64, status string) []byte {
}
// buildMemberJoin constructs a member_join broadcast for when a user comes online.
func buildMemberJoin(user *db.User) []byte {
func buildMemberJoin(user *db.User, roleName string) []byte {
var avatarVal any
if user.Avatar != nil {
avatarVal = *user.Avatar
@@ -55,21 +66,25 @@ func buildMemberJoin(user *db.User) []byte {
return buildJSON(map[string]any{
"type": "member_join",
"payload": map[string]any{
"id": user.ID,
"username": user.Username,
"avatar": avatarVal,
"status": "online",
"role_id": user.RoleID,
"user": map[string]any{
"id": user.ID,
"username": user.Username,
"avatar": avatarVal,
"role": roleName,
},
},
})
}
// buildChatMessage constructs a chat_message broadcast envelope.
func buildChatMessage(msgID, channelID, userID int64, username string, avatar *string, content string, timestamp string, replyTo *int64) []byte {
func buildChatMessage(msgID, channelID, userID int64, username string, avatar *string, content string, timestamp string, replyTo *int64, attachments []map[string]any) []byte {
var avatarVal any
if avatar != nil {
avatarVal = *avatar
}
if attachments == nil {
attachments = []map[string]any{}
}
return buildJSON(map[string]any{
"type": "chat_message",
"payload": map[string]any{
@@ -80,9 +95,10 @@ func buildChatMessage(msgID, channelID, userID int64, username string, avatar *s
"username": username,
"avatar": avatarVal,
},
"content": content,
"reply_to": replyTo,
"timestamp": timestamp,
"content": content,
"reply_to": replyTo,
"timestamp": timestamp,
"attachments": attachments,
},
})
}
+16 -9
View File
@@ -43,6 +43,12 @@ func ServeWS(hub *Hub, database *db.DB, allowedOrigins []string) http.HandlerFun
c := newClient(hub, conn, user, tokenHash)
hub.Register(c)
// Look up role name for protocol-compliant payloads.
roleName := "member"
if role, roleErr := database.GetRoleByID(user.RoleID); roleErr == nil && role != nil {
roleName = role.Name
}
slog.Info("websocket connected", "username", user.Username, "user_id", user.ID, "remote", r.RemoteAddr)
_ = database.LogAudit(user.ID, "ws_connect", "user", user.ID,
"WebSocket connected from "+r.RemoteAddr)
@@ -53,12 +59,12 @@ func ServeWS(hub *Hub, database *db.DB, allowedOrigins []string) http.HandlerFun
// Send auth_ok followed by the ready payload.
ctx := r.Context()
_ = conn.Write(ctx, websocket.MessageText, buildAuthOK(database, user))
_ = conn.Write(ctx, websocket.MessageText, buildAuthOK(database, user, roleName))
if ready, readyErr := buildReady(database); readyErr == nil {
_ = conn.Write(ctx, websocket.MessageText, ready)
}
hub.BroadcastToAll(buildMemberJoin(user))
hub.BroadcastToAll(buildMemberJoin(user, roleName))
hub.BroadcastToAll(buildPresenceMsg(user.ID, "online"))
// writePump runs in background; readPump blocks.
@@ -126,11 +132,11 @@ func authenticateConn(conn *websocket.Conn, database *db.DB) (*db.User, string,
var env envelope
if err := json.Unmarshal(raw, &env); err != nil {
_ = conn.Write(ctx, websocket.MessageText, buildErrorMsg("AUTH_ERROR", "invalid message"))
_ = conn.Write(ctx, websocket.MessageText, buildAuthError( "invalid message"))
return nil, "", fmt.Errorf("auth: invalid JSON: %w", err)
}
if env.Type != "auth" {
_ = conn.Write(ctx, websocket.MessageText, buildErrorMsg("AUTH_ERROR", "first message must be auth"))
_ = conn.Write(ctx, websocket.MessageText, buildAuthError( "first message must be auth"))
return nil, "", fmt.Errorf("auth: unexpected type %q", env.Type)
}
@@ -138,25 +144,25 @@ func authenticateConn(conn *websocket.Conn, database *db.DB) (*db.User, string,
Token string `json:"token"`
}
if err := json.Unmarshal(env.Payload, &p); err != nil || p.Token == "" {
_ = conn.Write(ctx, websocket.MessageText, buildErrorMsg("AUTH_ERROR", "missing token"))
_ = conn.Write(ctx, websocket.MessageText, buildAuthError( "missing token"))
return nil, "", fmt.Errorf("auth: missing token")
}
hash := auth.HashToken(p.Token)
sess, err := database.GetSessionByTokenHash(hash)
if err != nil || sess == nil {
_ = conn.Write(ctx, websocket.MessageText, buildErrorMsg("AUTH_ERROR", "invalid token"))
_ = conn.Write(ctx, websocket.MessageText, buildAuthError( "invalid token"))
return nil, "", fmt.Errorf("auth: invalid session")
}
if auth.IsSessionExpired(sess.ExpiresAt) {
_ = conn.Write(ctx, websocket.MessageText, buildErrorMsg("AUTH_ERROR", "session expired"))
_ = conn.Write(ctx, websocket.MessageText, buildAuthError( "session expired"))
return nil, "", fmt.Errorf("auth: session expired")
}
user, err := database.GetUserByID(sess.UserID)
if err != nil || user == nil {
_ = conn.Write(ctx, websocket.MessageText, buildErrorMsg("AUTH_ERROR", "user not found"))
_ = conn.Write(ctx, websocket.MessageText, buildAuthError( "user not found"))
return nil, "", fmt.Errorf("auth: user not found")
}
@@ -169,7 +175,7 @@ func authenticateConn(conn *websocket.Conn, database *db.DB) (*db.User, string,
}
// buildAuthOK constructs the auth_ok server→client message.
func buildAuthOK(database *db.DB, user *db.User) []byte {
func buildAuthOK(database *db.DB, user *db.User, roleName string) []byte {
serverName := "OwnCord Server"
motd := "Welcome!"
_ = database.QueryRow("SELECT value FROM settings WHERE key='server_name'").Scan(&serverName)
@@ -188,6 +194,7 @@ func buildAuthOK(database *db.DB, user *db.User) []byte {
"username": user.Username,
"avatar": avatarVal,
"status": user.Status,
"role": roleName,
},
"server_name": serverName,
"motd": motd,