fix(09): WR-03 refuse writes that the compiled list and form do not declare
This commit is contained in:
@@ -37,9 +37,9 @@ All paths are relative to `<prefix>/api/v1`. A controller ID `vendor.plugin.cont
|
||||
| GET `/navigation`, GET `/settings` | Navigation and settings entries the administrator may open. |
|
||||
| GET `/settings/{code}/schema`, GET and PUT `/settings/{code}` | Settings form schema, values and update. |
|
||||
| GET `/{vendor}/{plugin}/{controller}/schema/list`, `.../schema/form`, `.../schema/relation/{name}` | Localized list, form and relation schemas. |
|
||||
| GET and POST `/{vendor}/{plugin}/{controller}` | List records; create a record. |
|
||||
| GET, PUT and DELETE `/{vendor}/{plugin}/{controller}/{id}` | Show, update and delete a record. |
|
||||
| POST `/{vendor}/{plugin}/{controller}/bulk-delete` | Delete a set of records in one transaction. |
|
||||
| GET and POST `/{vendor}/{plugin}/{controller}` | List records; create a record (needs a form and `create` in `toolbar.buttons`). |
|
||||
| GET, PUT and DELETE `/{vendor}/{plugin}/{controller}/{id}` | Show, update and delete a record (update and delete need a form). |
|
||||
| POST `/{vendor}/{plugin}/{controller}/bulk-delete` | Delete a set of records in one transaction; needs `delete` in `toolbar.buttons`. |
|
||||
| POST `/{vendor}/{plugin}/{controller}/widgets/{field}` | Run the action of a `type: widget` field with an optional `record_id` and the fill snapshot; answers `{message, fill}`. |
|
||||
| POST `/{vendor}/{plugin}/{controller}/toolbar/{action}` | Run a registered toolbar action with an empty `{}` body; answers `{message, fill: {}}`. |
|
||||
| GET `/{vendor}/{plugin}/{controller}/partials/{name}` | Render a declared header or form partial as a node tree; `?id=` (form partials only) passes the scoped record to the view model. |
|
||||
|
||||
@@ -498,3 +498,63 @@ func containsString(items []string, want string) bool {
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// TestCRUDOperationsFollowDeclarations pins WR-03: the compiled list and form
|
||||
// decide which writes the server accepts, not only what the SPA shows.
|
||||
func TestCRUDOperationsFollowDeclarations(t *testing.T) {
|
||||
_, httpSvc, cc, db, hooks := hookFixture(t)
|
||||
ctx := principalCtx(hooks, superUser())
|
||||
created := crudCall(httpSvc, http.MethodPost, "", []byte(`{"name":"Ada"}`), ctx)
|
||||
if created.Code != http.StatusCreated {
|
||||
t.Fatalf("declared create=%d %s", created.Code, created.Body.String())
|
||||
}
|
||||
id := uintString(decodeData(t, created.Body.Bytes())["id"])
|
||||
bulk := func() *httptest.ResponseRecorder {
|
||||
req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(`{"ids":[`+id+`]}`)).WithContext(ctx)
|
||||
req.SetPathValue("vendor", "acme")
|
||||
req.SetPathValue("plugin", "demo")
|
||||
req.SetPathValue("controller", "records")
|
||||
rec := httptest.NewRecorder()
|
||||
httpSvc.bulkDelete(rec, req)
|
||||
return rec
|
||||
}
|
||||
forbidden := func(name string, rec *httptest.ResponseRecorder) {
|
||||
t.Helper()
|
||||
if rec.Code != http.StatusForbidden || !strings.Contains(rec.Body.String(), `"forbidden"`) {
|
||||
t.Fatalf("%s=%d %s, want 403 forbidden", name, rec.Code, rec.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// No toolbar create button: the create route is closed, update is not.
|
||||
list := *cc.List
|
||||
cc.List = &list
|
||||
cc.List.ToolbarButtons = []string{"delete"}
|
||||
forbidden("create without a toolbar create button", crudCall(httpSvc, http.MethodPost, "", []byte(`{"name":"Bea"}`), ctx))
|
||||
if got := crudCall(httpSvc, http.MethodPut, id, []byte(`{"name":"Cid"}`), ctx); got.Code != http.StatusOK {
|
||||
t.Fatalf("update=%d %s", got.Code, got.Body.String())
|
||||
}
|
||||
|
||||
// No toolbar delete button: bulk delete is closed.
|
||||
cc.List.ToolbarButtons = []string{"create"}
|
||||
forbidden("bulk delete without a toolbar delete button", bulk())
|
||||
if n := countCrud(t, db); n != 1 {
|
||||
t.Fatalf("rows=%d, a refused bulk delete removed data", n)
|
||||
}
|
||||
|
||||
// No form: nothing can be created, updated or deleted one by one.
|
||||
form := cc.Form
|
||||
cc.Form = nil
|
||||
forbidden("create without a form", crudCall(httpSvc, http.MethodPost, "", []byte(`{"name":"Dan"}`), ctx))
|
||||
forbidden("update without a form", crudCall(httpSvc, http.MethodPut, id, []byte(`{"name":"Eve"}`), ctx))
|
||||
forbidden("delete without a form", crudCall(httpSvc, http.MethodDelete, id, nil, ctx))
|
||||
if n := countCrud(t, db); n != 1 {
|
||||
t.Fatalf("rows=%d, a refused write changed data", n)
|
||||
}
|
||||
cc.Form = form
|
||||
|
||||
// Declared again: bulk delete works.
|
||||
cc.List.ToolbarButtons = []string{"create", "delete"}
|
||||
if got := bulk(); got.Code != http.StatusOK || deletedCount(t, got.Body.Bytes()) != 1 {
|
||||
t.Fatalf("declared bulk delete=%d %s", got.Code, got.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -47,6 +47,9 @@ const crudListConfig = `modelClass: Record
|
||||
list: ~/plugins/acme/demo/models/record/columns.yaml
|
||||
recordsPerPage: 20
|
||||
showSearch: true
|
||||
showCheckboxes: true
|
||||
toolbar:
|
||||
buttons: [create, delete]
|
||||
`
|
||||
|
||||
const crudColumns = `columns:
|
||||
|
||||
@@ -606,6 +606,9 @@ func (s *service) show(w http.ResponseWriter, r *http.Request) {
|
||||
|
||||
func (s *service) create(w http.ResponseWriter, r *http.Request) {
|
||||
s.protect(w, r, func(cc *CompiledController) {
|
||||
if !s.operationDeclared(w, r, cc, "create") {
|
||||
return
|
||||
}
|
||||
body, err := decodeObject(r)
|
||||
if err != nil {
|
||||
writeCRUDError(w, err)
|
||||
@@ -627,6 +630,9 @@ func (s *service) create(w http.ResponseWriter, r *http.Request) {
|
||||
|
||||
func (s *service) update(w http.ResponseWriter, r *http.Request) {
|
||||
s.protect(w, r, func(cc *CompiledController) {
|
||||
if !s.operationDeclared(w, r, cc, "update") {
|
||||
return
|
||||
}
|
||||
id, err := pathID(r)
|
||||
if err != nil {
|
||||
writeCRUDError(w, err)
|
||||
@@ -653,6 +659,9 @@ func (s *service) update(w http.ResponseWriter, r *http.Request) {
|
||||
|
||||
func (s *service) bulkDelete(w http.ResponseWriter, r *http.Request) {
|
||||
s.protect(w, r, func(cc *CompiledController) {
|
||||
if !s.operationDeclared(w, r, cc, "bulk-delete") {
|
||||
return
|
||||
}
|
||||
in, err := decodeBulk(r)
|
||||
if err != nil {
|
||||
writeCRUDError(w, err)
|
||||
@@ -684,6 +693,9 @@ func decodeBulk(r *http.Request) (BulkDeleteInput, error) {
|
||||
|
||||
func (s *service) deleteRecord(w http.ResponseWriter, r *http.Request) {
|
||||
s.protect(w, r, func(cc *CompiledController) {
|
||||
if !s.operationDeclared(w, r, cc, "delete") {
|
||||
return
|
||||
}
|
||||
id, err := pathID(r)
|
||||
if err != nil {
|
||||
writeCRUDError(w, err)
|
||||
@@ -759,6 +771,23 @@ func (s *service) protect(w http.ResponseWriter, r *http.Request, fn func(*Compi
|
||||
fn(cc)
|
||||
}
|
||||
|
||||
// operationDeclared refuses a write the controller's YAML does not declare, so
|
||||
// the compiled list and form are the capability, not a hint for the SPA. Create
|
||||
// needs a form and a toolbar `create` button; update and single-record delete
|
||||
// (the form screen's delete button, as in Winter) need a form; bulk delete needs
|
||||
// the toolbar `delete` button, which in turn needs showCheckboxes. The answer is
|
||||
// 403 with the same envelope as a permission failure.
|
||||
func (s *service) operationDeclared(w http.ResponseWriter, r *http.Request, cc *CompiledController, op string) bool {
|
||||
if cc.operationDeclared(op) {
|
||||
return true
|
||||
}
|
||||
if principal, _ := bouncer.User(r.Context()); principal != nil {
|
||||
s.logAuth(r, "denied", principal.ID)
|
||||
}
|
||||
WriteError(w, http.StatusForbidden, "forbidden", msgForbidden)
|
||||
return false
|
||||
}
|
||||
|
||||
func projectRow(row any, controller pact.AdminController, cols []ListColumn) map[string]any {
|
||||
v := reflect.ValueOf(row)
|
||||
for v.Kind() == reflect.Pointer {
|
||||
|
||||
@@ -107,6 +107,29 @@ func compileRegistry(items []controllerRef) (*Registry, error) {
|
||||
return &Registry{byID: byID, assets: assets}, nil
|
||||
}
|
||||
|
||||
// operationDeclared reports whether the controller's compiled list and form
|
||||
// declare the write operation: create, update, delete or bulk-delete.
|
||||
func (cc *CompiledController) operationDeclared(op string) bool {
|
||||
switch op {
|
||||
case "create":
|
||||
return cc.Form != nil && (cc.List == nil || hasToolbarButton(cc.List, "create"))
|
||||
case "update", "delete":
|
||||
return cc.Form != nil
|
||||
case "bulk-delete":
|
||||
return cc.List != nil && hasToolbarButton(cc.List, "delete")
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
func hasToolbarButton(list *ListSchema, name string) bool {
|
||||
for _, button := range list.ToolbarButtons {
|
||||
if button == name {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
func withoutAction(actions []string, drop string) []string {
|
||||
out := make([]string, 0, len(actions))
|
||||
for _, action := range actions {
|
||||
|
||||
@@ -25,6 +25,9 @@ modelClass: Record
|
||||
const p10ListConfig = `modelClass: Record
|
||||
list: ~/plugins/acme/demo/models/record/columns.yaml
|
||||
recordsPerPage: 20
|
||||
showCheckboxes: true
|
||||
toolbar:
|
||||
buttons: [create, delete]
|
||||
`
|
||||
|
||||
const p10Columns = `columns:
|
||||
|
||||
@@ -294,7 +294,11 @@ func phase09DeniedService() *service {
|
||||
controller := orderController{perms: []string{"acme.demo.access"}}
|
||||
return &service{reg: &Registry{
|
||||
byID: map[string]*CompiledController{
|
||||
"acme.demo.widgets": {Controller: controller, Relations: map[string]*CompiledRelation{}},
|
||||
"acme.demo.widgets": {
|
||||
Controller: controller, Relations: map[string]*CompiledRelation{},
|
||||
// Declare every write so the denial under test is the permission.
|
||||
Form: &FormSchema{}, List: &ListSchema{ToolbarButtons: []string{"create", "delete"}},
|
||||
},
|
||||
},
|
||||
settings: map[string]*CompiledSetting{
|
||||
"demo": {Item: pact.SettingsItem{Code: "demo", Permissions: []string{"acme.demo.manage_settings"}}},
|
||||
|
||||
Reference in New Issue
Block a user