fix(10.1): CR-01 answer a value that does not fit its column with a 422
lagoon.Fill now returns a *lagoon.FillTypeError naming the key when a requested value cannot be stored in its column (a fraction, exponent or overflow for an integer field, or a value of the wrong type). The admin save path maps it to a validation_failed 422 on that field instead of a 500 CapabilityError; genuine capability failures keep the 500.
This commit is contained in:
@@ -12,7 +12,7 @@ Schema-driven admin backend that compiles WinterCMS-style YAML list, form, filte
|
|||||||
|
|
||||||
- Boot-time schema compilation: `cabana.CompileList` and `cabana.CompileForm` read a controller's YAML from the plugin's embedded tree, check that `modelClass` matches the controller's model name, and cache a locale-neutral schema. Each request gets a translated copy (`cabana.ListSchema.Localize`, `cabana.FormSchema.Localize`, `cabana.RelationSchema.Localize`) through [phrasebook](../phrasebook/README.md), with CLDR plural forms for the SPA's messages.
|
- Boot-time schema compilation: `cabana.CompileList` and `cabana.CompileForm` read a controller's YAML from the plugin's embedded tree, check that `modelClass` matches the controller's model name, and cache a locale-neutral schema. Each request gets a translated copy (`cabana.ListSchema.Localize`, `cabana.FormSchema.Localize`, `cabana.RelationSchema.Localize`) through [phrasebook](../phrasebook/README.md), with CLDR plural forms for the SPA's messages.
|
||||||
- Generic CRUD with `cabana.CRUDService`: list, show, create, update, delete and bulk delete. Writes run in transactions, and reads and writes are scoped by the controller's `pact.ListExtendQuery` and `pact.FormExtendQuery` hooks. `cabana.ExecuteList` applies search, sort, filters and pagination only on columns declared in the schema, so request parameters never reach SQL directly.
|
- Generic CRUD with `cabana.CRUDService`: list, show, create, update, delete and bulk delete. Writes run in transactions, and reads and writes are scoped by the controller's `pact.ListExtendQuery` and `pact.FormExtendQuery` hooks. `cabana.ExecuteList` applies search, sort, filters and pagination only on columns declared in the schema, so request parameters never reach SQL directly.
|
||||||
- Mass-assignment protection: writable form fields are bound to model columns at activation (`cabana.BindWritableFields`), and `cabana.ProjectWritableFields` drops unknown keys, case variants, nested objects and protected columns from request bodies. Values are filled and validated through [lagoon](../lagoon/README.md), and the form lifecycle hooks declared in `pact` (before and after create, update and delete) run around each write.
|
- Mass-assignment protection: writable form fields are bound to model columns at activation (`cabana.BindWritableFields`), and `cabana.ProjectWritableFields` drops unknown keys, case variants, nested objects and protected columns from request bodies. Values are filled and validated through [lagoon](../lagoon/README.md); a value that does not fit its column (a `lagoon.FillTypeError`, such as a fraction for an integer field) is a 422 `validation_failed` on that field, and the form lifecycle hooks declared in `pact` (before and after create, update and delete) run around each write.
|
||||||
- Relations: `type: relation` form fields for belongsTo and belongsToMany (`cabana.FieldRelationProvider`, `cabana.FieldRelationContract`) with a paginated options endpoint and display labels in every record response; relation managers (`cabana.AdminRelationContractProvider`, `cabana.RelationContract`) served by `cabana.RelationService` for listing linked records and candidates and for linking and unlinking. Framework code never guesses table, pivot or foreign-key names: the controller supplies them.
|
- Relations: `type: relation` form fields for belongsTo and belongsToMany (`cabana.FieldRelationProvider`, `cabana.FieldRelationContract`) with a paginated options endpoint and display labels in every record response; relation managers (`cabana.AdminRelationContractProvider`, `cabana.RelationContract`) served by `cabana.RelationService` for listing linked records and candidates and for linking and unlinking. Framework code never guesses table, pivot or foreign-key names: the controller supplies them.
|
||||||
- Form widgets and controller actions: a `type: widget` field in `fields.yaml` names a plugin custom element (`widget:`, which must start with the owning plugin's `{vendor}-{plugin}-` prefix), the controller action it runs (`action:`, registered through `pact.HasAdminActions`) and the writable scalar fields of the same form the action may write back (`fill:`). The admin SPA posts the action to a cabana-owned route, so the CSRF check, permissions (the controller's plus the action's own) and record scoping (`pact.FormExtendQuery`) never depend on plugin code; the response carries only the declared fill keys with scalar values. Unknown keys, a foreign or invalid tag, an unregistered action or a fill key that is not a writable scalar field fail boot.
|
- Form widgets and controller actions: a `type: widget` field in `fields.yaml` names a plugin custom element (`widget:`, which must start with the owning plugin's `{vendor}-{plugin}-` prefix), the controller action it runs (`action:`, registered through `pact.HasAdminActions`) and the writable scalar fields of the same form the action may write back (`fill:`). The admin SPA posts the action to a cabana-owned route, so the CSRF check, permissions (the controller's plus the action's own) and record scoping (`pact.FormExtendQuery`) never depend on plugin code; the response carries only the declared fill keys with scalar values. Unknown keys, a foreign or invalid tag, an unregistered action or a fill key that is not a writable scalar field fail boot.
|
||||||
- Controller assets: a controller implementing `pact.AdminClientAssets` names JS (`.js`, `.mjs`) and CSS files under its plugin's `assets/` directory, Winter's `addJs`/`addCss`. They are read from the plugin's embedded tree at boot (a missing file fails boot; there is no disk override) and listed in the list and form schemas under `assets` as same-origin URLs with a `?v=` content hash. A form with a widget needs at least one JS file.
|
- Controller assets: a controller implementing `pact.AdminClientAssets` names JS (`.js`, `.mjs`) and CSS files under its plugin's `assets/` directory, Winter's `addJs`/`addCss`. They are read from the plugin's embedded tree at boot (a missing file fails boot; there is no disk override) and listed in the list and form schemas under `assets` as same-origin URLs with a `?v=` content hash. A form with a widget needs at least one JS file.
|
||||||
|
|||||||
@@ -325,6 +325,12 @@ func (s CRUDService) save(ctx context.Context, cc *CompiledController, id any, i
|
|||||||
}
|
}
|
||||||
projected := projectOperation(cc, in.Body, op)
|
projected := projectOperation(cc, in.Body, op)
|
||||||
if err := lagoon.Fill(target, fillAllowed(cc, target, op), projected, false); err != nil {
|
if err := lagoon.Fill(target, fillAllowed(cc, target, op), projected, false); err != nil {
|
||||||
|
// A value that does not fit its column is the admin's input,
|
||||||
|
// not a missing capability: answer it on the field.
|
||||||
|
var typed *lagoon.FillTypeError
|
||||||
|
if errors.As(err, &typed) {
|
||||||
|
return &ValidationError{Details: fillTypeDetails(typed.Key)}
|
||||||
|
}
|
||||||
return &CapabilityError{ControllerID: controllerID(cc)}
|
return &CapabilityError{ControllerID: controllerID(cc)}
|
||||||
}
|
}
|
||||||
if hook, ok := target.(lagoon.HasBeforeValidate); ok && hook != nil {
|
if hook, ok := target.(lagoon.HasBeforeValidate); ok && hook != nil {
|
||||||
@@ -737,6 +743,12 @@ func valuesForRules(model any, rules map[string]string) map[string]any {
|
|||||||
return out
|
return out
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// fillTypeDetails is the 422 detail for a value lagoon.Fill could not store
|
||||||
|
// in its column. Writable fill keys equal their form field names.
|
||||||
|
func fillTypeDetails(key string) map[string]any {
|
||||||
|
return map[string]any{key: []string{"The " + key + " field has an invalid value."}}
|
||||||
|
}
|
||||||
|
|
||||||
func validationDetails(msgs map[string][]string) map[string]any {
|
func validationDetails(msgs map[string][]string) map[string]any {
|
||||||
out := make(map[string]any, len(msgs))
|
out := make(map[string]any, len(msgs))
|
||||||
for key, messages := range msgs {
|
for key, messages := range msgs {
|
||||||
|
|||||||
@@ -469,3 +469,86 @@ func crudBody(t *testing.T, raw string) map[string]any {
|
|||||||
}
|
}
|
||||||
return body
|
return body
|
||||||
}
|
}
|
||||||
|
|
||||||
|
type crudYearRow struct {
|
||||||
|
ID uint `gorm:"column:id;primaryKey"`
|
||||||
|
Name string `gorm:"column:name"`
|
||||||
|
Year *int `gorm:"column:year"`
|
||||||
|
}
|
||||||
|
|
||||||
|
func (crudYearRow) TableName() string { return "cabana_crud_year_rows" }
|
||||||
|
func (crudYearRow) Fillable() []string { return []string{"name", "year"} }
|
||||||
|
func (crudYearRow) Rules() map[string]string { return map[string]string{"name": "required"} }
|
||||||
|
|
||||||
|
// TestCRUDFillTypeIsValidation covers CR-01: a value lagoon.Fill cannot store
|
||||||
|
// in its column (a fraction, an exponent or an overflow for an integer field,
|
||||||
|
// or a value of the wrong type) is the admin's input, so create and update
|
||||||
|
// answer 422 with a message on that field rather than a 500 capability error.
|
||||||
|
func TestCRUDFillTypeIsValidation(t *testing.T) {
|
||||||
|
_, db := newListService(t)
|
||||||
|
if err := db.Migrator().DropTable(&crudYearRow{}); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if err := db.AutoMigrate(&crudYearRow{}); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
fsys := crudFS()
|
||||||
|
fsys["models/record/fields.yaml"] = &fstest.MapFile{Data: []byte(`fields:
|
||||||
|
name:
|
||||||
|
label: Name
|
||||||
|
type: text
|
||||||
|
required: true
|
||||||
|
year:
|
||||||
|
label: Year
|
||||||
|
type: number
|
||||||
|
`)}
|
||||||
|
reg, err := compileRegistry([]controllerRef{{
|
||||||
|
plugin: formPlugin{fsys: fsys},
|
||||||
|
ctl: crudController{rec: func() any { return &crudYearRow{} }},
|
||||||
|
}})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("registry: %v", err)
|
||||||
|
}
|
||||||
|
cc, ok := reg.Get("acme.demo.records")
|
||||||
|
if !ok {
|
||||||
|
t.Fatal("compiled controller missing")
|
||||||
|
}
|
||||||
|
svc := CRUDService{DB: db}
|
||||||
|
ctx := context.Background()
|
||||||
|
count := func() int64 {
|
||||||
|
t.Helper()
|
||||||
|
var n int64
|
||||||
|
if err := db.Model(&crudYearRow{}).Count(&n).Error; err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
return n
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, raw := range []string{`{"name":"Ada","year":1977.5}`, `{"name":"Ada","year":1e21}`, `{"name":"Ada","year":99999999999999999999}`, `{"name":"Ada","year":"1977"}`} {
|
||||||
|
_, err := svc.Create(ctx, cc, RecordInput{Body: crudBody(t, raw)})
|
||||||
|
assertValidation(t, err, "year", "The year field has an invalid value.")
|
||||||
|
if n := count(); n != 0 {
|
||||||
|
t.Fatalf("%s: rows=%d, an invalid year persisted", raw, n)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
_, err = svc.Create(ctx, cc, RecordInput{Body: crudBody(t, `{"name":true}`)})
|
||||||
|
assertValidation(t, err, "name", "The name field has an invalid value.")
|
||||||
|
|
||||||
|
rec, err := svc.Create(ctx, cc, RecordInput{Body: crudBody(t, `{"name":"Ada","year":1977}`)})
|
||||||
|
if err != nil || rec["year"] == nil {
|
||||||
|
t.Fatalf("create=%#v err=%v, want year 1977", rec, err)
|
||||||
|
}
|
||||||
|
var stored crudYearRow
|
||||||
|
if err := db.Where("name = ?", "Ada").Take(&stored).Error; err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
_, err = svc.Update(ctx, cc, stored.ID, RecordInput{Body: crudBody(t, `{"year":1977.5}`)})
|
||||||
|
assertValidation(t, err, "year", "The year field has an invalid value.")
|
||||||
|
var after crudYearRow
|
||||||
|
if err := db.Take(&after, stored.ID).Error; err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if after.Year == nil || *after.Year != 1977 {
|
||||||
|
t.Fatalf("year after rejected update = %v, want 1977", after.Year)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -15,7 +15,7 @@ Postgres data layer: the shared GORM connection, per-plugin migrations, model he
|
|||||||
- One shared pool: `lagoon.Open`, `lagoon.Use` and `lagoon.OpenFromApp` return a `*sql.DB` and a `*gorm.DB` built on that same pool; `lagoon.Publish` makes both available on the `backpack.App`.
|
- One shared pool: `lagoon.Open`, `lagoon.Use` and `lagoon.OpenFromApp` return a `*sql.DB` and a `*gorm.DB` built on that same pool; `lagoon.Publish` makes both available on the `backpack.App`.
|
||||||
- Database check at connect time: `lagoon.CheckLocale` refuses a database whose default collation is not the ICU `pl-PL` locale, so ordering matches the database default without per-query `COLLATE`.
|
- Database check at connect time: `lagoon.CheckLocale` refuses a database whose default collation is not the ICU `pl-PL` locale, so ordering matches the database default without per-query `COLLATE`.
|
||||||
- Per-plugin migrations: `lagoon.Migrate` runs the framework's `system_files` set (`attach.Migrations`) and backend admin identity set (`lagoon.BackendAdminMigrations`), then every `pact.HasMigrations` set in plugin activation order, each in its own `summer_migrations_<plugin_id>` history table (`lagoon.HistoryTableName`). `lagoon.RollbackLast` and `lagoon.Status` cover rollback and history.
|
- Per-plugin migrations: `lagoon.Migrate` runs the framework's `system_files` set (`attach.Migrations`) and backend admin identity set (`lagoon.BackendAdminMigrations`), then every `pact.HasMigrations` set in plugin activation order, each in its own `summer_migrations_<plugin_id>` history table (`lagoon.HistoryTableName`). `lagoon.RollbackLast` and `lagoon.Status` cover rollback and history.
|
||||||
- Mass assignment: `lagoon.Fill` copies only allow-listed keys onto a model by GORM column name and silently drops the rest, logging each dropped key once outside production. A `json.Number` (from a decoder using `UseNumber`) fills integer, unsigned and float fields, and a fraction or an overflow is an error. `lagoon.HasFillable` and `lagoon.HasHidden` are the Go forms of `$fillable` and `$hidden`.
|
- Mass assignment: `lagoon.Fill` copies only allow-listed keys onto a model by GORM column name and silently drops the rest, logging each dropped key once outside production. A `json.Number` (from a decoder using `UseNumber`) fills integer, unsigned and float fields. A value that does not fit its column (a fraction, an exponent or an overflow for an integer field, or a value of the wrong type) is a `lagoon.FillTypeError` naming the key, so a caller can answer it as a validation failure on that field. `lagoon.HasFillable` and `lagoon.HasHidden` are the Go forms of `$fillable` and `$hidden`.
|
||||||
- Validation: `lagoon.Validate` accepts Laravel-style rule strings (`required`, `nullable`, `integer`, `numeric`, `between`, `min`, `max`, `in`, `unique`, `boolean`, `email`, `confirmed`, `different`, `mimes`) and returns a field-to-messages map, translated through phrasebook when a translator is given. Unknown rule tokens are an error.
|
- Validation: `lagoon.Validate` accepts Laravel-style rule strings (`required`, `nullable`, `integer`, `numeric`, `between`, `min`, `max`, `in`, `unique`, `boolean`, `email`, `confirmed`, `different`, `mimes`) and returns a field-to-messages map, translated through phrasebook when a translator is given. Unknown rule tokens are an error.
|
||||||
- Safe ordering: `lagoon.OrderBy` appends an ORDER BY only for an allow-listed column and an `asc` or `desc` direction.
|
- Safe ordering: `lagoon.OrderBy` appends an ORDER BY only for an allow-listed column and an `asc` or `desc` direction.
|
||||||
- Pagination: `lagoon.Paginate` builds a `lagoon.Page` with `data` and `meta` (`current_page`, `last_page`, `per_page`, `total`).
|
- Pagination: `lagoon.Paginate` builds a `lagoon.Page` with `data` and `meta` (`current_page`, `last_page`, `per_page`, `total`).
|
||||||
@@ -102,6 +102,7 @@ func (p *Plugin) Migrations() []*gormigrate.Migration {
|
|||||||
| `lagoon.Encrypted` | Encrypted text column; `lagoon.Encrypted.Reveal` is the only plaintext accessor. |
|
| `lagoon.Encrypted` | Encrypted text column; `lagoon.Encrypted.Reveal` is the only plaintext accessor. |
|
||||||
| `lagoon.Jsonable` | Generic JSON-as-TEXT column with NULL tracking. |
|
| `lagoon.Jsonable` | Generic JSON-as-TEXT column with NULL tracking. |
|
||||||
| `lagoon.Fill` | Allow-listed mass assignment by column name. |
|
| `lagoon.Fill` | Allow-listed mass assignment by column name. |
|
||||||
|
| `lagoon.FillTypeError` | Returned by `lagoon.Fill` when a requested value does not fit its column; `Key` names the column. |
|
||||||
| `lagoon.Validate` | Laravel-style rule validation with a `unique` database check. |
|
| `lagoon.Validate` | Laravel-style rule validation with a `unique` database check. |
|
||||||
| `lagoon.OrderBy` | Allow-listed ORDER BY. |
|
| `lagoon.OrderBy` | Allow-listed ORDER BY. |
|
||||||
| `lagoon.Paginate` | Builds a `lagoon.Page` with `lagoon.PageMeta`. |
|
| `lagoon.Paginate` | Builds a `lagoon.Page` with `lagoon.PageMeta`. |
|
||||||
|
|||||||
@@ -25,9 +25,25 @@ type HasHidden interface {
|
|||||||
|
|
||||||
var droppedKeys sync.Map
|
var droppedKeys sync.Map
|
||||||
|
|
||||||
|
// FillTypeError reports a requested value that cannot be stored in its
|
||||||
|
// column: a fraction, an exponent or an overflow for an integer field, or a
|
||||||
|
// value of the wrong type. Key is the column the value was requested for, so
|
||||||
|
// a caller can answer it as a validation failure on that field. Fill returns
|
||||||
|
// any other failure, such as a model that is not a struct pointer, as a
|
||||||
|
// plain error.
|
||||||
|
type FillTypeError struct {
|
||||||
|
Key string
|
||||||
|
Err error
|
||||||
|
}
|
||||||
|
|
||||||
|
func (e *FillTypeError) Error() string { return "lagoon: fill " + e.Key + ": " + e.Err.Error() }
|
||||||
|
|
||||||
|
func (e *FillTypeError) Unwrap() error { return e.Err }
|
||||||
|
|
||||||
// Fill copies requested keys onto model only when they are also in allowed.
|
// Fill copies requested keys onto model only when they are also in allowed.
|
||||||
// Unknown and non-fillable keys are dropped with no error (D-06). In
|
// Unknown and non-fillable keys are dropped with no error (D-06). In
|
||||||
// non-production, each type+key pair is logged once.
|
// non-production, each type+key pair is logged once. A value that does not
|
||||||
|
// fit its column is a *FillTypeError.
|
||||||
func Fill(model any, allowed []string, requested map[string]any, production bool) error {
|
func Fill(model any, allowed []string, requested map[string]any, production bool) error {
|
||||||
if model == nil {
|
if model == nil {
|
||||||
return fmt.Errorf("lagoon: fill model is nil")
|
return fmt.Errorf("lagoon: fill model is nil")
|
||||||
@@ -53,7 +69,7 @@ func Fill(model any, allowed []string, requested map[string]any, production bool
|
|||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
if err := setField(field, val); err != nil {
|
if err := setField(field, val); err != nil {
|
||||||
return fmt.Errorf("lagoon: fill %s: %w", key, err)
|
return &FillTypeError{Key: key, Err: err}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
return nil
|
return nil
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ package lagoon
|
|||||||
import (
|
import (
|
||||||
"bytes"
|
"bytes"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
|
"errors"
|
||||||
"log/slog"
|
"log/slog"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
@@ -193,3 +194,38 @@ func TestFillJSONNumber(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestFillTypeErrorNamesTheKey proves a value that does not fit its column
|
||||||
|
// comes back as a *FillTypeError carrying the requested key, so a caller can
|
||||||
|
// answer it as a validation failure on that field, while a bad model stays a
|
||||||
|
// plain error.
|
||||||
|
func TestFillTypeErrorNamesTheKey(t *testing.T) {
|
||||||
|
type numbers struct {
|
||||||
|
Year int `gorm:"column:year"`
|
||||||
|
Count uint8 `gorm:"column:count"`
|
||||||
|
Title string `gorm:"column:title"`
|
||||||
|
Price float64 `gorm:"column:price"`
|
||||||
|
}
|
||||||
|
allowed := []string{"year", "count", "title", "price"}
|
||||||
|
cases := map[string]any{
|
||||||
|
"year": json.Number("1977.5"),
|
||||||
|
"count": json.Number("1e21"),
|
||||||
|
"title": true,
|
||||||
|
"price": "cheap",
|
||||||
|
}
|
||||||
|
for key, value := range cases {
|
||||||
|
var row numbers
|
||||||
|
err := Fill(&row, allowed, map[string]any{key: value}, true)
|
||||||
|
var typed *FillTypeError
|
||||||
|
if !errors.As(err, &typed) || typed.Key != key || typed.Err == nil {
|
||||||
|
t.Fatalf("%s = %v: err = %#v, want *FillTypeError for %s", key, value, err, key)
|
||||||
|
}
|
||||||
|
if !strings.Contains(err.Error(), "lagoon: fill "+key+": ") {
|
||||||
|
t.Fatalf("%s: message = %q", key, err.Error())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
var typed *FillTypeError
|
||||||
|
if err := Fill(numbers{}, allowed, map[string]any{"year": 1}, true); err == nil || errors.As(err, &typed) {
|
||||||
|
t.Fatalf("non-pointer model: err = %v, want a plain error", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user