Make amount of group members in patch configurable.

changelog/unreleased/enhancement-add-configurable-group-members-patch-limit.md

@@ -0,0 +1,8 @@
+Enhancement: Make the group members addition limit configurable
+
+It's now possible to configure the limit of group members addition by PATCHing `/graph/v1.0/groups/{groupID}`.
+It still defaults to 20 as defined in the spec but it can be configured via `.graph.api.group_members_patch_limit`
+in `ocis.yaml` or via the `GRAPH_GROUP_MEMBERS_PATCH_LIMIT` environment variable.
+
+https://github.com/owncloud/ocis/pull/5357
+https://github.com/owncloud/ocis/issues/5262

services/graph/pkg/config/config.go

@@ -19,6 +19,8 @@ type Config struct {
 
 	HTTP HTTP `yaml:"http"`
 
+	API API `yaml:"api"`
+
 	Reva          *shared.Reva          `yaml:"reva"`
 	TokenManager  *TokenManager         `yaml:"token_manager"`
 	GRPCClientTLS *shared.GRPCClientTLS `yaml:"grpc_client_tls"`
@@ -85,6 +87,11 @@ type Identity struct {
 	LDAP    LDAP   `yaml:"ldap"`
 }
 
+// API represents API configuration parameters.
+type API struct {
+	GroupMembersPatchLimit int `yaml:"group_members_patch_limit" env:"GRAPH_GROUP_MEMBERS_PATCH_LIMIT" desc:"The amount of group members allowed to be added with a single patch request."`
+}
+
 // Events combines the configuration options for the event bus.
 type Events struct {
 	Endpoint             string `yaml:"endpoint" env:"GRAPH_EVENTS_ENDPOINT" desc:"The address of the event system. The event system is the message queuing service. It is used as message broker for the microservice architecture. Set to a empty string to disable emitting events."`

services/graph/pkg/config/defaults/defaultconfig.go

@@ -30,6 +30,9 @@ func DefaultConfig() *config.Config {
 		Service: config.Service{
 			Name: "graph",
 		},
+		API: config.API{
+			GroupMembersPatchLimit: 20,
+		},
 		Reva: shared.DefaultRevaConfig(),
 		Spaces: config.Spaces{
 			WebDavBase:   "https://localhost:9200",

services/graph/pkg/service/v0/groups.go

@@ -19,7 +19,6 @@ import (
 	"github.com/go-chi/render"
 )
 
-const memberRefsLimit = 20
 const memberTypeUsers = "users"
 
 // GetGroups implements the Service interface.
@@ -124,13 +123,13 @@ func (g Graph) PatchGroup(w http.ResponseWriter, r *http.Request) {
 
 	if memberRefs, ok := changes.GetMembersodataBindOk(); ok {
 		// The spec defines a limit of 20 members maxium per Request
-		if len(memberRefs) > memberRefsLimit {
+		if len(memberRefs) > g.config.API.GroupMembersPatchLimit {
 			logger.Debug().
 				Int("number", len(memberRefs)).
-				Int("limit", memberRefsLimit).
+				Int("limit", g.config.API.GroupMembersPatchLimit).
 				Msg("could not create group, exceeded members limit")
 			errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest,
-				fmt.Sprintf("Request is limited to %d members", memberRefsLimit))
+				fmt.Sprintf("Request is limited to %d members", g.config.API.GroupMembersPatchLimit))
 			return
 		}
 		memberIDs := make([]string, 0, len(memberRefs))

services/graph/pkg/service/v0/groups_test.go

@@ -6,6 +6,7 @@ import (
 	"encoding/json"
 	"errors"
 	"io"
+	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
 
@@ -284,8 +285,10 @@ var _ = Describe("Groups", func() {
 			It("fails when the number of users is exceeded - spec says 20 max", func() {
 				updatedGroup := libregraph.NewGroup()
 				updatedGroup.SetDisplayName("group1 updated")
-				updatedGroup.SetMembersodataBind([]string{"1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "12", "13", "14", "15", "16", "17", "18",
-					"19", "20", "21"})
+				updatedGroup.SetMembersodataBind([]string{
+					"1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "12", "13", "14", "15", "16", "17", "18",
+					"19", "20", "21",
+				})
 				updatedGroupJson, err := json.Marshal(updatedGroup)
 				Expect(err).ToNot(HaveOccurred())
 
@@ -295,6 +298,41 @@ var _ = Describe("Groups", func() {
 				r = r.WithContext(context.WithValue(revactx.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx))
 				svc.PatchGroup(rr, r)
 
+				resp, err := ioutil.ReadAll(rr.Body)
+				Expect(err).ToNot(HaveOccurred())
+
+				Expect(string(resp)).To(ContainSubstring("Request is limited to 20"))
+				Expect(rr.Code).To(Equal(http.StatusBadRequest))
+			})
+
+			It("succeeds when the number of users is over 20 but the limit is raised to 21", func() {
+				updatedGroup := libregraph.NewGroup()
+				updatedGroup.SetDisplayName("group1 updated")
+				updatedGroup.SetMembersodataBind([]string{
+					"1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "12", "13", "14", "15", "16", "17", "18",
+					"19", "20", "21",
+				})
+				updatedGroupJson, err := json.Marshal(updatedGroup)
+				Expect(err).ToNot(HaveOccurred())
+
+				cfg.API.GroupMembersPatchLimit = 21
+				svc = service.NewService(
+					service.Config(cfg),
+					service.WithGatewayClient(gatewayClient),
+					service.EventsPublisher(&eventsPublisher),
+					service.WithIdentityBackend(identityBackend),
+				)
+
+				r := httptest.NewRequest(http.MethodPatch, "/graph/v1.0/me/groups", bytes.NewBuffer(updatedGroupJson))
+				rctx := chi.NewRouteContext()
+				rctx.URLParams.Add("groupID", *newGroup.Id)
+				r = r.WithContext(context.WithValue(revactx.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx))
+				svc.PatchGroup(rr, r)
+
+				resp, err := ioutil.ReadAll(rr.Body)
+				Expect(err).ToNot(HaveOccurred())
+
+				Expect(string(resp)).To(ContainSubstring("Error parsing [email protected] values"))
 				Expect(rr.Code).To(Equal(http.StatusBadRequest))
 			})