From a71af41d9ecf321815f90cce0ca5471152eb56bc Mon Sep 17 00:00:00 2001 From: kristianwind Date: Fri, 23 Oct 2026 13:59:30 +0200 Subject: [PATCH] feat(app): CORS for appen, en billet til live-synk, og /ping siger hvad den er MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Andet trin mod iOS-appen. Appen henter sin flade fra capacitor://localhost og taler med den server, ejeren har skrevet ind, så hver anmodning er cross-origin. - CORS på /api/v1 for appens origin og ingen andres. Ingen Allow-Credentials: appen bærer sin session som bearer, aldrig som cookie. Alle andre origins får ingen headere, præcis som før. - POST /ws-ticket giver en engangsbillet, der lever 30 sekunder, og /ws?ticket=… åbner socket'en med den. En browser kan ikke sætte en header på et WebSocket-håndtryk, og tokenet selv i URL'en ville ligge i hver proxy-log i månedsvis. En socket med billet springer origin-tjekket over, fordi den ikke bærer nogen cookie, som en fremmed side kunne låne. - Et håndtryk fra appens origin uden billet afvises. Prøven fandt hvorfor: biblioteket sammenligner kun værtsnavne, og capacitor://localhost har samme værtsnavn som en instans på http://localhost. - /ping svarer med product, edition og api, så appen kan kende en Verdande fra en routers login-side, før nogen er logget ind. Ingen version: /version er bag et login med vilje. Fem plantede fejl, hver fanget af sin egen prøve: CORS til enhver origin, genbrugelig billet, billet uden udløb, app-origin på en cookie, og Allow-Credentials. Co-Authored-By: Claude Opus 5.5 --- docs/openapi.yaml | 40 +++++ internal/httpapi/app_cors_test.go | 202 ++++++++++++++++++++++++++ internal/httpapi/app_handlers.go | 169 +++++++++++++++++++++ internal/httpapi/middleware.go | 2 + internal/httpapi/realtime_handlers.go | 4 + internal/httpapi/server.go | 17 ++- 6 files changed, 430 insertions(+), 4 deletions(-) create mode 100644 internal/httpapi/app_cors_test.go create mode 100644 internal/httpapi/app_handlers.go diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 2e39a85..332f641 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -4024,14 +4024,51 @@ paths: Events: `task.created`, `task.updated`, `task.completed`, `task.reopened`, `task.moved`, `task.deleted`, `comment.created`, `notification`, `reminder`. + + A browser opens it with its session cookie and from the instance's own + origin. The iOS app cannot put a header on a handshake, so it asks + `/ws-ticket` first and opens `/ws?ticket=…`; a handshake from the app's + origin without one is refused. + parameters: + - name: ticket + in: query + required: false + schema: { type: string } + description: From `/ws-ticket`. Good for one connection, within 30 seconds. responses: "101": { description: Switching protocols. } "401": { $ref: "#/components/responses/Unauthorized" } + /api/v1/ws-ticket: + post: + tags: [integrations] + summary: A one-use ticket for opening the live-sync socket without a cookie. + description: | + For the iOS app, whose session is a bearer token that a WebSocket + handshake cannot carry. Putting the token itself in the URL would leave a + credential lasting months in every log the URL passes through; the ticket + lasts thirty seconds and opens one connection. + responses: + "200": + description: OK + content: + application/json: + schema: + type: object + properties: + ticket: { type: string } + expires_in: { type: integer, description: Seconds., example: 30 } + "401": { $ref: "#/components/responses/Unauthorized" } + /api/v1/ping: get: tags: [integrations] summary: A liveness check that does not touch the database. + description: | + Also how the iOS app tells a Verdande from anything else at an address + somebody typed in, before anyone has signed in, and which half of the + interface to show. It carries no version: `/version` is behind a login on + purpose, and a public number is a list of the instances running an old one. security: [] responses: "200": @@ -4042,6 +4079,9 @@ paths: type: object properties: status: { type: string, const: ok } + product: { type: string, const: verdande } + edition: { type: string, enum: [full, notes] } + api: { type: integer, const: 1, description: Raised when /api/v1 changes in a way an older app cannot follow. } # --- data ----------------------------------------------------------------- diff --git a/internal/httpapi/app_cors_test.go b/internal/httpapi/app_cors_test.go new file mode 100644 index 0000000..4b5c9b2 --- /dev/null +++ b/internal/httpapi/app_cors_test.go @@ -0,0 +1,202 @@ +package httpapi + +import ( + "context" + "net/http" + "strings" + "testing" + "time" + + "github.com/coder/websocket" +) + +func (ts *testServer) corsDo(t *testing.T, method, path, origin string, header map[string]string) *http.Response { + t.Helper() + req, err := http.NewRequest(method, ts.URL+path, nil) + if err != nil { + t.Fatal(err) + } + if origin != "" { + req.Header.Set("Origin", origin) + } + for k, v := range header { + req.Header.Set(k, v) + } + resp, err := (&http.Client{Timeout: 10 * time.Second}).Do(req) + if err != nil { + t.Fatal(err) + } + resp.Body.Close() + return resp +} + +func TestCORSAnswersTheAppAndNobodyElse(t *testing.T) { + ts := newTestServer(t) + ts.bootstrap(t) + preflight := map[string]string{ + "Access-Control-Request-Method": "POST", + "Access-Control-Request-Headers": "authorization, content-type", + } + + resp := ts.corsDo(t, "OPTIONS", "/api/v1/projects", "capacitor://localhost", preflight) + if resp.StatusCode != http.StatusNoContent { + t.Errorf("app preflight: status %d", resp.StatusCode) + } + if got := resp.Header.Get("Access-Control-Allow-Origin"); got != "capacitor://localhost" { + t.Errorf("app preflight: Allow-Origin %q", got) + } + if got := resp.Header.Get("Access-Control-Allow-Headers"); !strings.Contains(got, "Authorization") { + t.Errorf("app preflight: Allow-Headers %q lacks Authorization", got) + } + if got := resp.Header.Get("Access-Control-Allow-Credentials"); got != "" { + t.Errorf("credentials allowed (%q): the app uses a bearer, never a cookie", got) + } + + // An ordinary request from the app can be read. + resp = ts.corsDo(t, "GET", "/api/v1/ping", "capacitor://localhost", nil) + if got := resp.Header.Get("Access-Control-Allow-Origin"); got != "capacitor://localhost" { + t.Errorf("app GET: Allow-Origin %q", got) + } + + // Anybody else gets nothing, so the browser keeps them from reading. + for _, origin := range []string{"https://evil.example", "http://localhost", "capacitor://evil", "null"} { + resp = ts.corsDo(t, "OPTIONS", "/api/v1/projects", origin, preflight) + if got := resp.Header.Get("Access-Control-Allow-Origin"); got != "" { + t.Errorf("%s: preflight answered with Allow-Origin %q", origin, got) + } + resp = ts.corsDo(t, "GET", "/api/v1/ping", origin, nil) + if got := resp.Header.Get("Access-Control-Allow-Origin"); got != "" { + t.Errorf("%s: GET answered with Allow-Origin %q", origin, got) + } + } +} + +// The app asks this before anyone has signed in, to tell a Verdande from a +// router's login page. +func TestPingNamesTheProductAndEditionButNoVersion(t *testing.T) { + ts := newTestServer(t) + anonymous := &testServer{Server: ts.Server, db: ts.db, client: newJarClient(t)} + resp, body := anonymous.do(t, "GET", "/api/v1/ping", nil) + if resp.StatusCode != http.StatusOK { + t.Fatalf("ping: %d", resp.StatusCode) + } + if body["product"] != "verdande" || body["edition"] != "full" || body["api"] != float64(1) { + t.Errorf("ping = %v", body) + } + if _, ok := body["version"]; ok { + t.Error("ping reveals the version to anybody") + } +} + +func dialWS(ctx context.Context, url, origin string) (*websocket.Conn, *http.Response, error) { + h := http.Header{} + if origin != "" { + h.Set("Origin", origin) + } + return websocket.Dial(ctx, url, &websocket.DialOptions{HTTPHeader: h}) +} + +func TestTheAppOpensLiveSyncWithATicket(t *testing.T) { + ts := newTestServer(t) + ts.bootstrap(t) + _, body := appLogin(t, ts) + token := body["session_token"].(string) + wsURL := "ws" + strings.TrimPrefix(ts.URL, "http") + "/api/v1/ws" + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + // Without a ticket or a cookie: refused. + if _, resp, err := dialWS(ctx, wsURL, "capacitor://localhost"); err == nil { + t.Fatal("a socket opened with no credential") + } else if resp == nil || resp.StatusCode != http.StatusUnauthorized { + t.Errorf("no credential: %v", resp) + } + + resp, body := ts.appDo(t, token, "POST", "/api/v1/ws-ticket", nil) + ticket, _ := body["ticket"].(string) + if resp.StatusCode != http.StatusOK || ticket == "" { + t.Fatalf("ws-ticket: %d %v", resp.StatusCode, body) + } + + conn, _, err := dialWS(ctx, wsURL+"?ticket="+ticket, "capacitor://localhost") + if err != nil { + t.Fatalf("dial with a ticket: %v", err) + } + defer conn.CloseNow() + + // And it is the right person's stream: their own new project arrives on it. + ts.appDo(t, token, "POST", "/api/v1/projects", map[string]string{"name": "Live fra telefonen"}) + _, msg, err := conn.Read(ctx) + if err != nil { + t.Fatalf("read: %v", err) + } + if !strings.Contains(string(msg), "Live fra telefonen") { + t.Errorf("first event was %s", msg) + } + + // Spent: the same ticket does not open a second socket. + if _, resp, err := dialWS(ctx, wsURL+"?ticket="+ticket, "capacitor://localhost"); err == nil { + t.Error("a ticket opened two sockets") + } else if resp == nil || resp.StatusCode != http.StatusUnauthorized { + t.Errorf("reused ticket: %v", resp) + } +} + +func TestATicketExpires(t *testing.T) { + ts := newTestServer(t) + ts.bootstrap(t) + _, body := appLogin(t, ts) + _, body = ts.appDo(t, body["session_token"].(string), "POST", "/api/v1/ws-ticket", nil) + ticket := body["ticket"].(string) + + ts.api.wsTickets.mu.Lock() + v := ts.api.wsTickets.m[ticket] + v.expires = time.Now().Add(-time.Second) + ts.api.wsTickets.m[ticket] = v + ts.api.wsTickets.mu.Unlock() + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + wsURL := "ws" + strings.TrimPrefix(ts.URL, "http") + "/api/v1/ws?ticket=" + ticket + if _, resp, err := dialWS(ctx, wsURL, "capacitor://localhost"); err == nil { + t.Error("an expired ticket opened a socket") + } else if resp == nil || resp.StatusCode != http.StatusUnauthorized { + t.Errorf("expired ticket: %v", resp) + } +} + +// The cookie path is unchanged: a foreign page cannot open a socket that rides +// on the cookie, and the instance's own page can. +func TestACookieSocketStillChecksTheOrigin(t *testing.T) { + ts := newTestServer(t) + ts.bootstrap(t) + var cookie string + for _, c := range ts.client.Jar.Cookies(mustURL(t, ts.URL)) { + if c.Name == sessionCookieInsecure { + cookie = c.Value + } + } + wsURL := "ws" + strings.TrimPrefix(ts.URL, "http") + "/api/v1/ws" + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + dial := func(origin string) error { + h := http.Header{} + h.Set("Origin", origin) + h.Set("Cookie", sessionCookieInsecure+"="+cookie) + conn, _, err := websocket.Dial(ctx, wsURL, &websocket.DialOptions{HTTPHeader: h}) + if err == nil { + conn.CloseNow() + } + return err + } + if err := dial("https://evil.example"); err == nil { + t.Error("a foreign page opened a socket on the cookie") + } + if err := dial("capacitor://localhost"); err == nil { + t.Error("the app origin opened a socket on a cookie, without a ticket") + } + if err := dial("http://localhost"); err != nil { + t.Errorf("control: the instance's own origin: %v", err) + } +} diff --git a/internal/httpapi/app_handlers.go b/internal/httpapi/app_handlers.go new file mode 100644 index 0000000..f143490 --- /dev/null +++ b/internal/httpapi/app_handlers.go @@ -0,0 +1,169 @@ +package httpapi + +import ( + "context" + "crypto/rand" + "encoding/hex" + "net/http" + "sync" + "time" +) + +// What the iOS app needs from a server that a browser gets for free by being on +// the same origin. The app loads its interface from capacitor://localhost and +// talks to whichever server its owner typed in, so every request it makes is +// cross-origin: it needs CORS to read the answers, a way to open the live-sync +// socket without a cookie, and a way to ask "is this a Verdande?" before anyone +// has signed in. + +// appCORS answers the app's origin, and nobody else's. +// +// No Access-Control-Allow-Credentials: the app carries its session as a bearer +// token, never a cookie, so a response to a request with a cookie on it is not +// something this header ever needs to unlock. Every other origin gets no CORS +// headers at all, exactly as before — the browser then keeps a foreign page from +// reading anything, which is the default this must not loosen. +func appCORS(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + origin := r.Header.Get("Origin") + if !isAppOrigin(origin) { + next.ServeHTTP(w, r) + return + } + h := w.Header() + h.Set("Access-Control-Allow-Origin", origin) + h.Add("Vary", "Origin") + // Read by the client: a rate limit's wait, and a download's file name. + h.Set("Access-Control-Expose-Headers", "Retry-After, Content-Disposition") + + if r.Method == http.MethodOptions && r.Header.Get("Access-Control-Request-Method") != "" { + h.Set("Access-Control-Allow-Methods", "GET, POST, PUT, PATCH, DELETE") + h.Set("Access-Control-Allow-Headers", "Authorization, Content-Type") + h.Set("Access-Control-Max-Age", "600") + w.WriteHeader(http.StatusNoContent) + return + } + next.ServeHTTP(w, r) + }) +} + +// --- the live-sync socket without a cookie ----------------------------------- + +// A browser cannot put a header on a WebSocket handshake, so the app's bearer +// token has no way onto it. Putting the token in the URL would work and would +// leave a credential that lasts months in every proxy log the URL passes. A +// ticket is the usual answer: asked for with the token, good for one connection, +// gone in thirty seconds. + +const wsTicketTTL = 30 * time.Second + +type wsTicket struct { + userID string + expires time.Time +} + +type wsTickets struct { + mu sync.Mutex + m map[string]wsTicket +} + +func newWSTickets() *wsTickets { return &wsTickets{m: map[string]wsTicket{}} } + +func (t *wsTickets) issue(userID string) (string, error) { + raw := make([]byte, 32) + if _, err := rand.Read(raw); err != nil { + return "", err + } + ticket := hex.EncodeToString(raw) + + t.mu.Lock() + defer t.mu.Unlock() + // Swept on the way in, so the map is bounded by thirty seconds of asking. + now := time.Now() + for k, v := range t.m { + if now.After(v.expires) { + delete(t.m, k) + } + } + t.m[ticket] = wsTicket{userID: userID, expires: now.Add(wsTicketTTL)} + return ticket, nil +} + +// redeem spends a ticket. It is deleted before it is checked, so a second +// presentation finds nothing, whether the first one was in time or not. +func (t *wsTickets) redeem(ticket string) (string, bool) { + t.mu.Lock() + defer t.mu.Unlock() + v, ok := t.m[ticket] + delete(t.m, ticket) + if !ok || time.Now().After(v.expires) { + return "", false + } + return v.userID, true +} + +func (s *Server) handleWSTicket(w http.ResponseWriter, r *http.Request) { + ticket, err := s.wsTickets.issue(userFrom(r.Context()).ID) + if err != nil { + s.internal(w, r, "issue websocket ticket", err) + return + } + writeJSON(w, http.StatusOK, map[string]any{ + "ticket": ticket, + "expires_in": int(wsTicketTTL.Seconds()), + }) +} + +// handleWebSocketEntry opens the socket with a ticket, or falls through to the +// ordinary session check. +// +// A socket opened with a ticket skips the origin check, and that is the point +// rather than a gap: the origin check exists because a browser attaches the +// cookie to a handshake from any page. A ticket is attached by nobody — it has to +// be asked for with a credential and put in the URL on purpose — so there is no +// ambient authority for a foreign page to borrow. +func (s *Server) handleWebSocketEntry(w http.ResponseWriter, r *http.Request) { + ticket := r.URL.Query().Get("ticket") + if ticket == "" { + // The app never holds a cookie, so a handshake from its origin without a + // ticket is not the app. Refused here rather than left to the origin + // check, which compares host names only: capacitor://localhost and an + // instance served at http://localhost have the same one. + if isAppOrigin(r.Header.Get("Origin")) { + writeError(w, http.StatusUnauthorized, CodeUnauthorized, "the app opens this socket with a ticket") + return + } + s.requireAuth(http.HandlerFunc(s.handleWebSocket)).ServeHTTP(w, r) + return + } + userID, ok := s.wsTickets.redeem(ticket) + if !ok { + writeError(w, http.StatusUnauthorized, CodeUnauthorized, "that ticket is not valid") + return + } + user, err := s.db.UserByID(r.Context(), userID) + if err != nil { + s.storeError(w, r, "load user", err) + return + } + ctx := context.WithValue(withUser(r.Context(), user), ctxTicket, true) + s.handleWebSocket(w, r.WithContext(ctx)) +} + +// --- is this a Verdande? ----------------------------------------------------- + +// handlePing answers before anybody has signed in, which is when the app needs +// it: the person has typed an address and the app has to tell "that is a +// Verdande" from "that is a router's login page" before it shows a password +// field for it. The edition decides which half of the interface to show. +// +// No version. /version sits behind a login on purpose, and a public number is a +// list of the instances still running an old one. +func (s *Server) handlePing(w http.ResponseWriter, r *http.Request) { + writeJSON(w, http.StatusOK, map[string]any{ + "status": "ok", + "product": "verdande", + "edition": s.edition(), + "api": 1, + }) +} diff --git a/internal/httpapi/middleware.go b/internal/httpapi/middleware.go index 3aa3a45..dcaf042 100644 --- a/internal/httpapi/middleware.go +++ b/internal/httpapi/middleware.go @@ -18,6 +18,8 @@ type ctxKey int const ( ctxUser ctxKey = iota ctxSession + // ctxTicket marks a socket opened with a one-use ticket rather than a cookie. + ctxTicket ) // sessionCookie is prefixed with __Host- when served over HTTPS. That prefix is diff --git a/internal/httpapi/realtime_handlers.go b/internal/httpapi/realtime_handlers.go index d5c66e9..a62de42 100644 --- a/internal/httpapi/realtime_handlers.go +++ b/internal/httpapi/realtime_handlers.go @@ -45,11 +45,15 @@ func (s *Server) publishToOwner(r *http.Request, event string, payload any) { func (s *Server) handleWebSocket(w http.ResponseWriter, r *http.Request) { user := userFrom(r.Context()) + ticketed, _ := r.Context().Value(ctxTicket).(bool) conn, err := websocket.Accept(w, r, &websocket.AcceptOptions{ // The socket is same-origin only. Without this, any page on the internet // could open a socket carrying the user's cookie and watch their tasks — // the same-origin policy does not apply to WebSocket handshakes. OriginPatterns: s.allowedOrigins(), + // Except one opened with a ticket, which carries no cookie for a foreign + // page to borrow: see handleWebSocketEntry. + InsecureSkipVerify: ticketed, }) if err != nil { s.log.Debug("websocket handshake", "err", err) diff --git a/internal/httpapi/server.go b/internal/httpapi/server.go index aa500da..f73ddd9 100644 --- a/internal/httpapi/server.go +++ b/internal/httpapi/server.go @@ -60,6 +60,10 @@ type Server struct { // instance should not be unreachable because one of them cannot be offered. passkeys *auth.WebAuthn + // One-use tickets for opening the live-sync socket without a cookie — see + // app_handlers.go. + wsTickets *wsTickets + // Password guessing is the attack this application is actually exposed to, so // the endpoints that check a secret are limited separately and more tightly // than the rest of the API. @@ -115,6 +119,7 @@ func New(cfg *config.Config, db *store.DB, log *slog.Logger, web fs.FS) *Server // their own code gets it right on the first or second try; ten in an hour is // generous for a person and hopeless for a search through a million. totpLimiter: newLimiter(10, time.Hour), + wsTickets: newWSTickets(), } // Before the CSP, deliberately: the policy has to describe the bytes that go // out, and in the notes edition those are the renamed ones. @@ -273,11 +278,15 @@ func New(cfg *config.Config, db *store.DB, log *slog.Logger, web fs.FS) *Server } r.Route("/api/v1", func(r chi.Router) { + // Before the CSRF lock, so a preflight is answered and never reaches it. + r.Use(appCORS) r.Use(s.requireCSRF) - r.Get("/ping", func(w http.ResponseWriter, r *http.Request) { - writeJSON(w, http.StatusOK, map[string]string{"status": "ok"}) - }) + r.Get("/ping", s.handlePing) + + // The live-sync socket checks its own credential: a ticket, or the + // session as everywhere else. See handleWebSocketEntry. + r.Get("/ws", s.handleWebSocketEntry) // The beacon's receiving end, and the number it produces. Both without a // session, because the caller is a stranger's installation — that is the @@ -378,7 +387,7 @@ func New(cfg *config.Config, db *store.DB, log *slog.Logger, web fs.FS) *Server full := !s.cfg.NotesOnly() r.Use(s.requireAuth) - r.Get("/ws", s.handleWebSocket) + r.Post("/ws-ticket", s.handleWSTicket) // The MCP endpoint. Reachable with a personal API token, so a connector // sees exactly what its owner sees.