From ee2344502eaa139636afd97483b414f1f8d58145 Mon Sep 17 00:00:00 2001 From: Pascal Fischer <32096965+pascal-fischer@users.noreply.github.com> Date: Mon, 21 Sep 2026 17:26:52 +0200 Subject: [PATCH] [management] fix group resource validation (#7608) --- .../http/handlers/groups/groups_handler.go | 45 ++++++++++++------- .../handlers/groups/groups_handler_test.go | 43 +++++++++++++++++- 2 files changed, 71 insertions(+), 17 deletions(-) diff --git a/management/server/http/handlers/groups/groups_handler.go b/management/server/http/handlers/groups/groups_handler.go index f8d161a87..ed01e7c3d 100644 --- a/management/server/http/handlers/groups/groups_handler.go +++ b/management/server/http/handlers/groups/groups_handler.go @@ -148,13 +148,10 @@ func (h *handler) updateGroup(w http.ResponseWriter, r *http.Request) { peers = *req.Peers } - resources := make([]types.Resource, 0) - if req.Resources != nil { - for _, res := range *req.Resources { - resource := types.Resource{} - resource.FromAPIRequest(&res) - resources = append(resources, resource) - } + resources, err := resourcesFromAPIRequest(req.Resources) + if err != nil { + util.WriteError(r.Context(), err, w) + return } group := types.Group{ @@ -210,13 +207,10 @@ func (h *handler) createGroup(w http.ResponseWriter, r *http.Request) { peers = *req.Peers } - resources := make([]types.Resource, 0) - if req.Resources != nil { - for _, res := range *req.Resources { - resource := types.Resource{} - resource.FromAPIRequest(&res) - resources = append(resources, resource) - } + resources, err := resourcesFromAPIRequest(req.Resources) + if err != nil { + util.WriteError(r.Context(), err, w) + return } group := types.Group{ @@ -335,11 +329,30 @@ func toGroupResponse(peers []*nbpeer.Peer, group *types.Group) *api.Group { gr.PeersCount = len(gr.Peers) for _, res := range group.Resources { - resResp := res.ToAPIResponse() - gr.Resources = append(gr.Resources, *resResp) + if resResp := res.ToAPIResponse(); resResp != nil { + gr.Resources = append(gr.Resources, *resResp) + } } gr.ResourcesCount = len(gr.Resources) return &gr } + +func resourcesFromAPIRequest(req *[]api.Resource) ([]types.Resource, error) { + resources := make([]types.Resource, 0) + if req == nil { + return resources, nil + } + + for _, res := range *req { + if res.Id == "" || !types.ResourceType(res.Type).Valid() { + return nil, status.Errorf(status.InvalidArgument, "resource id shouldn't be empty and type must be one of: peer, domain, host, subnet") + } + resource := types.Resource{} + resource.FromAPIRequest(&res) + resources = append(resources, resource) + } + + return resources, nil +} diff --git a/management/server/http/handlers/groups/groups_handler_test.go b/management/server/http/handlers/groups/groups_handler_test.go index 57e238630..78e4a2578 100644 --- a/management/server/http/handlers/groups/groups_handler_test.go +++ b/management/server/http/handlers/groups/groups_handler_test.go @@ -8,8 +8,8 @@ import ( "fmt" "io" "net/http" - "net/netip" "net/http/httptest" + "net/netip" "strings" "testing" @@ -208,6 +208,33 @@ func TestWriteGroup(t *testing.T) { expectedStatus: http.StatusUnprocessableEntity, expectedBody: false, }, + { + name: "Write Group POST Empty Resource", + requestType: http.MethodPost, + requestPath: "/api/groups", + requestBody: bytes.NewBuffer( + []byte(`{"name":"With Resource","resources":[{}]}`)), + expectedStatus: http.StatusUnprocessableEntity, + expectedBody: false, + }, + { + name: "Write Group PUT Empty Resource", + requestType: http.MethodPut, + requestPath: "/api/groups/id-existed", + requestBody: bytes.NewBuffer( + []byte(`{"name":"With Resource","resources":[{"id":"","type":"host"}]}`)), + expectedStatus: http.StatusUnprocessableEntity, + expectedBody: false, + }, + { + name: "Write Group POST Unknown Resource Type", + requestType: http.MethodPost, + requestPath: "/api/groups", + requestBody: bytes.NewBuffer( + []byte(`{"name":"With Resource","resources":[{"id":"res-1","type":"banana"}]}`)), + expectedStatus: http.StatusUnprocessableEntity, + expectedBody: false, + }, { name: "Write Group PUT OK", requestType: http.MethodPut, @@ -376,6 +403,20 @@ func TestGetAllGroups(t *testing.T) { } } +func TestToGroupResponseSkipsEmptyResource(t *testing.T) { + group := &types.Group{ + ID: "id-resources", + Name: "Resources", + Issued: types.GroupIssuedAPI, + Resources: []types.Resource{{}, {ID: "res-1", Type: types.ResourceTypeHost}}, + } + + got := toGroupResponse(nil, group) + + assert.Equal(t, 1, got.ResourcesCount) + assert.Equal(t, []api.Resource{{Id: "res-1", Type: api.ResourceType(types.ResourceTypeHost)}}, got.Resources) +} + func TestDeleteGroup(t *testing.T) { tt := []struct { name string