From 92e1b4e09ce3ac30bfdbfe579ceb82ddf44abb72 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Sun, 4 Oct 2026 00:10:15 +0200 Subject: [PATCH] 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 --- README.md | 9 +- classes/classes_test.go | 78 ++++ classes/image_guard.go | 59 +++ classes/submission_store.go | 123 ++++++ classes/sync_g15office.go | 22 ++ controllers/api/feedback_api_controller.go | 116 ++++++ controllers/api/me_hidden_controller.go | 66 ++++ controllers/api/request.go | 146 +++++++ feedback_test.go | 425 ++++++++++++++++++++- go.mod | 2 +- plugin.go | 25 +- routes.go | 8 +- 12 files changed, 1073 insertions(+), 6 deletions(-) create mode 100644 classes/classes_test.go create mode 100644 classes/image_guard.go create mode 100644 classes/submission_store.go create mode 100644 classes/sync_g15office.go create mode 100644 controllers/api/me_hidden_controller.go create mode 100644 controllers/api/request.go diff --git a/README.md b/README.md index 6a84e26..f551341 100644 --- a/README.md +++ b/README.md @@ -20,6 +20,9 @@ The plugin ID, the table names, the config keys and the settings code are a cont - **Widget config.** `GET /_feedback/api/v1/{key}/config` answers the widget's position, whether visitors may hide it, its colors and its labels (`?lang=en` selects the English labels, anything else the Polish ones). A disabled widget, an empty key or a key that differs (compared in constant time) is 404 `{"error":true,"message":"Not found"}`. - **Origin gate.** A request without an `Origin` header passes. An `Origin` passes only when its URL host, lower-cased, is on the allowed-origins list (one entry per line; a line is reduced to its URL host when it has one). An empty list refuses every `Origin` with 403 `{"error":true,"message":"Origin not allowed"}`, so a widget that was never configured cannot be embedded anywhere. +- **Submissions.** `POST /_feedback/api/v1/{key}/submit` (multipart, from the widget) passes the same key and Origin gate, then validates the report as the WinterCMS plugin does: `message` (required, up to 5000 characters), `type` (`bug`, `feature` or `other`), `email` (optional, up to 255), `page_url` (required, up to 2000), `user_agent` (up to 500), `console_log` (up to 20000) and `screenshot` (an optional JPEG, PNG, GIF or WebP of up to 10240 KB). A failure is 422 `{"error":true,"message":"Validation failed","details":{...}}`. A screenshot whose bytes do not decode as one of those images (`classes.IsAllowedImage`) is 422 with `details.screenshot = ["The file is not a valid image."]`. The submission (status `pending`), its public screenshot (attachOne `screenshot`) and its G15Office job are written in one transaction, and the answer is 202 `{"success":true}`: the visitor never waits on G15Office. +- **Hiding the widget.** `PUT /_feedback/api/v1/me/hidden` (behind golem15.user's `jwt.auth`) stores the signed-in user's preference: `hidden` must be a Laravel boolean (`true`, `false`, `1`, `0`, `"1"`, `"0"`), else 422 `{"error":"Validation failed","errors":{...}}`. The answer is `{"hidden":bool}`. Every golem15.user payload (fetch, login, update) carries `feedback_widget_hidden` through a `GetApiArrayEvent` listener: the stored value, false without a row. +- **CORS.** The widget's preflights are answered by the framework's path-scoped CORS layer, so the plugin registers no `OPTIONS` route. - **Rate limits.** The named buckets `feedback-config` (60 a minute) and `feedback-submit` (10 a minute) count per client IP behind the configured trusted proxies. - **Settings.** `models.Settings` is the singleton row of `golem15_feedback_settings`. `models.Instance` creates it on first access with the WinterCMS defaults and a new widget key (`wk_` plus 32 random letters and digits). @@ -60,7 +63,8 @@ The plugin's own `go.mod` replaces the framework with `../../../../summercms.go` ## API reference - `feedback`: `Plugin`, the `golem15.feedback` plugin (config, lang, migrations, models, routes, buckets); `BucketConfig`, `BucketSubmit`. -- `controllers/api`: `Config`, `KeyMatches`, `OriginAllowed`. +- `controllers/api`: `Config`, `Submit`, `SubmitRules`, `MeHidden`, `MeHiddenRules`, `KeyMatches`, `OriginAllowed`. +- `classes`: `IsAllowedImage`, `SniffImageMIME`, `AllowedImageMIMEs`; `Screenshot`, `StoreSubmission`, `ScreenshotOf`; the job contract `SyncG15OfficeArgs`, `SyncG15OfficeKind`, `SyncG15OfficeQueue`, `SyncG15OfficeLabel`, `SyncG15OfficeAttempts`. - `models`: `Settings` (table `golem15_feedback_settings`), `SettingsID`, `SettingsCode`, `DefaultSettings`, `Instance`, `NewWidgetKey`, `Labels`, `URLHost`, `ASCIILower`; `Submission` (table `golem15_feedback_submissions`), `StatusPending`, `StatusSent`, `StatusFailed`, `TypeBug`, `TypeFeature`, `TypeOther`, `ScreenshotField`, `G15OfficeTypeMap`; `UserPreference` (table `golem15_feedback_user_preferences`), `IsWidgetHidden`, `SetWidgetHidden`; `Register`, `All`. - `updates`: the migrations (`202610030201_create_feedback_submissions`, `202610030202_create_feedback_user_preferences`, `202610030203_create_feedback_settings`), `Register`, `All`. @@ -78,7 +82,8 @@ Override them with `SUMMER_GOLEM15__FEEDBACK__G15_OFFICE__BASE_URL`, `SUMMER_GOL ## Dependencies -- The SummerCMS framework, `git.golem15.com/golem15/summercms` (modules `backpack`, `compass`, `lagoon`, `pact`, `party`, `surf`, `wire`). +- The SummerCMS framework, `git.golem15.com/golem15/summercms` (modules `backpack`, `bouncer`, `compass`, `conga`, `lagoon`, `pact`, `party`, `phrasebook`, `surf`, `wire`). +- gocloud.dev blob storage for the screenshots, through the application's uploads bucket. - The user plugin, `git.golem15.com/golem15/sm-user-plugin`. - GORM and gormigrate, as pinned in `go.mod`. - testcontainers-go for the Postgres-backed tests. diff --git a/classes/classes_test.go b/classes/classes_test.go new file mode 100644 index 0000000..63706a1 --- /dev/null +++ b/classes/classes_test.go @@ -0,0 +1,78 @@ +package classes_test + +import ( + "bytes" + "encoding/hex" + "image" + "image/color" + "image/gif" + "image/jpeg" + "image/png" + "testing" + + "git.golem15.com/golem15/sm-feedback-plugin/classes" + "git.golem15.com/golem15/summercms/modules/lagoon/attach" +) + +func mustHex(t *testing.T, s string) []byte { + t.Helper() + b, err := hex.DecodeString(s) + if err != nil { + t.Fatal(err) + } + return b +} + +func encoded(t *testing.T, enc func(*bytes.Buffer, image.Image) error) []byte { + t.Helper() + img := image.NewRGBA(image.Rect(0, 0, 3, 2)) + img.Set(1, 1, color.RGBA{R: 200, A: 255}) + var buf bytes.Buffer + if err := enc(&buf, img); err != nil { + t.Fatal(err) + } + return buf.Bytes() +} + +// TestImageGuardCopy pins the feedback copy of ImageContentGuard: the +// shared vectors (the PHP-recorded screenshot fixtures among them) get the +// verdicts PHP gave and the framework's own guard (attach.IsAllowedImage) +// gives, and the sniffed type of each accepted image is one of the four. +func TestImageGuardCopy(t *testing.T) { + cases := []struct { + name string + data []byte + want bool + }{ + {"png", encoded(t, func(b *bytes.Buffer, i image.Image) error { return png.Encode(b, i) }), true}, + {"jpeg", encoded(t, func(b *bytes.Buffer, i image.Image) error { return jpeg.Encode(b, i, nil) }), true}, + {"gif", encoded(t, func(b *bytes.Buffer, i image.Image) error { return gif.Encode(b, i, nil) }), true}, + // The parity fixtures: a PNG and a WebP PHP accepted, and a JPEG + // header without an image PHP refused ("The file is not a valid + // image."). + {"fixture-png", mustHex(t, "89504e470d0a1a0a0000000d49484452000000080000000808020000004b6d29dc0000001949444154789c626160a8906360c0442c20021b189c128001005509023d9970f43c0000000049454e44ae426082"), true}, + {"fixture-webp", mustHex(t, "524946464800000057454250565038203c0000005003009d012a180010003e6d2c9245a422a19804004006c4a009d3284600fc000108a000feec35853fff497f283d1d7ff3f3396879687ee0b7790000"), true}, + {"fixture-spoofed-jpeg", mustHex(t, "ffd8ffe000104a46494600010100000100010000706f6c79676c6f743a2061204a5045472068656164657220776974686f757420616e7920696d616765ffd9"), false}, + {"text", []byte("plain text, not a picture"), false}, + {"empty", nil, false}, + {"svg", []byte(``), false}, + {"bmp", append([]byte("BM"), make([]byte, 60)...), false}, + {"png-header-only", mustHex(t, "89504e470d0a1a0a"), false}, + } + for _, c := range cases { + got := classes.IsAllowedImage(c.data) + if got != c.want { + t.Errorf("%s: IsAllowedImage = %v, want %v", c.name, got, c.want) + } + if fw := attach.IsAllowedImage(c.data); fw != got { + t.Errorf("%s: the framework guard says %v, the copy %v", c.name, fw, got) + } + if got { + switch classes.SniffImageMIME(c.data) { + case "image/png", "image/jpeg", "image/gif", "image/webp": + default: + t.Errorf("%s: sniffed %q", c.name, classes.SniffImageMIME(c.data)) + } + } + } +} diff --git a/classes/image_guard.go b/classes/image_guard.go new file mode 100644 index 0000000..e2b3aa1 --- /dev/null +++ b/classes/image_guard.go @@ -0,0 +1,59 @@ +// Package classes holds the golem15.feedback services: the screenshot +// content guard, the G15Office client and the job that files a submission +// as a G15Office task. +package classes + +import ( + "bytes" + "image" + "net/http" + + // The decoders image.DecodeConfig needs for the allowed formats; the + // webp decoder is registered by lagoon/attach. + _ "image/gif" + _ "image/jpeg" + _ "image/png" + + _ "git.golem15.com/golem15/summercms/modules/lagoon/attach" +) + +// AllowedImageMIMEs is ImageContentGuard::ALLOWED_IMAGE_MIMES. +var AllowedImageMIMEs = []string{"image/jpeg", "image/png", "image/gif", "image/webp"} + +// SniffImageMIME is the finfo magic-byte sniff of ImageContentGuard: the +// content type the bytes themselves declare, independent of any file name +// or client header. +func SniffImageMIME(data []byte) string { + return http.DetectContentType(data) +} + +// IsAllowedImage ports ImageContentGuard::isAllowedImage: the bytes must +// sniff as JPEG, PNG, GIF or WebP and must decode as that image +// (getimagesize), which rejects a polyglot whose header alone looks right. +// It fails closed: empty or unreadable content is refused. The WinterCMS +// plugins keep one copy of the guard each, and so do their ports. +func IsAllowedImage(data []byte) bool { + if len(data) == 0 { + return false + } + sniffed := SniffImageMIME(data) + allowed := false + for _, m := range AllowedImageMIMEs { + if sniffed == m { + allowed = true + break + } + } + if !allowed { + return false + } + cfg, format, err := image.DecodeConfig(bytes.NewReader(data)) + if err != nil || cfg.Width <= 0 || cfg.Height <= 0 { + return false + } + switch format { + case "jpeg", "png", "gif", "webp": + return true + } + return false +} diff --git a/classes/submission_store.go b/classes/submission_store.go new file mode 100644 index 0000000..321f33a --- /dev/null +++ b/classes/submission_store.go @@ -0,0 +1,123 @@ +package classes + +import ( + "context" + "crypto/rand" + "encoding/hex" + "errors" + "mime" + "path/filepath" + "strconv" + "strings" + + "git.golem15.com/golem15/sm-feedback-plugin/models" + "git.golem15.com/golem15/summercms/modules/backpack" + "git.golem15.com/golem15/summercms/modules/conga" + "git.golem15.com/golem15/summercms/modules/lagoon" + "git.golem15.com/golem15/summercms/modules/lagoon/attach" + "gocloud.dev/blob" + "gorm.io/gorm" +) + +// Screenshot is a validated screenshot upload: the client's file name and +// the bytes, which passed IsAllowedImage. +type Screenshot struct { + Filename string + Data []byte +} + +// StoreSubmission saves a new submission (status pending), queues its +// G15Office job (SyncG15OfficeArgs, a summer_jobs row labelled +// SyncG15OfficeLabel) and, when there is one, stores the screenshot as a +// public file attached to the submission's screenshot relation, all in one +// transaction: when any step fails nothing is kept, no job is queued and +// the screenshot's blob is deleted again. +func StoreSubmission(ctx context.Context, app *backpack.App, gdb *gorm.DB, bucket *blob.Bucket, sub *models.Submission, shot *Screenshot) error { + if gdb == nil { + return errors.New("feedback: database is not configured") + } + if shot != nil && bucket == nil { + return errors.New("feedback: upload bucket is not configured") + } + jobs, err := conga.From(app) + if err != nil { + return err + } + sub.Status = models.StatusPending + var keys []string + err = lagoon.Transaction(ctx, gdb, func(ctx context.Context, tx *gorm.DB) error { + if err := tx.WithContext(ctx).Create(sub).Error; err != nil { + return err + } + if _, err := jobs.Dispatch(ctx, tx.Session(&gorm.Session{NewDB: true, Context: ctx}), SyncG15OfficeArgs{SubmissionID: sub.ID}, conga.DispatchOpts{ + Label: SyncG15OfficeLabel, + Queue: SyncG15OfficeQueue, + MaxAttempts: SyncG15OfficeAttempts, + }); err != nil { + return err + } + if shot == nil { + return nil + } + f, err := storeScreenshot(ctx, tx.Session(&gorm.Session{NewDB: true, Context: ctx}), bucket, sub.ID, shot) + if f != nil { + keys = attach.BlobKeys(*f) + } + return err + }) + if err != nil && len(keys) > 0 { + _ = attach.DeleteKeys(context.WithoutCancel(ctx), bucket, keys) + } + return err +} + +// storeScreenshot writes the screenshot under a fresh Winter-style disk +// name (22 hex characters plus the client file's extension, lower-cased) +// and inserts its system_files row attached to the submission, public and +// sorted by its own id as WinterCMS's Sortable trait does. The returned +// file is set once the blob is written, so a failed row can be cleaned up. +func storeScreenshot(ctx context.Context, db *gorm.DB, bucket *blob.Bucket, submissionID uint, shot *Screenshot) (*attach.File, error) { + raw := make([]byte, 11) + if _, err := rand.Read(raw); err != nil { + return nil, err + } + diskName := hex.EncodeToString(raw) + if ext := strings.ToLower(strings.TrimPrefix(filepath.Ext(shot.Filename), ".")); ext != "" { + diskName += "." + ext + } + contentType := SniffImageMIME(shot.Data) + if mt, _, err := mime.ParseMediaType(contentType); err == nil { + contentType = mt + } + if err := bucket.WriteAll(ctx, attach.BlobKey(diskName), shot.Data, &blob.WriterOptions{ContentType: contentType}); err != nil { + return nil, err + } + public := true + f := &attach.File{ + DiskName: diskName, + FileName: shot.Filename, + FileSize: int64(len(shot.Data)), + ContentType: contentType, + IsPublic: &public, + Field: models.ScreenshotField, + AttachmentType: models.Submission{}.MorphName(), + AttachmentID: strconv.FormatUint(uint64(submissionID), 10), + } + if err := db.Create(f).Error; err != nil { + return f, err + } + f.SortOrder = int(f.ID) + return f, db.Model(&attach.File{}).Where("id = ?", f.ID).Update("sort_order", f.SortOrder).Error +} + +// ScreenshotOf returns the submission's attached screenshot, or nil. +func ScreenshotOf(ctx context.Context, db *gorm.DB, submissionID uint) (*attach.File, error) { + var files []attach.File + err := db.WithContext(ctx). + Where("attachment_type = ? AND attachment_id = ? AND field = ?", models.Submission{}.MorphName(), strconv.FormatUint(uint64(submissionID), 10), models.ScreenshotField). + Order("sort_order, id").Limit(1).Find(&files).Error + if err != nil || len(files) == 0 { + return nil, err + } + return &files[0], nil +} diff --git a/classes/sync_g15office.go b/classes/sync_g15office.go new file mode 100644 index 0000000..8bcf411 --- /dev/null +++ b/classes/sync_g15office.go @@ -0,0 +1,22 @@ +package classes + +// The G15Office sync job's contract: SubmitFeedback queues one per stored +// submission, the worker files it (SyncFeedbackToG15Office). +const ( + // SyncG15OfficeKind is the River kind of the job. + SyncG15OfficeKind = "golem15.feedback.sync_g15office" + // SyncG15OfficeQueue is the job's queue. + SyncG15OfficeQueue = "feedback" + // SyncG15OfficeLabel is the summer_jobs label, the PHP job class. + SyncG15OfficeLabel = `Golem15\Feedback\Jobs\SyncFeedbackToG15Office` + // SyncG15OfficeAttempts is SyncFeedbackToG15Office::$tries. + SyncG15OfficeAttempts = 3 +) + +// SyncG15OfficeArgs is the job's argument: the submission to file. +type SyncG15OfficeArgs struct { + SubmissionID uint `json:"submission_id"` +} + +// Kind is the River job kind. +func (SyncG15OfficeArgs) Kind() string { return SyncG15OfficeKind } diff --git a/controllers/api/feedback_api_controller.go b/controllers/api/feedback_api_controller.go index 2e1961d..57a6780 100644 --- a/controllers/api/feedback_api_controller.go +++ b/controllers/api/feedback_api_controller.go @@ -4,10 +4,16 @@ package api import ( "crypto/subtle" + "io" + "log/slog" "net/http" + "git.golem15.com/golem15/sm-feedback-plugin/classes" "git.golem15.com/golem15/sm-feedback-plugin/models" "git.golem15.com/golem15/summercms/modules/backpack" + "git.golem15.com/golem15/summercms/modules/lagoon" + "git.golem15.com/golem15/summercms/modules/phrasebook" + "gocloud.dev/blob" "gorm.io/gorm" ) @@ -50,6 +56,116 @@ func Config(app *backpack.App) http.HandlerFunc { } } +// SubmitRules are FeedbackApiController::submit's validation rules. +var SubmitRules = []lagoon.RequestRule{ + {Field: "message", Rules: lagoon.ParseRules("required|string|max:5000")}, + {Field: "type", Rules: lagoon.ParseRules("required|in:bug,feature,other")}, + {Field: "email", Rules: lagoon.ParseRules("nullable|email|max:255")}, + {Field: "page_url", Rules: lagoon.ParseRules("required|string|max:2000")}, + {Field: "user_agent", Rules: lagoon.ParseRules("nullable|string|max:500")}, + {Field: "console_log", Rules: lagoon.ParseRules("nullable|string|max:20000")}, + {Field: "screenshot", Rules: lagoon.ParseRules("nullable|image|mimes:jpg,jpeg,png,gif,webp|max:10240")}, +} + +// validationBody is the widget routes' 422 envelope. +type validationBody struct { + Error bool `json:"error"` + Message string `json:"message"` + Details any `json:"details"` +} + +// Submit handles POST /_feedback/api/v1/{key}/submit (multipart, from the +// widget): after the widget gate it validates the report as PHP does (422 +// "Validation failed" with Laravel's errors under details), refuses a +// screenshot whose bytes are not an image, stores the submission with its +// public screenshot and queues the G15Office job in one transaction, and +// answers 202 {"success":true}. The visitor never waits on G15Office. +func Submit(app *backpack.App) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + if _, ok := gate(w, r, app); !ok { + return + } + input, ok := readInput(w, r) + if !ok { + return + } + ctx := r.Context() + gdb := database(app) + tr, _ := app.Lookup[*phrasebook.Translator]() + errs, err := lagoon.ValidateRequest(ctx, gdb.WithContext(ctx), input, SubmitRules, tr) + if err != nil { + writeOpaque500(w) + return + } + if len(errs) > 0 { + writeJSON(w, http.StatusUnprocessableEntity, validationBody{ + Error: true, Message: "Validation failed", + Details: orderedErrors{errs: errs, order: lagoon.ErrorKeys(errs, SubmitRules)}, + }) + return + } + var shot *classes.Screenshot + if upload, isFile := input["screenshot"].(lagoon.UploadedFile); isFile && upload.Open != nil { + data, err := readUpload(upload) + if err != nil { + writeOpaque500(w) + return + } + if !classes.IsAllowedImage(data) { + writeJSON(w, http.StatusUnprocessableEntity, validationBody{ + Error: true, Message: "Validation failed", + Details: map[string][]string{"screenshot": {"The file is not a valid image."}}, + }) + return + } + shot = &classes.Screenshot{Filename: upload.Filename, Data: data} + } + var bucket *blob.Bucket + if shot != nil { + if bucket, ok = app.Lookup[*blob.Bucket](); !ok || bucket == nil { + slog.Default().Error("golem15.feedback: upload bucket is not configured") + writeOpaque500(w) + return + } + } + sub := &models.Submission{ + Message: stringOr(inputString(input, "message")), + Type: stringOr(inputString(input, "type")), + Email: inputString(input, "email"), + PageURL: stringOr(inputString(input, "page_url")), + UserAgent: inputString(input, "user_agent"), + ConsoleLog: inputString(input, "console_log"), + } + if err := classes.StoreSubmission(ctx, app, gdb, bucket, sub, shot); err != nil { + slog.Default().Error("golem15.feedback: storing a submission failed", slog.Any("error", err)) + writeOpaque500(w) + return + } + writeJSON(w, http.StatusAccepted, successBody{Success: true}) + } +} + +type successBody struct { + Success bool `json:"success"` +} + +func stringOr(s *string) string { + if s == nil { + return "" + } + return *s +} + +// readUpload reads an uploaded file; the max:10240 rule already bounded it. +func readUpload(upload lagoon.UploadedFile) ([]byte, error) { + rc, err := upload.Open() + if err != nil { + return nil, err + } + defer rc.Close() + return io.ReadAll(rc) +} + // gate loads the settings (creating them on first access) and runs the // widget checks of both public routes in PHP's order: the key (404 // "Not found") then the Origin (403 "Origin not allowed"). diff --git a/controllers/api/me_hidden_controller.go b/controllers/api/me_hidden_controller.go new file mode 100644 index 0000000..9094ae8 --- /dev/null +++ b/controllers/api/me_hidden_controller.go @@ -0,0 +1,66 @@ +package api + +import ( + "net/http" + + "git.golem15.com/golem15/sm-feedback-plugin/models" + "git.golem15.com/golem15/summercms/modules/backpack" + "git.golem15.com/golem15/summercms/modules/bouncer" + "git.golem15.com/golem15/summercms/modules/lagoon" + "git.golem15.com/golem15/summercms/modules/phrasebook" +) + +// MeHiddenRules are MeHiddenController::update's validation rules. +var MeHiddenRules = []lagoon.RequestRule{ + {Field: "hidden", Rules: lagoon.ParseRules("required|boolean")}, +} + +type meHiddenErrors struct { + Error string `json:"error"` + Errors any `json:"errors"` +} + +type meHiddenBody struct { + Hidden bool `json:"hidden"` +} + +// MeHidden handles PUT /_feedback/api/v1/me/hidden (jwt.auth): the signed-in +// user's "hide the widget" preference. hidden must be a Laravel boolean +// (true, false, 1, 0, "1", "0"); anything else is 422 {"error":"Validation +// failed","errors":...}. Only the JWT principal's row is written, and the +// answer is {"hidden":bool}. The value also rides on every user payload +// (feedback_widget_hidden), so there is no read route. +func MeHidden(app *backpack.App) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + gdb := database(app) + user, ok := bouncer.User(r.Context()) + if gdb == nil || !ok || user == nil || user.ID == 0 { + writeOpaque500(w) + return + } + input, ok := readInput(w, r) + if !ok { + return + } + ctx := r.Context() + tr, _ := app.Lookup[*phrasebook.Translator]() + errs, err := lagoon.ValidateRequest(ctx, gdb.WithContext(ctx), input, MeHiddenRules, tr) + if err != nil { + writeOpaque500(w) + return + } + if len(errs) > 0 { + writeJSON(w, http.StatusUnprocessableEntity, meHiddenErrors{ + Error: "Validation failed", + Errors: orderedErrors{errs: errs, order: lagoon.ErrorKeys(errs, MeHiddenRules)}, + }) + return + } + hidden := laravelTrue(input["hidden"]) + if err := models.SetWidgetHidden(ctx, gdb, user.ID, hidden); err != nil { + writeOpaque500(w) + return + } + writeJSON(w, http.StatusOK, meHiddenBody{Hidden: hidden}) + } +} diff --git a/controllers/api/request.go b/controllers/api/request.go new file mode 100644 index 0000000..ab3b21d --- /dev/null +++ b/controllers/api/request.go @@ -0,0 +1,146 @@ +package api + +import ( + "encoding/json" + "errors" + "io" + "mime" + "net/http" + "net/url" + "strconv" + "strings" + + "git.golem15.com/golem15/summercms/modules/lagoon" +) + +// errBodyTooLarge is a body over the router's cap. +var errBodyTooLarge = errors.New("feedback: request body too large") + +// multipartMemory is how much of a multipart body is kept in memory; the +// rest spills to temporary files. The router's body cap bounds the total. +const multipartMemory = 32 << 20 + +// decodeInput is Laravel's $request->all(): the query string overlaid by +// the body. A JSON body decodes with numbers as json.Number; a form or +// multipart body gives strings (the last value of a repeated key) and +// lagoon.UploadedFile values for files. An unreadable body is an empty bag. +func decodeInput(r *http.Request) (map[string]any, error) { + out := map[string]any{} + mergeValues(out, r.URL.Query()) + if r.Body == nil || r.Method == http.MethodGet || r.Method == http.MethodHead { + return out, nil + } + mediaType, _, _ := mime.ParseMediaType(r.Header.Get("Content-Type")) + switch mediaType { + case "multipart/form-data": + if err := r.ParseMultipartForm(multipartMemory); err != nil { + if isBodyTooLarge(err) { + return nil, errBodyTooLarge + } + return out, nil + } + if r.MultipartForm != nil { + mergeValues(out, r.MultipartForm.Value) + for key, headers := range r.MultipartForm.File { + if len(headers) > 0 { + out[key] = lagoon.UploadedFileFromHeader(headers[len(headers)-1]) + } + } + } + return out, nil + case "application/x-www-form-urlencoded": + if err := r.ParseForm(); err != nil { + if isBodyTooLarge(err) { + return nil, errBodyTooLarge + } + return out, nil + } + mergeValues(out, r.PostForm) + return out, nil + default: + raw, err := io.ReadAll(r.Body) + if err != nil { + if isBodyTooLarge(err) { + return nil, errBodyTooLarge + } + return out, nil + } + if strings.TrimSpace(string(raw)) == "" { + return out, nil + } + dec := json.NewDecoder(strings.NewReader(string(raw))) + dec.UseNumber() + var body map[string]any + if err := dec.Decode(&body); err != nil { + // Laravel's json() of an invalid body is an empty bag. + return out, nil + } + for k, v := range body { + out[k] = v + } + return out, nil + } +} + +func mergeValues(out map[string]any, values url.Values) { + for key, vals := range values { + if len(vals) > 0 { + out[key] = vals[len(vals)-1] + } + } +} + +func isBodyTooLarge(err error) bool { + var mbe *http.MaxBytesError + if errors.As(err, &mbe) { + return true + } + // mime/multipart reports the reader error as text, not wrapped. + return err != nil && strings.Contains(err.Error(), "request body too large") +} + +// readInput decodes the request; a body over the router's cap is a 413. +func readInput(w http.ResponseWriter, r *http.Request) (map[string]any, bool) { + input, err := decodeInput(r) + if errors.Is(err, errBodyTooLarge) { + writeJSON(w, http.StatusRequestEntityTooLarge, map[string]any{"error": "Payload too large"}) + return nil, false + } + if err != nil { + input = map[string]any{} + } + return input, true +} + +// inputString is $request->input($key) for a validated string field: nil +// when absent or null, else the string as sent. +func inputString(input map[string]any, key string) *string { + switch v := input[key].(type) { + case string: + return &v + case json.Number: + s := v.String() + return &s + } + return nil +} + +// laravelTrue is (bool) of a value the boolean rule accepted (true, false, +// 1, 0, "1", "0"): true for true, 1 and "1". +func laravelTrue(v any) bool { + switch t := v.(type) { + case bool: + return t + case json.Number: + f, err := t.Float64() + return err == nil && f == 1 + case float64: + return t == 1 + case int: + return t == 1 + case string: + n, err := strconv.ParseFloat(strings.TrimSpace(t), 64) + return err == nil && n == 1 + } + return false +} diff --git a/feedback_test.go b/feedback_test.go index 7b3f720..f1fe592 100644 --- a/feedback_test.go +++ b/feedback_test.go @@ -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()) + } + } +} diff --git a/go.mod b/go.mod index 5526d40..68e90c1 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( git.golem15.com/golem15/summercms v0.0.0 github.com/go-gormigrate/gormigrate/v2 v2.1.7 github.com/testcontainers/testcontainers-go/modules/postgres v0.44.0 + gocloud.dev v0.46.0 gorm.io/gorm v1.31.2 ) @@ -102,7 +103,6 @@ require ( go.opentelemetry.io/otel/sdk/metric v1.44.0 // indirect go.opentelemetry.io/otel/trace v1.44.0 // indirect go.yaml.in/yaml/v3 v3.0.5 // indirect - gocloud.dev v0.46.0 // indirect golang.org/x/crypto v0.57.0 // indirect golang.org/x/image v0.46.0 // indirect golang.org/x/net v0.58.0 // indirect diff --git a/plugin.go b/plugin.go index b99c02d..7db2269 100644 --- a/plugin.go +++ b/plugin.go @@ -7,18 +7,22 @@ package feedback import ( + "context" "embed" + "errors" "io/fs" "net/http" "time" "git.golem15.com/golem15/sm-feedback-plugin/models" "git.golem15.com/golem15/sm-feedback-plugin/updates" + userclasses "git.golem15.com/golem15/sm-user-plugin/classes" "git.golem15.com/golem15/summercms/modules/backpack" "git.golem15.com/golem15/summercms/modules/pact" "git.golem15.com/golem15/summercms/modules/party" "git.golem15.com/golem15/summercms/modules/surf" "github.com/go-gormigrate/gormigrate/v2" + "gorm.io/gorm" ) var ( @@ -61,9 +65,28 @@ func (p *Plugin) Register(app *backpack.App) error { return nil } -// Boot keeps the application. +// Boot keeps the application and adds feedback_widget_hidden to every +// golem15.user payload (fetch, login, update...): the user's preference, +// false without a row. The user plugin itself is never changed. func (p *Plugin) Boot(app *backpack.App) error { p.app = app + if app != nil && app.Events != nil { + app.Events.Listen[*userclasses.GetApiArrayEvent]("golem15.feedback", func(ctx context.Context, e *userclasses.GetApiArrayEvent) error { + if e == nil || e.User == nil { + return nil + } + gdb, ok := app.Lookup[*gorm.DB]() + if !ok || gdb == nil { + return errors.New("feedback: user payload listener has no database") + } + hidden, err := models.IsWidgetHidden(ctx, gdb, e.User.ID) + if err != nil { + return err + } + e.Collected()["feedback_widget_hidden"] = hidden + return nil + }) + } return nil } diff --git a/routes.go b/routes.go index a91c7c0..d28b8b6 100644 --- a/routes.go +++ b/routes.go @@ -3,16 +3,22 @@ package feedback import ( "git.golem15.com/golem15/sm-feedback-plugin/controllers/api" "git.golem15.com/golem15/summercms/modules/pact" + "git.golem15.com/golem15/summercms/modules/surf" ) // Routes mounts the widget API. The public group has no auth: the widget // key in the path and the Origin allow-list are its gate, backed by the // per-IP buckets. CORS preflights (OPTIONS) on _feedback/api/* are answered // by surf's path-scoped CORS layer, which the application's http.cors.paths -// must list, so no OPTIONS route is registered. +// must list, so no OPTIONS route is registered. A second group on the same +// prefix carries the per-user preference behind golem15.user's jwt.auth. func (p *Plugin) Routes(r pact.Router) error { r.Group("/_feedback/api/v1", nil, func(g pact.Router) { g.Get("/{key}/config", api.Config(p.app), "throttle:"+BucketConfig) + g.Post("/{key}/submit", api.Submit(p.app), "throttle:"+BucketSubmit) + }) + r.Group("/_feedback/api/v1", surf.Use("jwt.auth"), func(g pact.Router) { + g.Put("/me/hidden", api.MeHidden(p.app)) }) return nil }