diff --git a/internal/handlers/event_body.go b/internal/handlers/event_body.go new file mode 100644 index 0000000..73dde40 --- /dev/null +++ b/internal/handlers/event_body.go @@ -0,0 +1,192 @@ +package handlers + +import ( + "database/sql" + "errors" + "net/http" + "strconv" + + "github.com/go-chi/chi" + "github.com/google/uuid" + "gorm.io/gorm" + "sneak.berlin/go/webhooker/internal/database" +) + +// eventBodyQuery reads one event's stored body as bytes. The cast +// to blob is what makes the driver hand back the stored bytes +// rather than a string conversion, so Content-Length taken from +// the result matches what goes on the wire. The soft-delete +// predicate is spelled out because Raw bypasses GORM's default +// scope, and it is what stops a reaped event still being +// downloadable. +const eventBodyQuery = "SELECT cast(body as blob) " + + "FROM events WHERE id = ? AND webhook_id = ? AND deleted_at IS NULL" + +// HandleEventBodyDownload serves one event's stored body in +// full, which the event log page cannot: it caps each rendered +// body at maxRenderedBodyBytes. +// +// The bytes are attacker-supplied — anyone who can reach the +// public receiver chooses them — and this route hands them back +// inside the operator's own authenticated origin, so the +// response is deliberately not renderable. Content-Disposition +// makes the browser download rather than display it, and the +// octet-stream type plus nosniff stop it being interpreted as +// HTML or script. Without those a stored payload would execute +// as the logged-in operator. The application's CSP does not +// help here: script-src allows 'unsafe-inline' from 'self', so +// a document served from this origin could run its own inline +// script. +func (h *Handlers) HandleEventBodyDownload() http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + webhook, ok := h.ownedWebhook(w, r) + if !ok { + return + } + + // Parsing the id before use serves two purposes: a + // malformed id can never reach the SQL or the response + // header, and the canonical form below is drawn from + // uuid's own fixed alphabet rather than from the + // request, so the Content-Disposition value cannot be + // steered by a client. + eventID, err := uuid.Parse(chi.URLParam(r, "eventID")) + if err != nil { + http.NotFound(w, r) + + return + } + + h.serveEventBody(w, r, webhook, eventID.String()) + } +} + +// serveEventBody writes the named event's stored body to w. +// +// The event must belong to webhook, which is what keeps this +// route from reading any event in the system by id alone. Two +// things enforce that and they are not equally strong. The +// operative one is that events live in a per-webhook SQLite +// file, so a sibling webhook's event is not in the database +// being queried at all. The webhook_id predicate on the query +// below is the second guard, and it is currently redundant +// against that isolation; it is there so the scoping survives +// any future change that puts more than one webhook's events in +// one file. +// +// The body is read in one query and held whole in memory while +// it is written. That is the bound: one body per concurrent +// download, and a body is capped at 1 MB when it is ingested, +// so a download cannot cost more than that. There is no +// cheaper bound available — database/sql exposes no incremental +// handle on a SQLite BLOB, and reading byte ranges with substr +// does not avoid the cost either, because SQLite materialises +// the whole column value to evaluate each substr call. Range +// reads only pay for that materialisation once per range. +// +// One consequence is worth keeping in view: the read finishes +// before the client is written to, so no read lock is held for +// the length of a slow download. These per-webhook databases +// run in SQLite's default journal mode rather than WAL, so a +// lock held that long would block the receiver from recording +// new events. +func (h *Handlers) serveEventBody( + w http.ResponseWriter, + r *http.Request, + webhook database.Webhook, + eventID string, +) { + if !h.dbMgr.DBExists(webhook.ID) { + http.NotFound(w, r) + + return + } + + webhookDB, err := h.dbMgr.GetDB(webhook.ID) + if err != nil { + h.serverError(w, "failed to get webhook database", err) + + return + } + + body, found, err := eventBody(webhookDB, webhook.ID, eventID) + if err != nil { + h.serverError(w, "failed to read event body", err) + + return + } + + // A miss is a 404 whether the event belongs to another + // webhook or does not exist at all, so the response does + // not report which. Reading the body before any header is + // written is also what keeps an event reaped mid-request + // from producing a torn response: either the read finds the + // row and the whole body is served, or it does not and the + // response is a clean 404. + if !found { + http.NotFound(w, r) + + return + } + + setEventBodyHeaders(w, eventID, int64(len(body))) + + _, err = w.Write(body) + if err != nil { + // The status and Content-Length are already committed, + // so the client sees a short download. There is no way + // to report a 500 from here; the log is the record. + h.log.Error( + "failed to write event body", + "webhook_id", webhook.ID, + "event_id", eventID, + "error", err, + ) + } +} + +// eventBody returns an event's stored body and whether the event +// exists within the webhook. +func eventBody( + webhookDB *gorm.DB, + webhookID, eventID string, +) ([]byte, bool, error) { + var body []byte + + err := webhookDB.Raw( + eventBodyQuery, eventID, webhookID, + ).Row().Scan(&body) + + if errors.Is(err, sql.ErrNoRows) { + return nil, false, nil + } + + if err != nil { + return nil, false, err + } + + return body, true, nil +} + +// setEventBodyHeaders applies the response headers that make +// this route safe to hand attacker-supplied bytes through. See +// HandleEventBodyDownload for why they are a security control +// and not a formatting choice. +// +// nosniff is also set by the global SecurityHeaders middleware. +// It is repeated here so the guarantee belongs to the route +// that needs it rather than to a middleware someone could +// reorder or scope away. +func setEventBodyHeaders( + w http.ResponseWriter, + eventID string, + size int64, +) { + w.Header().Set("Content-Type", "application/octet-stream") + w.Header().Set("X-Content-Type-Options", "nosniff") + w.Header().Set( + "Content-Disposition", + `attachment; filename="webhooker-event-`+eventID+`.bin"`, + ) + w.Header().Set("Content-Length", strconv.FormatInt(size, 10)) +} diff --git a/internal/handlers/event_body_test.go b/internal/handlers/event_body_test.go new file mode 100644 index 0000000..e4656ac --- /dev/null +++ b/internal/handlers/event_body_test.go @@ -0,0 +1,506 @@ +package handlers_test + +import ( + "context" + "net/http" + "net/http/httptest" + "net/url" + "strconv" + "strings" + "testing" + + "github.com/go-chi/chi" + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gorm.io/gorm/clause" + "sneak.berlin/go/webhooker/internal/database" + "sneak.berlin/go/webhooker/internal/handlers" + "sneak.berlin/go/webhooker/internal/session" +) + +// paramEventID is the chi URL parameter the body download +// handler reads. +const paramEventID = "eventID" + +// otherTestUserID owns webhooks the session user must not be +// able to read. +const otherTestUserID = "other-user-id" + +// seedWebhookFor inserts a webhook owned by the given user. +func seedWebhookFor( + t *testing.T, + db *database.Database, + userID string, +) *database.Webhook { + t.Helper() + + wh := &database.Webhook{ + UserID: userID, + Name: "wh-" + userID, + } + + require.NoError( + t, + db.DB().Omit(clause.Associations).Create(wh).Error, + ) + + return wh +} + +// fetchEventBody runs the real download handler as the test user +// for the given source and event ids. +func fetchEventBody( + t *testing.T, + h *handlers.Handlers, + sess *session.Session, + sourceID, eventID string, +) *httptest.ResponseRecorder { + t.Helper() + + // The path is escaped and the raw id goes in the route + // context, which is what chi hands a handler: the param is + // already percent-decoded by the time it is read. + req := httptest.NewRequestWithContext( + context.Background(), + http.MethodGet, + "/source/"+url.PathEscape(sourceID)+ + "/logs/"+url.PathEscape(eventID)+"/body", + nil, + ) + + for _, c := range authenticatedCookies( + t, sess, deleteTestUserID, deleteTestUsername, + ) { + req.AddCookie(c) + } + + rctx := chi.NewRouteContext() + rctx.URLParams.Add(paramSourceID, sourceID) + rctx.URLParams.Add(paramEventID, eventID) + + req = req.WithContext( + context.WithValue( + req.Context(), chi.RouteCtxKey, rctx, + ), + ) + + w := httptest.NewRecorder() + h.HandleEventBodyDownload().ServeHTTP(w, req) + + return w +} + +// TestHandleEventBodyDownload_ServesOversizeBodyInFull is the +// capability the render cap took away: a body far above what the +// event log page will show comes back whole and byte-identical, +// with the headers that keep it from being rendered. +func TestHandleEventBodyDownload_ServesOversizeBodyInFull( + t *testing.T, +) { + t.Parallel() + + var ( + h *handlers.Handlers + sess *session.Session + db *database.Database + dbMgr *database.WebhookDBManager + ) + + app := newTestApp(t, &h, &sess, &db, &dbMgr) + app.RequireStart() + + t.Cleanup(app.RequireStop) + + // Far above the render cap, with multibyte runes and a + // distinctive tail, so a body that the log page can only + // show a slice of comes back whole and in order. + const sentinel = "TAIL-SENTINEL-1f4a9c" + + stored := strings.Repeat("A", 200*1024) + + strings.Repeat(snowman, 1000) + sentinel + + wh := seedWebhook(t, db) + evt := seedEventWithBody(t, dbMgr, wh.ID, stored) + + w := fetchEventBody(t, h, sess, wh.ID, evt.ID) + + require.Equal(t, http.StatusOK, w.Code) + assert.Greater(t, len(stored), bodyCap) + assert.Equal(t, stored, w.Body.String()) + assert.Equal( + t, strconv.Itoa(len(stored)), + w.Header().Get("Content-Length"), + ) +} + +// TestHandleEventBodyDownload_BodiesRoundTripByteIdentical +// covers the sizes and byte values a stored body can actually +// take: empty, one byte, either side of the render cap, and +// bytes that are not text at all. Content-Length has to equal +// the bytes written in every case, since it is derived from the +// same read that produces them. +func TestHandleEventBodyDownload_BodiesRoundTripByteIdentical( + t *testing.T, +) { + t.Parallel() + + // A NUL, invalid UTF-8 and a multibyte rune, so nothing on + // the path can be treating the body as text. + binary := "\x00\x01\xff\xfe" + snowman + "\x00tail" + + cases := map[string]string{ + "empty": "", + "single byte": "x", + "one below cap": strings.Repeat("b", bodyCap-1), + "exactly cap": strings.Repeat("c", bodyCap), + "one above cap": strings.Repeat("d", bodyCap+1), + "binary": binary, + } + + for name, stored := range cases { + t.Run(name, func(t *testing.T) { + t.Parallel() + + var ( + h *handlers.Handlers + sess *session.Session + db *database.Database + dbMgr *database.WebhookDBManager + ) + + app := newTestApp(t, &h, &sess, &db, &dbMgr) + app.RequireStart() + + t.Cleanup(app.RequireStop) + + wh := seedWebhook(t, db) + evt := seedEventWithBody(t, dbMgr, wh.ID, stored) + + w := fetchEventBody(t, h, sess, wh.ID, evt.ID) + + require.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, stored, w.Body.String()) + assert.Equal( + t, strconv.Itoa(len(stored)), + w.Header().Get("Content-Length"), + ) + assert.Equal( + t, len(stored), w.Body.Len(), + "Content-Length must equal bytes written", + ) + }) + } +} + +// TestHandleEventBodyDownload_HeadersAreNotRenderable pins the +// response headers that stop attacker-supplied bytes executing +// in the operator's own origin. They are a security control, not +// presentation. +func TestHandleEventBodyDownload_HeadersAreNotRenderable( + t *testing.T, +) { + t.Parallel() + + var ( + h *handlers.Handlers + sess *session.Session + db *database.Database + dbMgr *database.WebhookDBManager + ) + + app := newTestApp(t, &h, &sess, &db, &dbMgr) + app.RequireStart() + + t.Cleanup(app.RequireStop) + + wh := seedWebhook(t, db) + evt := seedEventWithBody(t, dbMgr, wh.ID, `{"small":true}`) + + w := fetchEventBody(t, h, sess, wh.ID, evt.ID) + + require.Equal(t, http.StatusOK, w.Code) + assert.Equal( + t, "application/octet-stream", + w.Header().Get("Content-Type"), + ) + assert.Equal( + t, "nosniff", + w.Header().Get("X-Content-Type-Options"), + ) + + disposition := w.Header().Get("Content-Disposition") + assert.Equal( + t, + `attachment; filename="webhooker-event-`+evt.ID+`.bin"`, + disposition, + ) +} + +// TestHandleEventBodyDownload_ScriptBodyStaysInert proves a +// stored HTML payload is handed back as an attachment of opaque +// bytes rather than as anything a browser will execute. The +// bytes themselves are unaltered: this route reports what was +// delivered. +func TestHandleEventBodyDownload_ScriptBodyStaysInert( + t *testing.T, +) { + t.Parallel() + + var ( + h *handlers.Handlers + sess *session.Session + db *database.Database + dbMgr *database.WebhookDBManager + ) + + app := newTestApp(t, &h, &sess, &db, &dbMgr) + app.RequireStart() + + t.Cleanup(app.RequireStop) + + const payload = `` + + wh := seedWebhook(t, db) + evt := seedEventWithBody(t, dbMgr, wh.ID, payload) + + w := fetchEventBody(t, h, sess, wh.ID, evt.ID) + + require.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, payload, w.Body.String()) + + contentType := w.Header().Get("Content-Type") + assert.Equal(t, "application/octet-stream", contentType) + assert.NotContains(t, contentType, "html") + assert.NotContains(t, contentType, "xml") + assert.NotContains(t, contentType, "javascript") + assert.Contains( + t, w.Header().Get("Content-Disposition"), "attachment", + ) + assert.Equal( + t, "nosniff", + w.Header().Get("X-Content-Type-Options"), + ) +} + +// TestHandleEventBodyDownload_OtherUsersEvent404s is the +// authorization test the definition of done asks for: an event +// stored under a webhook the session user does not own is not +// readable, and the miss does not distinguish itself from a +// nonexistent one. +func TestHandleEventBodyDownload_OtherUsersEvent404s( + t *testing.T, +) { + t.Parallel() + + var ( + h *handlers.Handlers + sess *session.Session + db *database.Database + dbMgr *database.WebhookDBManager + ) + + app := newTestApp(t, &h, &sess, &db, &dbMgr) + app.RequireStart() + + t.Cleanup(app.RequireStop) + + const theirPayload = "OTHER-USERS-PAYLOAD-8b1d" + + theirs := seedWebhookFor(t, db, otherTestUserID) + evt := seedEventWithBody(t, dbMgr, theirs.ID, theirPayload) + + w := fetchEventBody(t, h, sess, theirs.ID, evt.ID) + + assert.Equal(t, http.StatusNotFound, w.Code) + assert.NotContains(t, w.Body.String(), theirPayload) +} + +// TestHandleEventBodyDownload_EventOfAnotherWebhook404s pins +// that holding a valid event id is not enough: the event has to +// belong to the webhook in the path. Both webhooks here are the +// session user's and both have event databases, so the +// ownership check cannot be what produces the 404. +// +// What does produce it is the per-webhook database file rather +// than the webhook_id predicate on the query — removing that +// predicate leaves this test green, because the sibling's event +// is in a different file. The test is kept as the behavioural +// guard the route owes; see serveEventBody for which mechanism +// is load-bearing. +func TestHandleEventBodyDownload_EventOfAnotherWebhook404s( + t *testing.T, +) { + t.Parallel() + + var ( + h *handlers.Handlers + sess *session.Session + db *database.Database + dbMgr *database.WebhookDBManager + ) + + app := newTestApp(t, &h, &sess, &db, &dbMgr) + app.RequireStart() + + t.Cleanup(app.RequireStop) + + const other = "BELONGS-TO-THE-OTHER-WEBHOOK-3c7e" + + mine := seedWebhook(t, db) + seedEventWithBody(t, dbMgr, mine.ID, `{"mine":true}`) + + sibling := seedWebhook(t, db) + evt := seedEventWithBody(t, dbMgr, sibling.ID, other) + + w := fetchEventBody(t, h, sess, mine.ID, evt.ID) + + assert.Equal(t, http.StatusNotFound, w.Code) + assert.NotContains(t, w.Body.String(), other) +} + +// TestHandleEventBodyDownload_UnknownEvent404s covers the plain +// miss, including an id that is not a uuid at all and so never +// reaches the query or the response header. +func TestHandleEventBodyDownload_UnknownEvent404s(t *testing.T) { + t.Parallel() + + var ( + h *handlers.Handlers + sess *session.Session + db *database.Database + dbMgr *database.WebhookDBManager + ) + + app := newTestApp(t, &h, &sess, &db, &dbMgr) + app.RequireStart() + + t.Cleanup(app.RequireStop) + + wh := seedWebhook(t, db) + seedEventWithBody(t, dbMgr, wh.ID, `{"mine":true}`) + + for _, id := range []string{ + uuid.New().String(), + `../../etc/passwd`, + "not-a-uuid", + `x"; rm -rf /`, + } { + w := fetchEventBody(t, h, sess, wh.ID, id) + + assert.Equal( + t, http.StatusNotFound, w.Code, + "event id %q", id, + ) + assert.Empty( + t, w.Header().Get("Content-Disposition"), + "event id %q must not reach a header", id, + ) + } +} + +// TestHandleEventBodyDownload_ReapedEvent404s pins what happens +// when the retention reaper takes an event out from under this +// route. The body is read in one query before any header is +// written, so a reaped event cannot produce a partial download: +// it is a clean 404 with no Content-Length and no +// Content-Disposition. Both removals the codebase performs are +// covered — the reaper hard-deletes, and a soft-deleted row is +// excluded by the query's own deleted_at predicate rather than +// by GORM's default scope, which Raw bypasses. +func TestHandleEventBodyDownload_ReapedEvent404s(t *testing.T) { + t.Parallel() + + for name, hard := range map[string]bool{ + "soft deleted": false, + "hard deleted": true, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + + var ( + h *handlers.Handlers + sess *session.Session + db *database.Database + dbMgr *database.WebhookDBManager + ) + + app := newTestApp(t, &h, &sess, &db, &dbMgr) + app.RequireStart() + + t.Cleanup(app.RequireStop) + + const payload = "REAPED-PAYLOAD-4d2a" + + wh := seedWebhook(t, db) + evt := seedEventWithBody(t, dbMgr, wh.ID, payload) + + webhookDB, err := dbMgr.GetDB(wh.ID) + require.NoError(t, err) + + del := webhookDB + if hard { + del = del.Unscoped() + } + + require.NoError( + t, + del.Delete(&database.Event{}, "id = ?", evt.ID). + Error, + ) + + w := fetchEventBody(t, h, sess, wh.ID, evt.ID) + + assert.Equal(t, http.StatusNotFound, w.Code) + assert.NotContains(t, w.Body.String(), payload) + assert.Empty(t, w.Header().Get("Content-Length")) + assert.Empty( + t, w.Header().Get("Content-Disposition"), + ) + }) + } +} + +// TestHandleSourceLogs_TruncationMarkerLinksToDownload proves +// the page tells the reader where the rest of the body is, and +// only when there is a rest to fetch. +func TestHandleSourceLogs_TruncationMarkerLinksToDownload( + t *testing.T, +) { + t.Parallel() + + var ( + h *handlers.Handlers + sess *session.Session + db *database.Database + dbMgr *database.WebhookDBManager + ) + + app := newTestApp(t, &h, &sess, &db, &dbMgr) + app.RequireStart() + + t.Cleanup(app.RequireStop) + + big := seedWebhook(t, db) + bigEvt := seedEventWithBody( + t, dbMgr, big.ID, strings.Repeat("A", 4*bodyCap), + ) + + page := renderSourceLogsPage(t, h, sess, big.ID) + assert.Contains( + t, page, + "/source/"+big.ID+"/logs/"+bigEvt.ID+"/body", + ) + + small := seedWebhook(t, db) + smallEvt := seedEventWithBody( + t, dbMgr, small.ID, `{"kept":"whole"}`, + ) + + page = renderSourceLogsPage(t, h, sess, small.ID) + assert.NotContains( + t, page, + "/source/"+small.ID+"/logs/"+smallEvt.ID+"/body", + ) +} diff --git a/internal/handlers/event_log_view_test.go b/internal/handlers/event_log_view_test.go index 7ffd2fb..e1d5a41 100644 --- a/internal/handlers/event_log_view_test.go +++ b/internal/handlers/event_log_view_test.go @@ -25,13 +25,14 @@ const bodyCap = handlers.MaxRenderedBodyBytesForTest const snowman = "☃" // seedEventWithBody records one event with the given body in the -// webhook's own database. +// webhook's own database and returns it, so a caller that needs +// the generated event id can have it. func seedEventWithBody( t *testing.T, dbMgr *database.WebhookDBManager, webhookID string, body string, -) { +) *database.Event { t.Helper() webhookDB, err := dbMgr.GetDB(webhookID) @@ -47,6 +48,8 @@ func seedEventWithBody( require.NoError(t, webhookDB.Omit( clause.Associations, ).Create(event).Error) + + return event } // seedAndProject stores one body and returns the projection the diff --git a/internal/handlers/source_management.go b/internal/handlers/source_management.go index b538bcd..0243b76 100644 --- a/internal/handlers/source_management.go +++ b/internal/handlers/source_management.go @@ -713,29 +713,55 @@ func (h *Handlers) evictArchiveWriterIfUnused(webhookID string) { h.evictArchiveWriter(webhookID) } +// ownedWebhook resolves the request's sourceID parameter to a +// webhook the session's user owns. +// +// Ownership and existence are decided by one query, so a +// webhook belonging to another user is indistinguishable from +// one that does not exist: both are a 404, and neither confirms +// the id. Callers that reach further into a webhook's data — +// the event log page and the event body download — share this +// one check rather than restating it, so the download cannot +// come to authorize differently from the page that links to it. +// +// It reports false once it has written the response, which is a +// redirect to the login page for an unauthenticated request and +// a 404 otherwise. The caller returns without writing more. +func (h *Handlers) ownedWebhook( + w http.ResponseWriter, + r *http.Request, +) (database.Webhook, bool) { + var webhook database.Webhook + + userID, ok := h.getUserID(r) + if !ok { + http.Redirect( + w, r, "/pages/login", http.StatusSeeOther, + ) + + return database.Webhook{}, false + } + + sourceID := chi.URLParam(r, "sourceID") + + err := h.db.DB().Where( + "id = ? AND user_id = ?", sourceID, userID, + ).First(&webhook).Error + if err != nil { + http.NotFound(w, r) + + return database.Webhook{}, false + } + + return webhook, true +} + // HandleSourceLogs shows the request/response logs for a // webhook. func (h *Handlers) HandleSourceLogs() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - userID, ok := h.getUserID(r) + webhook, ok := h.ownedWebhook(w, r) if !ok { - http.Redirect( - w, r, "/pages/login", http.StatusSeeOther, - ) - - return - } - - sourceID := chi.URLParam(r, "sourceID") - - var webhook database.Webhook - - err := h.db.DB().Where( - "id = ? AND user_id = ?", sourceID, userID, - ).First(&webhook).Error - if err != nil { - http.NotFound(w, r) - return } diff --git a/internal/server/routes.go b/internal/server/routes.go index b282e12..a8e4b01 100644 --- a/internal/server/routes.go +++ b/internal/server/routes.go @@ -146,6 +146,15 @@ func (s *Server) setupSourceRoutes() { r.Post("/edit", s.h.HandleSourceEditSubmit()) r.Post("/delete", s.h.HandleSourceDelete()) r.Get("/logs", s.h.HandleSourceLogs()) + // The log page renders each body only up to its cap, so + // this is the only route that serves a whole one. It + // belongs to this group for its RequireAuth and + // NoCache; see HandleEventBodyDownload for the headers + // that keep the bytes it returns inert. + r.Get( + "/logs/{eventID}/body", + s.h.HandleEventBodyDownload(), + ) r.Post( "/entrypoints", s.h.HandleEntrypointCreate(), diff --git a/internal/server/routes_test.go b/internal/server/routes_test.go index 1c5f72d..81b6cf0 100644 --- a/internal/server/routes_test.go +++ b/internal/server/routes_test.go @@ -7,6 +7,7 @@ import ( "net/http/httptest" "net/url" "regexp" + "strconv" "strings" "testing" @@ -14,6 +15,7 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/fx" "go.uber.org/fx/fxtest" + "gorm.io/gorm/clause" "sneak.berlin/go/webhooker/internal/config" "sneak.berlin/go/webhooker/internal/database" "sneak.berlin/go/webhooker/internal/delivery" @@ -48,6 +50,7 @@ type testEnv struct { router http.Handler sess *session.Session db *database.Database + dbMgr *database.WebhookDBManager } // newTestEnv wires the dependency graph with fx and builds the @@ -57,12 +60,13 @@ func newTestEnv(t *testing.T) *testEnv { t.Helper() var ( - log *logger.Logger - cfg *config.Config - mw *middleware.Middleware - hnd *handlers.Handlers - sess *session.Session - db *database.Database + log *logger.Logger + cfg *config.Config + mw *middleware.Middleware + hnd *handlers.Handlers + sess *session.Session + db *database.Database + dbMgr *database.WebhookDBManager ) app := fxtest.New( @@ -85,7 +89,7 @@ func newTestEnv(t *testing.T) *testEnv { middleware.New, handlers.New, ), - fx.Populate(&log, &cfg, &mw, &hnd, &sess, &db), + fx.Populate(&log, &cfg, &mw, &hnd, &sess, &db, &dbMgr), ) app.RequireStart() t.Cleanup(app.RequireStop) @@ -94,6 +98,7 @@ func newTestEnv(t *testing.T) *testEnv { router: server.NewRouterForTest(log.Get(), cfg, mw, hnd), sess: sess, db: db, + dbMgr: dbMgr, } } @@ -232,6 +237,49 @@ func (e *testEnv) seedUser( return user.ID, hash } +// seedWebhook creates a webhook owned by the given user. +func (e *testEnv) seedWebhook( + t *testing.T, + userID string, +) *database.Webhook { + t.Helper() + + wh := &database.Webhook{UserID: userID, Name: "routed"} + + require.NoError( + t, + e.db.DB().Omit(clause.Associations).Create(wh).Error, + ) + + return wh +} + +// seedEvent records one event with the given body in a webhook's +// own database. +func (e *testEnv) seedEvent( + t *testing.T, + webhookID, body string, +) *database.Event { + t.Helper() + + webhookDB, err := e.dbMgr.GetDB(webhookID) + require.NoError(t, err) + + event := &database.Event{ + WebhookID: webhookID, + Method: http.MethodPost, + Body: body, + ContentType: "application/octet-stream", + } + + require.NoError( + t, + webhookDB.Omit(clause.Associations).Create(event).Error, + ) + + return event +} + // storedHash reads the current password hash for a username. func (e *testEnv) storedHash(t *testing.T, username string) string { t.Helper() @@ -381,3 +429,95 @@ func TestPasswordChange_UnderLimit_Succeeds(t *testing.T) { "an under-limit password change should still apply", ) } + +// --- /source/{sourceID} group --- + +// TestSourceLogs_TruncationLinkDownloadsTheBody walks the whole +// feature the way a user does: render the event log page through +// the production router, take the download URL out of the markup +// the template emitted, and fetch that URL through the router +// again. Nothing here is hand-written, so a typo in either the +// route pattern or the template href fails this test — the +// handler-level tests cannot catch that, because they forge +// their own route context and assert a URL string they wrote +// themselves. +func TestSourceLogs_TruncationLinkDownloadsTheBody(t *testing.T) { + t.Parallel() + + env := newTestEnv(t) + + userID, _ := env.seedUser(t, "loguser", "somepassword") + cookies := env.authCookies(t, userID, "loguser") + + // Comfortably over the event log page's render cap, so the + // page truncates the body and renders the download link at + // all. The exact cap is the handlers package's business and + // is pinned by its own tests; this only needs to exceed it. + stored := strings.Repeat("Z", 64*1024) + + wh := env.seedWebhook(t, userID) + env.seedEvent(t, wh.ID, stored) + + page := env.get("/source/"+wh.ID+"/logs", cookies) + require.Equal(t, http.StatusOK, page.Code) + + link := regexp.MustCompile( + `href="(/source/[^"]+/body)"`, + ).FindStringSubmatch(page.Body.String()) + require.Len( + t, link, 2, + "truncated body should render a download link", + ) + + w := env.get(html.UnescapeString(link[1]), cookies) + + require.Equal( + t, http.StatusOK, w.Code, + "the link the page emits must be a live route", + ) + assert.Equal(t, stored, w.Body.String()) + assert.Equal( + t, strconv.Itoa(len(stored)), + w.Header().Get("Content-Length"), + ) + assert.Equal( + t, "application/octet-stream", + w.Header().Get("Content-Type"), + ) + assert.Contains( + t, w.Header().Get("Content-Disposition"), "attachment", + ) + assert.Equal( + t, "nosniff", w.Header().Get("X-Content-Type-Options"), + ) +} + +// TestSourceLogsBody_OtherUser404s pins that the download route +// as registered is behind the auth the group provides and the +// ownership check the handler applies: another logged-in user +// asking the real router for the same URL gets a 404, and an +// unauthenticated request never reaches the handler at all. +func TestSourceLogsBody_OtherUser404s(t *testing.T) { + t.Parallel() + + env := newTestEnv(t) + + ownerID, _ := env.seedUser(t, "owner", "somepassword") + wh := env.seedWebhook(t, ownerID) + + const payload = "OWNERS-PAYLOAD-77c1" + + evt := env.seedEvent(t, wh.ID, payload) + path := "/source/" + wh.ID + "/logs/" + evt.ID + "/body" + + intruderID, _ := env.seedUser(t, "intruder", "somepassword") + intruder := env.authCookies(t, intruderID, "intruder") + + w := env.get(path, intruder) + assert.Equal(t, http.StatusNotFound, w.Code) + assert.NotContains(t, w.Body.String(), payload) + + anon := env.get(path, nil) + assert.Equal(t, http.StatusSeeOther, anon.Code) + assert.Equal(t, "/pages/login", anon.Header().Get("Location")) +} diff --git a/templates/source_logs.html b/templates/source_logs.html index e523712..cf48983 100644 --- a/templates/source_logs.html +++ b/templates/source_logs.html @@ -38,7 +38,7 @@
{{.Body}}
{{if .BodyTruncated}} -

Body truncated for display: showing {{.BodyShownBytes}} of {{.BodyBytes}} bytes. The stored body is unchanged.

+

Body truncated for display: showing {{.BodyShownBytes}} of {{.BodyBytes}} bytes. The stored body is unchanged — download the full body.

{{end}}