fix(32): CR-02 gate field type uploads on extension visibility
The plugin field-type upload route resolved a form extension's field without checking the extension's Permissions and Visible, so an administrator who cannot see the extension could still run its Upload. The route now answers 404 for them, like the extension's widget actions and partials.
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user