diff --git a/Client/tauri-client/src/components/message-list/attachments.ts b/Client/tauri-client/src/components/message-list/attachments.ts index 0a40bca5..f4e5bdae 100644 --- a/Client/tauri-client/src/components/message-list/attachments.ts +++ b/Client/tauri-client/src/components/message-list/attachments.ts @@ -5,7 +5,6 @@ import { createElement, - setText, appendChildren, } from "@lib/dom"; import { createIcon } from "@lib/icons"; @@ -27,7 +26,7 @@ let _serverHost: string | null = null; /** Set the server host (called once from MainPage on connect). */ export function setServerHost(host: string): void { - _serverHost = host; + _serverHost = host.toLowerCase(); } /** Resolve a potentially relative URL to a full URL using the server host. */ @@ -66,8 +65,38 @@ export function isSafeUrl(url: string): boolean { // Image cache: memory + IndexedDB for persistence across restarts // --------------------------------------------------------------------------- -/** In-memory cache for instant re-render. */ +/** In-memory cache for instant re-render (LRU eviction at CACHE_MAX). */ const memoryCache = new Map(); +const CACHE_MAX = 200; + +/** Safe MIME types allowed in data: URIs — blocks script injection via crafted Content-Type. */ +const SAFE_MIME_TYPES = new Set([ + "image/png", "image/jpeg", "image/gif", "image/webp", "image/svg+xml", + "image/avif", "image/bmp", "video/mp4", "video/webm", "audio/mpeg", + "audio/ogg", "audio/wav", "application/pdf", +]); + +/** Sanitize a Content-Type header value for use in a data: URI. */ +function sanitizeContentType(raw: string): string { + const mime = raw.split(";")[0]?.trim() ?? ""; + return SAFE_MIME_TYPES.has(mime) ? raw : "application/octet-stream"; +} + +/** Check if a URL points to the configured OwnCord server. */ +function isServerUrl(url: string): boolean { + if (_serverHost === null) return false; + try { + const parsed = new URL(url); + return parsed.host === _serverHost; + } catch { + return false; + } +} + +/** Report whether a URL targets the configured OwnCord server host. */ +export function isTrustedServerUrl(url: string): boolean { + return isServerUrl(url); +} /** In-flight fetch promises to prevent duplicate concurrent requests. */ const inFlight = new Map>(); @@ -151,6 +180,10 @@ export function fetchImageAsDataUrl(url: string): Promise { // 3. IndexedDB cache (persists across restarts) const idbCached = await idbGet(url); if (idbCached !== null) { + if (memoryCache.size >= CACHE_MAX) { + const firstKey = memoryCache.keys().next().value; + if (firstKey !== undefined) memoryCache.delete(firstKey); + } memoryCache.set(url, idbCached); return idbCached; } @@ -162,17 +195,24 @@ export function fetchImageAsDataUrl(url: string): Promise { // chat messages containing internal URLs. Mitigated by: (1) isSafeUrl only allows // http/https, (2) responses are only used as image data, not executed. try { - const res = await tauriFetch(url, { - danger: { acceptInvalidCerts: true, acceptInvalidHostnames: false }, - } as RequestInit); + const useInsecure = isServerUrl(url); + const fetchOpts: RequestInit = useInsecure + ? { danger: { acceptInvalidCerts: true, acceptInvalidHostnames: false } } as RequestInit + : {}; + const res = await tauriFetch(url, fetchOpts); if (!res.ok) return null; - const contentType = res.headers.get("content-type") ?? "image/png"; + const rawCt = res.headers.get("content-type") ?? ""; + const contentType = sanitizeContentType(rawCt); const buffer = await res.arrayBuffer(); const base64 = uint8ToBase64(new Uint8Array(buffer)); const dataUrl = `data:${contentType};base64,${base64}`; - // Store in both caches + // Store in both caches (LRU eviction) + if (memoryCache.size >= CACHE_MAX) { + const firstKey = memoryCache.keys().next().value; + if (firstKey !== undefined) memoryCache.delete(firstKey); + } memoryCache.set(url, dataUrl); void idbPut(url, dataUrl); @@ -300,10 +340,12 @@ async function downloadFile(url: string, filename: string): Promise { const filePath = await save({ defaultPath: filename }); if (filePath === null) return; // User cancelled - // Fetch file data - const res = await tauriFetch(url, { - danger: { acceptInvalidCerts: true, acceptInvalidHostnames: false }, - } as RequestInit); + // Fetch file data — only accept invalid certs for the OwnCord server + const useInsecure = isServerUrl(url); + const fetchOpts: RequestInit = useInsecure + ? { danger: { acceptInvalidCerts: true, acceptInvalidHostnames: false } } as RequestInit + : {}; + const res = await tauriFetch(url, fetchOpts); if (!res.ok) return; const buffer = await res.arrayBuffer(); diff --git a/Client/tauri-client/src/components/message-list/embeds.ts b/Client/tauri-client/src/components/message-list/embeds.ts index f2053a84..0dae32e9 100644 --- a/Client/tauri-client/src/components/message-list/embeds.ts +++ b/Client/tauri-client/src/components/message-list/embeds.ts @@ -10,7 +10,7 @@ import { import { observeMedia } from "@lib/media-visibility"; import { fetch as tauriFetch } from "@tauri-apps/plugin-http"; import { createLogger } from "@lib/logger"; -import { isSafeUrl } from "./attachments"; +import { isSafeUrl, isTrustedServerUrl } from "./attachments"; const log = createLogger("embeds"); @@ -82,21 +82,58 @@ export function parseOgTags(html: string): OgMeta { /** Block link previews to private/internal IP ranges to prevent SSRF. * The connected OwnCord server host is NOT blocked (it's trusted). */ -function isPrivateHost(hostname: string): boolean { - // Block localhost variants - if (hostname === "localhost" || hostname === "127.0.0.1" || hostname === "::1" || hostname === "[::1]") return true; - // Block link-local, RFC1918, and cloud metadata endpoints - if (hostname.startsWith("10.") || hostname.startsWith("192.168.") || hostname === "169.254.169.254") return true; - if (hostname.startsWith("172.")) { - const second = parseInt(hostname.split(".")[1] ?? "", 10); - if (second >= 16 && second <= 31) return true; +function parseIPv4Literal(hostname: string): readonly [number, number, number, number] | null { + const parts = hostname.split("."); + if (parts.length !== 4) return null; + + const octets = parts.map((part) => { + if (!/^\d+$/.test(part)) return NaN; + return Number(part); + }); + if (octets.some((octet) => Number.isNaN(octet) || octet < 0 || octet > 255)) { + return null; } + + return [octets[0], octets[1], octets[2], octets[3]]; +} + +function isPrivateHost(hostname: string): boolean { + const h = hostname.replace(/^\[|\]$/g, "").toLowerCase(); + const isIPv6Literal = h.includes(":"); + const ipv4 = parseIPv4Literal(h); + + // Block localhost variants and unspecified address + if (h === "localhost") return true; + + if (isIPv6Literal) { + if (h === "::" || h === "::1") return true; + // IPv6 private ranges: fc00::/7 (fc.. and fd..), link-local fe80::/10. + if (h.startsWith("fc") || h.startsWith("fd") || /^fe[89ab]/.test(h)) return true; + // IPv4-mapped IPv6 addresses (::ffff:x.x.x.x). + if (h.startsWith("::ffff:")) return true; + return false; + } + + if (ipv4 !== null) { + const [first, second] = ipv4; + // Block loopback, unspecified, RFC1918, link-local, CGNAT, and benchmarking ranges. + if (first === 0 || first === 10 || first === 127) return true; + if (first === 169 && second === 254) return true; + if (first === 172 && second >= 16 && second <= 31) return true; + if (first === 192 && second === 168) return true; + if (first === 100 && second >= 64 && second <= 127) return true; + if (first === 198 && (second === 18 || second === 19)) return true; + } + return false; } function isBlockedForPreview(url: string): boolean { try { const parsed = new URL(url); + if (isTrustedServerUrl(parsed.toString())) { + return false; + } return isPrivateHost(parsed.hostname); } catch { return true; // Malformed URLs are blocked @@ -129,11 +166,14 @@ function fetchOgMeta(url: string): Promise { try { const controller = new AbortController(); const timer = setTimeout(() => controller.abort(), 5000); - const res = await tauriFetch(url, { + const fetchOpts: RequestInit = { signal: controller.signal, headers: { "User-Agent": "facebookexternalhit/1.1 (+http://www.facebook.com/externalhit_uatext.php)" }, - danger: { acceptInvalidCerts: true, acceptInvalidHostnames: false }, - } as RequestInit); + }; + if (isTrustedServerUrl(url)) { + fetchOpts.danger = { acceptInvalidCerts: true, acceptInvalidHostnames: false }; + } + const res = await tauriFetch(url, fetchOpts); clearTimeout(timer); if (!res.ok) { @@ -247,7 +287,7 @@ export function applyOgMeta( imgSrc = `${base.origin}${imgSrc}`; } catch { /* keep as-is */ } } - if (isSafeUrl(imgSrc)) { + if (isSafeUrl(imgSrc) && !isBlockedForPreview(imgSrc)) { const isGif = imgSrc.toLowerCase().endsWith(".gif"); const attrs: Record = { class: "msg-embed-link-img", @@ -263,8 +303,8 @@ export function applyOgMeta( imageWrap.style.display = "none"; }); if (isGif) { - (img as HTMLImageElement).addEventListener("load", () => { - observeMedia(img as HTMLImageElement, imgSrc, imageWrap); + (img).addEventListener("load", () => { + observeMedia(img, imgSrc, imageWrap); }, { once: true }); } imageWrap.appendChild(img); diff --git a/Client/tauri-client/tests/unit/embeds.test.ts b/Client/tauri-client/tests/unit/embeds.test.ts new file mode 100644 index 00000000..e052fffa --- /dev/null +++ b/Client/tauri-client/tests/unit/embeds.test.ts @@ -0,0 +1,110 @@ +import { + afterEach, + beforeEach, + describe, + expect, + it, + vi, +} from "vitest"; + +const { fetchMock } = vi.hoisted(() => ({ + fetchMock: vi.fn(), +})); + +vi.mock("@tauri-apps/plugin-http", () => ({ + fetch: fetchMock, +})); + +import { renderGenericLinkPreview } from "../../src/components/message-list/embeds"; +import { setServerHost } from "../../src/components/message-list/attachments"; + +function mockHtmlResponse(html: string) { + return { + ok: true, + headers: { + get(name: string) { + return name.toLowerCase() === "content-type" ? "text/html; charset=utf-8" : null; + }, + }, + text: vi.fn().mockResolvedValue(html), + }; +} + +describe("renderGenericLinkPreview", () => { + beforeEach(() => { + document.body.innerHTML = ""; + fetchMock.mockReset(); + setServerHost("example.com"); + }); + + afterEach(() => { + document.body.innerHTML = ""; + }); + + it("fetches OG metadata for public domains that begin with fd", async () => { + fetchMock.mockResolvedValue(mockHtmlResponse("F-Droid")); + + const card = renderGenericLinkPreview("https://fdroid.org/packages"); + document.body.appendChild(card); + + await vi.waitFor(() => { + expect(fetchMock).toHaveBeenCalledWith( + "https://fdroid.org/packages", + expect.objectContaining({ + headers: expect.objectContaining({ + "User-Agent": expect.stringContaining("facebookexternalhit"), + }), + }), + ); + }); + + await vi.waitFor(() => { + expect(card.querySelector(".msg-embed-link-title")?.textContent).toBe("F-Droid"); + }); + }); + + it("blocks previews for private IPv6 literals", async () => { + for (const url of ["https://[fd00::1]/", "https://[fe80::1]/", "https://[::ffff:127.0.0.1]/"]) { + document.body.innerHTML = ""; + const card = renderGenericLinkPreview(url); + document.body.appendChild(card); + await Promise.resolve(); + expect(card.querySelector(".msg-embed-link-title")?.textContent).toBeTruthy(); + } + + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it("blocks previews for loopback IPv4 literals beyond 127.0.0.1", async () => { + const card = renderGenericLinkPreview("https://127.0.0.2/internal"); + document.body.appendChild(card); + + await Promise.resolve(); + + expect(fetchMock).not.toHaveBeenCalled(); + expect(card.querySelector(".msg-embed-link-title")?.textContent).toBe("127.0.0.2"); + }); + + it("allows previews for the configured OwnCord server even on private hosts", async () => { + setServerHost("LOCALHOST:8080"); + fetchMock.mockResolvedValue(mockHtmlResponse("OwnCord Local")); + + const card = renderGenericLinkPreview("https://localhost:8080/docs"); + document.body.appendChild(card); + + await vi.waitFor(() => { + expect(fetchMock).toHaveBeenCalledWith( + "https://localhost:8080/docs", + expect.objectContaining({ + headers: expect.objectContaining({ + "User-Agent": expect.stringContaining("facebookexternalhit"), + }), + }), + ); + }); + + await vi.waitFor(() => { + expect(card.querySelector(".msg-embed-link-title")?.textContent).toBe("OwnCord Local"); + }); + }); +}); \ No newline at end of file diff --git a/Server/api/auth_handler.go b/Server/api/auth_handler.go index 152b9b65..1aede722 100644 --- a/Server/api/auth_handler.go +++ b/Server/api/auth_handler.go @@ -68,7 +68,7 @@ func MountAuthRoutes(r chi.Router, database *db.DB, limiter *auth.RateLimiter, t Post("/register", handleRegister(database)) r.With(RateLimitMiddleware(loginLimiter, 60, time.Minute, trustedProxies)). - Post("/login", handleLogin(database, limiter)) + Post("/login", handleLogin(database, limiter, trustedProxies)) r.With(AuthMiddleware(database)). Post("/logout", handleLogout(database)) @@ -105,6 +105,15 @@ func handleRegister(database *db.DB) http.HandlerFunc { return } + // Validate username format (length, no control/invisible chars). + if err := auth.ValidateUsername(req.Username); err != nil { + writeJSON(w, http.StatusBadRequest, errorResponse{ + Error: "INVALID_INPUT", + Message: err.Error(), + }) + return + } + // Validate password strength before anything else. if err := auth.ValidatePasswordStrength(req.Password); err != nil { writeJSON(w, http.StatusBadRequest, errorResponse{ @@ -125,21 +134,18 @@ func handleRegister(database *db.DB) http.HandlerFunc { return } - // Validate and consume invite atomically to prevent TOCTOU races. - if err := database.UseInviteAtomic(req.InviteCode); err != nil { - writeJSON(w, http.StatusBadRequest, genericAuthError) - return - } - - // Create user with default Member role. - uid, err := database.CreateUser(req.Username, hash, int(permissions.MemberRoleID)) + // Atomically consume the invite and create the user so failed + // registrations do not burn a valid invite code. + uid, err := database.CreateUserWithInvite(req.Username, hash, int(permissions.MemberRoleID), req.InviteCode) if err != nil { // UNIQUE constraint violation → duplicate username → 400. // Any other DB error → 500. - if strings.Contains(err.Error(), "UNIQUE constraint") { + if db.IsUniqueConstraintError(err) { + writeJSON(w, http.StatusBadRequest, genericAuthError) + } else if errors.Is(err, db.ErrNotFound) { writeJSON(w, http.StatusBadRequest, genericAuthError) } else { - slog.Error("CreateUser failed", "err", err, "username", req.Username) + slog.Error("CreateUserWithInvite failed", "err", err, "username", req.Username) writeJSON(w, http.StatusInternalServerError, errorResponse{ Error: "SERVER_ERROR", Message: "registration failed — please try again", @@ -189,7 +195,7 @@ func handleRegister(database *db.DB) http.HandlerFunc { } // handleLogin processes POST /api/v1/auth/login. -func handleLogin(database *db.DB, limiter *auth.RateLimiter) http.HandlerFunc { +func handleLogin(database *db.DB, limiter *auth.RateLimiter, trustedProxies []string) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { var req loginRequest if err := json.NewDecoder(r.Body).Decode(&req); err != nil { @@ -212,7 +218,7 @@ func handleLogin(database *db.DB, limiter *auth.RateLimiter) http.HandlerFunc { return } - ip := clientIP(r) + ip := clientIPWithProxies(r, trustedProxies) // Check lockout first. lockKey := "login_lock:" + ip diff --git a/Server/api/auth_handler_test.go b/Server/api/auth_handler_test.go index f9ef17fc..cbe6c6d8 100644 --- a/Server/api/auth_handler_test.go +++ b/Server/api/auth_handler_test.go @@ -35,8 +35,12 @@ func newAuthTestDB(t *testing.T) *db.DB { // buildAuthRouter returns a chi router with auth routes mounted on /api/v1/auth. func buildAuthRouter(database *db.DB, limiter *auth.RateLimiter) http.Handler { + return buildAuthRouterWithProxies(database, limiter, nil) +} + +func buildAuthRouterWithProxies(database *db.DB, limiter *auth.RateLimiter, trustedProxies []string) http.Handler { r := chi.NewRouter() - api.MountAuthRoutes(r, database, limiter, nil) + api.MountAuthRoutes(r, database, limiter, trustedProxies) return r } @@ -169,6 +173,34 @@ func TestRegister_InviteUsedUp(t *testing.T) { } } +func TestRegister_DuplicateUsername_DoesNotConsumeInvite(t *testing.T) { + database := newAuthTestDB(t) + limiter := auth.NewRateLimiter() + router := buildAuthRouter(database, limiter) + + ownerID, _ := database.CreateUser("owner4", "hash", 1) + _, _ = database.CreateUser("takenuser", "hash", 4) + code, _ := database.CreateInvite(ownerID, 1, nil) + + duplicate := postJSON(t, router, "/api/v1/auth/register", map[string]string{ + "username": "takenuser", + "password": "securePass1", + "invite_code": code, + }) + if duplicate.Code != http.StatusBadRequest { + t.Fatalf("duplicate username status = %d, want 400; body = %s", duplicate.Code, duplicate.Body.String()) + } + + success := postJSON(t, router, "/api/v1/auth/register", map[string]string{ + "username": "freshuser", + "password": "securePass2", + "invite_code": code, + }) + if success.Code != http.StatusCreated { + t.Fatalf("invite should remain usable after failed registration, status = %d, want 201; body = %s", success.Code, success.Body.String()) + } +} + func TestRegister_MissingFields(t *testing.T) { database := newAuthTestDB(t) limiter := auth.NewRateLimiter() @@ -257,6 +289,31 @@ func TestLogin_UnknownUser(t *testing.T) { } } +func TestLogin_LockoutUsesTrustedForwardedIP(t *testing.T) { + database := newAuthTestDB(t) + limiter := auth.NewRateLimiter() + router := buildAuthRouterWithProxies(database, limiter, []string{"127.0.0.0/8"}) + + for i := 0; i < 10; i++ { + req := httptest.NewRequest(http.MethodPost, "/api/v1/auth/login", bytes.NewReader([]byte(`{"username":"nobody","password":"wrongpass123"}`))) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-Forwarded-For", "198.51.100.10") + req.RemoteAddr = "127.0.0.1:9999" + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + } + + req := httptest.NewRequest(http.MethodPost, "/api/v1/auth/login", bytes.NewReader([]byte(`{"username":"nobody","password":"wrongpass123"}`))) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-Forwarded-For", "198.51.100.11") + req.RemoteAddr = "127.0.0.1:9999" + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + if rr.Code != http.StatusUnauthorized { + t.Fatalf("different forwarded client should not inherit another client's lockout, got %d", rr.Code) + } +} + func TestLogin_GenericErrorOnBadCredentials(t *testing.T) { database := newAuthTestDB(t) limiter := auth.NewRateLimiter() diff --git a/Server/api/channel_handler.go b/Server/api/channel_handler.go index e6a9b78e..fbe004e9 100644 --- a/Server/api/channel_handler.go +++ b/Server/api/channel_handler.go @@ -4,8 +4,11 @@ import ( "log/slog" "net/http" "strconv" + "strings" + "time" "github.com/go-chi/chi/v5" + "github.com/owncord/server/auth" "github.com/owncord/server/db" "github.com/owncord/server/permissions" ) @@ -15,9 +18,39 @@ const ( maxMessageLimit = 100 ) +func isInvalidSearchQueryError(err error) bool { + if err == nil { + return false + } + msg := strings.ToLower(err.Error()) + return strings.Contains(msg, "fts5") || + strings.Contains(msg, "unterminated string") || + strings.Contains(msg, "malformed") || + strings.Contains(msg, "syntax error") +} + +func searchRateLimitMiddleware(limiter *auth.RateLimiter, limit int, window time.Duration, trustedProxies []string) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ip := clientIPWithProxies(r, trustedProxies) + if !limiter.Allow("search:"+ip, limit, window) { + w.Header().Set("Retry-After", strconv.Itoa(int(window.Seconds()))) + writeJSON(w, http.StatusTooManyRequests, errorResponse{ + Error: "RATE_LIMITED", + Message: "too many requests, please slow down", + }) + return + } + + next.ServeHTTP(w, r) + }) + } +} + // MountChannelRoutes registers all channel-related routes onto r. -// All routes require authentication. -func MountChannelRoutes(r chi.Router, database *db.DB) { +// All routes require authentication. The limiter is used to rate-limit +// expensive endpoints like search. +func MountChannelRoutes(r chi.Router, database *db.DB, limiter *auth.RateLimiter, trustedProxies []string) { r.Route("/api/v1/channels", func(r chi.Router) { r.Use(AuthMiddleware(database)) r.Get("/", handleListChannels(database)) @@ -26,7 +59,10 @@ func MountChannelRoutes(r chi.Router, database *db.DB) { r.Post("/{id}/pins/{messageId}", handleSetPinned(database, true)) r.Delete("/{id}/pins/{messageId}", handleSetPinned(database, false)) }) - r.With(AuthMiddleware(database)).Get("/api/v1/search", handleSearch(database)) + r.With( + AuthMiddleware(database), + searchRateLimitMiddleware(limiter, 30, time.Minute, trustedProxies), + ).Get("/api/v1/search", handleSearch(database)) } // hasChannelPermREST checks whether the role has the given permission on the channel, @@ -264,6 +300,13 @@ func handleSearch(database *db.DB) http.HandlerFunc { results, err := database.SearchMessages(q, channelID, limit) if err != nil { + if isInvalidSearchQueryError(err) { + writeJSON(w, http.StatusBadRequest, errorResponse{ + Error: "BAD_REQUEST", + Message: "invalid search query", + }) + return + } slog.Error("handleSearch SearchMessages", "err", err, "query", q) writeJSON(w, http.StatusInternalServerError, errorResponse{ Error: "INTERNAL", @@ -281,26 +324,40 @@ func handleSearch(database *db.DB) http.HandlerFunc { overrides, oErr = database.GetAllChannelPermissionsForRole(role.ID) if oErr != nil { slog.Error("handleSearch GetAllChannelPermissionsForRole", "err", oErr) + writeJSON(w, http.StatusInternalServerError, errorResponse{ + Error: "INTERNAL", + Message: "search failed", + }) + return } } - // Build a cache of channel types so we can detect DM channels without - // repeated queries for the same channel ID. - channelTypeCache := map[int64]string{} + // Batch-fetch channel types in a single query to avoid N+1 lookups. + uniqueIDs := make(map[int64]struct{}, len(results)) for _, res := range results { - if _, seen := channelTypeCache[res.ChannelID]; !seen { - ch, chErr := database.GetChannel(res.ChannelID) - if chErr != nil || ch == nil { - channelTypeCache[res.ChannelID] = "" - continue - } - channelTypeCache[res.ChannelID] = ch.Type - } + uniqueIDs[res.ChannelID] = struct{}{} + } + channelIDs := make([]int64, 0, len(uniqueIDs)) + for id := range uniqueIDs { + channelIDs = append(channelIDs, id) + } + channelTypeCache, ctErr := database.GetChannelTypes(channelIDs) + if ctErr != nil { + slog.Error("handleSearch GetChannelTypes", "err", ctErr) + writeJSON(w, http.StatusInternalServerError, errorResponse{ + Error: "INTERNAL", + Message: "search failed", + }) + return } var filtered []db.MessageSearchResult for _, res := range results { - chType := channelTypeCache[res.ChannelID] + chType, ok := channelTypeCache[res.ChannelID] + if !ok { + // Fail closed if we cannot determine the channel type. + continue + } if chType == "dm" { // DM channels require participant-based auth. if user == nil { diff --git a/Server/api/channel_handler_test.go b/Server/api/channel_handler_test.go index decae231..c4c5d558 100644 --- a/Server/api/channel_handler_test.go +++ b/Server/api/channel_handler_test.go @@ -172,7 +172,7 @@ func newChannelTestDB(t *testing.T) *db.DB { func buildChannelRouter(database *db.DB) http.Handler { r := chi.NewRouter() - api.MountChannelRoutes(r, database) + api.MountChannelRoutes(r, database, auth.NewRateLimiter(), nil) return r } @@ -499,6 +499,20 @@ func TestSearch_InvalidLimit(t *testing.T) { } } +func TestSearch_InvalidFTSQuery(t *testing.T) { + database := newChannelTestDB(t) + router := buildChannelRouter(database) + token := chTestCreateToken(t, database, "badfts", 1) + user, _ := database.GetUserByUsername("badfts") + chID, _ := database.CreateChannel("fts", "text", "", "", 0) + _, _ = database.CreateMessage(chID, user.ID, "search seed", nil) + + rr := chGet(t, router, "/api/v1/search?q=%22", token) + if rr.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400; body: %s", rr.Code, rr.Body.String()) + } +} + func TestSearch_ZeroLimit(t *testing.T) { database := newChannelTestDB(t) router := buildChannelRouter(database) @@ -522,6 +536,84 @@ func TestSearch_LimitCappedAt100(t *testing.T) { } } +func TestSearch_ChannelTypeLookupFailure_FailsClosed(t *testing.T) { + database := newChannelTestDB(t) + router := buildChannelRouter(database) + token := chTestCreateToken(t, database, "searchfailclosed", 1) + user, _ := database.GetUserByUsername("searchfailclosed") + chID, _ := database.CreateChannel("searchable", "text", "", "", 0) + _, _ = database.CreateMessage(chID, user.ID, "closedlookupterm", nil) + + _, err := database.Exec(`ALTER TABLE channels RENAME TO channels_with_type`) + if err != nil { + t.Fatalf("rename channels: %v", err) + } + _, err = database.Exec(`CREATE TABLE channels ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + name TEXT NOT NULL + )`) + if err != nil { + t.Fatalf("recreate channels without type: %v", err) + } + _, err = database.Exec(`INSERT INTO channels (id, name) SELECT id, name FROM channels_with_type`) + if err != nil { + t.Fatalf("copy channels: %v", err) + } + + rr := chGet(t, router, "/api/v1/search?q=closedlookupterm", token) + if rr.Code != http.StatusInternalServerError { + t.Fatalf("status = %d, want 500; body: %s", rr.Code, rr.Body.String()) + } +} + +func TestSearch_ChannelOverrideLookupFailure_ReturnsError(t *testing.T) { + database := newChannelTestDB(t) + router := buildChannelRouter(database) + token := chTestCreateToken(t, database, "searchoverridefail", 4) + user, _ := database.GetUserByUsername("searchoverridefail") + chID, _ := database.CreateChannel("searchable", "text", "", "", 0) + _, _ = database.CreateMessage(chID, user.ID, "overridefailterm", nil) + + _, err := database.Exec(`DROP TABLE channel_overrides`) + if err != nil { + t.Fatalf("drop channel_overrides: %v", err) + } + + rr := chGet(t, router, "/api/v1/search?q=overridefailterm", token) + if rr.Code != http.StatusInternalServerError { + t.Fatalf("status = %d, want 500; body: %s", rr.Code, rr.Body.String()) + } +} + +func TestSearch_TrustedProxyRateLimitUsesForwardedIP(t *testing.T) { + database := newChannelTestDB(t) + r := chi.NewRouter() + limiter := auth.NewRateLimiter() + api.MountChannelRoutes(r, database, limiter, []string{"127.0.0.0/8"}) + token := chTestCreateToken(t, database, "proxysearch", 1) + + for i := 0; i < 30; i++ { + req := httptest.NewRequest(http.MethodGet, "/api/v1/search?q=test", nil) + req.Header.Set("Authorization", "Bearer "+token) + req.Header.Set("X-Forwarded-For", fmt.Sprintf("198.51.100.%d", i+1)) + req.RemoteAddr = "127.0.0.1:9999" + rr := httptest.NewRecorder() + r.ServeHTTP(rr, req) + if rr.Code != http.StatusOK { + t.Fatalf("request %d status = %d, want 200; body: %s", i, rr.Code, rr.Body.String()) + } + } + + req := httptest.NewRequest(http.MethodGet, "/api/v1/search?q=test", nil) + req.Header.Set("Authorization", "Bearer "+token) + req.Header.Set("X-Forwarded-For", "198.51.100.200") + req.RemoteAddr = "127.0.0.1:9999" + rr := httptest.NewRecorder() + r.ServeHTTP(rr, req) + if rr.Code != http.StatusOK { + t.Fatalf("independent forwarded client should not be throttled by shared proxy IP, got %d", rr.Code) + } +} // ─── Messages — before/after cursor ───────────────────────────────────────── @@ -554,4 +646,3 @@ func TestChannelMessages_InvalidLimit(t *testing.T) { t.Errorf("invalid limit status = %d, want 400", rr.Code) } } - diff --git a/Server/api/router.go b/Server/api/router.go index b1c3ada0..d0eb1728 100644 --- a/Server/api/router.go +++ b/Server/api/router.go @@ -19,9 +19,10 @@ import ( "github.com/owncord/server/ws" ) -// NewRouter builds and returns the fully configured HTTP handler and the -// WebSocket hub (so the caller can call hub.GracefulStop on shutdown). -func NewRouter(cfg *config.Config, database *db.DB, ver string, logBuf *admin.RingBuffer) (http.Handler, *ws.Hub) { +// NewRouter builds and returns the fully configured HTTP handler, the +// WebSocket hub (so the caller can call hub.GracefulStop on shutdown), and a +// cleanup function that stops background goroutines (e.g. rate-limiter cleanup). +func NewRouter(cfg *config.Config, database *db.DB, ver string, logBuf *admin.RingBuffer) (http.Handler, *ws.Hub, func()) { r := chi.NewRouter() // Middleware stack. @@ -71,7 +72,7 @@ func NewRouter(cfg *config.Config, database *db.DB, ver string, logBuf *admin.Ri MountInviteRoutes(r, database) // Channel and message REST routes. - MountChannelRoutes(r, database) + MountChannelRoutes(r, database, limiter, cfg.Server.TrustedProxies) // DM REST routes are mounted after hub creation (below) so the hub can // be passed as a DMBroadcaster for real-time close events. @@ -173,7 +174,19 @@ func NewRouter(cfg *config.Config, database *db.DB, ver string, logBuf *admin.Ri // Client auto-update endpoint (unauthenticated). MountClientUpdateRoute(r, u) - return r, hub + // Issue 15: Warn if AllowedOrigins contains wildcard. + for _, o := range cfg.Server.AllowedOrigins { + if o == "*" { + slog.Warn("AllowedOrigins contains wildcard '*' — consider restricting to specific origins for production use") + break + } + } + + cleanup := func() { + close(limiterStopCh) + } + + return r, hub, cleanup } // serverStartTime records when the process started; used for uptime in /health. diff --git a/Server/api/router_test.go b/Server/api/router_test.go index 0fa99358..cfc92ae0 100644 --- a/Server/api/router_test.go +++ b/Server/api/router_test.go @@ -32,7 +32,8 @@ func setupRouter(t *testing.T) http.Handler { }, } - handler, _ := api.NewRouter(cfg, database, "test", nil) + handler, _, cleanup := api.NewRouter(cfg, database, "test", nil) + t.Cleanup(cleanup) return handler } diff --git a/Server/auth/helpers.go b/Server/auth/helpers.go index 2ef5bc04..f816c7c9 100644 --- a/Server/auth/helpers.go +++ b/Server/auth/helpers.go @@ -1,13 +1,41 @@ package auth import ( + "fmt" "net/http" "strings" "time" + "unicode" "github.com/owncord/server/db" ) +// ValidateUsername checks that a (pre-trimmed) username meets naming rules: +// - Length 2-32 runes (after trim) +// - Only printable characters (no control chars, no zero-width chars) +// +// Returns a descriptive error on failure, nil on success. +func ValidateUsername(username string) error { + username = strings.TrimSpace(username) + n := len([]rune(username)) + if n < 2 { + return fmt.Errorf("username must be at least 2 characters") + } + if n > 32 { + return fmt.Errorf("username must be at most 32 characters") + } + for _, r := range username { + if unicode.IsControl(r) { + return fmt.Errorf("username must not contain control characters") + } + // Block zero-width and other invisible formatting characters. + if unicode.In(r, unicode.Cf) { + return fmt.Errorf("username must not contain invisible characters") + } + } + return nil +} + // ExtractBearerToken parses the "Authorization: Bearer " header from r // and returns the token and true. Returns "", false if the header is absent, // uses a scheme other than "bearer" (case-insensitive), or has an empty token. diff --git a/Server/db/auth_queries.go b/Server/db/auth_queries.go index a7b85dee..15be3d7d 100644 --- a/Server/db/auth_queries.go +++ b/Server/db/auth_queries.go @@ -23,6 +23,56 @@ func (d *DB) CreateUser(username, passwordHash string, roleID int) (int64, error return res.LastInsertId() } +// CreateUserWithInvite atomically consumes an invite and creates the user in +// the same transaction so a failed registration does not burn the invite. +func (d *DB) CreateUserWithInvite(username, passwordHash string, roleID int, inviteCode string) (int64, error) { + tx, err := d.sqlDB.Begin() + if err != nil { + return 0, fmt.Errorf("CreateUserWithInvite begin: %w", err) + } + committed := false + defer func() { + if !committed { + _ = tx.Rollback() + } + }() + + result, err := tx.Exec( + `UPDATE invites SET use_count = use_count + 1 + WHERE code = ? AND revoked = 0 + AND (max_uses IS NULL OR use_count < max_uses) + AND (expires_at IS NULL OR strftime('%s', expires_at) > strftime('%s', 'now'))`, + inviteCode, + ) + if err != nil { + return 0, fmt.Errorf("CreateUserWithInvite use invite: %w", err) + } + rows, err := result.RowsAffected() + if err != nil { + return 0, fmt.Errorf("CreateUserWithInvite invite rows: %w", err) + } + if rows == 0 { + return 0, fmt.Errorf("CreateUserWithInvite invite unavailable: %w", ErrNotFound) + } + + result, err = tx.Exec( + `INSERT INTO users (username, password, role_id) VALUES (?, ?, ?)`, + username, passwordHash, roleID, + ) + if err != nil { + return 0, fmt.Errorf("CreateUserWithInvite create user: %w", err) + } + uid, err := result.LastInsertId() + if err != nil { + return 0, fmt.Errorf("CreateUserWithInvite last insert id: %w", err) + } + if err := tx.Commit(); err != nil { + return 0, fmt.Errorf("CreateUserWithInvite commit: %w", err) + } + committed = true + return uid, nil +} + // GetUserByUsername returns the user with the given username (case-insensitive), // or nil if not found. func (d *DB) GetUserByUsername(username string) (*User, error) { diff --git a/Server/db/channel_queries.go b/Server/db/channel_queries.go index f4544fb3..97e5687e 100644 --- a/Server/db/channel_queries.go +++ b/Server/db/channel_queries.go @@ -4,6 +4,7 @@ import ( "database/sql" "errors" "fmt" + "strings" ) // ListChannels returns all channels ordered by position. @@ -202,3 +203,41 @@ func nullableString(s string) any { } return s } + +// GetChannelTypes returns a map of channel ID → type string for the given IDs +// in a single query, avoiding N+1 lookups. +func (d *DB) GetChannelTypes(ids []int64) (map[int64]string, error) { + if len(ids) == 0 { + return map[int64]string{}, nil + } + + // Build placeholders and args. + placeholders := make([]string, len(ids)) + args := make([]any, len(ids)) + for i, id := range ids { + placeholders[i] = "?" + args[i] = id + } + + query := fmt.Sprintf( + `SELECT id, type FROM channels WHERE id IN (%s)`, + strings.Join(placeholders, ","), + ) + + rows, err := d.sqlDB.Query(query, args...) + if err != nil { + return nil, fmt.Errorf("GetChannelTypes query: %w", err) + } + defer rows.Close() + + result := make(map[int64]string, len(ids)) + for rows.Next() { + var id int64 + var chType string + if err := rows.Scan(&id, &chType); err != nil { + return nil, fmt.Errorf("GetChannelTypes scan: %w", err) + } + result[id] = chType + } + return result, rows.Err() +} diff --git a/Server/db/errors.go b/Server/db/errors.go index 55dada85..d6ed299f 100644 --- a/Server/db/errors.go +++ b/Server/db/errors.go @@ -1,6 +1,9 @@ package db -import "errors" +import ( + "errors" + "strings" +) // Sentinel errors for the db package. Use errors.Is() to check. var ( @@ -21,3 +24,10 @@ var ( // without an administrator. ErrLastAdmin = errors.New("last admin cannot be deleted") ) + +// IsUniqueConstraintError reports whether err is a SQLite UNIQUE constraint +// violation. This centralizes the fragile string check so callers don't +// scatter strings.Contains calls throughout the codebase. +func IsUniqueConstraintError(err error) bool { + return err != nil && strings.Contains(err.Error(), "UNIQUE constraint") +}