diff --git a/docs/backend/admin-controllers.md b/docs/backend/admin-controllers.md index 7d3e999c..f4b3e4a7 100644 --- a/docs/backend/admin-controllers.md +++ b/docs/backend/admin-controllers.md @@ -616,7 +616,7 @@ An extension adds value fields (`text`, `textarea`, `number`, `checkbox`, `switc ### Permissions, widgets and partials -`Permissions` (backend permission codes some plugin registers) and `Visible` (asked per request) decide who sees the extension. An administrator who fails either gets none of its fields in the form schema or the record; a save drops their posted keys for it and does not call its `Save`, so stored values stay as they are. Its widget actions and partials answer 404 for them. +`Permissions` (backend permission codes some plugin registers) and `Visible` (asked per request) decide who sees the extension. An administrator who fails either gets none of its fields in the form schema or the record; a save drops their posted keys for it and does not call its `Save`, so stored values stay as they are. Its widget actions, partials and field type uploads answer 404 for them. A `widget` field's tag uses the contributing plugin's prefix (`acme-extras-...`), its `action` names one of the extension's `Actions` (a name the owner already registers stops the boot), and its `fill` keys name value fields of the same extension. Its element script comes from one of the plugin's field types (loaded on every admin page) or from the plugin's own `pact.AdminClientAssets`, which cabana adds to the target form's assets. A `partial` field renders `_.htm` from `Dir` in the contributing plugin's `AdminFS` through `PartialData`, with the same view model rules as `pact.AdminPartialData`; a partial name the owner already uses stops the boot. diff --git a/modules/cabana/field_type_upload.go b/modules/cabana/field_type_upload.go index 606a540b..afc32983 100644 --- a/modules/cabana/field_type_upload.go +++ b/modules/cabana/field_type_upload.go @@ -8,6 +8,7 @@ import ( "net/http" "strings" + "git.golem15.com/golem15/summercms/modules/bouncer" "git.golem15.com/golem15/summercms/modules/lagoon" "git.golem15.com/golem15/summercms/modules/pact" "gorm.io/gorm" @@ -28,7 +29,8 @@ import ( // the record transaction fails after Upload or the element stops waiting; // Upload stores under a name that is safe to leave behind. It answers // 201 {data: {url}}; a pact.FormFieldError is a 422 on the field, any other -// Upload error the opaque 500. +// Upload error the opaque 500. A form extension's field answers 404 to an +// administrator who does not see the extension. func (s *service) fieldUpload(w http.ResponseWriter, r *http.Request) { s.protect(w, r, func(cc *CompiledController) { id, err := pathID(r) @@ -49,6 +51,16 @@ func (s *service) fieldUpload(w http.ResponseWriter, r *http.Request) { writeNotFound(w, r) return } + // D-91: a form extension's field exists only for an administrator + // who sees the extension (its Permissions and Visible), as on its + // widget actions and partials. + if ext := cc.extensionOf(field.Name); ext != nil { + principal, _ := bouncer.User(r.Context()) + if !extensionVisible(r.Context(), principal, ext) { + writeNotFound(w, r) + return + } + } db, err := s.db() if err != nil { WriteError(w, http.StatusInternalServerError, "error", msgServerError) diff --git a/modules/cabana/form_extensions_postgres_test.go b/modules/cabana/form_extensions_postgres_test.go index 0285f8e9..de461df2 100644 --- a/modules/cabana/form_extensions_postgres_test.go +++ b/modules/cabana/form_extensions_postgres_test.go @@ -6,6 +6,7 @@ import ( "encoding/json" "errors" "io/fs" + "mime/multipart" "net/http" "net/http/httptest" "strconv" @@ -598,3 +599,78 @@ func TestFormExtensionRequired(t *testing.T) { t.Fatalf("editor update: status=%d body=%s", rec.Code, rec.Body.String()) } } + +// TestFormExtensionUpload (CR-02): the field upload route of an extension's +// plugin field type answers 404 to an administrator who does not see the +// extension, and its Upload never runs for them. +func TestFormExtensionUpload(t *testing.T) { + var uploads int + fileType := pact.FormFieldType{ + Name: "extrafile", Element: "acme-extra-file", Scripts: []string{"assets/js/extra.js"}, + Decode: func(_ context.Context, in pact.FormFieldValue) (pact.FormFieldDecoded, error) { + return pact.FormFieldDecoded{Value: in.Value}, nil + }, + Upload: func(_ context.Context, in pact.FormFieldUpload) (pact.FormFieldUploadResult, error) { + uploads++ + return pact.FormFieldUploadResult{URL: "/media/" + in.Filename}, nil + }, + } + extra := feContributor{ + id: "acme.extra", + fsys: fstest.MapFS{ + "extensions/posts.yaml": &fstest.MapFile{Data: []byte("fields:\n img:\n type: extrafile\n after: a\n")}, + "assets/js/extra.js": &fstest.MapFile{Data: []byte("// extra\n")}, + }, + types: []pact.FormFieldType{fileType}, + extensions: []pact.AdminActionExtension{{ + Controller: "acme.blog.posts", + Form: &pact.AdminFormExtension{Fields: "extensions/posts.yaml", Permissions: []string{"acme.extra.manage"}, Save: feNoopSave}, + }}, + } + env := newFEEnv(t, feOwner{fields: feOwnerFields}, extra) + seed := fePost{A: "A0"} + if err := env.db.Create(&seed).Error; err != nil { + t.Fatal(err) + } + upload := func(principal *bouncer.Principal, id string) *httptest.ResponseRecorder { + t.Helper() + var buf bytes.Buffer + mw := multipart.NewWriter(&buf) + part, err := mw.CreateFormFile("file", "a.png") + if err != nil { + t.Fatal(err) + } + if _, err := part.Write([]byte("png")); err != nil { + t.Fatal(err) + } + if err := mw.Close(); err != nil { + t.Fatal(err) + } + req := httptest.NewRequest(http.MethodPost, "/", &buf) + req.Header.Set("Content-Type", mw.FormDataContentType()) + req.Header.Set("X-Requested-With", "XMLHttpRequest") + for name, value := range map[string]string{"vendor": "acme", "plugin": "blog", "controller": "posts", "id": id, "field": "img"} { + req.SetPathValue(name, value) + } + req = req.WithContext(bouncer.WithUser(req.Context(), principal)) + rec := httptest.NewRecorder() + requireAjax(env.svc.fieldUpload)(rec, req) + return rec + } + id := strconv.FormatUint(uint64(seed.ID), 10) + for _, target := range []string{"0", id} { + if rec := upload(superUser(), target); rec.Code != http.StatusCreated || !strings.Contains(rec.Body.String(), "/media/a.png") { + t.Fatalf("visible upload to %s: status=%d body=%s", target, rec.Code, rec.Body.String()) + } + } + editor := &bouncer.Principal{ID: 2, Backend: true, PermissionGrants: map[string]bool{"acme.blog.access": true}} + before := uploads + for _, target := range []string{"0", id} { + if rec := upload(editor, target); rec.Code != http.StatusNotFound { + t.Fatalf("invisible upload to %s: status=%d body=%s", target, rec.Code, rec.Body.String()) + } + } + if uploads != before { + t.Fatalf("Upload ran for an administrator who does not see the extension") + } +}