diff --git a/Doku/api/rest.md b/Doku/api/rest.md index 638617d..c7a311a 100644 --- a/Doku/api/rest.md +++ b/Doku/api/rest.md @@ -16,8 +16,9 @@ Basis: `/api/v1`. Antworten sind JSON; Fehler haben die Form `{"error": "Text"}` | POST | `/auth/reset-password` | `{token, password}` (mind. 8 Zeichen) → neues Passwort, alle Sitzungen beendet; `400` bei ungültigem oder abgelaufenem Link | | POST | `/auth/quickstart` | Immer `403` (Gastzugang deaktiviert) | | POST | `/webhooks/ingest` | Einliefern per Webhook, Key als `X-API-Key` (empfohlen), Bearer oder `?token=`. Body: `title`/`content`/`text`/`message`, `priority`, `source`, `tags`, `dueDate`; `target` (oder `type`): `triage` (Standard, Inbox), `task` (optional `taskListId`), `list_item` (`listId`, `category`, `quantity`). Klartext-Body wird als Inhalt genommen. `403` ohne passenden Scope oder ohne Schreibrecht auf die Liste | -| GET | `/ws`, `/ws/spaces/{id}`, `/ws/lists/{id}` | WebSocket, siehe [websocket.md](websocket.md); Token als `?token=` nötig | -| GET | `/events` | SSE-Fallback; Token als `?token=` nötig | +| POST | `/auth/stream-ticket` | Einmal-Ticket (60 s) für WebSocket/SSE | +| GET | `/ws`, `/ws/spaces/{id}`, `/ws/lists/{id}` | WebSocket, siehe [websocket.md](websocket.md); `?ticket=` oder `Authorization`-Header | +| GET | `/events` | SSE-Fallback; `?ticket=` oder `Authorization`-Header | ## Konto und Allgemeines (angemeldet) diff --git a/Doku/api/websocket.md b/Doku/api/websocket.md index a1f1312..1a2badd 100644 --- a/Doku/api/websocket.md +++ b/Doku/api/websocket.md @@ -20,7 +20,7 @@ Der Client versucht zuerst WebSocket und wechselt in folgenden Fällen für die Ohne WebSocket und SSE nutzt die Notiz-Synchronisation Long-Polling (`GET /docs/{id}/changes`, bis 25 s), und wenn auch das dreimal scheitert, eine Abfrage alle 3 s (siehe [modules/workspace.md](../modules/workspace.md)). -**Authentifizierung:** Token als `?token=` oder `Authorization`-Header. Ohne gültiges Token antworten `/ws` und `/events` mit `401`. Das Token wird nur beim Verbinden geprüft ([FLAW-22](../flaws.md)). +**Authentifizierung:** Browser holen sich direkt vor dem Verbinden ein Einmal-Ticket (`POST /auth/stream-ticket` mit dem normalen `Authorization`-Header → `{ticket, expiresIn: 60}`) und hängen es als `?ticket=` an; es gilt 60 Sekunden und einmal und wird ungültig, sobald alle Sitzungen beendet werden. Andere Clients dürfen stattdessen den `Authorization`-Header schicken. Sitzungs-Tokens in der URL und API-Keys werden nicht angenommen (`401`). Bei Logout, „Alle Sitzungen beenden“, neuem Passwort und Deaktivierung trennt der Server alle Streams des Kontos; Clients mit gültiger Sitzung verbinden sich mit neuem Ticket neu. Mit gesetzter `BASE_URL` nimmt der WebSocket nur Verbindungen von dieser Adresse oder vom angefragten Host an ([FLAW-22](../flaws.md)). **Räume:** Jeder Beitritt (Raum in der URL, `JOIN_ROOM`, SSE-Startparameter `rooms`/`docId`, `POST /events/rooms`) wird geprüft: `doc:` nur mit Lesezugriff auf die Notiz, sonst muss die ID eine lesbare geteilte Liste sein. Nicht erlaubte Räume werden ignoriert, `POST /events/rooms` antwortet dann mit `403`. SSE-Räume kann nur der Besitzer der jeweiligen `sseId` ändern. diff --git a/Doku/flaws.md b/Doku/flaws.md index 0992085..52c9da1 100644 --- a/Doku/flaws.md +++ b/Doku/flaws.md @@ -31,7 +31,7 @@ Schweregrade: **Hoch** (Datenabfluss oder Rechteumgehung möglich), **Mittel** ( | FLAW-19 | Qualität | Frontend-Tests nur für den Outliner, `lint` ist ein Platzhalter | Niedrig | | FLAW-20 | Qualität | ~~Sehr große Dateien~~; duplizierte Zugriffslogik | Niedrig | | FLAW-21 | Repo | ~~`LICENSE` fehlt, obwohl README MIT angibt~~ | **Behoben** | -| FLAW-22 | Netzwerk | Token in der URL (WS/SSE), CORS `*`, `CheckOrigin` immer `true` | Niedrig | +| FLAW-22 | Netzwerk | ~~Token in der URL (WS/SSE), CORS `*`, `CheckOrigin` immer `true`~~ | **Behoben** | | FLAW-23 | SQLite | Single-Writer-Engpass bei vielen gleichzeitigen Schreibern | Niedrig | | FLAW-24 | Gruppen | ~~Organisations-Endpunkte ohne Berechtigungsprüfung~~ | **Behoben** | | FLAW-25 | Workspace | ~~Struktur-Aktionen an fremd gesperrten Bullets nur teilweise ausgeführt~~ | **Behoben** | @@ -174,6 +174,7 @@ Schweregrade: **Hoch** (Datenabfluss oder Rechteumgehung möglich), **Mittel** ( - README und Badge nennen die MIT-Lizenz und verlinken auf `LICENSE`; die Datei existiert nicht im Repository. ### FLAW-22: Netzwerk-Details +- **Status: Behoben.** WebSocket/SSE nutzen Einmal-Tickets (60 s, einmalig, an `token_version` gebunden) statt des Sitzungs-Tokens in der URL; `?token=` wird für Streams nicht mehr angenommen. Mit `BASE_URL` erlaubt CORS nur diese Adresse und der WebSocket nur sie oder den angefragten Host; Credentials sind generell aus (es gibt keine Cookies). Logout, „Alle Sitzungen beenden“, Passwortwechsel und Deaktivierung trennen alle Streams des Kontos (`Hub.DisconnectUser`). Tests: `streamticket_test.go`, `TestLogoutEndsLiveStreams`. Ohne `BASE_URL` bleibt die Origin-Prüfung aus, weil Proxys den `Host` oft umschreiben. - `authenticateUserFromReq` akzeptiert das Token als `?token=` (nötig für `WebSocket`/`EventSource`, die keine Header setzen). Tokens landen dadurch in Proxy- und Zugriffslogs. - `AllowedOrigins: ["*"]` (Auth läuft über `Authorization`-Header, nicht über Cookies, das Risiko ist daher begrenzt) und `CheckOrigin` liefert immer `true`. - Ein einmal verbundener WebSocket/SSE-Stream wird bei Logout oder Token-Widerruf nicht getrennt. diff --git a/Doku/reverse-proxy.md b/Doku/reverse-proxy.md index 05af823..2019fa1 100644 --- a/Doku/reverse-proxy.md +++ b/Doku/reverse-proxy.md @@ -236,7 +236,8 @@ curl -i -N \ -H "Origin: https://mindsift.example.com" \ -H "Sec-WebSocket-Key: SGVsbG8sIHdvcmxkIQ==" \ -H "Sec-WebSocket-Version: 13" \ - https://mindsift.example.com/api/v1/ws?token= + -H "Authorization: Bearer " \ + https://mindsift.example.com/api/v1/ws ``` *Erwartete Antwort:* `101 Switching Protocols` diff --git a/frontend/src/features/lists/SharedListsView.tsx b/frontend/src/features/lists/SharedListsView.tsx index 9bbf9c8..4272a7b 100644 --- a/frontend/src/features/lists/SharedListsView.tsx +++ b/frontend/src/features/lists/SharedListsView.tsx @@ -11,7 +11,7 @@ import { ShoppingCart, } from 'lucide-react'; import { ListItem, SharedList, ListShare, Team, User, WSMessage } from '../../lib/types'; -import { api, buildWebSocketURL, triggerHaptic, createRealtimeConnection, RealtimeTransport } from '../../lib/api'; +import { api, triggerHaptic, createRealtimeConnection, RealtimeTransport } from '../../lib/api'; import { UnifiedShareModal } from '../../components/UnifiedShareModal'; import { AvatarStack, AvatarPerson } from '../../components/AvatarStack'; import { CountBadge } from '../../components/CountBadge'; diff --git a/frontend/src/lib/__tests__/realtime.test.ts b/frontend/src/lib/__tests__/realtime.test.ts index 02f21ba..9f0f9bd 100644 --- a/frontend/src/lib/__tests__/realtime.test.ts +++ b/frontend/src/lib/__tests__/realtime.test.ts @@ -62,7 +62,7 @@ describe('realtime fallback', () => { vi.stubGlobal('EventSource', FakeEventSource); sessionStorage.clear(); localStorage.setItem('mindsift_jwt_token', 'test-token'); - vi.stubGlobal('fetch', vi.fn(async () => new Response('{}', { status: 200 }))); + vi.stubGlobal('fetch', vi.fn(async () => new Response('{"ticket":"t1"}', { status: 200 }))); }); afterEach(() => { vi.useRealTimers(); @@ -73,7 +73,7 @@ describe('realtime fallback', () => { const api = await loadApi(); const states: string[] = []; api.onRealtimeStatus((connected, transport) => states.push(`${connected}:${transport}`)); - api.connectWebSocket(); + await api.connectWebSocket(); FakeWebSocket.instances[0].open(); expect(api.getRealtimeTransport()).toBe('ws'); @@ -87,9 +87,17 @@ describe('realtime fallback', () => { api.disconnectWebSocket(); }); + it('connects with a one-time ticket, never with the session token in the URL', async () => { + const api = await loadApi(); + await api.connectWebSocket(); + expect(FakeWebSocket.instances[0].url).toContain('ticket=t1'); + expect(FakeWebSocket.instances[0].url).not.toContain('test-token'); + api.disconnectWebSocket(); + }); + it('a WebSocket answering PONG stays in use', async () => { const api = await loadApi(); - api.connectWebSocket(); + await api.connectWebSocket(); const ws = FakeWebSocket.instances[0]; ws.open(); for (let i = 0; i < 6; i++) { @@ -104,7 +112,7 @@ describe('realtime fallback', () => { it('SSE only counts as connected once the first event arrives', async () => { sessionStorage.setItem('mindsift_ws_blocked', 'true'); const api = await loadApi(); - api.connectWebSocket(); + await api.connectWebSocket(); const es = FakeEventSource.instances[0]; es.open(); expect(api.isRealtimeConnected()).toBe(false); @@ -118,7 +126,7 @@ describe('realtime fallback', () => { it('a buffering proxy (stream opens, no events) switches SSE off for this tab', async () => { sessionStorage.setItem('mindsift_ws_blocked', 'true'); const api = await loadApi(); - api.connectWebSocket(); + await api.connectWebSocket(); const es = FakeEventSource.instances[0]; es.open(); await vi.advanceTimersByTimeAsync(5_500); diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index 9609836..03887c5 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -1209,19 +1209,28 @@ let isRealtimeConnectedState = false; let manualDisconnect = false; let wsReconnectDelay = 2000; -export function buildWebSocketURL(listId?: string): string { - const protocol = window.location.protocol === 'https:' ? 'wss:' : 'ws:'; - const token = getToken() || ''; - if (listId) { - return `${protocol}//${window.location.host}/api/v1/ws/lists/${encodeURIComponent(listId)}?token=${encodeURIComponent(token)}`; +// WebSocket and EventSource cannot send headers, so instead of the session token (which would land in +// proxy logs) every connection gets a one-time ticket, valid for a minute (POST /auth/stream-ticket). +async function fetchStreamTicket(): Promise { + try { + const res = await request<{ ticket?: string }>('/api/v1/auth/stream-ticket', { method: 'POST' }); + return res?.ticket || null; + } catch { + return null; } - return `${protocol}//${window.location.host}/api/v1/ws?token=${encodeURIComponent(token)}`; } -export function buildSSEURL(listId?: string, docId?: string, rooms?: string[]): string { - const token = getToken() || ''; +export function buildWebSocketURL(ticket: string, listId?: string): string { + const protocol = window.location.protocol === 'https:' ? 'wss:' : 'ws:'; + if (listId) { + return `${protocol}//${window.location.host}/api/v1/ws/lists/${encodeURIComponent(listId)}?ticket=${encodeURIComponent(ticket)}`; + } + return `${protocol}//${window.location.host}/api/v1/ws?ticket=${encodeURIComponent(ticket)}`; +} + +export function buildSSEURL(ticket: string, listId?: string, docId?: string, rooms?: string[]): string { const params = new URLSearchParams(); - if (token) params.set('token', token); + params.set('ticket', ticket); if (listId) params.set('listId', listId); if (docId) params.set('docId', docId); if (rooms && rooms.length > 0) params.set('rooms', rooms.join(',')); @@ -1309,7 +1318,9 @@ function switchToSSE(reason: string) { connectSSE(); } -export function connectSSE(rooms?: string[]): void { +let sseTicketPending = false; + +export async function connectSSE(rooms?: string[]): Promise { if (rooms) { rooms.forEach((r) => globalSubscribedRooms.add(r)); } @@ -1328,8 +1339,20 @@ export function connectSSE(rooms?: string[]): void { activeGlobalSSE = null; } + if (sseTicketPending) return; + sseTicketPending = true; + const ticket = await fetchStreamTicket(); + sseTicketPending = false; + if (manualDisconnect || !getToken()) return; + if (!ticket) { + // Server unreachable or session gone: try again later like after a dropped stream + notifyRealtimeStatusChange(false, 'none'); + sseReconnectTimer = setTimeout(() => connectSSE(), sseReconnectDelay); + return; + } + try { - const url = buildSSEURL(undefined, undefined, Array.from(globalSubscribedRooms)); + const url = buildSSEURL(ticket, undefined, undefined, Array.from(globalSubscribedRooms)); const es = new EventSource(url); activeGlobalSSE = es; @@ -1402,15 +1425,16 @@ export function connectSSE(rooms?: string[]): void { } } -export function connectWebSocket(): void { +let wsTicketPending = false; + +export async function connectWebSocket(): Promise { const token = getToken(); if (!token) return; manualDisconnect = false; // If proxy blocked WebSocket previously, do not attempt failing WebSocket; stay on SSE if (isWsBlockedByProxy) { - connectSSE(); - return; + return connectSSE(); } if (activeGlobalWS && (activeGlobalWS.readyState === WebSocket.OPEN || activeGlobalWS.readyState === WebSocket.CONNECTING)) { @@ -1421,8 +1445,22 @@ export function connectWebSocket(): void { clearTimeout(wsReconnectTimer); clearTimeout(sseReconnectTimer); + if (wsTicketPending) return; + wsTicketPending = true; + const ticket = await fetchStreamTicket(); + wsTicketPending = false; + if (manualDisconnect || !getToken()) return; + if (activeGlobalWS && (activeGlobalWS.readyState === WebSocket.OPEN || activeGlobalWS.readyState === WebSocket.CONNECTING)) { + return; + } + if (!ticket) { + notifyRealtimeStatusChange(false, 'none'); + wsReconnectTimer = setTimeout(() => connectWebSocket(), wsReconnectDelay); + return; + } + try { - const url = buildWebSocketURL(); + const url = buildWebSocketURL(ticket); const ws = new WebSocket(url); activeGlobalWS = ws; diff --git a/internal/api/handlers_admin.go b/internal/api/handlers_admin.go index b7c884c..1b19db0 100644 --- a/internal/api/handlers_admin.go +++ b/internal/api/handlers_admin.go @@ -188,6 +188,9 @@ func (s *Server) handleAdminUpdateUser(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadRequest, err.Error()) return } + if req.IsActive != nil && !*req.IsActive { + s.wsHub.DisconnectUser(targetUserID) + } writeJSON(w, http.StatusOK, u) } @@ -222,6 +225,7 @@ func (s *Server) handleAdminResetPassword(w http.ResponseWriter, r *http.Request writeError(w, http.StatusInternalServerError, err.Error()) return } + s.wsHub.DisconnectUser(targetUserID) writeJSON(w, http.StatusOK, map[string]bool{"success": true}) } diff --git a/internal/api/handlers_auth.go b/internal/api/handlers_auth.go index bdbf922..8c286de 100644 --- a/internal/api/handlers_auth.go +++ b/internal/api/handlers_auth.go @@ -400,6 +400,8 @@ func (s *Server) handleLogout(w http.ResponseWriter, r *http.Request) { _ = s.store.RevokeToken(tokenStr, userID, exp) } } + // Streams are not tied to one session; other devices reconnect with a new ticket on their own + s.wsHub.DisconnectUser(userID) writeJSON(w, http.StatusOK, map[string]bool{"success": true}) } @@ -409,6 +411,7 @@ func (s *Server) handleRevokeAllSessions(w http.ResponseWriter, r *http.Request) writeError(w, http.StatusInternalServerError, "Sitzungen konnten nicht beendet werden") return } + s.wsHub.DisconnectUser(userID) writeJSON(w, http.StatusOK, map[string]bool{"success": true}) } @@ -454,6 +457,7 @@ func (s *Server) handleUpdateUserSettings(w http.ResponseWriter, r *http.Request return } _ = s.store.UpdateUserPassword(userID, hash) + s.wsHub.DisconnectUser(userID) } u, err := s.store.UpdateUserFull( userID, diff --git a/internal/api/password_reset.go b/internal/api/password_reset.go index f4f4767..94fd41f 100644 --- a/internal/api/password_reset.go +++ b/internal/api/password_reset.go @@ -87,6 +87,7 @@ func (s *Server) handleResetPassword(w http.ResponseWriter, r *http.Request) { return } s.loginGuard.unlockAccount(userID) + s.wsHub.DisconnectUser(userID) slog.Info("Passwort per E-Mail-Link zurückgesetzt", "user_id", userID) writeJSON(w, http.StatusOK, map[string]bool{"ok": true}) } diff --git a/internal/api/realtime.go b/internal/api/realtime.go index 27079db..4f2b1e0 100644 --- a/internal/api/realtime.go +++ b/internal/api/realtime.go @@ -9,14 +9,25 @@ import ( "github.com/sandrodach/mindsift/internal/auth" ) +// authenticateUserFromReq authenticates a live stream: a one-time ticket (?ticket=, browsers) or a +// session in the Authorization header (other clients). Session tokens in the URL are not accepted any +// more (FLAW-22), API keys never (streams carry the user's data, keys only deliver). func (s *Server) authenticateUserFromReq(r *http.Request) string { - token := r.URL.Query().Get("token") - if token == "" { - if authHdr := r.Header.Get("Authorization"); strings.HasPrefix(authHdr, "Bearer ") { - token = strings.TrimPrefix(authHdr, "Bearer ") + if id := r.URL.Query().Get("ticket"); id != "" { + tk, ok := s.streamTickets.consume(id) + if !ok { + return "" } + u, err := s.store.GetUserByID(tk.userID) + if err != nil || u == nil || !u.IsActive || u.TokenVersion != tk.tokenVersion { + return "" + } + return u.ID + } + token := "" + if authHdr := r.Header.Get("Authorization"); strings.HasPrefix(authHdr, "Bearer ") { + token = strings.TrimPrefix(authHdr, "Bearer ") } - // Live streams carry the user's data, so API keys (which only deliver) are not accepted here if user, keyScopes := s.authenticateToken(token); user != nil && keyScopes == nil { return user.ID } diff --git a/internal/api/realtime_test.go b/internal/api/realtime_test.go index 0f70c48..5029e72 100644 --- a/internal/api/realtime_test.go +++ b/internal/api/realtime_test.go @@ -62,7 +62,8 @@ func newTestUser(t *testing.T, store *storage.Store, email string) (*domain.User func openSSE(t *testing.T, ts *httptest.Server, token string) *sseStream { t.Helper() ctx, cancel := context.WithCancel(context.Background()) - req, _ := http.NewRequestWithContext(ctx, http.MethodGet, ts.URL+"/api/v1/events?token="+token, nil) + req, _ := http.NewRequestWithContext(ctx, http.MethodGet, ts.URL+"/api/v1/events", nil) + req.Header.Set("Authorization", "Bearer "+token) resp, err := http.DefaultClient.Do(req) if err != nil { cancel() @@ -271,8 +272,8 @@ func TestReorderRejectsForeignTasks(t *testing.T) { func dialWS(t *testing.T, ts *httptest.Server, token string) *websocket.Conn { t.Helper() - url := "ws" + strings.TrimPrefix(ts.URL, "http") + "/api/v1/ws?token=" + token - conn, _, err := websocket.DefaultDialer.Dial(url, nil) + url := "ws" + strings.TrimPrefix(ts.URL, "http") + "/api/v1/ws" + conn, _, err := websocket.DefaultDialer.Dial(url, http.Header{"Authorization": {"Bearer " + token}}) if err != nil { t.Fatalf("dial ws: %v", err) } @@ -336,3 +337,20 @@ func TestDocChangesArePushedToTheNoteRoom(t *testing.T) { t.Fatalf("no DOC_SEQ push for the note: %+v", msg) } } + +func TestLogoutEndsLiveStreams(t *testing.T) { + srv, _, ts, owner, token := startRealtimeServer(t) + st := openSSE(t, ts, token) + srv.wsHub.DisconnectUser(owner.ID) + deadline := time.After(2 * time.Second) + for { + select { + case _, ok := <-st.events: + if !ok { + return // stream closed by the server + } + case <-deadline: + t.Fatal("stream still open after DisconnectUser") + } + } +} diff --git a/internal/api/server.go b/internal/api/server.go index bcb17a2..76cbab0 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -30,7 +30,8 @@ type Server struct { leases *collab.LeaseManager // block leases and open sessions of notes notifier *collab.Notifier // wakes long-poll requests on note changes - demo demoState // periodic reset in demo mode (demo.go) + demo demoState // periodic reset in demo mode (demo.go) + streamTickets streamTickets // one-time tickets for WebSocket/SSE (streamticket.go) loginGuard *loginGuard // brute-force protection (loginguard.go) } @@ -45,6 +46,7 @@ func NewServer(cfg *config.Config, store *storage.Store, wsHub *ws.Hub, staticFS } srv.loginGuard = newLoginGuard(!cfg.DisableLoginProtection) wsHub.SetRoomAuthorizer(srv.canJoinRoom) + wsHub.SetOriginCheck(srv.streamOriginAllowed) // Next occurrences of repeating tasks appear live on every connected client store.OnTaskCreated = func(th *domain.Thought) { srv.wsHub.BroadcastToUsers(srv.taskAudience(th), "TASK_CREATED", th) @@ -81,12 +83,18 @@ func (s *Server) Router() http.Handler { r.Use(s.demoResetGate) r.Use(middleware.Recoverer) r.Use(s.apiLoggingMiddleware) + // Browsers of other sites may only call the API when no BASE_URL is set (auth runs over the + // Authorization header, never cookies, so credentials are not allowed either way) + allowedOrigins := []string{"*"} + if s.cfg.BaseURLSet { + allowedOrigins = []string{strings.TrimRight(s.cfg.BaseURL, "/")} + } r.Use(cors.Handler(cors.Options{ - AllowedOrigins: []string{"*"}, + AllowedOrigins: allowedOrigins, AllowedMethods: []string{"GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"}, AllowedHeaders: []string{"Accept", "Authorization", "Content-Type", "X-API-Key"}, ExposedHeaders: []string{"Link"}, - AllowCredentials: true, + AllowCredentials: false, MaxAge: 300, })) @@ -112,6 +120,7 @@ func (s *Server) Router() http.Handler { r.Get("/api/v1/auth/me", s.handleMe) r.Post("/api/v1/events/rooms", s.handleSSERooms) + r.Post("/api/v1/auth/stream-ticket", s.handleStreamTicket) r.Post("/api/v1/auth/logout", s.handleLogout) r.Post("/api/v1/auth/revoke-all", s.handleRevokeAllSessions) r.Patch("/api/v1/auth/settings", s.handleUpdateUserSettings) diff --git a/internal/api/server_test.go b/internal/api/server_test.go index 66257b5..dd84824 100644 --- a/internal/api/server_test.go +++ b/internal/api/server_test.go @@ -891,7 +891,8 @@ func TestSSEGlobalEndpoint(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - req = httptest.NewRequest(http.MethodGet, "/api/v1/events?token="+token, nil).WithContext(ctx) + req = httptest.NewRequest(http.MethodGet, "/api/v1/events", nil).WithContext(ctx) + req.Header.Set("Authorization", "Bearer "+token) rec = httptest.NewRecorder() go func() { @@ -1147,7 +1148,8 @@ func TestTokenBlacklistingAndTokenVersionRevocation(t *testing.T) { } // Also check authenticateUserFromReq rejects token1 - req = httptest.NewRequest(http.MethodGet, "/api/v1/ws?token="+token1, nil) + req = httptest.NewRequest(http.MethodGet, "/api/v1/ws", nil) + req.Header.Set("Authorization", "Bearer "+token1) if uid := srv.authenticateUserFromReq(req); uid != "" { t.Fatalf("authenticateUserFromReq should reject blacklisted token1, got uid %q", uid) } @@ -1468,7 +1470,8 @@ func TestAPIKeysOnlyDeliverData(t *testing.T) { t.Fatalf("key must not write into lists its owner cannot see, got %d", code) } // Keys never open live streams (those carry the user's data) - streamReq := httptest.NewRequest(http.MethodGet, "/api/v1/events?token="+full.RawKey, nil) + streamReq := httptest.NewRequest(http.MethodGet, "/api/v1/events", nil) + streamReq.Header.Set("Authorization", "Bearer "+full.RawKey) if uid := srv.authenticateUserFromReq(streamReq); uid != "" { t.Fatalf("API key must not authenticate a live stream") } diff --git a/internal/api/streamticket.go b/internal/api/streamticket.go new file mode 100644 index 0000000..b491478 --- /dev/null +++ b/internal/api/streamticket.go @@ -0,0 +1,97 @@ +package api + +import ( + "crypto/rand" + "encoding/hex" + "net/http" + "net/url" + "strings" + "sync" + "time" + + "github.com/sandrodach/mindsift/internal/auth" +) + +// WebSocket and EventSource cannot send an Authorization header, so the browser used to put its +// 30-day session token into the URL, where it ends up in proxy and access logs (FLAW-22). Instead the +// app now asks for a stream ticket (POST /auth/stream-ticket, with the normal header) right before it +// connects and passes that: random, valid for 60 seconds, usable once, bound to the session's token +// version. A ticket in a log is worthless. + +const streamTicketTTL = 60 * time.Second + +type streamTicket struct { + userID string + tokenVersion int + expires time.Time +} + +type streamTickets struct { + mu sync.Mutex + tickets map[string]streamTicket +} + +func (t *streamTickets) issue(userID string, tokenVersion int) (string, error) { + buf := make([]byte, 24) + if _, err := rand.Read(buf); err != nil { + return "", err + } + id := hex.EncodeToString(buf) + now := time.Now() + t.mu.Lock() + defer t.mu.Unlock() + if t.tickets == nil { + t.tickets = map[string]streamTicket{} + } + for k, v := range t.tickets { + if now.After(v.expires) { + delete(t.tickets, k) + } + } + t.tickets[id] = streamTicket{userID: userID, tokenVersion: tokenVersion, expires: now.Add(streamTicketTTL)} + return id, nil +} + +// consume returns the ticket and removes it; ok is false for unknown or expired tickets. +func (t *streamTickets) consume(id string) (streamTicket, bool) { + t.mu.Lock() + defer t.mu.Unlock() + tk, ok := t.tickets[id] + delete(t.tickets, id) + if !ok || time.Now().After(tk.expires) { + return streamTicket{}, false + } + return tk, true +} + +func (s *Server) handleStreamTicket(w http.ResponseWriter, r *http.Request) { + user, err := s.store.GetUserByID(auth.GetUserID(r.Context())) + if err != nil || user == nil { + writeError(w, http.StatusUnauthorized, "Nicht autorisiert") + return + } + ticket, err := s.streamTickets.issue(user.ID, user.TokenVersion) + if err != nil { + writeError(w, http.StatusInternalServerError, "Ticket konnte nicht erstellt werden") + return + } + writeJSON(w, http.StatusOK, map[string]interface{}{"ticket": ticket, "expiresIn": int(streamTicketTTL.Seconds())}) +} + +// streamOriginAllowed: with BASE_URL set, browsers may open WebSockets only from that address or the +// requested host itself. Requests without Origin (non-browser clients) pass; they need a ticket anyway. +func (s *Server) streamOriginAllowed(r *http.Request) bool { + origin := r.Header.Get("Origin") + if origin == "" || !s.cfg.BaseURLSet { + return true + } + o, err := url.Parse(origin) + if err != nil { + return false + } + if strings.EqualFold(o.Host, r.Host) { + return true + } + base, err := url.Parse(s.cfg.BaseURL) + return err == nil && strings.EqualFold(o.Host, base.Host) +} diff --git a/internal/api/streamticket_test.go b/internal/api/streamticket_test.go new file mode 100644 index 0000000..e410645 --- /dev/null +++ b/internal/api/streamticket_test.go @@ -0,0 +1,73 @@ +package api + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/sandrodach/mindsift/internal/config" +) + +func TestStreamTicketsReplaceTokensInURLs(t *testing.T) { + srv, store, token := setupTestServer(t) + router := srv.Router() + ticket := func(tok string) string { + req := httptest.NewRequest(http.MethodPost, "/api/v1/auth/stream-ticket", nil) + req.Header.Set("Authorization", "Bearer "+tok) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + var out struct{ Ticket string } + _ = json.Unmarshal(rec.Body.Bytes(), &out) + return out.Ticket + } + stream := func(query string) string { + return srv.authenticateUserFromReq(httptest.NewRequest(http.MethodGet, "/api/v1/events?"+query, nil)) + } + + tk := ticket(token) + if tk == "" { + t.Fatal("no ticket issued") + } + if stream("ticket="+tk) == "" { + t.Fatal("a fresh ticket must open the stream") + } + if stream("ticket="+tk) != "" { + t.Fatal("a ticket works only once") + } + if stream("token="+token) != "" { + t.Fatal("session tokens in the URL are no longer accepted") + } + + // Ending all sessions makes unused tickets worthless as well + tk = ticket(token) + uid := stream("ticket=" + ticket(token)) + _ = store.RevokeAllUserTokens(uid) + if stream("ticket="+tk) != "" { + t.Fatal("ticket from before 'all sessions ended' must be refused") + } +} + +func TestStreamOriginCheck(t *testing.T) { + srv := &Server{cfg: &config.Config{BaseURL: "https://notes.example.com", BaseURLSet: true}} + check := func(host, origin string) bool { + r := httptest.NewRequest(http.MethodGet, "/api/v1/ws", nil) + r.Host = host + if origin != "" { + r.Header.Set("Origin", origin) + } + return srv.streamOriginAllowed(r) + } + if !check("mindsift:8080", "https://notes.example.com") { + t.Error("BASE_URL origin must pass even if the proxy rewrote Host") + } + if !check("notes.example.com", "https://notes.example.com") { + t.Error("same host must pass") + } + if check("notes.example.com", "https://evil.example.org") { + t.Error("foreign origin must be refused") + } + if !check("notes.example.com", "") { + t.Error("non-browser clients without Origin pass (they still need a ticket)") + } +} diff --git a/internal/ws/hub.go b/internal/ws/hub.go index fca2e72..0d2cfbd 100644 --- a/internal/ws/hub.go +++ b/internal/ws/hub.go @@ -17,10 +17,22 @@ var upgrader = websocket.Upgrader{ ReadBufferSize: 4096, WriteBufferSize: 4096, CheckOrigin: func(r *http.Request) bool { - return true // Self-hosted reverse proxy compatible + return originCheck(r) }, } +// originCheck decides which browser origins may open a WebSocket (see SetOriginCheck). Without a +// configured check every origin is accepted: proxies often rewrite the Host header, so a plain +// same-host comparison would break working setups. +var originCheck = func(*http.Request) bool { return true } + +// SetOriginCheck installs the origin rule for WebSocket upgrades. +func (h *Hub) SetOriginCheck(f func(*http.Request) bool) { + if f != nil { + originCheck = f + } +} + type Client struct { hub *Hub conn *websocket.Conn @@ -371,6 +383,19 @@ func (h *Hub) BroadcastToUser(userID, event string, payload interface{}) { } } +// DisconnectUser ends every live connection of a user (logout, all sessions ended, new password, +// account deactivated). Clients reconnect with a fresh ticket if their session is still valid. +func (h *Hub) DisconnectUser(userID string) { + h.mu.RLock() + defer h.mu.RUnlock() + for client := range h.userRooms[userID] { + client.closeSend() + } + for sseClient := range h.sseUserRooms[userID] { + sseClient.closeSend() + } +} + // BroadcastSystem sends an event to every connection. Only for events without user data (e.g. the // demo reset notice). func (h *Hub) BroadcastSystem(event string) {