feat: feedback submissions with screenshots, the hide-widget preference and its user payload key

- POST /_feedback/api/v1/{key}/submit validates as PHP, refuses non-image screenshots (ImageContentGuard copy) and stores the submission, its public screenshot and the G15Office job in one transaction
- PUT /_feedback/api/v1/me/hidden (jwt.auth) stores the caller's preference; feedback_widget_hidden joins every golem15.user payload
- preflights stay with the framework CORS layer
This commit is contained in:
Jakub Zych
2026-10-04 00:10:15 +02:00
parent b4b4135ad3
commit 92e1b4e09c
12 changed files with 1073 additions and 6 deletions

View File

@@ -1,18 +1,28 @@
package feedback
import (
"bytes"
"context"
"encoding/json"
"image"
"image/png"
"mime/multipart"
"net/http"
"net/http/httptest"
"net/textproto"
"os"
"path/filepath"
"regexp"
"strconv"
"strings"
"testing"
"git.golem15.com/golem15/sm-feedback-plugin/classes"
"git.golem15.com/golem15/sm-feedback-plugin/controllers/api"
"git.golem15.com/golem15/sm-feedback-plugin/internal/pgtest"
"git.golem15.com/golem15/sm-feedback-plugin/models"
usercontrollers "git.golem15.com/golem15/sm-user-plugin/controllers"
usermodels "git.golem15.com/golem15/sm-user-plugin/models"
"git.golem15.com/golem15/summercms/modules/backpack"
"git.golem15.com/golem15/summercms/modules/compass"
"git.golem15.com/golem15/summercms/modules/lagoon"
@@ -20,6 +30,8 @@ import (
"git.golem15.com/golem15/summercms/modules/party"
"git.golem15.com/golem15/summercms/modules/phrasebook"
"git.golem15.com/golem15/summercms/modules/surf"
"gocloud.dev/blob"
_ "gocloud.dev/blob/memblob"
"gorm.io/gorm"
)
@@ -35,7 +47,7 @@ func testConfig(t testing.TB) *compass.Config {
if err := os.WriteFile(filepath.Join(dir, "app.yaml"), []byte("name: feedback-test\n"), 0o644); err != nil {
t.Fatal(err)
}
httpYAML := "body_limits:\n default_bytes: 1048576\n upload_bytes: 16777216\ncors:\n paths:\n - _feedback/api/*\n allowed_methods: [\"*\"]\n allowed_origins: [\"*\"]\n allowed_headers: [\"*\"]\n"
httpYAML := "body_limits:\n default_bytes: 33554432\n upload_bytes: 33554432\ncors:\n paths:\n - _feedback/api/*\n allowed_methods: [\"*\"]\n allowed_origins: [\"*\"]\n allowed_headers: [\"*\"]\n"
if err := os.WriteFile(filepath.Join(dir, "http.yaml"), []byte(httpYAML), 0o644); err != nil {
t.Fatal(err)
}
@@ -359,3 +371,414 @@ func TestAllowedOriginHosts(t *testing.T) {
t.Fatalf("nil origins gave %v", hosts)
}
}
// pngBytes is a valid 2x1 PNG, optionally padded to size bytes with
// trailing data (a decoder reads the header only, so it stays an image).
func pngBytes(t *testing.T, size int) []byte {
t.Helper()
img := image.NewRGBA(image.Rect(0, 0, 2, 1))
var buf bytes.Buffer
if err := png.Encode(&buf, img); err != nil {
t.Fatal(err)
}
out := buf.Bytes()
if size > len(out) {
out = append(out, make([]byte, size-len(out))...)
}
return out
}
type part struct {
name, value, filename, ctype string
data []byte
}
func multipartRequest(t *testing.T, target string, parts []part, origin string) *http.Request {
t.Helper()
var buf bytes.Buffer
mw := multipart.NewWriter(&buf)
for _, p := range parts {
if p.filename == "" {
if err := mw.WriteField(p.name, p.value); err != nil {
t.Fatal(err)
}
continue
}
h := textproto.MIMEHeader{}
h.Set("Content-Disposition", `form-data; name="`+p.name+`"; filename="`+p.filename+`"`)
h.Set("Content-Type", p.ctype)
w, err := mw.CreatePart(h)
if err != nil {
t.Fatal(err)
}
if _, err := w.Write(p.data); err != nil {
t.Fatal(err)
}
}
if err := mw.Close(); err != nil {
t.Fatal(err)
}
req := httptest.NewRequest(http.MethodPost, target, &buf)
req.Header.Set("Content-Type", mw.FormDataContentType())
req.Header.Set("Accept", "application/json")
if origin != "" {
req.Header.Set("Origin", origin)
}
return req
}
func validParts(overrides map[string]string) []part {
fields := []string{"message", "type", "email", "page_url", "user_agent", "console_log"}
values := map[string]string{
"message": "The cover is wrong.",
"type": "bug",
"email": "reporter@example.test",
"page_url": "https://widget.example.test/albums/7",
"user_agent": "Mozilla/5.0 Test",
"console_log": "[error] x is undefined",
}
for k, v := range overrides {
values[k] = v
}
out := make([]part, 0, len(fields))
for _, f := range fields {
if v, ok := values[f]; ok && v != "\x00" {
out = append(out, part{name: f, value: v})
}
}
return out
}
func countRows(t *testing.T, gdb *gorm.DB, query string, args ...any) int64 {
t.Helper()
var n int64
if err := gdb.Raw(query, args...).Scan(&n).Error; err != nil {
t.Fatal(err)
}
return n
}
// TestFeedbackSubmit drives POST {key}/submit: each validation limit one
// step either side, the image guard, the stored submission with its public
// screenshot, the G15Office job queued in the same transaction, a rolled
// back write queueing nothing, and the per-IP throttle.
func TestFeedbackSubmit(t *testing.T) {
submitURL := "/_feedback/api/v1/" + testWidgetKey + "/submit"
email255 := strings.Repeat("a", 64) + "@" + strings.Repeat("b", 63) + "." + strings.Repeat("c", 63) + "." + strings.Repeat("d", 61)
t.Run("limits", func(t *testing.T) {
cases := []struct {
name string
parts []part
status int
field string
}{
{"message-5000", validParts(map[string]string{"message": strings.Repeat("ą", 5000)}), 202, ""},
{"message-5001", validParts(map[string]string{"message": strings.Repeat("ą", 5001)}), 422, "message"},
{"page-url-2000", validParts(map[string]string{"page_url": strings.Repeat("p", 2000)}), 202, ""},
{"page-url-2001", validParts(map[string]string{"page_url": strings.Repeat("p", 2001)}), 422, "page_url"},
{"user-agent-500", validParts(map[string]string{"user_agent": strings.Repeat("u", 500)}), 202, ""},
{"user-agent-501", validParts(map[string]string{"user_agent": strings.Repeat("u", 501)}), 422, "user_agent"},
{"console-log-20000", validParts(map[string]string{"console_log": strings.Repeat("c", 20000)}), 202, ""},
{"console-log-20001", validParts(map[string]string{"console_log": strings.Repeat("c", 20001)}), 422, "console_log"},
{"email-255", validParts(map[string]string{"email": email255}), 202, ""},
{"email-256", validParts(map[string]string{"email": "a" + email255}), 422, "email"},
{"type-other", validParts(map[string]string{"type": "other"}), 202, ""},
{"type-unknown", validParts(map[string]string{"type": "question"}), 422, "type"},
{"screenshot-10240-kb", append(validParts(nil), part{name: "screenshot", filename: "screenshot.png", ctype: "image/png", data: pngBytes(t, 10240*1024)}), 202, ""},
{"screenshot-10241-kb", append(validParts(nil), part{name: "screenshot", filename: "screenshot.png", ctype: "image/png", data: pngBytes(t, 10240*1024+1)}), 422, "screenshot"},
{"screenshot-not-an-image", append(validParts(nil), part{name: "screenshot", filename: "screenshot.png", ctype: "image/png", data: []byte("plain text, not a picture")}), 422, "screenshot"},
{"screenshot-header-only", append(validParts(nil), part{name: "screenshot", filename: "screenshot.jpg", ctype: "image/jpeg", data: []byte{0xFF, 0xD8, 0xFF, 0xE0, 0, 0x10, 'J', 'F', 'I', 'F', 0}}), 422, "screenshot"},
}
h, _, gdb := newHandler(t)
resetSettings(t, gdb, "https://widget.example.test", nil)
for i, c := range cases {
if i > 0 && i%9 == 0 {
// A fresh router per nine posts: the bucket allows ten a minute.
h, _, _ = newHandler(t)
}
rec := do(t, h, multipartRequest(t, submitURL, c.parts, "https://widget.example.test"))
if rec.Code != c.status {
t.Fatalf("%s: status %d want %d: %s", c.name, rec.Code, c.status, rec.Body.String())
}
if c.status == 202 {
if rec.Body.String() != `{"success":true}` {
t.Fatalf("%s: body %s", c.name, rec.Body.String())
}
continue
}
body := decode(t, rec)
details, _ := body["details"].(map[string]any)
if body["error"] != true || body["message"] != "Validation failed" || details[c.field] == nil || len(details) != 1 {
t.Fatalf("%s: body %s", c.name, rec.Body.String())
}
}
})
t.Run("guard-message", func(t *testing.T) {
h, _, gdb := newHandler(t)
resetSettings(t, gdb, "", nil)
spoofed := append([]byte{0xFF, 0xD8, 0xFF, 0xE0, 0, 0x10, 'J', 'F', 'I', 'F', 0}, []byte(strings.Repeat("x", 40))...)
rec := do(t, h, multipartRequest(t, submitURL, append(validParts(nil), part{name: "screenshot", filename: "screenshot.jpg", ctype: "image/jpeg", data: spoofed}), ""))
if rec.Code != 422 || rec.Body.String() != `{"error":true,"message":"Validation failed","details":{"screenshot":["The file is not a valid image."]}}` {
t.Fatalf("spoofed: %d %s", rec.Code, rec.Body.String())
}
})
t.Run("missing-fields-in-rule-order", func(t *testing.T) {
h, _, gdb := newHandler(t)
resetSettings(t, gdb, "", nil)
rec := do(t, h, multipartRequest(t, submitURL, []part{{name: "email", value: "x"}}, ""))
var raw struct {
Details json.RawMessage `json:"details"`
}
if err := json.Unmarshal(rec.Body.Bytes(), &raw); err != nil || rec.Code != 422 {
t.Fatalf("%d %s", rec.Code, rec.Body.String())
}
keys := regexp.MustCompile(`"([a-z_]+)":\[`).FindAllStringSubmatch(string(raw.Details), -1)
var got []string
for _, k := range keys {
got = append(got, k[1])
}
if strings.Join(got, ",") != "message,type,email,page_url" {
t.Fatalf("details keys %v", got)
}
})
t.Run("gate", func(t *testing.T) {
h, _, gdb := newHandler(t)
resetSettings(t, gdb, "https://widget.example.test", nil)
wantError := func(rec *httptest.ResponseRecorder, status int, message string) {
t.Helper()
if rec.Code != status || rec.Body.String() != `{"error":true,"message":"`+message+`"}` {
t.Fatalf("%d %s", rec.Code, rec.Body.String())
}
}
wantError(do(t, h, multipartRequest(t, "/_feedback/api/v1/wk_wrong/submit", validParts(nil), "")), 404, "Not found")
wantError(do(t, h, multipartRequest(t, submitURL, validParts(nil), "https://evil.test")), 403, "Origin not allowed")
resetSettings(t, gdb, "", nil)
wantError(do(t, h, multipartRequest(t, submitURL, validParts(nil), "https://widget.example.test")), 403, "Origin not allowed")
})
t.Run("stored-attached-and-queued", func(t *testing.T) {
h, _, gdb := newHandler(t)
resetSettings(t, gdb, "https://widget.example.test", nil)
jobsBefore := countRows(t, gdb, `SELECT COUNT(*) FROM river_job WHERE kind = ?`, classes.SyncG15OfficeKind)
shot := pngBytes(t, 0)
rec := do(t, h, multipartRequest(t, submitURL, append(validParts(map[string]string{"email": ""}), part{name: "screenshot", filename: "Screen Shot.PNG", ctype: "image/png", data: shot}), "https://widget.example.test"))
if rec.Code != http.StatusAccepted || rec.Body.String() != `{"success":true}` {
t.Fatalf("%d %s", rec.Code, rec.Body.String())
}
var sub models.Submission
if err := gdb.Order("id DESC").Take(&sub).Error; err != nil {
t.Fatal(err)
}
if sub.Status != models.StatusPending || sub.Type != "bug" || sub.Message != "The cover is wrong." || sub.Email == nil || *sub.Email != "" ||
sub.PageURL != "https://widget.example.test/albums/7" || sub.UserAgent == nil || *sub.ConsoleLog != "[error] x is undefined" {
t.Fatalf("submission %+v", sub)
}
file, err := classes.ScreenshotOf(t.Context(), gdb, sub.ID)
if err != nil || file == nil {
t.Fatalf("screenshot %v %v", file, err)
}
if !file.Public() || file.FileName != "Screen Shot.PNG" || file.ContentType != "image/png" || file.FileSize != int64(len(shot)) ||
!strings.HasSuffix(file.DiskName, ".png") || file.SortOrder != int(file.ID) {
t.Fatalf("file %+v", file)
}
var args string
if err := gdb.Raw(`SELECT args::text FROM river_job WHERE kind = ? ORDER BY id DESC LIMIT 1`, classes.SyncG15OfficeKind).Scan(&args).Error; err != nil {
t.Fatal(err)
}
if countRows(t, gdb, `SELECT COUNT(*) FROM river_job WHERE kind = ?`, classes.SyncG15OfficeKind) != jobsBefore+1 ||
!strings.Contains(args, `"submission_id": `+strconv.FormatUint(uint64(sub.ID), 10)) {
t.Fatalf("job args %s", args)
}
var queue string
var maxAttempts int
if err := gdb.Raw(`SELECT queue, max_attempts FROM river_job WHERE kind = ? ORDER BY id DESC LIMIT 1`, classes.SyncG15OfficeKind).Row().Scan(&queue, &maxAttempts); err != nil {
t.Fatal(err)
}
if queue != classes.SyncG15OfficeQueue || maxAttempts != 3 {
t.Fatalf("queue %q attempts %d", queue, maxAttempts)
}
if countRows(t, gdb, `SELECT COUNT(*) FROM summer_jobs WHERE label = ?`, classes.SyncG15OfficeLabel) == 0 {
t.Fatal("no summer_jobs row")
}
})
t.Run("rolled-back-write-queues-nothing", func(t *testing.T) {
_, app, gdb := newHandler(t)
subsBefore := countRows(t, gdb, `SELECT COUNT(*) FROM golem15_feedback_submissions`)
jobsBefore := countRows(t, gdb, `SELECT COUNT(*) FROM river_job WHERE kind = ?`, classes.SyncG15OfficeKind)
recordsBefore := countRows(t, gdb, `SELECT COUNT(*) FROM summer_jobs WHERE label = ?`, classes.SyncG15OfficeLabel)
closed, err := blob.OpenBucket(t.Context(), "mem://")
if err != nil {
t.Fatal(err)
}
_ = closed.Close()
sub := &models.Submission{Message: "x", Type: "bug", PageURL: "https://widget.example.test/"}
err = classes.StoreSubmission(t.Context(), app, gdb, closed, sub, &classes.Screenshot{Filename: "s.png", Data: pngBytes(t, 0)})
if err == nil {
t.Fatal("a failed screenshot write was stored")
}
if countRows(t, gdb, `SELECT COUNT(*) FROM golem15_feedback_submissions`) != subsBefore ||
countRows(t, gdb, `SELECT COUNT(*) FROM river_job WHERE kind = ?`, classes.SyncG15OfficeKind) != jobsBefore ||
countRows(t, gdb, `SELECT COUNT(*) FROM summer_jobs WHERE label = ?`, classes.SyncG15OfficeLabel) != recordsBefore {
t.Fatal("the rolled-back submission left a row or a job behind")
}
})
t.Run("throttle", func(t *testing.T) {
h, _, gdb := newHandler(t)
resetSettings(t, gdb, "", nil)
for i := 1; i <= 11; i++ {
rec := do(t, h, multipartRequest(t, submitURL, []part{{name: "message", value: "x"}}, ""))
if i <= 10 && rec.Code != 422 {
t.Fatalf("post %d: %d", i, rec.Code)
}
if i == 11 && rec.Code != http.StatusTooManyRequests {
t.Fatalf("post 11: %d %s", rec.Code, rec.Body.String())
}
}
})
}
func createUser(t *testing.T, gdb *gorm.DB, email string) *usermodels.User {
t.Helper()
if err := gdb.Exec(`DELETE FROM golem15_feedback_user_preferences WHERE user_id IN (SELECT id FROM users WHERE email = ?)`, email).Error; err != nil {
t.Fatal(err)
}
if err := gdb.Unscoped().Where("email = ?", email).Delete(&usermodels.User{}).Error; err != nil {
t.Fatal(err)
}
u := &usermodels.User{Email: email, IsActivated: true}
if err := gdb.Create(u).Error; err != nil {
t.Fatal(err)
}
return u
}
func bearer(t *testing.T, app *backpack.App, u *usermodels.User) string {
t.Helper()
tok, err := usercontrollers.IssueToken(app, u, httptest.NewRequest(http.MethodGet, "http://localhost/", nil))
if err != nil {
t.Fatal(err)
}
return "Bearer " + tok
}
func putHidden(t *testing.T, h http.Handler, auth, body string) *httptest.ResponseRecorder {
t.Helper()
req := httptest.NewRequest(http.MethodPut, "/_feedback/api/v1/me/hidden", strings.NewReader(body))
req.Header.Set("Content-Type", "application/json")
req.Header.Set("Accept", "application/json")
if auth != "" {
req.Header.Set("Authorization", auth)
}
return do(t, h, req)
}
// TestMeHidden: PUT me/hidden accepts Laravel booleans, answers
// {"hidden":bool}, writes only the caller's row and refuses other values
// and a missing token.
func TestMeHidden(t *testing.T) {
h, app, gdb := newHandler(t)
alice := createUser(t, gdb, "alice-hidden@example.test")
bob := createUser(t, gdb, "bob-hidden@example.test")
aliceAuth := bearer(t, app, alice)
if err := models.SetWidgetHidden(t.Context(), gdb, bob.ID, true); err != nil {
t.Fatal(err)
}
for _, c := range []struct {
body string
want bool
}{
{`{"hidden":true}`, true}, {`{"hidden":false}`, false}, {`{"hidden":"1"}`, true},
{`{"hidden":0}`, false}, {`{"hidden":1}`, true}, {`{"hidden":"0"}`, false},
} {
rec := putHidden(t, h, aliceAuth, c.body)
if rec.Code != http.StatusOK || rec.Body.String() != `{"hidden":`+strconv.FormatBool(c.want)+`}` {
t.Fatalf("%s: %d %s", c.body, rec.Code, rec.Body.String())
}
got, err := models.IsWidgetHidden(t.Context(), gdb, alice.ID)
if err != nil || got != c.want {
t.Fatalf("%s: stored %v (%v)", c.body, got, err)
}
}
if n := countRows(t, gdb, `SELECT COUNT(*) FROM golem15_feedback_user_preferences WHERE user_id = ?`, alice.ID); n != 1 {
t.Fatalf("alice has %d preference rows", n)
}
if hidden, _ := models.IsWidgetHidden(t.Context(), gdb, bob.ID); !hidden {
t.Fatal("bob's preference changed")
}
for _, body := range []string{`{"hidden":"yes"}`, `{"hidden":2}`, `{}`, `{"hidden":null}`, `{"hidden":"true"}`} {
rec := putHidden(t, h, aliceAuth, body)
out := decode(t, rec)
errs, _ := out["errors"].(map[string]any)
if rec.Code != http.StatusUnprocessableEntity || out["error"] != "Validation failed" || errs["hidden"] == nil {
t.Fatalf("%s: %d %s", body, rec.Code, rec.Body.String())
}
}
if rec := putHidden(t, h, "", `{"hidden":true}`); rec.Code != http.StatusUnauthorized {
t.Fatalf("no token: %d %s", rec.Code, rec.Body.String())
}
}
// TestFeedbackApiArrayHook: the user plugin's payload carries
// feedback_widget_hidden, false without a row and true after PUT me/hidden,
// both through GET /_user/api/v1/fetch and the payload builder.
func TestFeedbackApiArrayHook(t *testing.T) {
h, app, gdb := newHandler(t)
u := createUser(t, gdb, "carol-hidden@example.test")
auth := bearer(t, app, u)
fetch := func() any {
t.Helper()
req := httptest.NewRequest(http.MethodGet, "/_user/api/v1/fetch", nil)
req.Header.Set("Authorization", auth)
req.Header.Set("Accept", "application/json")
rec := do(t, h, req)
if rec.Code != http.StatusOK {
t.Fatalf("fetch %d %s", rec.Code, rec.Body.String())
}
return decode(t, rec)["user"].(map[string]any)["feedback_widget_hidden"]
}
if got := fetch(); got != false {
t.Fatalf("without a row: %v", got)
}
if rec := putHidden(t, h, auth, `{"hidden":true}`); rec.Code != http.StatusOK {
t.Fatalf("put %d", rec.Code)
}
if got := fetch(); got != true {
t.Fatalf("after PUT: %v", got)
}
payload, err := usercontrollers.APIArray(t.Context(), app, u)
if err != nil || payload["feedback_widget_hidden"] != true {
t.Fatalf("APIArray %v %v", payload["feedback_widget_hidden"], err)
}
}
// TestFeedbackOptionsPreflight: OPTIONS on the feedback paths is answered
// 204 with an empty body by surf's CORS layer (the plugin registers no
// OPTIONS route), a preflight with the requested method echoed.
func TestFeedbackOptionsPreflight(t *testing.T) {
h, _, gdb := newHandler(t)
resetSettings(t, gdb, "", nil)
pre := httptest.NewRequest(http.MethodOptions, "/_feedback/api/v1/"+testWidgetKey+"/submit", nil)
pre.Header.Set("Origin", "https://widget.example.test")
pre.Header.Set("Access-Control-Request-Method", "POST")
pre.Header.Set("Access-Control-Request-Headers", "content-type")
rec := do(t, h, pre)
if rec.Code != http.StatusNoContent || rec.Body.Len() != 0 {
t.Fatalf("preflight %d %q", rec.Code, rec.Body.String())
}
if rec.Header().Get("Access-Control-Allow-Origin") != "*" || rec.Header().Get("Access-Control-Allow-Methods") != "POST" ||
rec.Header().Get("Access-Control-Allow-Headers") != "content-type" || rec.Header().Get("Cache-Control") != "no-cache, private" {
t.Fatalf("preflight headers %v", rec.Header())
}
for _, path := range []string{"/_feedback/api/v1/anything/else", "/_feedback/api/v1/me/hidden"} {
rec := do(t, h, httptest.NewRequest(http.MethodOptions, path, nil))
if rec.Code != http.StatusNoContent || rec.Body.Len() != 0 || rec.Header().Get("Cache-Control") != "no-cache, private" {
t.Fatalf("OPTIONS %s: %d %q %v", path, rec.Code, rec.Body.String(), rec.Header())
}
}
}