diff --git a/compose/service/module.go b/compose/service/module.go index b77b0d6ee..e8c29b135 100644 --- a/compose/service/module.go +++ b/compose/service/module.go @@ -273,7 +273,7 @@ func (svc module) Create(ctx context.Context, new *types.Module) (*types.Module, new.DeletedAt = nil if new.Fields != nil { - _ = new.Fields.Walk(func(f *types.ModuleField) error { + err = new.Fields.Walk(func(f *types.ModuleField) error { f.ID = nextID() f.ModuleID = new.ID f.NamespaceID = new.NamespaceID @@ -287,8 +287,15 @@ func (svc module) Create(ctx context.Context, new *types.Module) (*types.Module, f.Expressions.Validators[i] = v } + if !handle.IsValid(f.Name) { + return ModuleErrInvalidHandle() + } + return nil }) + if err != nil { + return + } } aProps.setModule(new) @@ -613,6 +620,12 @@ func updateModuleFields(ctx context.Context, s store.Storer, new, old *types.Mod return ModuleErrFieldNameReserved() } + // backward compatible; we didn't check for valid handle. + // if a field already existed and the handle is invalid we ignore the error. + if !handle.IsValid(f.Name) && old.Fields.FindByName(f.Name) == nil { + return ModuleErrInvalidHandle() + } + if f.ModuleID != new.ID { return fmt.Errorf("module id of field %q does not match the module", f.Name) } diff --git a/tests/compose/module_test.go b/tests/compose/module_test.go index 22422257f..fe6ed85c0 100644 --- a/tests/compose/module_test.go +++ b/tests/compose/module_test.go @@ -193,6 +193,25 @@ func TestModuleCreate(t *testing.T) { End() } +func TestModuleCreateInvalidField(t *testing.T) { + h := newHelper(t) + h.clearModules() + + helpers.AllowMe(h, types.NamespaceRbacResource(0), "read", "modules.search") + helpers.AllowMe(h, types.NamespaceRbacResource(0), "module.create") + + ns := h.makeNamespace("some-namespace") + + h.apiInit(). + Post(fmt.Sprintf("/namespace/%d/module/", ns.ID)). + JSON(`{ "name": "mod", "fields": [{ "name": "a", "kind": "Number" }] }`). + Header("Accept", "application/json"). + Expect(t). + Status(http.StatusOK). + Assert(helpers.AssertError("module.errors.invalidHandle")). + End() +} + func TestModuleUpdateForbidden(t *testing.T) { h := newHelper(t) h.clearModules() @@ -267,6 +286,30 @@ func TestModuleFieldsUpdate(t *testing.T) { h.a.Equal(m.Fields[1].Kind, "DateTime") } +func TestModuleFieldsUpdate_invalidHandle(t *testing.T) { + h := newHelper(t) + h.clearModules() + + helpers.AllowMe(h, types.NamespaceRbacResource(0), "read", "modules.search") + helpers.AllowMe(h, types.NamespaceRbacResource(0), "module.create") + helpers.AllowMe(h, types.ModuleRbacResource(0, 0), "update") + + ns := h.makeNamespace("some-namespace") + mod := h.makeModule(ns, "mod", &types.ModuleField{ + Name: "a", + Kind: "String", + }) + + h.apiInit(). + Post(fmt.Sprintf("/namespace/%d/module/%d", ns.ID, mod.ID)). + JSON(`{ "name": "mod", "fields": [{ "name": "a", "kind": "String" }] }`). + Header("Accept", "application/json"). + Expect(t). + Status(http.StatusOK). + Assert(helpers.AssertNoErrors). + End() +} + func TestModuleUpdateWithReservedFieldName(t *testing.T) { h := newHelper(t) h.clearModules()