fix(security): stream tickets instead of session tokens in URLs (FLAW-22)
MindSift CI & Docker Release / Build, Test in Container & Push (push) Successful in 1m32s
MindSift CI & Docker Release / Build, Test in Container & Push (push) Successful in 1m32s
WebSocket and EventSource put the 30-day session token into the URL, where proxies log it. The app now gets a one-time ticket right before connecting (POST /auth/stream-ticket, 60 s, bound to the token version); ?token= is no longer accepted for streams, non-browser clients may use the Authorization header. With BASE_URL set, CORS allows only that origin and WebSocket upgrades only it or the requested host; credentials are off (no cookies exist). Logout, ending all sessions, a new password and deactivation close all live streams of the account; clients with a valid session reconnect with a fresh ticket. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
5c711c2cf7
commit
f57a9884d1
17 files changed
+335
-41
No files matched your search
@@ -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})
|
||||
}
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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})
|
||||
}
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
}
|
||||
+12
-3
@@ -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)
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
@@ -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)")
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user